Re: [BUG] Symbolic links break "git fast-export"?

From: Elijah Newren <newren@gmail.com>
Date: Jun 24, 2019, 12:33 UTC
Message-ID: <CABPp-BE8um5g98jqWawsuG2dAvO6AZcR54vrRzAkJbq+L3K6Zw@mail.gmail.com>
In-Reply-To: <95EF0665-9882-4707-BB6A-94182C01BE91@gmail.com>

On Mon, Jun 24, 2019 at 5:05 AM Lars Schneider <larsxschneider@gmail.com> wrote:

Show 47 quoted lines

>
> Hi folks,
>
> Is my understanding correct, that `git fast-export | git fast-import`
> should not modify the repository? If yes, then we might have a bug in
> `git fast-export` if symbolic directory links are removed and converted
> to a real directory.
>
> Consider this test case:
>
>     # Create test repo
>     git init .
>     mkdir foo
>     echo "foo" >foo/baz
>     git add .
>     git commit -m "add foo dir"
>     ln -s foo bar
>     git add .
>     git commit -m "add bar dir as link"
>     rm bar
>     mkdir bar
>     echo "bar" >bar/baz
>     git add .
>     git commit -m "remove link and make bar dir real"
>
>     printf "BEFORE: "
>     git rev-parse HEAD
>
>     # Fast export, import ... that should not change anything!
>     git fast-export --no-data --all --signed-tags=warn-strip \
>         --tag-of-filtered-object=rewrite | git fast-import --force --quiet
>
>     printf "AFTER: "
>
> I would assume that the BEFORE/AFTER hashes match. Unfortunately, with
> Git 2.22.0 they do no. The problem is this export output I think:
>
>     remove link and make bar dir real
>     from :2
>     M 100644 5716ca5987cbf97d6bb54920bea6adde242d87e6 bar/baz
>     D bar
>
> The new file in the `bar` directory is added to the repo first and
> afterwards the path `bar` is deleted. I think that deletes the entire
> directory `bar`?
>
> If you confirm that this is a bug, then I will try to provide a fix.

My first reaction was, "we regressed on this again?", but it looks like my original fix for directory/file changes only handled one direction. Thus, my commit 060df6242281 ("fast-export: Fix output order of D/F changes", 2010-07-09) probably *caused* this bug. We should probably just sort not based on filename, but on changetype -- send all the deletes to fast-import before we send the modifies.

We should probably also make a corresponding improvement to fast-import; it also makes some attempts to be smart about handling order of modifies and deletes, but misses this case. See commit 253fb5f8897d ("fast-import: Improve robustness when D->F changes provided in wrong order", 2010-07-09). It'd be nice if fast-import could go through the list of changes, apply the deletes first, then the modifies -- although I'm not sure where renames go in the order off the top of my head.

Thanks for flagging this and working on it.

Elijah