Navigation Menu

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

Jump to bottom

cmd/go: TestScript/build_acl_windows fails on windows-arm builder #30711

Closed
alexbrainman opened this issue Mar 10, 2019 · 5 comments
Closed

cmd/go: TestScript/build_acl_windows fails on windows-arm builder #30711

alexbrainman opened this issue Mar 10, 2019 · 5 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone
Go1.13

Comments

@alexbrainman
Copy link
Member

windows-arm builders seems to be broken

https://build.golang.org/log/2579a41ab2b6bafce55876919188aa9933e746d2

with

--- FAIL: TestScript (0.08s)
    --- FAIL: TestScript/build_acl_windows (12.53s)
        script_test.go:189: 
            # Create $WORK\guest and give the Guests group full access.
            # Files created within that directory will have different security attributes by default. (3.085s)
            # Build a binary using the guest directory as an intermediate (6.094s)
            # Build the same binary, but write it to the guest directory. (3.162s)
            # Read ACLs for the files. (0.072s)
            > exec powershell -Command 'Get-Acl main.exe | Select -expand AccessToString'
            [stderr]
            S�t�a�r�t�i�n�g� �t�h�e� �C�L�R� �f�a�i�l�e�d� �w�i�t�h� �H�R�E�S�U�L�T� �7�e�.�
�
            �[exit status -65536]
            FAIL: testdata\script\build_acl_windows.txt:20: unexpected command failure
            
FAIL
FAIL	cmd/go	918.021s

Probably since https://go-review.googlesource.com/c/go/+/165752 was submitted.

The error message is not very helpful. And I do not have windows-arm computer to debug. @jordanrh1 can you, please, help to debug this? Thank you.

Alternatively, maybe we could revert CL 165752 ? Leaving for @bcmills to decide.

Alex
@ALTree ALTree added this to the Go1.13 milestone Mar 10, 2019
@ALTree ALTree added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 10, 2019
@bcmills
Copy link
Contributor

bcmills commented Mar 11, 2019

The difference is probably due to the more restricted environment for script tests. The setup for that environment is here:

go/src/cmd/go/script_test.go

Lines 101 to 132 in fd080ea

ts.env = []string{
"WORK=" + ts.workdir, // must be first for ts.abbrev
"PATH=" + testBin + string(filepath.ListSeparator) + os.Getenv("PATH"),
homeEnvName() + "=/no-home",
"CCACHE_DISABLE=1", // ccache breaks with non-existent HOME
"GOARCH=" + runtime.GOARCH,
"GOCACHE=" + testGOCACHE,
"GOOS=" + runtime.GOOS,
"GOPATH=" + filepath.Join(ts.workdir, "gopath"),
"GOPROXY=" + proxyURL,
"GOROOT=" + testGOROOT,
"GONOVERIFY=*",
tempEnvName() + "=" + filepath.Join(ts.workdir, "tmp"),
"devnull=" + os.DevNull,
"goversion=" + goVersion(ts),
":=" + string(os.PathListSeparator),
}
if runtime.GOOS == "plan9" {
ts.env = append(ts.env, "path="+testBin+string(filepath.ListSeparator)+os.Getenv("path"))
}
if runtime.GOOS == "windows" {
ts.env = append(ts.env, "exe=.exe")
} else {
ts.env = append(ts.env, "exe=")
}
for _, key := range extraEnvKeys {
if val := os.Getenv(key); val != "" {
ts.env = append(ts.env, key+"="+val)
}
}

Given the generally shaky state of the windows-arm builder, I'm inclined to add a skip until the test is fixed rather than roll back the change.

@bcmills
Copy link
Contributor

bcmills commented Mar 11, 2019

Looks like exec.LookPath uses the PATHEXT variable on Windows. We should probably preserve that.

go/src/os/exec/lp_windows.go

Line 61 in fd080ea

x := os.Getenv(`PATHEXT`)

@alexbrainman
Copy link
Member Author

The difference is probably due to the more restricted environment for script tests.

I agree, this could well be the problem here. It just we don't know which environment variable must not be skipped.

Looks like exec.LookPath uses the PATHEXT variable on Windows. We should probably preserve that.

I don't see what you you mean by that. script_test.go uses standard os/exec package to implement everything - it should just work. No?

Alex

@bcmills bcmills added the Testing An issue that has been verified to require only test changes, not just a test failure. label Mar 14, 2019
@gopherbot
Copy link

Change https://golang.org/cl/167778 mentions this issue: cmd/go: skip broken TestScript/build_acl_windows on arm

gopherbot pushed a commit that referenced this issue Mar 15, 2019
@alexbrainman
cmd/go: skip broken TestScript/build_acl_windows on arm
7765576 
Updates #30711

Change-Id: I280f7effaf488d5d9908d9d0cd1e0e99c22f91ca
Reviewed-on: https://go-review.googlesource.com/c/go/+/167778
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@gopherbot
Copy link

Change https://golang.org/cl/168859 mentions this issue: cmd/go: keep WINDIR during TestScript

@alexbrainman alexbrainman mentioned this issue Mar 26, 2019
Open
@golang golang locked and limited conversation to collaborators Mar 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Milestone
Go1.13
Development

No branches or pull requests
4 participants
@ALTree @bcmills @gopherbot @alexbrainman