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

Suppress goroot path in tool builds #2952

Merged
merged 1 commit into from
Sep 22, 2021

Conversation

adobke
Copy link
Contributor

@adobke adobke commented Sep 15, 2021

The goroot is encoded in the output binary, but the goroot is different
in different workspaces. As a result tool binaries (such as builder) are
not reproducible. Bazel will have cache misses for build involving
builder even if the resulting output from the build is reproducible.

Similar to go_binary builds, suppress the goroot path in go_tool_binary
output so that the result in reproducible.

What type of PR is this?

Bug fix

What does this PR do? Why is it needed?
Like go_binary output, ensure go_tool_binary output is reproducible

Which issues(s) does this PR fix?

Fixes #2951

@google-cla google-cla bot added the cla: yes label Sep 15, 2021
@adobke adobke force-pushed the reproducible_builder branch from a5c4191 to 6b9b36c Compare September 15, 2021 03:36
@sluongng
Copy link
Contributor

Simple fix, simple test. This change LGTM.

Very interesting find! I am very curious on how you came about and got down to troubleshooting this issue.
I tried to reproduce the issue and diffing the hex dump of the 2 builder binaries but couldnt make much sense of it.

There is a missing link somewhere between the go_sdk being encoded inside the binary and the GOROOT_FINAL being set.

I know the GOROOT string value was picked to be consistent with the go_binary rule, but could you confirm that the value does not matter here as long as it's a fixed non-empty string?

Thanks!

Copy link
Contributor

@steeve steeve left a comment

Choose a reason for hiding this comment

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

Thank you! Minor nitpick regarding gofmt

tests/integration/reproducibility/reproducibility_test.go Outdated Show resolved Hide resolved
The goroot is encoded in the output binary, but the goroot is different
in different workspaces. As a result tool binaries (such as builder) are
not reproducible. Bazel will have cache misses for build involving
builder even if the resulting output from the build is repoducible.

Similar go_binary builds, suppress the goroot path.
@adobke adobke force-pushed the reproducible_builder branch from 6b9b36c to c960c88 Compare September 16, 2021 05:32
@adobke
Copy link
Contributor Author

adobke commented Sep 16, 2021

Simple fix, simple test. This change LGTM.

Very interesting find! I am very curious on how you came about and got down to troubleshooting this issue.
I tried to reproduce the issue and diffing the hex dump of the 2 builder binaries but couldnt make much sense of it.

Indeed one minor change in the binary seems to end up making the hex dumps totally diverge. The tool that was useful here was to compare the output of strings on the each binary which revealed the embedded and different paths.

There is a missing link somewhere between the go_sdk being encoded inside the binary and the GOROOT_FINAL being set.

I know the GOROOT string value was picked to be consistent with the go_binary rule, but could you confirm that the value does not matter here as long as it's a fixed non-empty string?

Confirmed, changing to a random string seemed to make not difference (e.g. all tests pass)

@adobke adobke requested a review from steeve September 16, 2021 05:39
@steeve
Copy link
Contributor

steeve commented Sep 22, 2021

Thank you! Merging

@steeve steeve merged commit 777b972 into bazel-contrib:master Sep 22, 2021
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.

Tool binaries (like builder) are not reproducible and break the cache
3 participants