os: some functions fail if full paths are long (more than about 250 chars) on windows #3358
Comments
I am not sure how this should be fixed. It might not be easy. I never dealt with these in the past. But after some investigation, here is what I see is happening (http://goo.gl/th2fJ for details). All Windows apis, that take file name, accept it in a form that needs to be processed before it gets passed to correspondent file system: - it can contains relative paths, then "file name processing" code tucks in current directory in front; - all / are converted into \; - it deals with . and ..; - ... and so on ... It appears that this "file name processor" can only deal with files with absolute path less then ~ 260 characters long. So even if we pass file name like "alex.txt" to it, it might fail, because current directory full path might be too long already. There is one way to overcome ~ 260 length limitation - it is to skip "file name processor", and pass file name directly to file system (the file length limit there is ~32000). The way to do it is to prefix file name with \\?\, so c:\tmp\alex.txt would become \\?\c:\tmp\alex.txt and so on. The down side to this solution is that we loose all "file name processing" along the way, so we need to do it all manually. All file names have to be full path (absolute), we must reject file names like . and .. by hand and so on. I do not think we should "convert" all our file names into \\?\... in syscall by default. It could be quite dangerous, because it is possible to create files with weird names, like . and .. and such. Normal shell utilities and GUI wouldn't be able to deal with those. We might discover that, while perfectly accepted by Windows api, these might be rejected by other programs / apis. It could be confusing for some users to find that they pass "alex.txt" into Go and to see that Windows api gets \\?\c:\tmp\alex.txt. We might let things be the way they are, but monitor for particular Windows api errors and call api second time with \\?\... form of the file name. But again, it feels too magical to me. On the other hand, nothing stops our users from doing os.Create(`\\?\c:\tmp\alex.txt`), if they want to. So, I think, the issue here are not long file names as such, but more the fact that our Go builder hit this problem http://goo.gl/FbblU. So, perhaps we can change go command in such way that it is unlikely to happen. Looking at particular error: # ..\test\bench\go1 # _/C_/Users/ADMINI~1/AppData/Local/Temp/2/gobuilder/windows-amd64-0449267813c1/go/test/bench/go1 mkdir C:\Users\ADMINI~1\AppData\Local\Temp\2\go-build317224146\_\C_\Users\ADMINI~1\AppData\Local\Temp\2\gobuilder\windows-amd64-0449267813c1\go\test\bench\go1\_test\_\C_\Users\ADMINI~1\AppData\Local\Temp\2\gobuilder\windows-amd64-0449267813c1\go\test\bench\: The filename or extension is too long. it happens during go test ..\test\bench\go1 If we break long path, we could see what is happening. It starts with TEMP directory C:\Users\ADMINI~1\AppData\Local\Temp\2\ we created a temp directory there go-build317224146\ then we converted full path of ..\test\bench\go1 into import path (because it is not in GOPATH) _\C_\Users\ADMINI~1\AppData\Local\Temp\2\gobuilder\windows-amd64-0449267813c1\go\test\bench\go1\ (watch full TEMP path here again that was created by gobuilder), then we get _test + import-path again directory that holds transient test files _\C_\Users\ADMINI~1\AppData\Local\Temp\2\gobuilder\windows-amd64-0449267813c1\go\test\bench\ Perhaps, some of these rules can be changed to shorten the path. Maybe go test ..\test\bench\go1 on gobuilder is not important, but I am concerned with what happens if people start creating deep directories inside their GOPATH. Especially encouraged by "go get ..." conventions. Something like that: C:\a\code\src\code.google.com\p\gowingui\code.google.com\p\codesearch Alex |
The real issue is that the Windows API needs to allow long paths. But I agree with Alex that it is far too invasive a change to be considering so close to Go 1.1. We can tweak the go command to shorten some of the temporary paths, so that it is at least not doubled. I created issue #5027 for that. Labels changed: added go1.2, removed go1.1, suggested. |
> On the other hand, nothing stops our users from doing os.Create(`\\?\c:\tmp\alex.txt`), if they want to. That'd mean making stdlib users write OS-dependent code even without using syscall (which doesn't sound desirable); moreover, *not* writing that OS-dependent code would only be noticed on corner cases and would imply lots of boilerplate around every single call that deals with the filesystem, which is specially harsh when dealing with relative paths. If making os handling such conversion automatically looks overkill, then maybe adding a function to path/filepath that can create such a "super-absolute" path, but returns a regular absolute path on non-windows? It would also need documentation that it's needed to handle long paths (or long CWDs) on windows, and possibly a reference on the os package's filesystem functions. As an additional note, "native Cygwin" apps are not hit by this restriction and can access the full 32k character paths with stdio by internally always doing the path conversion to "super-absolute" paths. I was hit by this when running a Go program against files that were created by a cygwin program. |
As far as I can tell, resolving this would require mangling the paths, while recognizing device and UNC patterns as well. So long as this was done at the same time as the UTF-8 to UTF-16 conversion, this shouldn't introduce any additional mallocs I think. * If this was implemented, would reviewers want to see an environment variable or an easy switch to turn it off? |
|
Issue #6300 has been merged into this issue. |
|
Issue #6300 has been merged into this issue. |
|
Issue #6300 has been merged into this issue. |
I have run into this problem as well on Windows, and believe that I might have a relatively simple solution. On this line: https://code.google.com/p/go/codesearch#go/src/cmd/go/test.go&l=579 testDir is sometimes creating very long directory names for a test. This becomes a problem when p.ImportPath has been created by this function: https://code.google.com/p/go/codesearch#go/src/cmd/go/pkg.go&l=212 and could appear like in the example given in the code: _/c_/home/gopher/my/pkg My proposed fix would be this: If p.ImportPath already starts with an underscore, then simply leave it out of the creation of "testDir" altogether. So this: https://code.google.com/p/go/codesearch#go/src/cmd/go/test.go&l=579 could look something like this: addPath := "_test" if !strings.HasPrefix(p.ImportPath, "_") { addPath = filepath.FromSlash(p.ImportPath+"/_test") } testDir := filepath.Join(b.work, addPath) |
|
@iafan, what matter is being correct and consistent, and providing a simple interface. The speed is a detail which can be fixed. We shouldn't make an ugly API just for speed reasons. |
|
A compromise solution: We make all the std library function (exclude syscall) that manipulate We can even document this. |
|
Further activity by Microsoft on Windows 10 updates has rendered some of the underlying MAX_PATH/250-char limit stuff less troublesome. For your GO app to take advantage of the "I am large path aware" feature in Windows, a manifest file must be attached to your app. It would be nice if Go made adding that manifest easy on Windows. Manifests are also useful for other purposes like specifying that you require a particular version of some system library, and may be useful to some very small amount of users to have a custom manifest, but a default manifest that is at least marking the app as "large path aware" might be a useful thing to think of doing. |
|
FWIW - We have been successfully using a nice wrapper on windows for Minio, same code path works transparently for both Unixes and Windows. Relevant windows translation code. |
This https://github.com/lxn/walk/blob/master/README.mdown#create-manifest-testmanifest is one one way to do it. Alex |
|
Even though Windows 10 will "fix" this, not everyone has it, and not everyone will have it for quite a while. It seems like we should probably fix it too, by transparently flipping to the ?\ syntax for absolute paths passed to the os calls. This is worth prototyping at least. |
All operations work transparently other than helpers like MkdirAll and RemoveAll which needs a custom wrapper. But it is neatly doable Should i make a PR? |
|
CL https://golang.org/cl/32451 mentions this issue. |
|
CL https://golang.org/cl/32886 mentions this issue. |
Notably, don't allocate. Follow-up to https://golang.org/cl/32451 which added long path cleaning. Updates #3358 Change-Id: I89c59cbd660d0a030f31b6acd070fa9f3250683b Reviewed-on: https://go-review.googlesource.com/32886 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Russ Cox <rsc@golang.org>
|
I have tested this on 1.8 beta 2 and still have issues with large go build calls: I can open this as a separate bug if that works better. |
|
@cretz, please file a separate bug. My guess is that problem is separate, since the magic we added here to the |
This program creates directory, but fails to create file inside (CAREFUL IF CHANGING THIS PROGRAM - YOU MIGHT END UP WITH FILES/DIRECTORIES YOU WILL NOT BE ABLE TO DELETE WITH STANDARD SHELL TOOLS): package main import ( "fmt" "log" "os" "strings" ) func main() { // root := `\\?\c:\` root := `c:\` dir := root + strings.Repeat("d", 200) file := dir + `\` + strings.Repeat("f", 200) err := os.Mkdir(dir, 0777) if err != nil { log.Fatalf("Mkdir(%s): %v", dir, err) } f, err := os.Create(file) if err != nil { log.Fatalf("Create(%s): %v", file, err) } _, err = fmt.Fprintf(f, "Hello world") if err != nil { log.Fatalf("Fprintf(%s): %v", file, err) } err = f.Close() if err != nil { log.Fatalf("Close(%s): %v", file, err) } err = os.Remove(file) if err != nil { log.Fatalf("Remove(%s): %v", file, err) } err = os.Remove(dir) if err != nil { log.Fatalf("Remove(%s): %v", dir, err) } } This can be handled properly if we use \\?\ path prefix (uncomment line in the code). But \\?\ prefixed paths needs to be "normalized": they have to be absolute, no . or .. allowed, / is not allowed, UNC paths needs to be handled properly. See http://goo.gl/th2fJ for details, especially "Maximum Path Length Limitation" section. This issue has come up on our own builder: http://goo.gl/FbblU. Perhaps our own paths do not need to be that long. I did not look into that yet. AlexThe text was updated successfully, but these errors were encountered: