public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox•com>
To: Clemens Buchacher <drizzd@aon•at>
Cc: git@vger•kernel.org
Subject: Re: [PATCH] ignore trailing slash when creating leading directories
Date: Tue, 02 Sep 2008 13:36:38 -0700	[thread overview]
Message-ID: <7vod36jne1.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <20080902191322.GA11172@localhost> (Clemens Buchacher's message of "Tue, 2 Sep 2008 21:13:22 +0200")

Clemens Buchacher <drizzd@aon•at> writes:

> Unfortunately, if we simply add strerror to the error message, in place of
>
> fatal: could not create work tree dir 'path/'.
>
> the new version would print
>
> fatal: could not create work tree dir 'path/': File exists.
>
> which makes things worse IMO. We could of course strip trailing slashes in
> builtin-clone.c for now and revert that as soon as the cleanup patch is in,

I think the above statement is a wrong way to think about it.

This particular caller (and nobody else), even when it does not want the
"create all directories" behaviour, is giving such an instruction to the
existing function, without validating the user input.  That is a bug that
needs to be fixed, regardless of what the final semantics of c-l-d should
be, isn't it?

> but I think it's not worth the trouble.

What I think is not worth the trouble is, after fixing this caller, to
revert that fix, when/if we decide that c-l-d should strip trailing
slashes for all callers.

I am not saying the current semantics of c-l-d is optimal.  I am just
worried about breaking people's private patches that may depend on the
current behaviour.  It would be nice if we can clean it up without
breaking people, and after doing so, if somebody really want to have
"create all directories, the last one is also a directory", they can
invent a cleaner function that takes that insn as a separate paremeter,
not as an obscure "trailing slash means create all", which may be cute but
not clean nor clear at all, which is what c-l-d does.

  parent reply	other threads:[~2008-09-02 20:37 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-09-02  8:19 [PATCH] ignore trailing slash when creating leading directories Clemens Buchacher
2008-09-02 18:38 ` Junio C Hamano
2008-09-02 19:13   ` Clemens Buchacher
2008-09-02 20:07     ` [PATCH v2] ignore trailing slashes " Clemens Buchacher
2008-09-02 20:36     ` Junio C Hamano [this message]
2008-09-02 21:10       ` [PATCH] ignore trailing slash " Junio C Hamano
2008-09-03 19:02         ` Clemens Buchacher
2008-09-03 18:55   ` [PATCH] clone: fix creation of explicitly named target directory Clemens Buchacher
2008-09-03 19:24     ` 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=7vod36jne1.fsf@gitster.siamese.dyndns.org \
    --to=gitster@pobox$(echo .)com \
    --cc=drizzd@aon$(echo .)at \
    --cc=git@vger$(echo .)kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox