Cherry-pick with symlinks fails horribly

Cherry-pick with symlinks fails horribly

[Alexander Gladysh]

Hi, list!

OS X 10.6.2
Git 1.7.0.2

I'm complaining about Git symlink handling again. This time it is cherry-pick.

In my repo I have a symlink pointing to a directory.

I swap symlink with the directory in a single commit.

Now, if I try to cherry-pick any later commit from the branch that has
that swap commit to a branch that have not, cherry-pick fails
horribly.

See script to reproduce the bug below (run it in a clean directory).

Output example:

$ git cherry-pick <SHA>

Automatic cherry-pick failed.  After resolving the conflicts,
mark the corrected paths with 'git add <paths>' or 'git rm <paths>'
and commit the result with:

        git commit -c 6a398597ce7a00fe05f43ff88808303eb151dfb5

$ git status # Note the "Untracked files" section

# On branch master
# Changes to be committed:
#   (use "git reset HEAD <file>..." to unstage)
#
#	renamed:    a/f -> f1
#
# Unmerged paths:
#   (use "git reset HEAD <file>..." to unstage)
#   (use "git add/rm <file>..." as appropriate to mark resolution)
#
#	added by us:        b/a
#
# Untracked files:
#   (use "git add <file>..." to include in what will be committed)
#
#	b/a~HEAD

(Also I've seen git reset --hard to fail afterwards, complaining it
can't delete a directory, but I can't reproduce it now.)

I see a similar behaviour if I try to do interactive rebase accross
symlink swap commit.

Alexander.

#! /bin/bash

git init

mkdir a
touch a/f
git add a
git commit -m "a"

mkdir b
ln -s ../a b/a
git add b
git commit -m "b"

git checkout -b branch
rm b/a
mv a b/
ln -s b/a a
git add .
git commit -m "swap"

touch f1
git add f1
git commit -m "f1"

git checkout master

git cherry-pick `git rev-parse branch` # This one breaks horribly

Re: Cherry-pick with symlinks fails horribly

[Alexander Gladysh]

Sorry to nag, but... any help?

Alexander.

On Tue, Mar 9, 2010 at 04:28, Alexander Gladysh <agladysh@gmail.com> wrote:
> Hi, list!
>
> OS X 10.6.2
> Git 1.7.0.2
>
> I'm complaining about Git symlink handling again. This time it is cherry-pick.
>
> In my repo I have a symlink pointing to a directory.
>
> I swap symlink with the directory in a single commit.
>
> Now, if I try to cherry-pick any later commit from the branch that has
> that swap commit to a branch that have not, cherry-pick fails
> horribly.
>
> See script to reproduce the bug below (run it in a clean directory).
>
> Output example:
>
> $ git cherry-pick <SHA>
>
> Automatic cherry-pick failed.  After resolving the conflicts,
> mark the corrected paths with 'git add <paths>' or 'git rm <paths>'
> and commit the result with:
>
>        git commit -c 6a398597ce7a00fe05f43ff88808303eb151dfb5
>
> $ git status # Note the "Untracked files" section
>
> # On branch master
> # Changes to be committed:
> #   (use "git reset HEAD <file>..." to unstage)
> #
> #       renamed:    a/f -> f1
> #
> # Unmerged paths:
> #   (use "git reset HEAD <file>..." to unstage)
> #   (use "git add/rm <file>..." as appropriate to mark resolution)
> #
> #       added by us:        b/a
> #
> # Untracked files:
> #   (use "git add <file>..." to include in what will be committed)
> #
> #       b/a~HEAD
>
> (Also I've seen git reset --hard to fail afterwards, complaining it
> can't delete a directory, but I can't reproduce it now.)
>
> I see a similar behaviour if I try to do interactive rebase accross
> symlink swap commit.
>
> Alexander.
>
> #! /bin/bash
>
> git init
>
> mkdir a
> touch a/f
> git add a
> git commit -m "a"
>
> mkdir b
> ln -s ../a b/a
> git add b
> git commit -m "b"
>
> git checkout -b branch
> rm b/a
> mv a b/
> ln -s b/a a
> git add .
> git commit -m "swap"
>
> touch f1
> git add f1
> git commit -m "f1"
>
> git checkout master
>
> git cherry-pick `git rev-parse branch` # This one breaks horribly
>

