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

Update golang.org/x/tools release and third-party "extra" patch #2556

Closed
bartle-stripe opened this issue Jul 8, 2020 · 6 comments · Fixed by #2559
Closed

Update golang.org/x/tools release and third-party "extra" patch #2556

bartle-stripe opened this issue Jul 8, 2020 · 6 comments · Fixed by #2559

Comments

@bartle-stripe
Copy link
Contributor

What version of rules_go are you using?

5ecc2d3

What version of gazelle are you using?

0.21.0

What version of Bazel are you using?

3.0.0

Does this issue reproduce with the latest releases of all the above?

Yes

What operating system and processor architecture are you using?

macOS Catalina

What did you do?

Vendor golang.org/x/tools at/past golang/tools@5c6ccfd

What did you expect to see?

bazel build @org_golang_x_tools//... works.

What did you see instead?

/org_golang_x_tools/go/internal/gcimporter/gcimporter.go:213:18: undefined: BImportData
/org_golang_x_tools/go/internal/gcimporter/iexport.go:36:22: undefined: internalError
/org_golang_x_tools/go/internal/gcimporter/iexport.go:55:21: undefined: predeclared
/org_golang_x_tools/go/internal/gcimporter/iexport.go:59:9: undefined: internalErrorf
/org_golang_x_tools/go/internal/gcimporter/iexport.go:222:10: undefined: internalErrorf
/org_golang_x_tools/go/internal/gcimporter/iexport.go:255:10: undefined: internalErrorf
/org_golang_x_tools/go/internal/gcimporter/iexport.go:270:9: undefined: internalErrorf
/org_golang_x_tools/go/internal/gcimporter/iexport.go:302:15: undefined: deltaNewFile
/org_golang_x_tools/go/internal/gcimporter/iexport.go:306:11: undefined: deltaNewFile
/org_golang_x_tools/go/internal/gcimporter/iimport.go:189:16: undefined: fakeFileSet
/org_golang_x_tools/go/internal/gcimporter/iexport.go:306:11: too many errors

The commit above refactored things by adding a couple new files (bimport.go and bexport.go), which isn't reflected in https://github.com/bazelbuild/rules_go/blob/master/third_party/org_golang_x_tools-extras.patch.

It'd be great to also include a README file in https://github.com/bazelbuild/rules_go/tree/master/third_party that explains how one might generate these patches (in particular, the "extra" patches), or a BUILD rules that can generate them automatically.

@bartle-stripe
Copy link
Contributor Author

I wonder if the extra patch could be made more robust by changing srcs to something like srcs = glob(["*.go"], exclude = ["*_test.go"]).

@jayconrod
Copy link
Contributor

I'm confused about what's happening here exactly. You mentioned you're vendoring golang.org/x/tools. How are you using the vendored copy together with org_golang_x_tools? They will conflict.

Overriding dependencies is the documentation for using alternate versions of repositories declared in go_rules_dependencies. It's a good idea to point to that document from //third_party. I'll add a README file there.

About the extras patches: I don't want spend too much time automating generation of these: I'd rather avoid the need for them by fixing #2374. Unfortunately, that's still blocked on bazelbuild/bazel#11291.

@jayconrod
Copy link
Contributor

I'll update org_golang_x_tools together with the others in preparation for the v0.24.0 release next month. I won't update dependencies outside of that cycle, since it's a fair amount of work to get everything to work together, and it doesn't benefit many people until it's released.

@bartle-stripe
Copy link
Contributor Author

We have a setup where we basically drop a WORKSPACE file in e.g. vendor/golang.org/x/tools, named org_golang_x_tools, before we call go_rules_dependencies(). But, I'd expect following the approach you linked would result in the same problem?

@jayconrod
Copy link
Contributor

Seems like that should work. The patches will need to be applied manually, and they'll need to be updated for the version you're using, but I can't think of any other issues.

@bartle-stripe
Copy link
Contributor Author

Yup, for now I'm actually just applying a slightly altered version of the patch:

--- tools/build/org_golang_x_tools-extras.patch	2020-07-08 12:16:12.000000000 -0700
+++ src/rules_go/third_party/org_golang_x_tools-extras.patch	2020-05-13 23:17:26.000000000 -0700
@@ -1185,15 +1185,13 @@

  go_library(
      name = "go_default_library",
-@@ -14,6 +14,22 @@
+@@ -14,6 +14,20 @@
      visibility = ["//go:__subpackages__"],
  )

 +go_tool_library(
 +    name = "go_tool_library",
 +    srcs = [
-+        "bexport.go",
-+        "bimport.go",
 +        "exportdata.go",
 +        "gcimporter.go",
 +        "iexport.go",

jayconrod pushed a commit to jayconrod/rules_go that referenced this issue Jul 8, 2020
jayconrod pushed a commit that referenced this issue Jul 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants