git-cvsimport "you may need to merge manually"

git-cvsimport "you may need to merge manually"

[Randal L. Schwartz]

Starting recently, git-cvsimport has always ended with "you
may need to merge manually".  Why?  It worked before.  What
is it doing now?

    $ cd /home/merlyn/Git/stonehenge.git || exit 1
    $ git-cvsimport -v -k -o master -d /web/cvs stonehenge
    .
    .
    .
    skip patchset 323: 1142297290 before 1142351976
    Fetching htdocs/courses.html   v 1.4
    Update htdocs/courses.html: 8767 bytes
    Fetching htdocs/rates.html   v 1.2
    Update htdocs/rates.html: 2043 bytes
    Tree ID 38280334da5eaa4fd80fe1011e63db4b527f1d13
    Parent ID e93de754181fe963b8623423f509540021caead0
    Committed patch 324 (master 2006-03-16 18:14:31)
    Commit ID 985ca72d4e8a07d4189794231f035bac63c9e91d
    DONE; you may need to merge manually.
    $ git-status
    #
    # Updated but not checked in:
    #   (will commit)
    #
    #       modified: htdocs/courses.html
    #       modified: htdocs/rates.html
    #

Right... why didn't it commit those?  I'm having to issue "fake"
commits now, so I've lost the cvs-log comments that were formerly
very complete and cool.

-- 
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Perl/Unix/security consulting, Technical writing, Comedy, etc. etc.
See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training!

Re: git-cvsimport "you may need to merge manually"

[Junio C Hamano]

merlyn@stonehenge.com (Randal L. Schwartz) writes:

> Starting recently, git-cvsimport has always ended with "you
> may need to merge manually".  Why?  It worked before.  What
> is it doing now?
>
>     $ cd /home/merlyn/Git/stonehenge.git || exit 1
>     $ git-cvsimport -v -k -o master -d /web/cvs stonehenge
>     .
>     .
>     .
>     skip patchset 323: 1142297290 before 1142351976
>     Fetching htdocs/courses.html   v 1.4
>     Update htdocs/courses.html: 8767 bytes
>     Fetching htdocs/rates.html   v 1.2
>     Update htdocs/rates.html: 2043 bytes
>     Tree ID 38280334da5eaa4fd80fe1011e63db4b527f1d13
>     Parent ID e93de754181fe963b8623423f509540021caead0
>     Committed patch 324 (master 2006-03-16 18:14:31)
>     Commit ID 985ca72d4e8a07d4189794231f035bac63c9e91d
>     DONE; you may need to merge manually.
>     $ git-status
>     #
>     # Updated but not checked in:
>     #   (will commit)
>     #
>     #       modified: htdocs/courses.html
>     #       modified: htdocs/rates.html
>     #
>
> Right... why didn't it commit those?  I'm having to issue "fake"
> commits now, so I've lost the cvs-log comments that were formerly
> very complete and cool.

I do not use cvsimport but I wonder what Commit ID 985ca7
contains.  Can you try "git show 985ca7" and see if that is the
commit you want?

Also, which branch are you on when you run git-cvsimport, and
which branch did the commit 985ca7 go?  I suspect it is storing
the tip commit 985ca7 to a branch that you are _not_ on
currently, but refraining from merging that to your current
branch, or something like that.

Re: git-cvsimport "you may need to merge manually"

[Randal L. Schwartz]

>>>>> "Junio" == Junio C Hamano <junkio@cox.net> writes:

>> Right... why didn't it commit those?  I'm having to issue "fake"
>> commits now, so I've lost the cvs-log comments that were formerly
>> very complete and cool.

Junio> I do not use cvsimport but I wonder what Commit ID 985ca7
Junio> contains.  Can you try "git show 985ca7" and see if that is the
Junio> commit you want?

To bring the tree clean, I issued "git commit -a -m 'cvs sync'", and
"git-whatchanged -p" shows the most recent commit with the diff actually
*backed out* the most recent change in CVS, and the next change down was
actually the good one.  Weird.  Do I need to start all over again?

Junio> Also, which branch are you on when you run git-cvsimport, and
Junio> which branch did the commit 985ca7 go?  I suspect it is storing
Junio> the tip commit 985ca7 to a branch that you are _not_ on
Junio> currently, but refraining from merging that to your current
Junio> branch, or something like that.

Nope.  I'm on "master", and it should be writing "master".

-- 
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Perl/Unix/security consulting, Technical writing, Comedy, etc. etc.
See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training!

Re: git-cvsimport "you may need to merge manually"

[Randal L. Schwartz]

>>>>> "Randal" == Randal L Schwartz <merlyn@stonehenge.com> writes:

Randal> To bring the tree clean, I issued "git commit -a -m 'cvs sync'", and
Randal> "git-whatchanged -p" shows the most recent commit with the diff actually
Randal> *backed out* the most recent change in CVS, and the next change down was
Randal> actually the good one.  Weird.  Do I need to start all over again?

Junio> Also, which branch are you on when you run git-cvsimport, and
Junio> which branch did the commit 985ca7 go?  I suspect it is storing
Junio> the tip commit 985ca7 to a branch that you are _not_ on
Junio> currently, but refraining from merging that to your current
Junio> branch, or something like that.

Randal> Nope.  I'm on "master", and it should be writing "master".

And now for grins, I did "git-reset --hard 'HEAD^^^^^^^'", verified
that the data was in a sensible state from a few weeks back, then
did the git-cvsimport, and it replayed the CVS changes to bring it current.
git-status showed that seven files were dirty, and "git-reset --hard"
cleared that, and the files were in the right state.

Huh?

-- 
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Perl/Unix/security consulting, Technical writing, Comedy, etc. etc.
See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training!

Re: git-cvsimport "you may need to merge manually"

[Junio C Hamano]

merlyn@stonehenge.com (Randal L. Schwartz) writes:

> Junio> I do not use cvsimport but I wonder what Commit ID 985ca7
> Junio> contains.  Can you try "git show 985ca7" and see if that is the
> Junio> commit you want?
>
> To bring the tree clean, I issued "git commit -a -m 'cvs sync'", and
> "git-whatchanged -p" shows the most recent commit with the diff actually
> *backed out* the most recent change in CVS, and the next change down was
> actually the good one.  Weird.  Do I need to start all over again?

So the commit ID reported before the command says Done. was
actually what you wanted, but that latest tree was not checked
out cvsimport, so "git status" reported "you have these changes"
(it should have said "you have these backed-out", but there is
no way for it to know), and obviously your commit on top of that
is to back it out.

I just tried it on a copy of my day-job CVS repository.  Indeed
what it does seem quite strange.

    ...
    skip patchset 1019: 1142562992 before 1142562992
    skip patchset 1020: 1142563093 before 1142563093
    Fetching Makefile   v 1.10
    Update Makefile: 871 bytes
    Tree ID 4c51717e01fb08b6d15bc6e35d48142d2d5b94e7
    Parent ID 98d49aa57b93552cea82ce20c880bad4bcc5ebfc
    Committed patch 1021 (master 2006-03-17 03:04:31)
    Commit ID ad6c4fb908df7057d5564bf22cf9e0f9f3e743f2
    DONE


It updated the 'master' branch head, it reported the tip commit
ID before saying DONE, and that commit ID is stored in
refs/heads/master.  However, it does not seem to touch index nor
working tree files (I did not get "you may need to do merge"
message, by the way).

Since I know I was just trying things out, I do not care what is
in my index nor in the working tree files, so I could at this
point say:

	$ git reset --hard

to sync the master tree, but in general you may be running
cvsimport into a dirty tree, in which case you are screwed.

I think reverting this commit would help, but I do not remember
offhand what the breakage this patch was trying to fix.

    commit a541211ef4136eb7464c4466d20b60b8580efc44
    Author: Matthias Urlichs <smurf@smurf.noris.de>
    Date:   Tue Mar 7 10:08:34 2006 +0100

        cvsimport: Remove master-updating code

        The code which tried to update the master branch was somewhat broken.
        => People should do that manually, with "git merge".

        Signed-off-by: Matthias Urlichs <smurf@smurf.noris.de>
        Signed-off-by: Junio C Hamano <junkio@cox.net>

