Re: [PATCH] Fix safe_create_leading_directories() for Windows

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 2, 2014, 21:08 UTC
Message-ID: <xmqqha9mozvc.fsf@gitster.dls.corp.google.com>
In-Reply-To: <52C5D0AB.7050309@gmail.com>

Sebastian Schuberth <sschuberth@gmail.com> writes:

Show 15 quoted lines

> On 02.01.2014 20:55, Junio C Hamano wrote:
>
>> Thanks; the conclusion is correct --- you need a good commit
>> message in the recorded history.  That does not have anything to do
>> with integrating with pulling from subsystem maintainers, which we
>> regularly do.
>
> I'll send a v2 which adds a proper commits message inline.
>
>> Perhaps with s|Linux|POSIX|, but more importantly, was there a
>> specific error scenario that triggered this change?
>
> Yes, cloning to a non-existent directory with Windows paths fails like:
>
> fatal: could not create work tree dir 'c:\foo\bar'.: No such file or directory

OK. That was why I wanted to see (and Dscho correctly guessed) a good description in the proposed log message to see what problem the change is trying to address, so that we can judge if the change is tackling the right problem.

> Seems like the path to clone to is taken as-is from argv in
> cmd_clone(). So maybe another solution would be to replace all
> backslashes with forward slashes already there?

That sounds like a workable alternative, and it might even be a preferable solution but if and only if clone's use of the function to create paths that lead to a new working tree location is the only (ab)use of the function. That was what I meant when I said "it may be that it is a bug in the specific caller". AFAIK, the function was meant to be used only on paths we internally generate, and the paths we internally generate all are slash separated, in line with how paths are stored in the index.

If we are going to change the meaning of the function so that it can now take any random path in platform-specific convention that may be incompatible with the internal notion of paths Git has (i.e. what is passed to safe_create_leading_directories() may have to be massaged into a slash-separated form before it can be used in the index and parsed to be stuffed into trees), it is fine to do so as long as all the codepaths understands the new world order, but my earlier "git grep" hits did not tell me that such a change is warranted.

Thanks.