-
Notifications
You must be signed in to change notification settings - Fork 18k
x/build/cmd/release: fails for -target=linux-amd64 due to non-default GOROOT_FINAL value causing TestScript/test_race_install_cgo test to fail #39385
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
Comments
I don't have any particular insight, but I am curious to see whether this ends up being related to #33598. |
This seems to affect only the 64-bit targets, not 32-bit ones. Something that I can't explain yet is that this is affecting both linux/amd64 and linux/arm64. The former is configured with the race detector on, the latter without. Compare:
And:
TestScript documentation says "[race] is whether the race detector can be used". I'm not sure how that's determined exactly yet. |
As part of the sequence of commands in Removing The "! stale cmd/go" check was added in CL 223746 on March 18th, so this is the first time it's going through the unique way that I can reproduce locally (after removing the
|
The above is likely relevant, but not a complete explanation. |
I've asked @bcmills to help look into this, and we're investigating this now. We suspect it's some sort of difference between the way the @bcmills, I've been using --- a/cmd/release/release.go
+++ b/cmd/release/release.go
@@ -587,6 +587,7 @@ func (b *Build) make() error {
Output: execOut,
ExtraEnv: env,
Args: bc.AllScriptArgs(),
+ Debug: true,
})
if err != nil {
return err |
This looks like a bug in Minimal steps to reproduce:
|
Great find @bcmills, thank you! I'll unassign myself, leaving just you, and update this to NeedsFix, since we now know where the problem is and how to reproduce it without If the full fix needs more time, we have the option of splitting that into an okay-after-beta1 release blocker issue in the |
Well, it's not just a Lines 181 to 188 in d282b0f
That means it's not straightforward to make More work needed. 😞 |
Thanks for the analysis Bryan. Given the timing, should we factor out that problem into a separate issue, and resolve this one by reverting the "! stale cmd/cgo" check added to As I understand, this is now a known problem that we can fix after releasing Go 1.15 Beta 1, which can serve the purpose of helping us uncover unknown problems even without this fix. |
Change https://golang.org/cl/236818 mentions this issue: |
Change https://golang.org/cl/236819 mentions this issue: |
A proper fix turned out not to be terribly invasive. The comment in Fortunately, the fixes for those are straightforward: either explicitly unset |
The removed line assumed that the script's WORK directory is not a child of any directory containing version-control metadata. While that assumption does hold in most cases, it does not hold when, for example, $TMPDIR is $HOME/tmp and $HOME/.git/config exists. A similar situation may or may not arise when using golang.org/x/build/cmd/release. Either way, the assertion is incorrect and was interfering with local testing for #39385. Updates #39385 Fixes #39431 Change-Id: I67813d7ce455aa9b56a6eace6eddebf48d0f7fa6 Reviewed-on: https://go-review.googlesource.com/c/go/+/236818 Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Change https://golang.org/cl/237359 mentions this issue: |
This removes the same logic from run.bat that was removed from cmd/dist in CL 236819. The duplicated logic was removed from run.bash and run.rc in CL 6531, but that part of run.bat was apparently missed (and not noticed because its effect was redundant). Also fix a path-separator bug in cmd/addr2line.TestAddr2Line that was exposed as a result. Fixes #39478 Updates #39385 Change-Id: I00054966cf92ef92a03681bf23de7f45f46fbb5e Reviewed-on: https://go-review.googlesource.com/c/go/+/237359 Run-TryBot: Bryan C. Mills <bcmills@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
Edit: This is an issue for a specific test. See #39386 for the high level tracking issue.
This issue requires investigation. The problem is either in how the
release
command sets up the environment and executes commands on the builder, or in the builder definition, or in the test, or some combination. Depending on what we learn, we may end up deciding this is okay to resolve after beta1 (but before the final release) and skip this test failure in order to make the Go 1.15 beta 1 available for testing and finding unknown issues sooner.This can be reproduced (reliably) if your account has permissions needed to run
releasebot
(they are documented at https://github.com/golang/build/tree/master/cmd/releasebot#permissions) with:Complete Output
(The release command creates a tarball and places it in a staging directory; it doesn't publish anything, so it is safe to run with
-version=go1.15beta1
flag.)/cc @golang/osp-team @bcmills @jayconrod @matloob
The text was updated successfully, but these errors were encountered: