Disable GOCACHE and only enable -shared/-dynlink on selected platforms #1696
Conversation
go/private/mode.bzl
Outdated
| "darwin/amd64": None, | ||
| } | ||
|
|
||
| def add_link_arg(go, args): |
There was a problem hiding this comment.
It should be possible to reduce constraints on how this function is called. It looks like it just needs a mode, and it could return a list of strings which could be added by the caller with args.add_all.
Also, the name add_link_arg doesn't quite seem right, since this affects code generation. Maybe link_mode_args would be a better name?
go/private/context.bzl
Outdated
| @@ -241,6 +241,7 @@ def go_context(ctx, attr = None): | |||
| env.update({ | |||
| "GOARCH": mode.goarch, | |||
| "GOOS": mode.goos, | |||
| "GOCACHE": "off", # bazel has it's own caching | |||
There was a problem hiding this comment.
As was mentioned in another comment, this will probably break in Go 1.12. I think they want to integrate the cache more tightly into go build.
GoStdLib should be the only action that needs this. The other actions all read inputs and write outputs directly. So maybe GOCACHE could just be set in stdlib.go?
I share your concern about interacting with the user's cache outside of Bazel's workspace. It's probably okay as long as sandboxing or remote execution are in use, but that's not guaranteed. It's probably better to create a temp directory, set GOCACHE to that, then delete it at the end of the action.
It's probably best for all that to be in a separate PR though. Or feel free to file an issue, and I can fix it next week.
There was a problem hiding this comment.
sure thing, I'll remove it
|
I'm going to rebase on master |
Signed-off-by: Steeve Morin <steeve@zen.ly>
go/private/mode.bzl
Outdated
| args = [] | ||
| if mode.link == LINKMODE_C_ARCHIVE: | ||
| if platform in _LINK_C_ARCHIVE_PLATFORMS or mode.goos in _LINK_C_ARCHIVE_GOOS: | ||
| if platform == "linux/ppc64": |
There was a problem hiding this comment.
This return could cause problems. Can the condition be folded into the if above? ... and platform != "linux/ppc64"
|
Sorry I got sidetracked, thank you for making the change! |
Signed-off-by: Steeve Morin <steeve@zen.ly>
Go 1.11 has its own build cache, which is enabled by default. Disable it because it doesn't make sense with bazel. See https://golang.org/cmd/go/#hdr-Build_and_test_caching
Also, only enable
-shared/-dynlinkon selected platforms, mimicking whatgo builddoes.