Re: [RFC/PATCH 1/4] Add git-sequencer shell prototype

Hi,

On Fri, Jul 04, 2008 at 03:03:37AM +0200, Johannes Schindelin wrote:

On Fri, 4 Jul 2008, Stephan Beyer wrote:

quoted

On Fri, Jul 04, 2008 at 01:53:21AM +0200, Johannes Schindelin wrote:

quoted

On Thu, 3 Jul 2008, Stephan Beyer wrote:

quoted

Btw, another root commit problem is btw that it's not possible to 
cherry-pick root commits.

That is a problem to be fixed in cherry-pick, not in sequencer.  Care 
to take care of that?

Not at the moment but that's one of the things I note down for later ;-)

Well, logically, it should come _before_ you use it in sequencer.  And you 
should use it in sequencer.

Yet nobody seems to have asked for a cherry-pick that is able to pick
root commits and sequencer is not closed source after GSoC, so this can
be added whenever there is need and time for it.
That's what I wanted to say with "note down for later". ;-)

quoted

I don't get the light bulb.  You're talking about "the merge", I am
talking about fast-forward on picks.

Ooops.  I think I was talking about a later comment of Junio's.

The thing is, if you try to pick a commit, and the current revision is 
already the parent of that commit, I think it would be wrong to redo the 
commit, pretending that the current time and committer were applying that 
commit.

Right.

IMO the --signoff should check if the sign off is present (must be the 
last one, as if we redid that commit), and if it is, fast-forward.

If it is missing, we need to redo the commit.

That's a good point.
Checked that and it's even a bug in the sequencer: on fast-forwards, no
signoff is added.

This should fix that:

--- a/git-sequencer.sh
+++ b/git-sequencer.sh

@@ -219,8 +219,6 @@ pick_one () {
 		fi
 		;;
 	esac
-	test -n "$SIGNOFF" &&
-		what="$what -s"
 	$use_perform git $what "$@"
 }
 

@@ -973,16 +971,21 @@ insn_pick () {
 		$use_perform git commit --amend $EDIT $signoff --no-verify \
 			--author "$GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>" \
 			--message="$MESSAGE"
-	else
+	elif test -n "$AUTHOR"
+	then
 		# correct author if AUTHOR is set
-		test -n "$AUTHOR" &&
-			$use_perform git commit --amend $EDIT --no-verify -C HEAD \
-				--author "$GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>"
-		# XXX: a git-cherry-pick --author could be nicer here
+		$use_perform git commit --amend $EDIT --no-verify -C HEAD \
+			--author "$GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>"
+	elif test -n "$MESSAGE"
+	then
 		# correct commit message if MESSAGE is set
-		test -n "$MESSAGE" &&
-			$use_perform git commit --amend $EDIT $signoff --no-verify \
-				-C HEAD --message="$MESSAGE"
+		$use_perform git commit --amend $EDIT $signoff --no-verify \
+			-C HEAD --message="$MESSAGE"
+	elif test -n "$SIGNOFF"
+	then
+		# only add signoff
+		$use_perform git commit --amend $EDIT $signoff --no-verify \
+			-C HEAD
 	fi
 
 	return 0

###

Ah, and that is poorly tested, but not untested. :) See:
	$ git checkout -b seq-proto-dev3 HEAD^
	Switched to a new branch "seq-proto-dev3"
	$ git sequencer
	mark :1
	pick seq-proto-dev2
	ref refs/test/no
	reset :1
	pick --signoff seq-proto-dev2
Now: seq-proto-dev3 has signoff and seq-proto-dev2 and refs/test/no have the
same SHA1.  And the test suite passes.

Regards,
  Stephan

-- 
Stephan Beyer [off-list ref], PGP 0x6EDDD207FCC5040F