Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow dots and dashes at repo names in labels #1139

Merged
merged 1 commit into from
Nov 24, 2021

Conversation

facundominguez
Copy link
Contributor

Bug fix
cmd/gazelle

This PR has label.Parse accept repo names with dashes and dots. gazelle needs it to process bazel repositories that use these characters in repository names.

Fixes #1082

@facundominguez
Copy link
Contributor Author

facundominguez commented Nov 24, 2021

Bazel source code does not seem to allow leading dots and dashes in repository names:
https://github.com/bazelbuild/bazel/blob/cbad324d2c8520195d2ddafcabeb2d2ca96da0a1/src/main/java/com/google/devtools/build/lib/packages/WorkspaceGlobals.java#L53

However, when declaring repositories with http_archive I was able to define and reference repositories like -- and ... I tested bazelbuild/bazel@cbad324.

@blico
Copy link
Contributor

blico commented Nov 24, 2021

Thanks for the fix!

However, when declaring repositories with http_archive I was able to define and reference repositories like -- and ... I tested bazelbuild/bazel@cbad324.

http_archive rules do not create a new WORKSPACE, explaining why you do not see a failure. If you name a go_repository rule .. then you will see:

Error in repository_rule: invalid repository name '@..': workspace names are not allowed to be '@..'

Given that we cannot differentiate between repository rules that create a new WORKSPACE and those that don't, we should match the pattern in Bazel's source code and require repo name's start with a letter.

@blico blico self-requested a review November 24, 2021 14:20
@facundominguez
Copy link
Contributor Author

Given that we cannot differentiate between repository rules that create a new WORKSPACE and those that don't, we should match the pattern in Bazel's source code and require repo name's start with a letter.

I feel inclined to argue for the opposite. Given that we cannot differentiate between repository rules that create a new WORKSPACE and those that don't, gazelle must accept leading dots and dashes so it can digest the most repos that might be encountered in the wild.

Perhaps there is some reason for having gazelle being stricter than bazel, which I'm failing to see.

@blico
Copy link
Contributor

blico commented Nov 24, 2021

Good point, from the perspective of Gazelle's usage of label.Parse it makes sense for it to be less strict. It is possible there are other consumers of label.Parse that would prefer it to be more strict, but it is unlikely.

@blico blico closed this Nov 24, 2021
@blico blico reopened this Nov 24, 2021
@blico blico merged commit 6d3fa1c into bazel-contrib:master Nov 24, 2021
@facundominguez facundominguez deleted the fd/repo-dashdots branch November 24, 2021 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

label.Parse fails on labels with hyphens in the repository name
2 participants