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

cmd/go: TestScript/version_buildvcs_git_gpg (if enabled) fails on linux longtest builders #57034

Closed
cherrymui opened this issue Dec 1, 2022 · 9 comments
Assignees
Labels
FrozenDueToAge GoCommand cmd/go NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone

Comments

@cherrymui
Copy link
Member

Split from #49649 . Filing a new issue to investigate/decide.

After setting up the linux-arm64-longtest builder, we saw cmd/go's TestScript/version_buildvcs_git_gpg fails.
https://build.golang.org/log/7b3bdfcc706b20aeb4deb0a6b7fb6c73d75670d9

On 386 and amd64, linux-386-longtest and linux-amd64-longtest builders didn't actually have gpg command installed, so the test was actually skipped. @cagedmantis 's CL installs the command. And after that, the test fails on those builders, too. https://build.golang.org/log/30e41a5b422d7ca512aa244816e1ee578dfa9285
So we currently rolled back the builder update.

Interestingly, the test passes under SSH, but fails in the post-submit run and under gomote run. From #49649 (comment), it seems TMPDIR or some other environment variable settings may cause the difference.

cc @bcmills

@bcmills bcmills added Testing An issue that has been verified to require only test changes, not just a test failure. NeedsFix The path to resolution is known, but the work has not been done. GoCommand cmd/go labels Dec 2, 2022
@bcmills bcmills self-assigned this Dec 2, 2022
@bcmills bcmills modified the milestones: Go1.21, Go1.20 Dec 2, 2022
@bcmills
Copy link
Contributor

bcmills commented Dec 2, 2022

@gopherbot, please backport to Go 1.19 and 1.18: this test may fail spuriously due to problems with a fragile third-party tool.

@gopherbot
Copy link

Backport issue(s) opened: #57054 (for 1.18), #57055 (for 1.19).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

@gopherbot
Copy link

Change https://go.dev/cl/454502 mentions this issue: cmd/go: remove TestScript/version_buildvcs_git_gpg

@gopherbot
Copy link

Change https://go.dev/cl/454955 mentions this issue: [release-branch.go1.19] cmd/go: remove TestScript/version_buildvcs_git_gpg

@gopherbot
Copy link

Change https://go.dev/cl/454956 mentions this issue: [release-branch.go1.18] cmd/go: remove TestScript/version_buildvcs_git_gpg

@cagedmantis
Copy link
Contributor

I will update the linux-amd64-longtest and linux-386-longtest builder images to include gpg-agent and gpg next week.

@bcmills
Copy link
Contributor

bcmills commented Dec 6, 2022

@cagedmantis, I don't think you need to bother. I don't plan to try to restore the test — the complexity of the workarounds for the fragile socket behavior does not seem worth it for the incremental value of the test.

@cagedmantis
Copy link
Contributor

@bcmills I will remove gpg and gpg-agent from the images then.

@gopherbot
Copy link

Change https://go.dev/cl/455860 mentions this issue: env/linux-arm64-bullseye: remove gpg from container images

gopherbot pushed a commit to golang/build that referenced this issue Dec 8, 2022
The tests which require gpg and gpg-agent have been removed. This
change removes them from the container images.

For golang/go#57034

Change-Id: Id33dbc9881bb9a688955612a66f1f549822f3866
Reviewed-on: https://go-review.googlesource.com/c/build/+/455860
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Carlos Amedee <carlos@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
gopherbot pushed a commit that referenced this issue Dec 9, 2022
…t_gpg

This was a regression test added for a 'git' command line
used for build stamping. Unfortunately, 'gpg' has proved to
be extremely fragile:

* In recent versions, it appears to always require 'gpg-agent' to be
  installed for anything involving secret keys, but for some reason is
  not normally marked as requiring gpg-agent in Debian's package
  manager.

* It tries to create a Unix domain socket in a subdirectory of $TMPDIR
  without checking the path length, which fails when $TMPDIR is too
  long to fit in the 'sun_path' field of a sockaddr_un struct (which
  typically tops out somewhere between 92 and 108 bytes).

We could theoretically address those by artificially reducing the
script's TMPDIR length and checking for gpg-agent in addition to gpg,
but arguably those should both be fixed upstream instead. On balance,
the incremental value that this test provides does not seem worth the
complexity of dealing with such a fragile third-party tool.

Updates #50675.
Updates #48802.
Updates #57034.
Fixes #57054.

Change-Id: Ia3288c2f84f8db86ddfa139b4d1c0112d67079ef
Reviewed-on: https://go-review.googlesource.com/c/go/+/454502
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
(cherry picked from commit 45f5ef4)
Reviewed-on: https://go-review.googlesource.com/c/go/+/454956
gopherbot pushed a commit that referenced this issue Dec 9, 2022
…t_gpg

This was a regression test added for a 'git' command line
used for build stamping. Unfortunately, 'gpg' has proved to
be extremely fragile:

* In recent versions, it appears to always require 'gpg-agent' to be
  installed for anything involving secret keys, but for some reason is
  not normally marked as requiring gpg-agent in Debian's package
  manager.

* It tries to create a Unix domain socket in a subdirectory of $TMPDIR
  without checking the path length, which fails when $TMPDIR is too
  long to fit in the 'sun_path' field of a sockaddr_un struct (which
  typically tops out somewhere between 92 and 108 bytes).

We could theoretically address those by artificially reducing the
script's TMPDIR length and checking for gpg-agent in addition to gpg,
but arguably those should both be fixed upstream instead. On balance,
the incremental value that this test provides does not seem worth the
complexity of dealing with such a fragile third-party tool.

Updates #50675.
Updates #48802.
Updates #57034.
Fixes #57055.

Change-Id: Ia3288c2f84f8db86ddfa139b4d1c0112d67079ef
Reviewed-on: https://go-review.googlesource.com/c/go/+/454502
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
(cherry picked from commit 45f5ef4)
Reviewed-on: https://go-review.googlesource.com/c/go/+/454955
@golang golang locked and limited conversation to collaborators Dec 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge GoCommand cmd/go NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Development

No branches or pull requests

4 participants