All of lore.kernel.org
 help / color / mirror / Atom feed
From: Clemens Buchacher <drizzd@aon.at>
To: Junio C Hamano <gitster@pobox.com>
Cc: Johannes Sixt <j6t@kdbg.org>, git@vger.kernel.org
Subject: Re: [PATCH] do not overwrite untracked symlinks
Date: Tue, 22 Feb 2011 20:26:32 +0100	[thread overview]
Message-ID: <20110222192632.GB4881@localhost> (raw)
In-Reply-To: <7v62scftjl.fsf@alter.siamese.dyndns.org>

On Mon, Feb 21, 2011 at 10:54:38PM -0800, Junio C Hamano wrote:
> 
> Yes, I agree with that reading.  I'd squash in this on top, first making
> sure that "do not overwrite untracked symlinks" (which is the title of
> this patch) won't be broken, and then the original test that wanted to
> make sure that checking out a/b/c/d when a/b is pointing to an unrelated
> part of the tree does not nuke whatever is pointed at that symbolic link.

Ok, perfect.

> My unhappiness with the undescriptive "check_leading_path()" still
> remains, though...

Ok. If I can't figure anything out this weekend, I'll rename it.
Promise.

It's just that I'm even more unhappy about the actual code rather
than the function name.  And each time I look at it I end up trying
to fix it, only to find out that it affects so many parts of the
code that don't make any sense. 

For example, we already have a function check_path() in entry.c,
which does something very similar and is called by
checkout_entry(), which in turn does something similar to
verify_absent(). In fact, I suspect that both functions are called
twice by the same code path around unpack_trees, for the same paths
and for the same reason.

I strongly feel that we should separate the merge process into two
steps.

 - First, do everything in the index, ignoring the work tree.

 - Second, checkout the index to the work tree while making sure no
   changes or untracked files get overwritten.

I know I have talked about this before, and I don't know if I will
ever find the time to implement such a major change. But I can't
get myself to feel good about fixing the function name, but not the
function.

Clemens

  reply	other threads:[~2011-02-22 19:24 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-02 19:25 untracked symlinks are less precious than untracked files? Johannes Sixt
2011-02-02 20:03 ` Junio C Hamano
2011-02-02 22:24   ` Johannes Sixt
2011-02-05 18:18     ` [PATCH] Demonstrate breakage: checkout overwrites untracked symlink with directory Johannes Sixt
2011-02-05 18:33       ` Clemens Buchacher
2011-02-09 23:48         ` Junio C Hamano
2011-02-10 21:49           ` Clemens Buchacher
2011-02-20 12:13         ` [PATCH] do not overwrite untracked symlinks Clemens Buchacher
2011-02-21  7:15           ` Junio C Hamano
2011-02-21 19:46             ` Clemens Buchacher
2011-02-22  6:54               ` Junio C Hamano
2011-02-22 19:26                 ` Clemens Buchacher [this message]
2011-02-22 20:01                   ` Junio C Hamano
2011-02-15  7:24       ` [PATCH] Demonstrate breakage: checkout overwrites untracked symlink with directory Johannes Sixt

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=20110222192632.GB4881@localhost \
    --to=drizzd@aon.at \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j6t@kdbg.org \
    /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.