Windows: Use absolute and shortened path for GOROOT environment variable

[Xjs]

Maybe it's because my name is so long, but I frequently ran into errors where filepaths to Go packages exceeded 255 characters when trying to build under Windows, which caused GoStdlib and GoLink to fail. The modifications I made here make it possible for me to build Go binaries under Windows.

The GoLink issue was particularly tricky to spot because it seems only to occur when supplying a relative GOROOT, see also golang/go#26917. It could probably also be fixed by simply submitting an absolute GOROOT from Bazel, without needing to touch the Go wrapper program, but I didn't know how to make an absolute path out of a relative one in Bazel. It would of course also be mitigated by a potential Go toolchain fix as suggested in golang/go#26917, but who knows whether and when that might actually happen.

There are probably other ways to fix this, too. This one worked for me, but I'm open to suggestions.

[jayconrod]

I'm working on a fix for #1637, and I expect it will resolve the issue you're seeing, too. When that's fixed, paths to archives will be passed to the compiler and linker through -importcfg files (instead of on the command line). The paths will also be shorter, since they won't need to include the full import path.

Let's leave this PR open and re-evaluate when that fix is submitted. If this change is still needed, the Windows-specific code will need to be extracted into a function in a file ending with _windows.go (so it will only be built on Windows, and there will need to be another version of the function in another file with // +build !windows. I'm hoping we don't need that complexity though.

[Xjs]

Thank you very much for your reply, @jayconrod. What you're saying sounds good, let's see how it works out. Given that golang/go#26917 was assigned to the 1.12 milestone, I expect care will have to be taken when writing the importcfg files, so that the paths will be openable for the linker on Windows. I will be ready to re-evaluate ;)

Re windows-specific code: I totally overlooked that when focusing on fixing the issue at hand, and only today remembered that I should have extracted it to a specific file. Sorry about that. Anyway, let's hope the fix won't be needed anymore, indeed.)

[jayconrod]

@Xjs Ok, fixes are in. Please try out master and see if that fixes the issue you were seeing.

[Xjs]

@jayconrod Thank you very much for getting back to me! :)

Unfortunately I'm still getting the same error as before, one that the first part of my fix addresses:

ERROR: C:/users/jannis.schnitzer/_bazel_jannis.schnitzer/2gjvkcc3/external/io_bazel_rules_go/BUILD.bazel:8:1: GoStdlib external/io_bazel_rules_go/windows_amd64_pure_stripped/stdlib%/pkg failed (Exit 1)
go build vendor/golang_org/x/crypto/chacha20poly1305/internal/chacha20: C:\users\jannis.schnitzer\_bazel_jannis.schnitzer\2gjvkcc3\execroot\internal_project_workspace_name\bazel-out\host\bin\external\io_bazel_rules_go\go\tools\builders\windows_amd64_stripped\filter_buildid.exe: fork/exec C:\users\jannis.schnitzer\_bazel_jannis.schnitzer\2gjvkcc3\execroot\internal_project_workspace_name\bazel-out\host\bin\external\io_bazel_rules_go\go\tools\builders\windows_amd64_stripped\filter_buildid.exe: The directory name is invalid.

I had already traced this error back to the go build tool setting the working directory of -toolexec programs to subdirectories of the GOROOT, which in turn are too long for Windows (there is no way to make the lpCurrentDirectory argument of the CreateProcessW syscall accept a long path, as already noticed by @laszlocsomor under [1]). The »shorten GOROOT« commit in my fix (94ca818) addresses this; I don't know if you see another way to do it. If not, I'm happy to refactor my fix such that the Windows-specific code is in a specific file.

[1] bazelbuild/bazel#2181 (comment)

[laszlocsomor]

@Xjs : there is a trick: you can create a junction pointing to the long directory, and set lpCurrentDirectory to the junction.

[jayconrod]

@Xjs CreateProcessW doesn't accept long paths? That's ... very unfortunate.

Will GetShortPathName actually work for all paths? If I'm understanding the documentation correctly, the shorted path could still exceed the maximum length if there are a large number of path components. It doesn't seem like it buys us that much extra length. Maybe the junction workaround that @laszlocsomor mentioned would work in more cases?

Moving forward with this:

• I think the change in link.go can be reverted. The linker itself should not read GOROOT, and all the paths in the importcfg file should be absolute (which means os.Open can convert them to long paths on Windows).
• The change for stdlib.go will probably still be needed. We'll need a function that preprocesses the directory, either by shortening a path or creating a junction. The non-Windows implementation of that function can just return the original path.

[Xjs]

@jayconrod Thanks for your reply. (Yes, it is very unfortunate.)

Shortening the path was the solution employed by Bazel (bazelbuild/bazel@44ecf9a), so I went for that one as well, since we're running in the bazel ecosystem anyway. Creating a junction might work, too. (Does a os.Symlink transparently produce a junction on Windows?) The question would be where to create the link, which is not obvious to me at the moment. Is it for you?

I agree with your assessment of the link.go change.

[Xjs]

@jayconrod I pushed one small change to link.go where a slash was still used in path construction in the importcfg file (https://github.com/bazelbuild/rules_go/blob/2df290816913fc27e77b58cbb203d8f5bda107d1/go/tools/builders/link.go#L158). Can't test on a production Windows machine until tomorrow, but I'm fairly confident that this will do then.

[jayconrod]

Looks good. Thanks for working on this!

[laszlocsomor]

@jayconrod : you're right that GetShortPathName doesn't help with too deep directories. Also, the user may disable short path generation on the volume, meaning GetShortPathName would return the path unchanged.
To place junctions, I suggest creating a temp directory under %TMP% or %TEMP%. (Note: unfortunately the default value for these envvars is %WINDIR%\temp which is readonly.)