My guess is that problem is that the magic we added to the os package doesn't help the syscall package, since syscall (where the exec happens) doesn't use os.

Maybe we need to move the os package's support routines (building the UNC paths) into an internal support package usable by both os and syscall.

/cc @quentinmit

And /cc @alexbrainman

@bradfitz - I think this would be handled by the caller of Exec instead of in syscall directly since they are opaque strings and the difference between a file path and any other arbitrary string is determined by the caller. I do agree that this and the os package should share the same path-fixing code. I also agree it should be an internal function for now until more thought is given to expose (maybe os/exec.OSSafePath but it's really annoying to abstract a Windows -only issue).

Why not have filepath.LongPath() function to be used for these situations?

@harshavardhana If the go std lib handles windows long filepaths, which I think is great, I don't think it would be good to expose it anywhere except (maybe) in syscall, as this is a really platform specific issue where the fix doesn't need to affect API.

@harshavardhana If the go std lib handles windows long filepaths, which I think is great, I don't think it would be good to expose it anywhere except (maybe) in syscall, as this is a really platform specific issue where the fix doesn't need to affect API.

👍

We very consciously did not make syscall transparently handle long paths, because it would be confusing if the resulting syscall was called with different arguments than the caller intended.

Probably the right thing to do is to teach os/exec to use the same long path handling as os.

Actually, os/exec just uses os.StartProcess. So this is in the os package after all.

I'll send out a fix.

CL https://golang.org/cl/34725 mentions this issue.

Not trivial, it turns out. Somebody should tackle it during Go 1.9. https://golang.org/cl/34725 might be a good starting point. It has some debugging notes in it and a test.

If I was to guess, the UNC meses with the quote logic; guess only.

Note: #21145, just closed as a dup of this issue, has an handy Powershell script that creates a folder that triggers the problem.

After reviewing #21145 I think the CL that Brad had started ( https://golang.org/cl/34725 ) is barking up the wrong tree. In that CL it attempts to fixup the process name to use the UNC path equivalent that the rest of the os functions can use. However, as noted in the CL, CreateProcess doesn't accept UNC paths for the process name.

Looking at this issue again the process name is short C:\dls\go1.8\go\pkg\tool\windows_amd64\compile.exe and less then 250 chars. This means that no long path fixup would be applied even if it could work.

I suspect that the error is actually referring to the argument length (Windows doesn't really differentiate between the two in error message).

From https://msdn.microsoft.com/en-us/library/windows/desktop/ms682425(v=vs.85).aspx

lpCommandLine [in, out, optional]
The command line to be executed. The maximum length of this string is 32,768 characters, including the Unicode terminating null character. If lpApplicationName is NULL, the module name portion of lpCommandLine is limited to MAX_PATH characters.

Two ways to fix this would be to instrument the Go compilers to take a new argument "-args-on-stdin=true" flag where the exec would read from stdin for args or pass the argument list file to the executable "@C:\Temp\arglist" where the given file is read off of disk and used as arguments.

It would be nice to solve the general problem, but perhaps Windows does not provide a mechanism for that. If we can only solve the problem for cmd/go, then of course we should do that.

For the record, GCC and friends do this by treating any argument that starts with '@' as a response file. The file is turned into a sequence of white-space separated strings, and the response file argument is replaced by the resulting set of arguments. This seems to be a standard Windows concept, documented at https://msdn.microsoft.com/en-us/library/windows/desktop/aa367156(v=vs.85).aspx .

I’m running into this issue and I’d like to contribute a fix.

If the maintainers would like me to teach the toolchain about response files, I’m happy to take a swing at it, but I have some questions:

1. It seems like it may be worthwhile to teach the flag package how to understand response files. If that is reasonable, my approach would be when looking for flags, also treat strings that start with @ specially. Is it reasonable to make that change or would it need to be guarded behind some check to ensure existing invocations which pass strings start start with @ in a position where a flag would be legal continue to work? It so, I assume adding a new function like AllowResponseFile(bool) is the way to go?
2. How do you want to do word splitting in the response file? Would we want to support multiple arguments per line in the response file or say that each argument is delimited by a newline. Regardless of what we choose, how does someone escape a character that would cause word breaking?
3. If adding this feature to the flag package is unacceptable should I just teach the compiler a new argument like -sourcefiles ?

@ellismg before deciding how to fix this, can you provide me with steps to reproduce it.

Thank you

Alex

@alexbrainman

Sure. In the case I'm hitting, the problem is just that we construct a command line that is too long (>32K characters) when we try to exec into the compiler.

This works:

C:\Users\Matt G. Ellis\go\src\github.com\ellismg\scratch>dir /B
empty.go
hello.go

C:\Users\Matt G. Ellis\go\src\github.com\ellismg\scratch>type empty.go
package main
C:\Users\Matt G. Ellis\go\src\github.com\ellismg\scratch>type hello.go
package main

import "fmt"

func main() {
    fmt.Printf("hello, world\n")
}
C:\Users\Matt G. Ellis\go\src\github.com\ellismg\scratch>go build

C:\Users\Matt G. Ellis\go\src\github.com\ellismg\scratch>

If you add a bunch of additional files (depending on how deep your source code is, you may need more copies of the dummy file, to construct a command line that is over the windows limit):

C:\Users\Matt G. Ellis\go\src\github.com\ellismg\scratch>for /l %i IN (1,1,2048) DO copy /y empty.go %i.g
o
[...lots of copy spew elided...]

Then go build will fail when trying to run compiler.exe

C:\Users\Matt G. Ellis\go\src\github.com\ellismg\scratch>go build
go build github.com/ellismg/scratch: C:\Go\pkg\tool\windows_amd64\compile.exe: fork/exec C:\Go\pkg\tool\w
indows_amd64\compile.exe: The filename or extension is too long.

Here's my system information:

C:\Users\Matt G. Ellis\go\src\github.com\ellismg\scratch>go version
go version go1.9 windows/amd64

C:\Users\Matt G. Ellis\go\src\github.com\ellismg\scratch>go env
set GOARCH=amd64
set GOBIN=
set GOEXE=.exe
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=C:\Users\Matt G. Ellis\go
set GORACE=
set GOROOT=C:\Go
set GOTOOLDIR=C:\Go\pkg\tool\windows_amd64
set GCCGO=gccgo
set CC=gcc
set GOGCCFLAGS=-m64 -mthreads -fmessage-length=0
set CXX=g++
set CGO_ENABLED=1
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config

Locally, I did a hacky thing where I just taught compiler.exe about a -srcfiles argument that would read a response file and then passed every source file via that list. With that set of changes, I was able to build.

Nothing long path specific here, as @kardianos surmised. Just generating a very long command that ends up being over a Windows limit.

Something like flag.(*FlagSet).ResponseFiles(bool) makes sense to me, with flag.ResponseFiles calling that method on CommandLine.

The response files should be parsed exactly as the Microsoft tools parse them. I believe they permit multiple options on a single line, and permit quoting with backslash, single quote, and double quote.

@arizvisa, thanks for testing and noting the .exe bug. I've uploaded a new version and also using it with "link".

@bradfitz, hey man. I noticed that one of the reviewers of your patch suggested the idea of going the path of response files. I made a branch that goes down this route, and adds '@'-prefixed response files to the same place that your patch added your fix.

I essentially re-implemented the buildargv function (from libiberty in gcc) in golang, but at the moment it doesn't do recursive response files which I believe gnu-tools supports. It also doesn't explicitly test the tool that's being dispatched to as your patch does.

At the moment its dev'd against go1.10.1. If the community prefers this solution for some reason, though, I can rebase it against master and create a PR so it can be merged upstream. However, I could use another pair of eyes to sanity check how I'm escaping things when writing to the response file (since it's pretty common to have mistakes here).

The branch I'm referring to is at arizvisa/golang#GH-18468

Actually, another update. I didn't realize go1.10.2 came out, so I rebased it for that tag. lmk..

@arizvisa if you want your change looked at, you have to use tools that this this repo uses - https://golang.org/doc/contribute.html on how to contribute. Or just create a PR here on master branch. It was @ianlancetaylor who suggested to use response files on https://golang.org/cl/110395. If you post your change here, maybe it will be prefered to CL 110395.

Alex

@alexbrainman, I don't want to step on bradfitz's toes as it was his original fix, I was just offering some direction as it was based on his original work and his response to my complaining. It works for me, but I also don't really want to maintain it or anything ;-) as I'm not really a developer.

Plus, I had realized that I implemented it from a GPL-licensed piece of software which I'm pretty sure is a licensing conflict.

It works for me,

Sounds good to me. I gave Brad +2 now, so he can submit his CL.

I had realized that I implemented it from a GPL-licensed piece of software which I'm pretty sure is a licensing conflict.

It could well be.

Alex

I've submitted 17fbb83 (https://go-review.googlesource.com/c/go/+/110395). @arizvisa, please double check that it works for you?

I at least made it such that when it runs on our build farm, 10% of the compile/link invocations go through this code path, so it will get some exercise even for non-large build arguments.

@bradfitz, sorry to get back to you so late as I've been ill for a bit.

It seems to work without issue. So, thanks!

One thing not too important but worth thinking about in the future, the way you split lines in expandArgs just culls out the <CR>. I know it's dickheadish, but <CR> is actually a valid character in a filename. Regardless of that weirdness tho, bufio.Scanner might be able to be used in order to scan for newlines more effectively. That was the reason why I ripped buildargv out of libiberty anyways.

Regardless though. Thanks for getting this fixed!

<CR> is not really a valid character in a Windows filename per https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx which says...

Characters whose integer representations are in the range from 1 through 31, except for alternate data streams where these characters are allowed.

I don't think we need to care about "alternate data streams", which I assume are like Mac "resource forks"?

Ah, the api call suceeds. (Although I'm using touch to avoid writing a program).

[528] user@skinny ~/build/go1.10.2$ uname -a
MSYS_NT-6.3 skinny 2.10.0(0.325/5/3) 2018-02-09 15:25 x86_64 Msys
[529] user@skinny ~/build/go1.10.2$ touch $'hola\nmundo.'
[530] user@skinny ~/build/go1.10.2$ ls -l $'hola\nmundo.'
-rw-r--r-- 1 user None 0 May  6 16:02 'hola'$'\n''mundo.'
[531] user@skinny ~/build/go1.10.2$ echo hola*
hola
mundo.

Result of the file in the old cmd prompt.

C:\users\user\build\go1.10.2\src>dir /x ..\hola*
 Volume in drive C has no label.
 Volume Serial Number is FA87-0BF0

 Directory of C:\users\user\build\go1.10.2

05/06/2018  04:02 PM                 0 HOLAMU~1     hola?mundo.
               1 File(s)              0 bytes
               0 Dir(s)  13,513,748,480 bytes free

Then a streams which isn't related, plus I couldn't get a newline added as a stream (even using the api)

C:\users\user\build\go1.10.2>echo hi > stream:woot
C:\users\user\build\go1.10.2>dir /r stream
 Volume in drive C has no label.
 Volume Serial Number is FA87-0BF0

 Directory of C:\users\user\build\go1.10.2

05/06/2018  04:12 PM                 0 stream
                                     5 stream:woot:$DATA
               1 File(s)              0 bytes
               0 Dir(s)  13,508,710,400 bytes free

I'm not really a mac guy so my exp w/ resource forks are pretty spotty. but alternate data streams are typically used for things like metadata as they have no particular data structure. NTFS exposes some interesting default ADS which one can use to get some pretty interesting side effects out of software on windows. But as an example for a legitimate thing, browsers will add an ADS to a downloaded file so that a file can be noted as being "unsafe".

But regardless about platform differences/semantics, I'm just happy things are fixed and I just have tendencies to note side effects about things which is what that stripping of <CR> looked like to me. :)

The implementation of buildargv that I ripped attempted to parse out these things by handling quoting and backslash-escapes. However in order to use that implementation effectively, when I had to build the response file I spent most of the time trying to figure out how to properly quote, escape, and separate each argument in the response file so that buildargv would properly decode those characters that were explicitly specified which allows for their compiler to work independently of filesystem contraints. That's how the gnu folk handle it anyways.

Again though, thanks for fixing this!

(edited to fix some type-o's and the missing backticks around CR)

Is this supposed to be fixed in 1.10.2? I just upgraded to the latest version and I'm still getting the error.
Edit: I just saw the 1.11 milestone, how can I have access to this fix before the release?

Edit: I just saw the 1.11 milestone, how can I have access to this fix before the release?

You could patch it in and recompile Go if it's urgent.

But I suppose we could also backport it to Go 1.10.x if we do another one.

@gopherbot, please backport.

Backport issue(s) opened: #25292 (for 1.10).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.