All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Sebastian Schuberth <sschuberth@gmail.com>
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Git Mailing List <git@vger.kernel.org>
Subject: Re: [PATCH] Fix safe_create_leading_directories() for Windows
Date: Thu, 02 Jan 2014 13:08:55 -0800	[thread overview]
Message-ID: <xmqqha9mozvc.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <52C5D0AB.7050309@gmail.com> (Sebastian Schuberth's message of "Thu, 02 Jan 2014 21:48:43 +0100")

Sebastian Schuberth <sschuberth@gmail.com> writes:

> 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.

  parent reply	other threads:[~2014-01-02 21:09 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-02 17:22 [PATCH] Fix safe_create_leading_directories() for Windows Sebastian Schuberth
2014-01-02 17:33 ` Johannes Schindelin
2014-01-02 18:11   ` Sebastian Schuberth
2014-01-02 18:18     ` John Keeping
2014-01-02 20:37       ` Sebastian Schuberth
2014-01-02 20:46     ` Johannes Schindelin
2014-01-07 15:43       ` Erik Faye-Lund
2014-01-07 17:56         ` Johannes Schindelin
2014-01-07 21:49           ` Sebastian Schuberth
2014-01-02 19:55   ` Junio C Hamano
2014-01-02 20:42     ` Johannes Schindelin
2014-01-02 20:48     ` Sebastian Schuberth
2014-01-02 20:54       ` [PATCH v2] " Sebastian Schuberth
2014-01-02 21:10         ` Johannes Schindelin
2014-01-02 21:24         ` Junio C Hamano
2014-01-02 21:51           ` Junio C Hamano
2014-01-02 22:12             ` Junio C Hamano
2014-01-02 21:08       ` Junio C Hamano [this message]
2014-01-02 21:19         ` [PATCH] " Johannes Schindelin
2014-01-19  7:00           ` Sebastian Schuberth
2014-01-19  7:26         ` Sebastian Schuberth
2014-01-21 20:09           ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqha9mozvc.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=sschuberth@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.