Re: Cherry-pick with symlinks fails horribly

[Christian Couder]

On Tuesday 09 March 2010 02:28:30 Alexander Gladysh wrote:
> Hi, list!
> 
> OS X 10.6.2
> Git 1.7.0.2
> 
> I'm complaining about Git symlink handling again. This time it is
>  cherry-pick.
> 
> In my repo I have a symlink pointing to a directory.
> 
> I swap symlink with the directory in a single commit.
> 
> Now, if I try to cherry-pick any later commit from the branch that has
> that swap commit to a branch that have not, cherry-pick fails
> horribly.
> 
> See script to reproduce the bug below (run it in a clean directory).
> 
> Output example:
> 
> $ git cherry-pick <SHA>
> 
> Automatic cherry-pick failed.  After resolving the conflicts,
> mark the corrected paths with 'git add <paths>' or 'git rm <paths>'
> and commit the result with:
> 
>         git commit -c 6a398597ce7a00fe05f43ff88808303eb151dfb5
> 
> $ git status # Note the "Untracked files" section
> 
> # On branch master
> # Changes to be committed:
> #   (use "git reset HEAD <file>..." to unstage)
> #
> #	renamed:    a/f -> f1
> #
> # Unmerged paths:
> #   (use "git reset HEAD <file>..." to unstage)
> #   (use "git add/rm <file>..." as appropriate to mark resolution)
> #
> #	added by us:        b/a
> #
> # Untracked files:
> #   (use "git add <file>..." to include in what will be committed)
> #
> #	b/a~HEAD
> 
> (Also I've seen git reset --hard to fail afterwards, complaining it
> can't delete a directory, but I can't reproduce it now.)
> 
> I see a similar behaviour if I try to do interactive rebase accross
> symlink swap commit.
> 
> Alexander.
> 
> #! /bin/bash
> 
> git init
> 
> mkdir a
> touch a/f
> git add a
> git commit -m "a"
> 
> mkdir b
> ln -s ../a b/a
> git add b
> git commit -m "b"
> 
> git checkout -b branch
> rm b/a
> mv a b/
> ln -s b/a a
> git add .
> git commit -m "swap"
> 
> touch f1
> git add f1
> git commit -m "f1"
> 
> git checkout master
> 
> git cherry-pick `git rev-parse branch` # This one breaks horribly

I can reproduce the bug here on Linux. And Git v1.6.0 has the same bug.
So I suspect an old bug in unpack_trees.c. I will try to have another look at 
it this evening, but I am not familiar with that code.

Thanks for the report,
Christian.

Re: Cherry-pick with symlinks fails horribly

[Alexander Gladysh]

On Thu, Mar 11, 2010 at 07:57, Christian Couder <chriscool@tuxfamily.org> wrote:
> On Tuesday 09 March 2010 02:28:30 Alexander Gladysh wrote:
>> I'm complaining about Git symlink handling again. This time it is
>>  cherry-pick.

> I can reproduce the bug here on Linux. And Git v1.6.0 has the same bug.
> So I suspect an old bug in unpack_trees.c. I will try to have another look at
> it this evening, but I am not familiar with that code.

I have found my old bug-report. There is even some patch in that thread.

http://thread.gmane.org/gmane.comp.version-control.git/120741/

Not sure if it is the same issue or if the patch was even merged in though...

HTH,
Alexander.

Re: Cherry-pick with symlinks fails horribly

[Christian Couder]

On Thursday 11 March 2010 13:16:09 Alexander Gladysh wrote:
> On Thu, Mar 11, 2010 at 07:57, Christian Couder <chriscool@tuxfamily.org> 
wrote:
> > On Tuesday 09 March 2010 02:28:30 Alexander Gladysh wrote:
> >> I'm complaining about Git symlink handling again. This time it is
> >>  cherry-pick.
> >
> > I can reproduce the bug here on Linux. And Git v1.6.0 has the same bug.
> > So I suspect an old bug in unpack_trees.c. I will try to have another
> > look at it this evening, but I am not familiar with that code.
> 
> I have found my old bug-report. There is even some patch in that thread.
> 
> http://thread.gmane.org/gmane.comp.version-control.git/120741/
> 
> Not sure if it is the same issue or if the patch was even merged in
>  though...

