Skip to content

Conversation

@illicitonion
Copy link
Contributor

@illicitonion illicitonion commented May 9, 2022

Empirically, this works, and some rulesets (e.g. rules_nodejs)
explicitly create packages whose names contain @ characters.

We should document this reality (or forbid it, but that would be a
massively breaking change to the ecosystem).

Additional context: https://bazelbuild.slack.com/archives/CEZUUKQ6P/p1651850227929389?thread_ts=1651832164.144609&cid=CEZUUKQ6P

Emperically, this works, and some rulesets (e.g. `rules_nodejs`)
explicitly create packages whose names contain `@` characters.

We should document this reality (or forbid it, but that would be a
massively breaking change to the ecosystem).
illicitonion added a commit to illicitonion/bazel-gazelle that referenced this pull request May 9, 2022
See bazelbuild/bazel#15428 for context.

tl;dr: Bazel actually allows this, and some common rulesets (e.g.
`rules_nodejs`) rely heavily on it.
@sgowroji sgowroji added team-Documentation Documentation improvements that cannot be directly linked to other team labels awaiting-review PR is awaiting review from an assigned reviewer labels May 9, 2022
Copy link
Member

@Wyverald Wyverald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! You can clearly see that @ is an allowed character in the code

.or(CharMatcher.anyOf(" !\"#$%&'()*+,-./;<=>?@[]^_`{|}~"))
, so this is a no-brainer.

@Wyverald Wyverald added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels May 11, 2022
@bazel-io bazel-io closed this in 1941dc1 May 13, 2022
@sgowroji sgowroji removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label May 13, 2022
@illicitonion illicitonion deleted the package-name-ats branch May 13, 2022 12:43
achew22 pushed a commit to bazel-contrib/bazel-gazelle that referenced this pull request May 13, 2022
See bazelbuild/bazel#15428 for context.

tl;dr: Bazel actually allows this, and some common rulesets (e.g.
`rules_nodejs`) rely heavily on it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

team-Documentation Documentation improvements that cannot be directly linked to other team labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants