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

Alexander Gavrilov [off-list ref] writes:

Although it does not matter for Git itself, tools that
export to systems that explicitly track copies and
renames can benefit from such information.

This patch makes fast-export output correct action
logs when -M or -C are enabled.

Signed-off-by: Alexander Gavrilov <redacted>
---

	On Sunday 27 July 2008 00:21:03 Shawn O. Pearce wrote:
	> Do you mean to say git-fast-export in the end of the first line of
	> that last paragraph?

	Yes, of course. Thank you.

Alexander, Shawn, what is the status of this patch?  Has it been reviewed
sufficiently and is ready for application?

I think it is more fashionable to do what 3240240 (Docs: Use
"-l::\n--long\n" format in OPTIONS sections, 2008-06-08) did these days,
i.e.:

	-M::
        -C::
        	Description...

++
+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.

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).

These are somewhat unusual.  Was there any reason for these exports and
config?

Junio C Hamano [off-list ref] wrote:

Alexander Gavrilov [off-list ref] writes:

quoted

Although it does not matter for Git itself, tools that
export to systems that explicitly track copies and
renames can benefit from such information.

This patch makes fast-export output correct action
logs when -M or -C are enabled.

Signed-off-by: Alexander Gavrilov <redacted>
---

	On Sunday 27 July 2008 00:21:03 Shawn O. Pearce wrote:
	> Do you mean to say git-fast-export in the end of the first line of
	> that last paragraph?

	Yes, of course. Thank you.

Alexander, Shawn, what is the status of this patch?  Has it been reviewed
sufficiently and is ready for application?

It looked OK to me on the surface, except the one minor remark in
the documentation I noted above.  Other than that I am not very good
with the internal diff machinary so my ACK/NACK on such matters is
worth very little, if anything.
 
-- 
Shawn.

On Tue, Jul 29, 2008 at 8:11 PM, Junio C Hamano [off-list ref] wrote:

Alexander Gavrilov [off-list ref] writes:

quoted

++
+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.

quoted

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.