Skip to content

cmd/go: use the correct linker config in the buildID hash #40296

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 5 commits into from
Closed

cmd/go: use the correct linker config in the buildID hash #40296

wants to merge 5 commits into from

Conversation

lxop
Copy link
Contributor

@lxop lxop commented Jul 20, 2020

The linker config is hashed into the buildID; however,
the GOROOT_FINAL environment variable that is
actually used when -trimpath is specified was not
reflected in that hash. This change fixes that.

Fixes #38989

@googlebot googlebot added the cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change. label Jul 20, 2020
@gopherbot
Copy link
Contributor

This PR (HEAD: db76934) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/243557 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 1:

Congratulations on opening your first change. Thank you for your contribution!

Next steps:
A maintainer will review your change and provide feedback. See
https://golang.org/doc/contribute.html#review for more info and tips to get your
patch through code review.

Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.

During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11 or adds a tag like "wait-release", it means that this CL will be
reviewed as part of the next development cycle. See https://golang.org/s/release
for more details.

Please don’t reply on this GitHub thread. Visit golang.org/cl/243557.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Jay Conrod:

Patch Set 1: Run-TryBot+1

(3 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/243557.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 1:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=ac9b8cda

Please don’t reply on this GitHub thread. Visit golang.org/cl/243557.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 1: TryBot-Result+1

TryBots are happy.

Please don’t reply on this GitHub thread. Visit golang.org/cl/243557.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR (HEAD: 026e2aa) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/243557 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Jay Conrod:

Patch Set 2: Run-TryBot+1

(4 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/243557.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 2:

SlowBots beginning. Status page: https://farmer.golang.org/try?commit=191265a9

Please don’t reply on this GitHub thread. Visit golang.org/cl/243557.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 2: TryBot-Result+1

SlowBots are happy.
SlowBot builds that ran:

  • linux-amd64-longtest
  • windows-amd64-longtest

Please don’t reply on this GitHub thread. Visit golang.org/cl/243557.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Jay Conrod:

Patch Set 2: Code-Review+2

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/243557.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Alex Opie:

Patch Set 2:

(4 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/243557.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Alex Opie:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/243557.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Jay Conrod:

Patch Set 2: Trust+1

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/243557.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Bryan C. Mills:

Patch Set 2: Trust+1

Please don’t reply on this GitHub thread. Visit golang.org/cl/243557.
After addressing review feedback, remember to publish your drafts!

lxop added 5 commits September 17, 2020 16:18
@lxop
add test to confirm that builds are reproducible
fa414f8 
This test builds the same source twice, from two
different GOROOT paths, and checks that both
results share the same BuildID string to confirm
that the binaries are identical.

Updates: 38989
@lxop
use the actual linker config in the buildID hash
8923af0 
The linker config is hashed into the buildID; however,
the GOROOT_FINAL environment variable that is
actually used when -trimpath is specified was not
reflected in that hash. This change fixes that.

Fixes: 38989
@lxop
only run the test if symlinks are supported
4d38b15 
The test makes use of symlinks, so should be skipped
if the environment doesn't support them.
@lxop
run the test with modules enabled
cf7ef76 
There's no reason they should be disabled
@lxop
describe the test's purpose
4cf8292
@lxop lxop force-pushed the 38989-match-linker-config branch from 026e2aa to 4cf8292 Compare September 17, 2020 04:30
@gopherbot
Copy link
Contributor

This PR (HEAD: 4cf8292) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/243557 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Jay Conrod:

Patch Set 3: Run-TryBot+1

Please don’t reply on this GitHub thread. Visit golang.org/cl/243557.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 3:

SlowBots beginning. Status page: https://farmer.golang.org/try?commit=da863b89

Please don’t reply on this GitHub thread. Visit golang.org/cl/243557.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 3:

Build is still in progress...
This change failed on linux-amd64-longtest:
See https://storage.googleapis.com/go-build-log/da863b89/linux-amd64-longtest_dd960ac2.log

Other builds still in progress; subsequent failure notices suppressed until final report. Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.

Please don’t reply on this GitHub thread. Visit golang.org/cl/243557.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 3: TryBot-Result-1

1 of 22 SlowBots failed:
Failed on linux-amd64-longtest: https://storage.googleapis.com/go-build-log/da863b89/linux-amd64-longtest_dd960ac2.log

Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.SlowBot builds that ran:

  • linux-amd64-longtest
  • windows-amd64-longtest

Please don’t reply on this GitHub thread. Visit golang.org/cl/243557.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR is being closed because golang.org/cl/243557 has been merged.

@gopherbot gopherbot closed this Sep 17, 2020
gopherbot pushed a commit that referenced this pull request Sep 17, 2020
@lxop
cmd/go: use the correct linker config in the buildID hash
0f7ac9b 
The linker config is hashed into the buildID; however,
the GOROOT_FINAL environment variable that is
actually used when -trimpath is specified was not
reflected in that hash. This change fixes that.

Fixes #38989

Change-Id: I418a21a9f6293ca63c101d22b501dfdba8e91ac6
GitHub-Last-Rev: 4cf8292
GitHub-Pull-Request: #40296
Reviewed-on: https://go-review.googlesource.com/c/go/+/243557
Run-TryBot: Jay Conrod <[email protected]>
Reviewed-by: Jay Conrod <[email protected]>
Trust: Jay Conrod <[email protected]>
Trust: Bryan C. Mills <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

cmd/link: Mismatch between hashed and actual linker config prevents reproducible builds
3 participants
@lxop @gopherbot @googlebot