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: TestScripts/svn fails on Windows #56555

Closed
qmuntal opened this issue Nov 3, 2022 · 6 comments
Closed

cmd/go: TestScripts/svn fails on Windows #56555

qmuntal opened this issue Nov 3, 2022 · 6 comments
Labels
FrozenDueToAge GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows
Milestone
Go1.20

Comments

@qmuntal
Copy link
Contributor

qmuntal commented Nov 3, 2022

What version of Go are you using (go version)?

$ go version
go version devel go1.20-651437e04e Mon Oct 17 19:38:03 2022 +0200 windows/amd64

Does this issue reproduce with the latest release?

Tip only

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\qmuntaldiaz\AppData\Local\go-build
set GOENV=C:\Users\qmuntaldiaz\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\qmuntaldiaz\go\pkg\mod
set GONOPROXY=github.com/microsoft/*
set GONOSUMDB=github.com/microsoft/*
set GOOS=windows
set GOPATH=C:\Users\qmuntaldiaz\go
set GOPRIVATE=github.com/microsoft/*
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:\Users\qmuntaldiaz\code\golang-go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=C:\Users\qmuntaldiaz\code\golang-go\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=devel go1.20-651437e04e Mon Oct 17 19:38:03 2022 +0200
set GCCGO=gccgo
set GOAMD64=v1
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=C:\Users\qmuntaldiaz\code\golang-go\src\go.mod
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 -fdebug-prefix-map=C:\Users\QMUNTA~1\AppData\Local\Temp\go-build369430030=/tmp/go-build -gno-record-gcc-switches

What did you do?

go test -v -run TestScripts/svn/hello -count 1 ./cmd/go/internal/vcweb/vcstest

What did you expect to see?

ok cmd/go/internal/vcweb/vcstest 0.155s

What did you see instead?

2022/11/03 19:37:17 serving svn on svn://127.0.0.1:50670
2022/11/03 19:37:19 serving /
--- FAIL: TestScripts (0.03s)
    --- FAIL: TestScripts/svn/hello.txt (2.20s)
        vcstest_test.go:136: 2022/11/03 19:37:19 hello.txt:
            > handle svn
            # Ensure SVN displays dates using UTC. (2.153s)
            > env TZ=''
            > mkdir db/transactions
            > mkdir db/txn-protorevs
            > chmod 0755 hooks/pre-revprop-change
            > env ROOT=$PWD
            > cd .checkout
            > svn checkout file://$ROOT .
            [stderr]
            svn: E170000: Illegal repository URL 'file://c:%5cusers%5cqmunta~1%5cappdata%5clocal%5ctemp%5ctestscripts3141470295%5c001%5csvn%5chello'


        vcstest_test.go:142: hello.txt:12: svn checkout file://C:\Users\QMUNTA~1\AppData\Local\Temp\TestScripts3141470295\001\svn\hello .: exit status 1
FAIL
FAIL    cmd/go/internal/vcweb/vcstest   5.654s
FAIL

Additional info

@bcmills
@mdempsky mdempsky added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Nov 3, 2022
@mdempsky mdempsky added this to the Go1.20 milestone Nov 3, 2022
@qmuntal
Copy link
Contributor Author

qmuntal commented Nov 3, 2022

I've identified 3 separate issues:

  • svn checkout file://$ROOT . does not work. It should be svn checkout file:///$ROOT . (notice the additional slash).
  • env TZ='' does not change the time zone on Windows, so the commit log does not have the appropriate format.
  • hooks/pre-revprop-change.bat hook file is mandatory when changing revision properties, but it is not there.

The first one is an easy fix, the second could be somehow fixable, and the third should be as easy as adding:

-- hooks/pre-revprop-change.bat --
@exit

The problem is that for a reason I don't understand, SVN doesn't like to execute bat files from within the Script framework, while it works when done from the command line.

I've spent the whole day on these without much luck. Unless someone wants to take it, I propose to skip the test on Windows for now.

@bcmills
Copy link
Contributor

bcmills commented Nov 3, 2022 •
edited

At a high level: I think I had assumed that svn and or svnserve would never be present at all on Windows — which SVN implementation do you have installed?

Probably the simplest approach would be to change (*vcweb.svnHandler).Available to always report false on Windows.

―

svn checkout file://$ROOT . does not work. It should be svn checkout file:///$ROOT . (notice the additional slash).

Agreed (specifically on Windows); compare #32456.

env TZ='' does not change the time zone on Windows, so the commit log does not have the appropriate format.

It might be possible to fix that using replace or cmpenv, or perhaps to configure svn not to print the timestamps at all if the tests aren't sensitive to them?

for a reason I don't understand, SVN doesn't like to execute bat files from within the Script framework, while it works when done from the command line.

Something to do with the PATHEXT environment variable, maybe? I don't think I included it here:
https://cs.opensource.google/go/go/+/master:src/cmd/go/internal/vcweb/script.go;l=125-135;drc=1ae93e4c201af78b000dccef0c2489bf7fb879ca

@erifan
Copy link

erifan commented Nov 4, 2022

On Linux, if the tzdata package is not installed, the test also fails. It would be good to report some prompt information if setting the time zone fails.

@erifan
Copy link

erifan commented Nov 4, 2022

Ignore my comment above, this has been fixed.

@gopherbot
Copy link

Change https://go.dev/cl/447935 mentions this issue: cmd/go: fix svn vctest on Windows and make them timezone agnostic

@qmuntal
Copy link
Contributor Author

qmuntal commented Nov 4, 2022 •
edited

At a high level: I think I had assumed that svn and or svnserve would never be present at all on Windows.

Microsoft internal builders come with SVN pre-installed, probably for historical reasons.

which SVN implementation do you have installed?

Tried with these: SlikSvn v1.14.2 and Win32Svn v1.8.15.

It might be possible to fix that using replace or cmpenv, or perhaps to configure svn not to print the timestamps at all if the tests aren't sensitive to them?

svn log --xml formats dates using UTC, which seems more robust that relying on TZ. I've given a try in CL 447935.

Something to do with the PATHEXT environment variable, maybe? I don't think I included it here:

Good shoot, the environment variable missing was ComSpec.

@golang golang locked and limited conversation to collaborators Nov 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows
Projects
None yet
Milestone
Go1.20
Development

No branches or pull requests
5 participants
@mdempsky @qmuntal @bcmills @gopherbot @erifan