On Sun, Jun 30, 2019 at 8:01 AM Lars Schneider <larsxschneider@xxxxxxxxx> wrote:
> > On Jun 24, 2019, at 5:33 AM, Elijah Newren <newren@xxxxxxxxx> wrote:
> >
> > On Mon, Jun 24, 2019 at 5:05 AM Lars Schneider <larsxschneider@xxxxxxxxx> wrote:
> >>
> >> 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.
> >>
> >> ...
> >
> > 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.
>
> 060df6242281 is interesting! If I revert the changes in builtin/fast-export.c,
> then the "t9350:directory becomes symlink" test still passes nowadays.
>
> Plus, my my new test case passes too:
>
> test_expect_success 'when transforming a symlink to a directory' '
> test_create_repo src &&
>
> mkdir src/foo &&
> echo a_line >src/foo/file.txt &&
> git -C src add foo/file.txt &&
> git -C src commit -m 1st_commit &&
>
> ln -s src/foo src/bar &&
> git -C src add bar &&
> git -C src commit -m 2nd_commit &&
>
> rm src/bar &&
> mkdir src/bar &&
> echo b_line >src/bar/b_file.txt &&
> git -C src add . &&
> git -C src commit -m 3rd_commit &&
>
> test_create_repo dst &&
> git -C src fast-export --all &&
> git -C src fast-export --all | git -C dst fast-import &&
> git -C src show >expected &&
> git -C dst show >actual &&
> test_cmp expected actual
> '
>
> Do you think it would make sense to revert the qsort change
> in fast-export? I haven't bisected yet which other change made
> the qsort change obsolete.
No need to bisect; it was the other commit I pointed out in my last
email, commit 253fb5f8897d ("fast-import: Improve robustness when D->F
changes
provided in wrong order", 2010-07-09). The original bug was
fixed/worked around in two places, because Shawn specifically said the
fast-import side being more robust was okay but that fast-export was
buggy and needed fixing. See
https://public-inbox.org/git/20100706193455.GA19476@xxxxxxxxxxx/ and
the thread around there. (It'd be really nice to be able to cc Shawn
on this...sigh.) Reverting the fast-export change would be okay if we
replaced it with a better change (something like sorting deletes
before modifies, though maybe with extra work around renames as
discussion elsewhere in this thread is touching on), but if we
straight up revert that change it would leave fast-export in violation
of the stream format.
Hope that helps,
Elijah