The patch was merged:

commit 77716755cbdf970fa0814a5f77c884b1f17693de
Author: Kjetil Barvik <barvik@broadpark.no>
Date:   Sun Jun 14 15:08:28 2009 +0200

    lstat_cache: guard against full match of length of 'name' parameter

so I think it is a different issue, but feel free to test.

Anyway when looking at t/t6035-merge-dir-to-symlink.sh, we can see that there 
are still 2 broken tests:

$ ./t6035-merge-dir-to-symlink.sh
*   ok 1: create a commit where dir a/b changed to symlink
*   ok 2: keep a/b-2/c/d across checkout
*   ok 3: checkout should not have deleted a/b-2/c/d
*   ok 4: setup for merge test
*   ok 5: do not lose a/b-2/c/d in merge (resolve)
*   still broken 6: do not lose a/b-2/c/d in merge (recursive)
*   ok 7: setup a merge where dir a/b-2 changed to symlink
*   ok 8: merge should not have conflicts (resolve)
*   still broken 9: merge should not have conflicts (recursive)
* still have 2 known breakage(s)
* passed all remaining 7 test(s)

So it looks like breakages in this area are known, though perhaps not your 
particular breakage.

Best regards,
Christian.

Re: Cherry-pick with symlinks fails horribly

[Junio C Hamano]

Christian Couder <chriscool@tuxfamily.org> writes:

> Anyway when looking at t/t6035-merge-dir-to-symlink.sh, we can see that
> there are still 2 broken tests:
>
> $ ./t6035-merge-dir-to-symlink.sh
> ...
> *   ok 5: do not lose a/b-2/c/d in merge (resolve)
> *   still broken 6: do not lose a/b-2/c/d in merge (recursive)
> *   ok 7: setup a merge where dir a/b-2 changed to symlink
> *   ok 8: merge should not have conflicts (resolve)
> *   still broken 9: merge should not have conflicts (recursive)
> * still have 2 known breakage(s)
> * passed all remaining 7 test(s)
>
> So it looks like breakages in this area are known, though perhaps not your 
> particular breakage.

The above shows that resolve passes the same tests that recursive fails,
which means that the breakage is likely to be in recursive, and not in
unpack-trees, as you seemt to have guessed earlier.  If cherry-pick were
still a shell script, we could easily test that conjecture by letting you
try running it using resolve instead of recursive, but things like that
has got a lot harder to do these days since many things were rewritten in
C (sigh).

It might not be a bad idea to teach a hidden primarily-for-debugging
option to "cherry-pick" to let it use resolve instead of recursive for
cases like this.

Re: Cherry-pick with symlinks fails horribly

[Christian Couder]

On Friday 12 March 2010 06:49:23 Junio C Hamano wrote:
> Christian Couder <chriscool@tuxfamily.org> writes:
> > Anyway when looking at t/t6035-merge-dir-to-symlink.sh, we can see that
> > there are still 2 broken tests:
> >
> > $ ./t6035-merge-dir-to-symlink.sh
> > ...
> > *   ok 5: do not lose a/b-2/c/d in merge (resolve)
> > *   still broken 6: do not lose a/b-2/c/d in merge (recursive)
> > *   ok 7: setup a merge where dir a/b-2 changed to symlink
> > *   ok 8: merge should not have conflicts (resolve)
> > *   still broken 9: merge should not have conflicts (recursive)
> > * still have 2 known breakage(s)
> > * passed all remaining 7 test(s)
> >
> > So it looks like breakages in this area are known, though perhaps not
> > your particular breakage.
> 
> The above shows that resolve passes the same tests that recursive fails,
> which means that the breakage is likely to be in recursive, and not in
> unpack-trees, as you seemt to have guessed earlier.  

Yes, you are right the breakage is in recursive as it works with resolve.

> If cherry-pick were
> still a shell script, we could easily test that conjecture by letting you
> try running it using resolve instead of recursive, but things like that
> has got a lot harder to do these days since many things were rewritten in
> C (sigh).
> 
> It might not be a bad idea to teach a hidden primarily-for-debugging
> option to "cherry-pick" to let it use resolve instead of recursive for
> cases like this.

I will send an RFC patch series to do that. I used it to check that the test 
case works with the resolve strategy.

Best regards,
Christian.