... Ugh, and after I wrote all of the above, the version of git
I have at work predates that commit X-<.  So maybe this was
fixed with that commit, and you are expected to say:

	$ git pull . origin

assuming that you are on "master" branch and cvsimoprt tracks
CVS head with "origin" branch, that is.

Smurf, help?

Re: git-cvsimport "you may need to merge manually"

[Randal L. Schwartz]

>>>>> "Junio" == Junio C Hamano <junkio@cox.net> writes:

Junio> Since I know I was just trying things out, I do not care what is
Junio> in my index nor in the working tree files, so I could at this
Junio> point say:

Junio> 	$ git reset --hard

Junio> to sync the master tree, but in general you may be running
Junio> cvsimport into a dirty tree, in which case you are screwed.

Yeah, this doesn't make sense.  It used to "Just Work".  I can
certainly add "git reset --hard" to my workflow, if that's the real
work around.  And if so, the manpage should document that.

Junio> ... Ugh, and after I wrote all of the above, the version of git
Junio> I have at work predates that commit X-<.  So maybe this was
Junio> fixed with that commit, and you are expected to say:

Junio> 	$ git pull . origin

Junio> assuming that you are on "master" branch and cvsimoprt tracks
Junio> CVS head with "origin" branch, that is.

Again, if that's the case, the manpage should say what's really
happening, so as not to confuse dumb people like me. :)

-- 
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Perl/Unix/security consulting, Technical writing, Comedy, etc. etc.
See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training!

Re: git-cvsimport "you may need to merge manually"

[Junio C Hamano]

merlyn@stonehenge.com (Randal L. Schwartz) writes:

> Yeah, this doesn't make sense.  It used to "Just Work".  I can
> certainly add "git reset --hard" to my workflow, if that's the real
> work around.  And if so, the manpage should document that.

That should _not_ be the solution.

I think the behaviour we are seeing does not make much sense.
I'll take a deeper look at it tonight (or tomorrow if I am
unlucky), if nobody beats me to it.

Sorry about the breakage.

Re: git-cvsimport "you may need to merge manually"

[Randal L. Schwartz]

>>>>> "Junio" == Junio C Hamano <junkio@cox.net> writes:

Junio> I think the behaviour we are seeing does not make much sense.
Junio> I'll take a deeper look at it tonight (or tomorrow if I am
Junio> unlucky), if nobody beats me to it.

Junio> Sorry about the breakage.

No, *thanks* for looking at it.  Your tireless contributions
are well appreciated.

-- 
Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095
<merlyn@stonehenge.com> <URL:http://www.stonehenge.com/merlyn/>
Perl/Unix/security consulting, Technical writing, Comedy, etc. etc.
See PerlTraining.Stonehenge.com for onsite and open-enrollment Perl training!

Re: git-cvsimport "you may need to merge manually"

[Junio C Hamano]

I tested the version in today's "next" branch, and it fixes the
problem for me in my tests that use my day-job CVS repository as
a guinea pig.

In case you are not brave enough to switch to "next" wholesale,
but are kind enough to try out the relevant cvsimport change for
us, here is a patch that applies on top of "master".  It is a
roll-up of two commits on my jc/cvsimport branch.

-- >8 --
cvsimport: honor -i and non -i upon subsequent imports

Documentation says -i is "import only", so without it,
subsequent import should update the current branch and working
tree files in a sensible way.

"A sensible way" defined by this commit is "act as if it is a
git pull from foreign repository which happens to be CVS not
git".  So:

 - If importing into the current branch (note that cvsimport
   requires the tracking branch is pristine -- you checked out
   the tracking branch but it is your responsibility not to make
   your own commits there), fast forward the branch head and
   match the index and working tree using two-way merge, just
   like "git pull" does.

 - If importing into a separate tracking branch, update that
   branch head, and merge it into your current branch, again,
   just like "git pull" does.

