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

Remove patching for WORKSPACE file #2355

Merged
merged 1 commit into from
Jan 30, 2020
Merged

Remove patching for WORKSPACE file #2355

merged 1 commit into from
Jan 30, 2020

Conversation

meteorcloudy
Copy link
Contributor

@meteorcloudy meteorcloudy commented Jan 30, 2020

Bazel will always override the external repository's WORKSPACE file by workspace_file, workspace_file_content or a created file with "workspace(name =<name in repo rule>)"

Bazel will always override the external repository's WORKSPACE file by `workspace_file`, `workspace_file_content` or an created file with "workspace(name =`<name in repo rule>`)"
@meteorcloudy
Copy link
Contributor Author

This will fix bazelbuild/bazel#10681 for rules_go

@philwo
Copy link
Contributor

philwo commented Jan 30, 2020

Thank you, Yun!

To clarify, this means that the problematic part of the patch was simply ignored until now (Bazel would patch the WORKSPACE file as requested and then replace it with a completely new one), so you can just remove it. Bazel@HEAD contains a bugfix for this behavior, so that patches now get applied after replacing the WORKSPACE file - which means that if the patch was not valid, it will now fail.

@jayconrod jayconrod merged commit 5474789 into master Jan 30, 2020
@jayconrod
Copy link
Contributor

Seems fine. I think this was necessary at some point, but not anymore.

What goes into the replacement WORKSPACE file?

@philwo
Copy link
Contributor

philwo commented Jan 30, 2020

@jayconrod The default WORKSPACE (if neither workspace_file nor workspace_file_content are specified on the http_repository) simply contains one line:

workspace(name = "{name}")

where {name} is the name attribute of the http_repository.

This is implemented here: https://github.com/bazelbuild/bazel/blob/master/tools/build_defs/repo/utils.bzl#L55

@philwo philwo deleted the meteorcloudy-patch-2 branch January 30, 2020 19:09
@nlopezgi
Copy link
Contributor

nlopezgi commented Feb 3, 2020

Hi, could we have a patch release with this commit included. See bazelbuild/rules_k8s#517 (comment)

jayconrod pushed a commit that referenced this pull request Feb 3, 2020
Bazel will always override the external repository's WORKSPACE file by `workspace_file`, `workspace_file_content` or an created file with "workspace(name =`<name in repo rule>`)"
jayconrod pushed a commit that referenced this pull request Feb 3, 2020
Bazel will always override the external repository's WORKSPACE file by `workspace_file`, `workspace_file_content` or an created file with "workspace(name =`<name in repo rule>`)"
@jayconrod
Copy link
Contributor

@nlopezgi Just tagged releases v0.20.6 and v0.21.2 with this.

@nlopezgi
Copy link
Contributor

nlopezgi commented Feb 3, 2020

thanks!

jayconrod pushed a commit to jayconrod/bazel-gazelle that referenced this pull request Feb 26, 2020
Normally, Gazelle detects the repository name by reading the WORKSPACE
file and looking for a workspace rule. In go_googleapis, the proper
workspace name is com_google_googleapis. Before
bazel-contrib/rules_go#2355, we patched the WORKSPACE file before running
Gazelle so the repository name would be set to
go_googleapis. Unfortunately, in newer versions of Bazel, WORKSPACE is
overwritten, so the patch doesn't apply.

This change just adds a hack to set the repository name to
go_googleapis if com_google_googleapis is detected.

Updates bazel-contrib/rules_go#2387
jayconrod pushed a commit to bazel-contrib/bazel-gazelle that referenced this pull request Feb 26, 2020
Normally, Gazelle detects the repository name by reading the WORKSPACE
file and looking for a workspace rule. In go_googleapis, the proper
workspace name is com_google_googleapis. Before
bazel-contrib/rules_go#2355, we patched the WORKSPACE file before running
Gazelle so the repository name would be set to
go_googleapis. Unfortunately, in newer versions of Bazel, WORKSPACE is
overwritten, so the patch doesn't apply.

This change just adds a hack to set the repository name to
go_googleapis if com_google_googleapis is detected.

Updates bazel-contrib/rules_go#2387
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants