From: Junio C Hamano <gitster@pobox•com>
To: Linus Torvalds <torvalds@linux-foundation•org>
Cc: "Pickens\, James E" <james.e.pickens@intel•com>,
"git\@vger.kernel.org" <git@vger•kernel.org>,
Kjetil Barvik <barvik@broadpark•no>,
Michael J Gruber <git@drmicha•warpmail.net>
Subject: Re: [PATCH v3] Demonstrate bugs when a directory is replaced with a symlink
Date: Wed, 29 Jul 2009 23:05:06 -0700 [thread overview]
Message-ID: <7vfxceln8t.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <alpine.LFD.2.01.0907291440480.3161@localhost.localdomain> (Linus Torvalds's message of "Wed\, 29 Jul 2009 15\:08\:12 -0700 \(PDT\)")
Linus Torvalds <torvalds@linux-foundation•org> writes:
> This patch should fix the 'checkout' issue.
>
> I made it use a new generic helper function ("check_path()"), since there
> are other cases like this that use just 'lstat()', and I bet we want to
> change that.
>
> The 'merge' issue is different, though: it's not due to a blind 'lstat()',
> but due to a blind 'unlink()' done by 'remove_path()'. I think
> 'remove_path()' should be taught to look for symlinks, and remove just the
> symlink - but that's a bit more work, especially since the symlink cache
> doesn't seem to expose any way to get the "what is the first symlink path"
> information.
I think this is a good thing to do regardless, but the James's "checkout"
test fails for an unrelated reason.
The tree has
120000 blob a36b773 a/b -> b-2
100644 blob e69de29 a/b-2/c/d
100644 blob e69de29 a/x
checked out, and wants to switch to
100644 blob e69de29 a/b-2/c/d
100644 blob e69de29 a/b/c/d
100644 blob e69de29 a/x
checkout_entry() is called to check out "a/b/c/d". If "a/b" symlink
were still there, the lstat() you fixed will be fooled.
But in James's test, because the symlink "a/b" is tracked in the
switched-from commit and is being obliterated by switching to a tree that
has a directory there, we (should) have called deleted_entry() on a/b to
mark it for removal, and inside check_updates() before going into the loop
to call checkout_entry(), we would have already removed the symlink "a/b"
that is going away inside unlink_entry().
The problem is that has_symlink_or_noent_leading_path() called from there
lies, without Kjetil's fix c52dc70 (lstat_cache: guard against full match
of length of 'name' parameter, 2009-06-14) that is in 'pu'.
If the original tree in the test did not have "a/b" tracked, but has an
untracked symlink "a/b" that points at b-2, then "a/b" will stay in the
work tree when the codepath your patch touches is reached, and the problem
will be demonstrated. Your patch will fix that issue.
So both fixes are necessary, and we need a separate test to illustrate
what your patch fixes.
I'll push out some updates.
prev parent reply other threads:[~2009-07-30 6:05 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-07-28 22:13 More symlink/directory troubles James Pickens
2009-07-28 22:13 ` [PATCH 1/2] Demonstrate bugs when a directory is replaced with a symlink James Pickens
2009-07-28 22:13 ` [PATCH 2/2] Demonstrate merge failure " James Pickens
2009-07-29 8:29 ` Michael J Gruber
2009-07-29 16:39 ` Pickens, James E
2009-07-29 8:19 ` [PATCH 1/2] Demonstrate bugs " Michael J Gruber
2009-07-29 8:33 ` Junio C Hamano
2009-07-29 16:57 ` Pickens, James E
2009-07-29 17:48 ` [PATCH v2] " Pickens, James E
2009-07-29 18:31 ` Junio C Hamano
2009-07-29 21:02 ` [PATCH v3] " Pickens, James E
2009-07-29 22:08 ` Linus Torvalds
2009-07-29 22:44 ` Junio C Hamano
2009-07-29 23:01 ` Kjetil Barvik
2009-07-29 23:58 ` Linus Torvalds
2009-07-30 1:06 ` Linus Torvalds
2009-07-30 3:26 ` Junio C Hamano
2009-07-30 6:05 ` Junio C Hamano [this message]
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=7vfxceln8t.fsf@alter.siamese.dyndns.org \
--to=gitster@pobox$(echo .)com \
--cc=barvik@broadpark$(echo .)no \
--cc=git@drmicha$(echo .)warpmail.net \
--cc=git@vger$(echo .)kernel.org \
--cc=james.e.pickens@intel$(echo .)com \
--cc=torvalds@linux-foundation$(echo .)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