Initial round of the updated code read the tip of the current
branch before and after the import runs, but forgot to chomp
what we read from the command.  The read-tree command did not
them with the trailing LF.  This has been fixed.

---

diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 02d1928..3728294 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -453,6 +453,7 @@ chdir($git_tree);
 my $last_branch = "";
 my $orig_branch = "";
 my %branch_date;
+my $tip_at_start = undef;
 
 my $git_dir = $ENV{"GIT_DIR"} || ".git";
 $git_dir = getwd()."/".$git_dir unless $git_dir =~ m#^/#;
@@ -487,6 +488,7 @@ unless(-d $git_dir) {
 		$last_branch = "master";
 	}
 	$orig_branch = $last_branch;
+	$tip_at_start = `git-rev-parse --verify HEAD`;
 
 	# populate index
 	system('git-read-tree', $last_branch);
@@ -873,7 +875,22 @@ if (defined $orig_git_index) {
 
 # Now switch back to the branch we were in before all of this happened
 if($orig_branch) {
-	print "DONE; you may need to merge manually.\n" if $opt_v;
+	print "DONE.\n" if $opt_v;
+	if ($opt_i) {
+		exit 0;
+	}
+	my $tip_at_end = `git-rev-parse --verify HEAD`;
+	if ($tip_at_start ne $tip_at_end) {
+		for ($tip_at_start, $tip_at_end) { chomp; }
+		print "Fetched into the current branch.\n" if $opt_v;
+		system(qw(git-read-tree -u -m),
+		       $tip_at_start, $tip_at_end);
+		die "Fast-forward update failed: $?\n" if $?;
+	}
+	else {
+		system(qw(git-merge cvsimport HEAD), "refs/heads/$opt_o");
+		die "Could not merge $opt_o into the current branch.\n" if $?;
+	}
 } else {
 	$orig_branch = "master";
 	print "DONE; creating $orig_branch branch\n" if $opt_v;

Re: git-cvsimport "you may need to merge manually"

[smurf]

[-- Attachment #1: Type: text/plain, Size: 910 bytes --]

Hi,

Randal L. Schwartz:
> Yeah, this doesn't make sense.  It used to "Just Work".  I can
> certainly add "git reset --hard" to my workflow, if that's the real
> work around.  And if so, the manpage should document that.
> 
The real workaround is not to import into live branches,
which is not a git-cvs-specific problem, so I didn't add that
to its manpage.

I'm not opposed to an appropriate patch if you think its necessary.
If you do, keep in mind that there are other git-*import scripts out
there which presumably have the same problem. ;-)

-- 
Matthias Urlichs   |   {M:U} IT Design @ m-u-it.de   |  smurf@smurf.noris.de
Disclaimer: The quote was selected randomly. Really. | http://smurf.noris.de
 - -
That which is good to be done, cannot be done too soon; and if it is
neglected to be done early, it will frequently happen that it will not be done
at all.
					-- Bishop Mant

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 191 bytes --]

Re: git-cvsimport "you may need to merge manually"

[Junio C Hamano]

smurf@smurf.noris.de writes:

> The real workaround is not to import into live branches,
> which is not a git-cvs-specific problem, so I didn't add that
> to its manpage.

Yes.  I sufferred the same problem when I did git-fetch/git-pull,
and there is a clever/ugly workaround for that.

Re: git-cvsimport "you may need to merge manually"

[smurf]

[-- Attachment #1: Type: text/plain, Size: 782 bytes --]

Hi,

Junio C Hamano:
> ... Ugh, and after I wrote all of the above, the version of git
> I have at work predates that commit X-<.  So maybe this was
> fixed with that commit, and you are expected to say:
> 
> 	$ git pull . origin
> 
Exactly.

> assuming that you are on "master" branch and cvsimoprt tracks
> CVS head with "origin" branch, that is.
> 
> Smurf, help?
> 
What for? You got it, after all. *g*

-- 
Matthias Urlichs   |   {M:U} IT Design @ m-u-it.de   |  smurf@smurf.noris.de
Disclaimer: The quote was selected randomly. Really. | http://smurf.noris.de
 - -
Sally sued for support; she was claimin'
Phil had fathered her baby (named Damon).
    She said, "I ought to know,"
    As she pointed below.
"'Cause this is the box that he came in."

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 191 bytes --]

Re: git-cvsimport "you may need to merge manually"

[Junio C Hamano]

smurf@smurf.noris.de writes:

> Junio C Hamano:
>> ..., and you are expected to say:
>> 
>> 	$ git pull . origin
>> 
> Exactly.

I think the second and subsequent run of "git cvsimport"
currently is similar to "git fetch", but earlier one that tried
to do checkout was probably similar to "git pull".  I think most
people would expect it to behave more like "git pull",
i.e. fetch from the upstream (that happens to be CVS) and merge
that into your branch.  It may not be operated that way
correctly and that might have been the reason we removed the
"master updates" code, but if that is the case I'd rather fix it
properly.

>> assuming that you are on "master" branch and cvsimoprt tracks
>> CVS head with "origin" branch, that is.
>> 
>> Smurf, help?
>> 
> What for? You got it, after all. *g*

Not really, I am afraid.  There is one snag _if_ you use the
current branch as the tracking branch.  Merlyn's setup is
exactly that -- he has "master" which is given to the command
with -o flag.  The branch head commit is already updated, but
the index and working tree is not.

Now, unlike git-fetch, git-cvsimport _requires_ you to have a
pristine tracking branch (otherwise we cannot discard already
seen patchsets from what we read from CVSPS), and leaving that
tracking branch checked out is calling for trouble because you
might be tempted to make your own commit on top of it.  So we
could argue that one solution would be to forbid importing into
the current branch.

But that breaks well behaving people who are used to leave a
tracking branch checked out _and_ promises not to touch that
branch head from the git side.

So what I would suggest is to do something like this:

 - Before starting to interpret CVSPS output, keep the commit
   object name of the current branch tip.

 - After we are done, read the current branch tip.  If they are
   different, we updated the current branch tip without matching
   the index and working tree, so we match them just like
   git-pull does.  Otherwise, we run 'git-merge' to merge the
   $opt_o branch into the current branch.

That is, perhaps, like this untested patch.  What do you think?

-- >8 --
cvsimport: act more like pull, not fetch

After updating tracking branches with upstream CVS changes, if
the current branch is one of the tracking branches, match the
index and working tree just like "git-pull" that was started
with one of the tracking branches checked out.  Otherwise, merge
the trunk ($opt_o) branch into the current branch.  This would
match users' expectation more closely.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 02d1928..b9cebaf 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -453,6 +453,7 @@ chdir($git_tree);
 my $last_branch = "";
 my $orig_branch = "";
 my %branch_date;
+my $tip_at_start = undef;
 
 my $git_dir = $ENV{"GIT_DIR"} || ".git";
 $git_dir = getwd()."/".$git_dir unless $git_dir =~ m#^/#;
@@ -487,6 +488,7 @@ unless(-d $git_dir) {
 		$last_branch = "master";
 	}
 	$orig_branch = $last_branch;
+	$tip_at_start = `git-rev-parse --verify HEAD`;
 
 	# populate index
 	system('git-read-tree', $last_branch);
@@ -873,7 +875,18 @@ if (defined $orig_git_index) {
 
 # Now switch back to the branch we were in before all of this happened
 if($orig_branch) {
-	print "DONE; you may need to merge manually.\n" if $opt_v;
+	print "DONE.\n" if $opt_v;
+	my $tip_at_end = `git-rev-parse --verify HEAD`;
+	if ($tip_at_start ne $tip_at_end) {
+		print "Fetched into the current branch.\n" if $opt_v;
+		system(qw(git-read-tree -u -m),
+		       $tip_at_start, $tip_at_end);
+		die "Fast-forward update failed: $?\n" if $?;
+	}
+	else {
+		system(qw(git-merge cvsimport HEAD), "refs/heads/$opt_o");
+		die "Could not merge $opt_o into the current branch.\n" if $?;
+	}
 } else {
 	$orig_branch = "master";
 	print "DONE; creating $orig_branch branch\n" if $opt_v;