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.

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

Full stdlib file paths in binary #969

Closed
ash2k opened this issue Oct 28, 2017 · 1 comment · Fixed by #3984
Closed

Full stdlib file paths in binary #969

ash2k opened this issue Oct 28, 2017 · 1 comment · Fixed by #3984

Comments

@ash2k
Copy link
Contributor

ash2k commented Oct 28, 2017

Seems like paths to standard library packages are put into the binary. That means builds are not reproducible, right?

==================
WARNING: DATA RACE
Write at 0x00c4205d00cc by main goroutine:
  internal/race.Write()
      /private/var/tmp/_bazel_ash2k/ea451d98a8bdaad3872b7aa995bea02c/bazel-sandbox/1936151865359060414/execroot/__main__/bazel-out/host/bin/external/go_stdlib_darwin_amd64_cgo_race/src/internal/race/race.go:41 +0x38
  sync.(*WaitGroup).Wait()
      /private/var/tmp/_bazel_ash2k/ea451d98a8bdaad3872b7aa995bea02c/bazel-sandbox/1936151865359060414/execroot/__main__/bazel-out/host/bin/external/go_stdlib_darwin_amd64_cgo_race/src/sync/waitgroup.go:129 +0xf4
  github.com/ash2k/stager/wait.(*Group).Wait()
      vendor/github.com/ash2k/stager/wait/group.go:14 +0x3a
  github.com/atlassian/smith/pkg/controller.(*BundleController).Run()
      pkg/controller/controller.go:104 +0x51f
  github.com/atlassian/smith/cmd/smith/app.(*App).Run()
      cmd/smith/app/app.go:148 +0x169f
  main.runWithContext()
      cmd/smith/main.go:81 +0x417
  main.run()
      cmd/smith/main.go:36 +0xd9
  main.main()
      cmd/smith/main.go:26 +0x33

Previous read at 0x00c4205d00cc by goroutine 115:
  internal/race.Read()
      /private/var/tmp/_bazel_ash2k/ea451d98a8bdaad3872b7aa995bea02c/bazel-sandbox/1936151865359060414/execroot/__main__/bazel-out/host/bin/external/go_stdlib_darwin_amd64_cgo_race/src/internal/race/race.go:37 +0x38
  sync.(*WaitGroup).Add()
      /private/var/tmp/_bazel_ash2k/ea451d98a8bdaad3872b7aa995bea02c/bazel-sandbox/1936151865359060414/execroot/__main__/bazel-out/host/bin/external/go_stdlib_darwin_amd64_cgo_race/src/sync/waitgroup.go:71 +0x16f
  github.com/ash2k/stager/wait.(*Group).Start()
      vendor/github.com/ash2k/stager/wait/group.go:35 +0x43
  github.com/ash2k/stager/wait.(*Group).StartWithChannel()
      vendor/github.com/ash2k/stager/wait/group.go:20 +0xd0
  github.com/atlassian/smith/pkg/controller.(*crdEventHandler).ensureWatch()
      pkg/controller/controller_crd_event_handler.go:111 +0xaaf
  github.com/atlassian/smith/pkg/controller.(*crdEventHandler).OnAdd()
      pkg/controller/controller_crd_event_handler.go:36 +0x57
  k8s.io/client-go/tools/cache.(*processorListener).run()
      vendor/k8s.io/client-go/tools/cache/shared_informer.go:545 +0x3b2
  k8s.io/client-go/tools/cache.(*processorListener).(k8s.io/client-go/tools/cache.run)-fm()
      vendor/k8s.io/client-go/tools/cache/shared_informer.go:381 +0x41
  k8s.io/apimachinery/pkg/util/wait.(*Group).Start.func1()
      vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:71 +0x5c

Goroutine 115 (running) created at:
  k8s.io/apimachinery/pkg/util/wait.(*Group).Start()
      vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:69 +0x6f
  k8s.io/client-go/tools/cache.(*sharedProcessor).addAndStartListener()
      vendor/k8s.io/client-go/tools/cache/shared_informer.go:381 +0x2dd
  k8s.io/client-go/tools/cache.(*sharedIndexInformer).AddEventHandlerWithResyncPeriod()
      vendor/k8s.io/client-go/tools/cache/shared_informer.go:331 +0x281
  k8s.io/client-go/tools/cache.(*sharedIndexInformer).AddEventHandler()
      vendor/k8s.io/client-go/tools/cache/shared_informer.go:267 +0x67
  github.com/atlassian/smith/pkg/controller.(*BundleController).Run()
      pkg/controller/controller.go:89 +0x2e3
  github.com/atlassian/smith/cmd/smith/app.(*App).Run()
      cmd/smith/app/app.go:148 +0x169f
  main.runWithContext()
      cmd/smith/main.go:81 +0x417
  main.run()
      cmd/smith/main.go:36 +0xd9
  main.main()
      cmd/smith/main.go:26 +0x33
==================
Found 1 data race(s)
@ash2k ash2k changed the title Full file paths in binary Full stdlib file paths in binary Oct 28, 2017
jayconrod pushed a commit to jayconrod/rules_go that referenced this issue Oct 30, 2017
Strip stdlib paths from binaries
9c6ff18 
Set GOROOT_FINAL to a constant string ('GOROOT') when
linking. Normally, the actual path to GOROOT is set in linked
binaries, which results in non-reproducible builds. GOROOT_FINAL
provides a constant replacement.

Also, added a test for common reproducibility problems.

Fixes bazel-contrib#969
jayconrod pushed a commit to jayconrod/rules_go that referenced this issue Oct 30, 2017
Strip stdlib paths from binaries
3bba90a 
Set GOROOT_FINAL to a constant string ('GOROOT') when
linking. Normally, the actual path to GOROOT is set in linked
binaries, which results in non-reproducible builds. GOROOT_FINAL
provides a constant replacement.

Also, added a test for common reproducibility problems.

Fixes bazel-contrib#969
@jayconrod jayconrod mentioned this issue Oct 30, 2017
Merged
jayconrod added a commit that referenced this issue Nov 1, 2017
@jayconrod
Strip stdlib paths from binaries (#971)
a931a1e 
Set GOROOT_FINAL to a constant string ('GOROOT') when
linking. Normally, the actual path to GOROOT is set in linked
binaries, which results in non-reproducible builds. GOROOT_FINAL
provides a constant replacement.

Also, added a test for common reproducibility problems.

Fixes #969
@JacobOaks
Copy link
Contributor

JacobOaks commented Jul 8, 2024 •
edited
Loading

FYI I was debugging why our test wrapper that @linzhp posted in #2370 stopped setting up go environment variables correctly in Go 1.23rc1, and realized that GOROOT was no longer set to "GOROOT" within our tests, instead being set to an actual sdk within bazel sandbox. Seems like golang/go#62047 may have caused this issue to come back. Not sure if there's a good replacement solution or not yet cc: @jayconrod

JacobOaks added a commit to JacobOaks/rules_go that referenced this issue Jul 8, 2024
@JacobOaks
Clear GOROOT when linking
a86b2da 
Due to golang/go#62047, Go 1.23 won't support `GOROOT_FINAL`.
This means that bazel-contrib#971 will no longer fix bazel-contrib#969,
meaning linked binaries will contain full GOROOT paths, making them non-reproducible.

Instead of using `GOROOT_FINAL`, this change sets `GOROOT` when invoking the linker,
which will cause the linker to continue to write `GOROOT` into the binary,
keeping builds consistent.

This works on Go 1.23rc1 as well.

I'm not a rules_go expert and I'm not married to this particular solution,
I just wanted to bring attention to the issue.
If there's a clever way we can set `-trimpath` when invoking the compiler,
that might be better - but we already use that flag to trim off the bazel sandbox I believe.
@JacobOaks JacobOaks mentioned this issue Jul 8, 2024
Merged
fmeum pushed a commit that referenced this issue Jul 16, 2024
@JacobOaks
Break reliance on GOROOT_FINAL (#3984)
1a32e03 
* Clear GOROOT when linking

Due to golang/go#62047, Go 1.23 won't support `GOROOT_FINAL`.
This means that #971 will no longer fix #969,
meaning linked binaries will contain full GOROOT paths, making them non-reproducible.

Instead of using `GOROOT_FINAL`, this change sets `GOROOT` when invoking the linker,
which will cause the linker to continue to write `GOROOT` into the binary,
keeping builds consistent.

This works on Go 1.23rc1 as well.

I'm not a rules_go expert and I'm not married to this particular solution,
I just wanted to bring attention to the issue.
If there's a clever way we can set `-trimpath` when invoking the compiler,
that might be better - but we already use that flag to trim off the bazel sandbox I believe.

* switch based on runtime.Version
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Break reliance on GOROOT_FINAL JacobOaks/rules_go
2 participants
@ash2k @JacobOaks