Skip to content
This repository has been archived by the owner on Nov 9, 2017. It is now read-only.

[issue52] adding support for long path names on Windows #85

Closed
wants to merge 2 commits into from

Conversation

nitram509
Copy link

Hi,

Two Co-workers and I created a patch, which enables msysgit
to handle long path names.

We consider the patch as "proposal" and want to get some feedback.
Applying the patch to v.1.8.3 will pass every integration test,
which un-patched version also does.

AFAIK, these things are still to do:

  • writing a test
  • split xutftowcs_path into separate functions for character
    conversion and prefixing - so de-prefixing can be avoided
    at some places
  • checking upper array boundaries

Please, give this patch a try and send feedback.

Thanks
Andi, Eric, Martin

@buildhive
Copy link

MSysGit - the development behind Git for Windows » git #111 SUCCESS
This pull request looks good
(what's this?)

@vangdfang
Copy link

The question I have is, will this allow files to be created that conceivably Windows (or any other application using the normal Windows APIs) cannot access? I can see use to a patch that allows full use to up to this limit--but use beyond that just seems like you're asking for trouble.

@kblees
Copy link

kblees commented Aug 7, 2013

@vangdfang Typical Windows applications use the shell's open file dialog (GetOpenFileName[AW]), which doesn't support long file names. I don't know any editor or IDE that can handle such files (while Java supposedly can open long file names, Java based IDEs such as Eclipse cannot). See the discussion in the related msysgit/git issue #24

This means that long file name support in git is indeed kinda pointless.

However, some people still seem to find this feature useful, and I've no problem with a patch if they are willing to spend their time on the issue.

@vangdfang
Copy link

True. In the issue that appeared to apply to @nitram509 originally, it appears that the path was 228 (plus something for an absolute path?) characters long -- just 20 characters shy of the 248 character limit. So, maybe there is an actual bug somewhere under the surface, preventing either MSYS tools or git from reaching the full 248 character limit by bailing early (perhaps a temporary buffer is too short, especially during the conversions to/from UCS-2). If that's the case, I think everyone would be much better served by fixing the actual bug than masking it (I'm especially paranoid because a fix I devised rather similar in concepts to this appeared to pass all regression tests that the stock build would pass, but resulted in rather quirky behavior in certain commands following the patch).

For grins, I wonder what would happen if PATH_MAX / MAX_PATH (sorry--the names kinda make me go @.@) were redefined much larger, say 1024. Not suggesting this be committed, necessarily, but would it resolve the error?

@nitram509
Copy link
Author

I've did an reality check and tried to open such a long file name (.txt) in different environments.
These are my results:

notepad (win8) - no
wordpad (win8) - no
eclipse 4.3 - no
sublime text 2 - no
chrome 28.0 - no
firefox 22.0 - no
libre office 4.1 - no
ms word 2010 - no
python 2.7.5 - no
node.js 0.10.8 - no
windows 7 explorer - no
windows 7 zip explorer - no

Total Commander 8 - YES
Total Commander 8 & builtin ZIP - YES
IntelliJ Idea 12 - YES
Java 7 - YES
Java Tools (Maven, Groovy, etc.) - 99.99% YES
Cygwin - YES
Netbeans 7.1 - YES

Considering the fact, that I'm working ~95% of my time in the Java universe and IntelliJ is my preferred IDE,
I'm fine with that. Especially in our financial domain, we prefer meaningful, long names and regularly hitting the limit.
So I'm motivated to get at least an 80% solution ;-)

@nitram509
Copy link
Author

@vangdfang
What do you mean by '... would be much better served by fixing the actual bug than masking it ...'?
What's the actual bug in your opinion?

@vangdfang
Copy link

@nitram509 Cool, thanks for the tests! That's nice knowing that there are some utilities out there that will work -- I can understand the usefulness (even if it's just say, some project with a really long file path on Linux coming over to Windows).

I was referring to the original path you posted in the original issue -- it was just shy of the 248 character limit. If accessing that has problems, then I'd consider that a bug unrelated to the long path support. But if the path really was longer than 248 characters, I can see the usefulness of this as well. If you have a look at #86, you will probably see a lot of similarity--though the issue we experienced in our environment was recursive submodules could exceed the path length due to an excessively-long relative path.

One specific issue I know that came about was "git status" from a directory like /c/foo/bar/baz would return the wrong error message. It would error (as expected), but the correct message is "fatal: Not a git repository (or any of the parent directories): .git". The other one that I didn't see (but a user reported when I had a patch that prefixed ?) was a "git fetch" operation causing a non-fatal message "warning: cannot open /etc/mailname: Invalid argument" (even though he had his user.name and user.email set globally) -- when I traced the code as best as I could, I could only reason that EINVAL came back somewhere instead of ENOENT.

…or comperations; re-formated comments C-style;
@buildhive
Copy link

MSysGit - the development behind Git for Windows » git #113 SUCCESS
This pull request looks good
(what's this?)

@dscho
Copy link
Member

dscho commented Dec 17, 2013

Hmm... So, how are we going to proceed about this pull request and #86? Which one is preferable?

@dscho
Copy link
Member

dscho commented Dec 30, 2013

I am afraid that this won't make it into our 1.8.5.2 preview.

static const int delay[] = { 0, 1, 10, 20, 40 };
unsigned int _CRT_fmode = _O_BINARY;

static wchar_t * deprefix(wchar_t *path) {
if ( path && wcsncmp(path, L"\\\\?\\", 4) == 0 )
Copy link
Member

Choose a reason for hiding this comment

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

This code style disagrees seriously with the code style all around in git.git...

@dscho
Copy link
Member

dscho commented Dec 31, 2013

@nitram509 it would be nice if you could have a look at #86 (which I just merged). I would totally love it if you could adapt this pull request to match in style, and to rebase it on top of 'master'!

@nitram509
Copy link
Author

@dscho Thanks for your explanation in detail, now I understand more "why".

I will have a look at #86 ...

PS: Thanks for putting a little pressure on me. My motivation went down, since I discovered, that there is relatively little interest within this community about this topic.

@vangdfang
Copy link

Hi @nitram509 -- please have a look at #110 as well... it attempts to integrate some of the work you have done along with some of the other problems I encountered while testing similar patches into a concise form. If you have additional improvements to make on it, you are absolutely welcome to comment, revise, and improve for yourself.

@marklagendijk
Copy link

@nitram509 +1!
There is another use case for which this patch would be very welcome: Node.js packages. The reason for this is that the dependencies of a package are downloaded inside the package. So you get directory trees like node_modules\package1\node_modules\dependency1\node_modules\subdependency1 etc.
Although Node.js itself can handle long paths, it becomes a problem when you try to check the packages into Git.

@cdubois
Copy link

cdubois commented Jan 15, 2014

@marklagendijk, Node.js installed modules are exactly why I am following this path issue. This is a huge problem on Windows and this is really the only item that is preventing us from migrating away from Subversion to git across the board.

@dscho
Copy link
Member

dscho commented Jan 15, 2014

@marklagendijk @cdubois understood.

However, keep in mind that at least one person participating on the solution is under serious time constraints (yes, I am talking about myself: I have not had the opportunity to spend any time on Git for Windows this week so far).

So please consider this as an encouragement to become more active yourself. I would appreciate any help in resolving the issues raised with this pull request as well as #110.

@nitram509
Copy link
Author

@vangdfang Is there still a need, that I look at #110 ?

@nitram509
Copy link
Author

@dscho Is there still a need, that I look at #86 ?

@dscho
Copy link
Member

dscho commented Jan 17, 2014

@nitram509 yes. The \\?\ prefix is still not part of https://github.com/msysgit/git, and even worse: we'll have to undo @vangdfang's branch and my workaround and start all over again (because there are circumstances when we should definitely not canonicalize).

@kblees kblees mentioned this pull request Jan 18, 2014
@brian-slate
Copy link

@marklagendijk +1!!

Any news on this? I'm working on a windows node app and I can't add my node_modules folder until this is patched. Thanks guys!

@donaldpipowitch
Copy link

+1

@dscho
Copy link
Member

dscho commented Feb 17, 2014

@unknpwn would be nice if you gave it a shot. It's as easy as installing the net installer and using the Git compiled inside that build environment to clone/check out.

@dscho
Copy link
Member

dscho commented Feb 17, 2014

Closed in favor of #122.

@dscho dscho closed this Feb 17, 2014
@caralluma
Copy link

Can someone provide me the Git preview version number which has this fix. I downloaded the latest Git-1.9.0-preview20140217.exe but the issue -Filename too long still exists with this. Is this fixed ?

error: unable to create file FunctionalTests/opalDomain/CheckOut/src/test/resources/FitNesseRoot/FunctionalTests/CheckOut/opalTests/ProcessOrderForCheckoutAsGuestUser/AdjustedQuantityScenarios/QualifiedToQuantityNormalItemUnavailableSplitItemAvailable/properties.xml (Filename too long)

@the-Arioch
Copy link

@SunilPrabhu for the future readers: Long Path functionality is disabled by default. How to enable it read in comments down the msysgit/msysgit#52

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.