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/link/internal/ld: testcarchive fails on Windows with MinGW: doesn't handle "\r" in output #68121

Closed
dagood opened this issue Jun 21, 2024 · 5 comments

Comments

@dagood
Copy link
Contributor

dagood commented Jun 21, 2024

Go version

go version go1.23rc1 windows/amd64

Output of go env in your module/workspace:

set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\dagood\AppData\Local\go-build
set GOENV=C:\Users\dagood\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\dagood\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\dagood\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:\Users\dagood\sdk\go1.23rc1
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLCHAIN=local
set GOTOOLDIR=C:\Users\dagood\sdk\go1.23rc1\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.23rc1
set GODEBUG=
set GOTELEMETRY=local
set GOTELEMETRYDIR=C:\Users\dagood\AppData\Roaming\go\telemetry
set GCCGO=gccgo
set GOAMD64=v1
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=NUL
set GOWORK=
set CGO_CFLAGS=-O2 -g
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-O2 -g
set CGO_FFLAGS=-O2 -g
set CGO_LDFLAGS=-O2 -g
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=C:\Users\dagood\AppData\Local\Temp\go-build2043855877=/tmp/go-build -gno-record-gcc-switches

What did you do?

go1.23rc1 test cmd/cgo/internal/testcarchive -run TestInstall -v

Also repros with go version devel go1.23-0af2148fdc Fri Jun 21 21:01:30 2024 +0000 windows/amd64 (latest master, 0af2148)

What did you see happen?

=== RUN   TestInstall
    carchive_test.go:487: [go tool cgo -objdir C:\Users\dagood\AppData\Local\Temp\carchive_test2892176246\_obj2451795512 -exportheader p.h p\p.go]
    carchive_test.go:489: [go install -buildmode=c-archive ./libgo]
    carchive_test.go:489: # testcarchive/libgo
        C:\Users\dagood\sdk\go1.23rc1\pkg\tool\windows_amd64\link.exe: running C:/Program Files/mingw-w64/x86_64-11.2.0-release-win32-seh-rt_v9-rev1/mingw64/bin failed: exec: "C:/Program Files/mingw-w64/x86_64-11.2.0-release-win32-seh-rt_v9-rev1/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/11.2.0/../../../../x86_64-w64-mingw32/bin/ar.exe\r": file does not exist

    carchive_test.go:489: exit status 1
--- FAIL: TestInstall (4.66s)
FAIL
FAIL    cmd/cgo/internal/testcarchive   5.423s
FAIL

Note: bin/ar.exe\r": file does not exist.

I'm not sure if all builds of MinGW use \r\n in their output, but I confirmed that a few other versions also fail in the same way:

func (ctxt *Link) findExtLinkTool(toolname string) string {
var cc []string
cc = append(cc, ctxt.extld()...)
cc = append(cc, hostlinkArchArgs(ctxt.Arch)...)
cc = append(cc, "--print-prog-name", toolname)
out, err := exec.Command(cc[0], cc[1:]...).CombinedOutput()
if err != nil {
Exitf("%s: finding %s failed: %v\n%s", os.Args[0], toolname, err, out)
}
cmdpath := strings.TrimSuffix(string(out), "\n")
return cmdpath
}

Changing the strings.TrimSuffix to strings.TrimRight(string(out), "\r\n") fixes the test. I'm planning to send a CL but need to dust off the contrib tools. 🙂 Submitted https://go-review.googlesource.com/c/go/+/593916 with this change.

What did you expect to see?

Success.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/593916 mentions this issue: cmd/link/internal/ld: handle "\r" in MinGW "--print-prog-name" output

@dagood dagood changed the title cmd/cgo/internal/testcarchive: tests fail on Windows with MinGW: doesn't handle "\r" in output cmd/link/internal/ld: testcarchive fails on Windows with MinGW: doesn't handle "\r" in output Jun 21, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/594855 mentions this issue: cmd/link/internal/ld: handle "\r" in MinGW "--print-prog-name" output

@dagood
Copy link
Contributor Author

dagood commented Jun 25, 2024

go version go1.23rc1 windows/amd64

I realized that a cherry-pick is necessary to fix this in 1.23, so took a look at https://go.dev/wiki/Go-Release-Cycle#timeline, and I think what I need to do is open a cherry-pick CL and reopen this issue because it still applies to 1.23.

@dagood dagood reopened this Jun 25, 2024
@cherrymui
Copy link
Member

@dagood a cherry-pick may not be necessary. The main branch is not opened for general development, so everything in the main branch should be still included in Go 1.23. We may just do a merge to release branch, instead of cherry-picking each CL. Thanks.

Mchnan pushed a commit to Mchnan/go-sylixos that referenced this issue Jul 9, 2024
Fix the "gcc --print-prog-name" output parser to handle "\r\n", not only
"\n". The MinGW compiler on Windows uses "\r\n" as line endings, causing
the existing parser to create paths like
".../x86_64-w64-mingw32/bin/ar.exe\r", which is not correct. By trimming
the "\r\n" cutset, both types of line endings are handled correctly.

Fixes golang#68121

Change-Id: I04b8bf9b6a5b29a1e59a6aa07fa4faa4c5bdeee6
Reviewed-on: https://go-review.googlesource.com/c/go/+/593916
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Commit-Queue: Ian Lance Taylor <iant@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants