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

x/build/cmd/release: test/winbatch.go test fails for -target=darwin-amd64 #39391

Closed
dmitshur opened this issue Jun 3, 2020 · 4 comments
Closed

x/build/cmd/release: test/winbatch.go test fails for -target=darwin-amd64 #39391

dmitshur opened this issue Jun 3, 2020 · 4 comments
Assignees
Milestone

Comments

@dmitshur
Copy link
Member

@dmitshur dmitshur commented Jun 3, 2020

This is an issue for a specific test. See #39386 for the high level tracking issue.

This can be reproduced if your account has permissions needed to run releasebot (documented here) with:

$ release -target=darwin-amd64 -version=go1.15beta1 -watch -rev=09e791feb1dfbfc05578a17cd9c035ef82af4033
[...]

##### ../test
# go run run.go -- winbatch.go
exit status 1
Windows batch file /private/var/folders/kh/5zzynz152r94t18yzstnrwx80000gn/T/workdir-host-darwin-10_15/go/src/all.bat does not contain CRLF line termination.
Try running git checkout src/*.bat to fix this.

FAIL	winbatch.go	3.709s
2020/06/03 15:12:55 Failed: exit status 1

[...]
FAILED

/cc @toothrot @cagedmantis @andybons @golang/osp-team

@dmitshur
Copy link
Member Author

@dmitshur dmitshur commented Jun 4, 2020

This test was added in CL 96495 for issue #37791.

@rasky, is it sufficient for the test to run on GOOS=windows only, and skip for other GOOSes?

/cc @alexbrainman

@dmitshur
Copy link
Member Author

@dmitshur dmitshur commented Jun 4, 2020

Ok, I think I have a pretty good sense of what's going on.

The fix implemented in in CL 96495 isn't having an effect on the *.bat files, and the test is correctly catching a problem.

The reason for this is related to how release pushes the source to a buildlet:

// Push source to buildlet.
b.logf("Pushing source to buildlet.")
...
tar := "https://go.googlesource.com/go/+archive/" + *rev + ".tar.gz"
if err := client.PutTarFromURL(ctx, tar, goDir); err != nil {
	b.logf("failed to put tarball %q into dir %q: %v", tar, goDir, err)
	return err
}

This came up as a question in #37791:

Will the Gerrit *.tar.gz serving endpoint respect this gitattributes file? I'm not sure.

What kind of test do we want? How can we verify Gerrit *.tar.gz?

The answer is by downloading a .tar.gz from a URL as shown above. For example:

https://go.googlesource.com/go/+archive/bdf76ca045c989bcff9a0111819559d632a4a39a.tar.gz

The all.bat file in it has Unix LF endings and the winbatch.go test reports a problem.

Another place to place a test for this could be in release, but it doesn't seem to be necessary since the current test works.

@dmitshur dmitshur added NeedsFix and removed NeedsInvestigation labels Jun 4, 2020
@dmitshur
Copy link
Member Author

@dmitshur dmitshur commented Jun 4, 2020

Sent CL 236437 which fixes this issue.

@gopherbot
Copy link

@gopherbot gopherbot commented Jun 4, 2020

Change https://golang.org/cl/236437 mentions this issue: all: treat all files as binary, but check in .bat with CRLF

@gopherbot gopherbot closed this in ee379d2 Jun 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.