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

Windows integration tests: git file path quirk fix #1291

Merged
merged 1 commit into from
Sep 10, 2020

Conversation

pda
Copy link
Member

@pda pda commented Sep 10, 2020

Here's a test failure I experienced repeatably in ad-hoc Windows testing, but wasn't showing up in CI Windows testing:

C:\pda\agent>go test ./bootstrap/integration -run TestPreExitHooksRunsAfterCommandFails
--- FAIL: TestPreExitHooksRunsAfterCommandFails (3.24s)
panic: exit status 128 [recovered]
        panic: exit status 128

goroutine 21 [running]:
testing.tRunner.func1.1(0x207a3e0, 0xc0002acfa0)
        c:/go/src/testing/testing.go:1057 +0x310
testing.tRunner.func1(0xc0000c0900)
        c:/go/src/testing/testing.go:1060 +0x43a
panic(0x207a3e0, 0xc0002acfa0)
        c:/go/src/runtime/panic.go:969 +0x176
github.com/buildkite/agent/v3/bootstrap/integration.(*BootstrapTester).ExpectLocalHook(0xc000332000, 0x20d8c35, 0x8, 0xc0003700a0)
        C:/pda/agent/bootstrap/integration/bootstrap_tester.go:214 +0x467
github.com/buildkite/agent/v3/bootstrap/integration.TestPreExitHooksRunsAfterCommandFails(0xc0000c0900)
        C:/pda/agent/bootstrap/integration/command_integration_test.go:31 +0x205
testing.tRunner(0xc0000c0900, 0x21290f8)
        c:/go/src/testing/testing.go:1108 +0xef
created by testing.(*T).Run
        c:/go/src/testing/testing.go:1159 +0x397
FAIL    github.com/buildkite/agent/v3/bootstrap/integration     3.429s
FAIL

Digging into ExpectLocalHook for exit status 128 found it was a failing git add … command. I added some debug output:

2020/09/09 15:02:15 $ git [add C:\Users\ADMINI~1\AppData\Local\Temp\2\git-repo275254366\.buildkite\hooks\pre-exit.bat]
2020/09/09 15:02:15 Result: exit status 128 fatal: C:\Users\ADMINI~1\AppData\Local\Temp\2\git-repo275254366\.buildkite\hooks\pre-exit.bat: 'C:\Users\ADMINI~1\AppData\Local\Temp\2\git-repo275254366\.buildkite\hooks\pre-exit.bat' is outside repository

That file is not outside the repository, but the ADMINI~1 8.3 shortened directory name is suspicious.

Indeed, git add of an absolute path containing an 8.3-shortening fails:

C:\Users\Administrator\AppData\Local\Temp\2\git-repo162512502>git add C:\Users\ADMINI~1\AppData\Local\Temp\2\git-repo162512502\.buildkite\hooks\pre-exit.bat
fatal: C:\Users\ADMINI~1\AppData\Local\Temp\2\git-repo162512502\.buildkite\hooks\pre-exit.bat: 'C:\Users\ADMINI~1\AppData\Local\Temp\2\git-repo162512502\.buildkite\hooks\pre-exit.bat' is outside repository

While the same git add with the expanded long directory name works fine:

C:\Users\Administrator\AppData\Local\Temp\2\git-repo162512502>git add C:\Users\Administrator\AppData\Local\Temp\2\git-repo162512502\.buildkite\hooks\pre-exit.bat
(ok)

The tilde path is coming from ioutil.TempDir("", "git-repo") which is documented at https://golang.org/pkg/io/ioutil/#TempDir but doesn't mention Windows things.

A practical solution seems to be to resolve the output of TempDir to a long filename path somehow. Some attempts from package filepath:

// filepath.Abs:          C:\Users\ADMINI~1\AppData\Local\Temp\2\git-repo275254366
// filepath.Clean:        C:\Users\ADMINI~1\AppData\Local\Temp\2\git-repo275254366
// filepath.EvalSymlinks: C:\Users\Administrator\AppData\Local\Temp\2\git-repo275254366

So, this PR runs the output of ioutil.TempDir through filepath.EvalSymlinks to expand Windows tilde-paths. It runs on all platforms, not just Windows, but I don't think resolving symlinks will do any harm? And, it's test code, not production code.

@pda pda requested a review from lox September 10, 2020 01:43
Copy link
Contributor

@yob yob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't run this change myself on windows (really must setup a VM), but this is well explained an makes sense.

It's incredible that 8.3 filenames are still a thing in 2020, I'm always impressed (and a little horrified) at Microsoft's impressive backwards compatibility.

👍

@pda pda merged commit a3b432c into master Sep 10, 2020
@pda pda deleted the windows-fix-git-tilde-path branch September 10, 2020 02:09
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

Successfully merging this pull request may close these issues.

None yet

2 participants