Re: [PATCH v2] Support copy and rename detection in fast-export.

["Alexander Gavrilov" <angavrilov@gmail.com>]

On Tue, Jul 29, 2008 at 8:11 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Alexander Gavrilov <angavrilov@gmail.com> writes:
>> ++
>> +Note that these options were always accepted by git-fast-export,
>> +but before a certain version it silently produced wrong results.
>> +You should always check the git version before using them.
>> +
>
> I do not quite follow the mention of "before a certain version", but I
> think it is talking about the earlier "fast-export" that took any diff
> options but did not act differently upon renamed/copied entries.  If that
> is the case, I think we can say something like this instead:
>
>        Note that earlier versions of this command did not complain and
>        produced incorrect results if you gave these options.
>
> because docs always talk about the current version.  My reading of Dscho's
> original 'show_filemodify' suggests me that "wrong results" does not just
> mean missing rename/copy information but a renamed old entity did not get
> removed correctly, resulting in an incorrect tree in the commit, right?
> Maybe we would want to be a bit more explicit about what kind of breakage
> you are talking about here.

Yes, broken renames is what I've been thinking of when I wrote that.

As fast-export is mainly meant to be used by third-party conversion
scripts, which are not bundled together with git, unsuspecting users
might try to run them using an old git version. The main point of my
note is that scripts should always check the version if they want to
use these options. It probably should also specify the exact value to
compare to, e.g:

        Note that before git 1.6 this command did not complain and produced
        incorrect results if you gave these options. Your scripts should always
        check the version before using them.


> If you see a copied or renamed entry, you emit "this old path to that old
> path" first, and then say that same path got modified.  It appears from my
> quick glance of fast-import that touching the same path more than once in
> a same commit like this sequence does would work fine (it will involve two
> calls to tree_content_set() for the same path but that is not something it
> has to forbid, and the function doesn't).

I'm sorry, but I don't quite understand what are you suggesting by
this paragraph. Yes, fast-import understands double modification, and
my test includes a check for this case. Of course, conversion scripts
for dumber targets might need to do one-line lookahead to eliminate
non-100% copies.


>> diff --git a/t/t9301-fast-export.sh b/t/t9301-fast-export.sh
>> index f18eec9..bb595b7 100755
>> --- a/t/t9301-fast-export.sh
>> +++ b/t/t9301-fast-export.sh
>> @@ -162,4 +162,50 @@ test_expect_success 'submodule fast-export | fast-import' '
>>
>>  '
>>
>> +export GIT_AUTHOR_NAME='A U Thor'
>> +export GIT_COMMITTER_NAME='C O Mitter'
>> +
>> +test_expect_success 'setup copies' '
>> +
>> +     git config --unset i18n.commitencoding &&
>
> These are somewhat unusual.  Was there any reason for these exports and
> config?
>

t9301-fast-export.sh earlier changes these parameters to test
automatic conversion to utf8. I reset them back before my test, in
order to be able to test the import by comparing SHA-1 (encoding
conversion changes it). There may be a better way to do it, though.