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

From: Junio C Hamano <junkio@cox.net>
Date: Mar 17, 2006, 08:06 UTC
Message-ID: <7vk6at1on6.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <20060317064618.GG14562@smurf.noris.de>

smurf@smurf.noris.de writes:

Show 6 quoted lines

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

Show 6 quoted lines

>> 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;