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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



smurf@xxxxxxxxxxxxxx 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@xxxxxxx>
---

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;

-
: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]