[PATCH 0/4] ack recoding in commit log

[PATCH 0/4] ack recoding in commit log

[Michael S. Tsirkin]

As a maintainer, I often get patches by mail, then
acked-by,reviewed-by etc responses are sent by separate
mail.

I like making acks commits,
this way they are easy to keep track of
as part of git history.


Since response mail happens to have appropriate
subject matching the patch, it's a natural fit to
then use git rebase mechanics if we want to smash
these acks into the original commit.

I have been using these patches without any problems
for a while now, and find the approach very convenient.

Included:
	rebase: new ack! action to handle ack commits
		this part seems ready for merge to me,
		please review and comment

	git-ack: new tool to record an ack
		this does not have proper documentation
		and tests yet, I definitely intend to
		do this but wanted to see whether people
		like the UI first.
		posting for early review and feedback


Note: yes, I think notes+git replace could be used for this too,
my workflow always includes a rebase with --autosquash
before publishing anyway, so dealing with ack as with any
other commit is nicer.


Michael S. Tsirkin (4):
  rebase -i: add ack action
  git-rebase: document ack
  rebase: test ack
  git-ack: record an ack

 Documentation/git-rebase.txt | 45 ++++++++++++++++++++++---
 contrib/git-ack              | 79 ++++++++++++++++++++++++++++++++++++++++++++
 git-rebase--interactive.sh   | 34 +++++++++++++++----
 t/t3415-rebase-autosquash.sh | 15 +++++++++
 4 files changed, 161 insertions(+), 12 deletions(-)
 create mode 100755 contrib/git-ack

-- 
MST

[PATCH 1/4] rebase -i: add ack action

[Michael S. Tsirkin]

This implements a new ack! action for git rebase -i
It is essentially a middle ground between fixup! and squash!:
- commits are squashed silently without editor being started
- commit logs are concatenated (with action line being discarded)
- because of the above, empty commits aren't discarded,
  their log is also included.

I am using it as follows:
	git am -s < mailbox #creates first commit
	hack ...
	get mail with Ack
	git commit --allow-empty -m `cat <<-EOF
	ack! first

	Acked-by: maintainer
	EOF`
	repeat cycle
	git rebase --autosquash -i origin/master
	before public branch push

The "cat" command above is actually a script that
parses the Ack mail to create the empty commit -
to be submitted separately.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 git-rebase--interactive.sh | 34 +++++++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 6ec9d3c..821872c 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -140,6 +140,7 @@ Commands:
  r, reword = use commit, but edit the commit message
  e, edit = use commit, but stop for amending
  s, squash = use commit, but meld into previous commit
+ a, ack = like "squash", but append commit body only to previous commit
  f, fixup = like "squash", but discard this commit's log message
  x, exec = run command (the rest of the line) using shell
 
@@ -412,6 +413,15 @@ update_squash_messages () {
 		echo
 		commit_message $2
 		;;
+	ack)
+		if test -f "$fixup_msg"
+		then
+			commit_message $2 | git stripspace --strip-comments | sed -e '1,2d' >> "$fixup_msg"
+		fi
+		printf '%s\n' "$comment_char This is the $(nth_string $count) commit message:"
+		echo
+		commit_message $2
+		;;
 	fixup)
 		echo
 		printf '%s\n' "$comment_char The $(nth_string $count) commit message will be skipped:"
@@ -453,7 +463,7 @@ record_in_rewritten() {
 	echo "$oldsha1" >> "$rewritten_pending"
 
 	case "$(peek_next_command)" in
-	squash|s|fixup|f)
+	squash|s|fixup|f|ack|a)
 		;;
 	*)
 		flush_rewritten_pending
@@ -521,8 +531,11 @@ do_next () {
 		warn "Stopped at $sha1... $rest"
 		exit_with_patch $sha1 0
 		;;
-	squash|s|fixup|f)
+	squash|s|fixup|f|ack|a)
 		case "$command" in
+		ack|a)
+			squash_style=ack
+			;;
 		squash|s)
 			squash_style=squash
 			;;
@@ -546,7 +559,7 @@ do_next () {
 			die_failed_squash $sha1 "$rest"
 		fi
 		case "$(peek_next_command)" in
-		squash|s|fixup|f)
+		squash|s|fixup|f|ack|a)
 			# This is an intermediate commit; its message will only be
 			# used in case of trouble.  So use the long version:
 			do_with_author output git commit --amend --no-verify -F "$squash_msg" \
@@ -557,7 +570,7 @@ do_next () {
 			# This is the final command of this squash/fixup group
 			if test -f "$fixup_msg"
 			then
-				do_with_author git commit --amend --no-verify -F "$fixup_msg" \
+				do_with_author git commit --quiet --amend --no-verify -F "$fixup_msg" \
 					${gpg_sign_opt:+"$gpg_sign_opt"} ||
 					die_failed_squash $sha1 "$rest"
 			else
@@ -690,7 +703,7 @@ skip_unnecessary_picks () {
 	done <"$todo" >"$todo.new" 3>>"$done" &&
 	mv -f "$todo".new "$todo" &&
 	case "$(peek_next_command)" in
-	squash|s|fixup|f)
+	squash|s|fixup|f|ack|a)
 		record_in_rewritten "$onto"
 		;;
 	esac ||
@@ -732,7 +745,7 @@ rearrange_squash () {
 	while read -r pick sha1 message
 	do
 		case "$message" in
-		"squash! "*|"fixup! "*)
+		"squash! "*|"fixup! "*|"ack! "*)
 			action="${message%%!*}"
 			rest=$message
 			prefix=
@@ -740,7 +753,7 @@ rearrange_squash () {
 			while :
 			do
 				case "$rest" in
-				"squash! "*|"fixup! "*)
+				"squash! "*|"fixup! "* |"ack! "*)
 					prefix="$prefix${rest%%!*},"
 					rest="${rest#*! }"
 					;;
@@ -975,6 +988,13 @@ do
 		comment_out=
 	fi
 
+	# keep empty ack! commits around: useful to add text to commit log
+	case "$rest" in
+	"ack! "*)
+		comment_out=
+		;;
+	esac
+
 	if test t != "$preserve_merges"
 	then
 		printf '%s\n' "${comment_out}pick $shortsha1 $rest" >>"$todo"
-- 
MST

[PATCH 2/4] git-rebase: document ack

[Michael S. Tsirkin]

document ack! behaviour and use

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 Documentation/git-rebase.txt | 45 +++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 40 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 2a93c64..c27aef4 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -384,7 +384,7 @@ or by giving more than one `--exec`:
 +
 If `--autosquash` is used, "exec" lines will not be appended for
 the intermediate commits, and will only appear at the end of each
-squash/fixup series.
+squash/fixup/ack series.
 
 --root::
 	Rebase all commits reachable from <branch>, instead of
@@ -398,13 +398,13 @@ squash/fixup series.
 
 --autosquash::
 --no-autosquash::
-	When the commit log message begins with "squash! ..." (or
-	"fixup! ..."), and there is a commit whose title begins with
+	When the commit log message begins with "squash! ..." ("fixup! ..."
+	or "ack! ..."), and there is a commit whose title begins with
 	the same ..., automatically modify the todo list of rebase -i
 	so that the commit marked for squashing comes right after the
 	commit to be modified, and change the action of the moved
-	commit from `pick` to `squash` (or `fixup`).  Ignores subsequent
-	"fixup! " or "squash! " after the first, in case you referred to an
+	commit from `pick` to `squash` (`fixup` or `ack`).  Ignores subsequent
+	"ack! ", "fixup! " or "squash! " after the first, in case you referred to an
 	earlier fixup/squash with `git commit --fixup/--squash`.
 +
 This option is only valid when the '--interactive' option is used.
@@ -624,6 +624,41 @@ consistent (they compile, pass the testsuite, etc.) you should use
 'git stash' to stash away the not-yet-committed changes
 after each commit, test, and amend the commit if fixes are necessary.
 
+----------------
+RECORDING ACKS
+----------------
+
+Interactive mode with --autosquash can be used to concatenate
+commit log for several commits, which is useful to record
+extra information about the commit, such as ack signatures.
+This allows, for example, the following workflow:
+
+1. receive patches by mail and commit
+2. receive by mail ack signatures for the patches
+3. prepare a series for submission
+4. submit
+
+where point 2. consists of several instances of
+	i) create a (possibly empty) commit with signature
+	  in the commit message
+
+Sometimes the ack signature added in i. cannot be amended to the
+commit it acks, because that commit is buried deeply in a
+patch series.  That is exactly what rebase --autosquash
+option is for: use it
+after plenty of "i"s, to automaticlly rearrange
+commits, and squashing multiple sign-off commits into
+the commit that is signed.
+
+Start it with the last commit you want to retain as-is:
+
+	git rebase --autosquash -i <after-this-commit>
+
+An editor will be fired up with all the commits in your current branch
+which come after the given commit. Ack commits will be
+re-arranged to come after the commit that is acked,
+and the action will be utomticlly changed from `pick` to `ack`
+to cause them to be squashed into the acked commit.
 
 RECOVERING FROM UPSTREAM REBASE
 -------------------------------
-- 
MST

[PATCH 3/4] rebase: test ack

[Michael S. Tsirkin]

test ack! handling

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 t/t3415-rebase-autosquash.sh | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
index 41370ab..9d7db13 100755
--- a/t/t3415-rebase-autosquash.sh
+++ b/t/t3415-rebase-autosquash.sh
@@ -74,6 +74,21 @@ test_expect_success 'auto squash (option)' '
 	test_auto_squash final-squash --autosquash
 '
 
+test_expect_success 'auto ack' '
+	ack="Acked-by: xyz"
+	msg=$(test_write_lines "ack! first commit" "" "$ack")
+	git reset --hard base &&
+	git commit --allow-empty -m "$msg" -- &&
+	git tag ack &&
+	test_tick &&
+	git rebase --autosquash -i HEAD^^^ &&
+	git log --oneline >actual &&
+	git show -s first-commit | grep -v ^commit > expected-msg &&
+	echo "    $ack" >> expected-msg &&
+	git show -s HEAD^ | grep -v ^commit > actual-msg &&
+	diff actual-msg expected-msg
+'
+
 test_expect_success 'auto squash (config)' '
 	git config rebase.autosquash true &&
 	test_auto_squash final-squash-config-true &&
-- 
MST

[PATCH 4/4] git-ack: record an ack

[Michael S. Tsirkin]

This is a simple script that I use by piping
incoming mail with an ack to it.
It produces an empty ack commit suitable for
squshing with git rebase -i -autosquash.

Works best if people ack individual commits: you simply
pipe each ack to git ack, before pushing your branch,
rebase.

Some people ack series by responding to cover letter
or to commit 1.
To address this usecase, there are two additional
flags: -s saves the ack signature in a file (you can
save several in a row), -r creates an ack for
a given patch using the saved signature.
Thus: pipe ack(s) to git ack -s, then select and pipe
each individual patch to git ack -r.
I don't use these flags much so they likely work
less well.

If it's found useful, this script can either
become a first-class command (with documentation
and tests) or be integrated as a flag into git am.

Limitations: requires that index is clean, this is
so we can create an empty commit recording the ack.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 contrib/git-ack | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 84 insertions(+)
 create mode 100755 contrib/git-ack

diff --git a/contrib/git-ack b/contrib/git-ack
new file mode 100755
index 0000000..4aeb16a
--- /dev/null
+++ b/contrib/git-ack
@@ -0,0 +1,84 @@
+msg=`mktemp`
+patch=`mktemp`
+info=`git mailinfo $msg $patch`
+subj=`echo "$info"|sed -n 's/^Subject: //p'`
+author=`echo "$info"|sed -n 's/^Author: //p'`
+email=`echo "$info"|sed -n 's/^Email: //p'`
+auth="$author <$email>"
+date=`echo "$info"|sed -n 's/^Date: //p'`
+sign=`mktemp`
+echo "ack! $subj" > $sign
+echo "" >> $sign
+if
+    git diff --cached HEAD
+then
+    nop < /dev/null
+else
+    echo "DIFF in cache. Not acked, reset or commit!"
+    exit 1
+fi
+GIT_DIR=`pwd`/${GIT_DIR}
+
+usage () {
+	echo "Usage: git ack " \
+            "[-s|--save|-a|--append|-r|--restore |-c|--clear]\n" >& 2;
+        exit 1;
+}
+
+append=
+save=
+clear=
+
+while test $# != 0
+do
+	case "$1" in
+	-a|--append)
+		append="y"
+		;;
+	-s|--s)
+		save="y"
+		;;
+	-r|--restore)
+		restore="y"
+		;;
+	-c|--clear)
+		clear="y"
+                ;;
+	*)
+		usage ;;
+	esac
+	shift
+done
+
+if
+    test "$clear"
+then
+    rm -f "${GIT_DIR}/ACKS"
+fi
+
+if
+    test "$save"
+then
+    if
+        test "$append"
+    then
+        cat $msg >> "${GIT_DIR}/ACKS"
+    else
+        cat $msg > "${GIT_DIR}/ACKS"
+    fi
+fi
+
+if
+    test "$restore"
+then
+    msg = ${GIT_DIR}/ACKS
+fi
+
+if
+    grep '^[A-Z][A-Za-z-]*-by:' $msg >> $sign
+then
+    git commit --allow-empty -F $sign --author="$auth" --date="$date"
+else
+    echo "No signature found!"
+    exit 2
+fi
-- 
MST

Re: [PATCH 2/4] git-rebase: document ack

[Eric Sunshine]

On Sun, May 18, 2014 at 5:17 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> document ack! behaviour and use
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  Documentation/git-rebase.txt | 45 +++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 40 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 2a93c64..c27aef4 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -384,7 +384,7 @@ or by giving more than one `--exec`:
>  +
>  If `--autosquash` is used, "exec" lines will not be appended for
>  the intermediate commits, and will only appear at the end of each
> -squash/fixup series.
> +squash/fixup/ack series.
>
>  --root::
>         Rebase all commits reachable from <branch>, instead of
> @@ -398,13 +398,13 @@ squash/fixup series.
>
>  --autosquash::
>  --no-autosquash::
> -       When the commit log message begins with "squash! ..." (or
> -       "fixup! ..."), and there is a commit whose title begins with
> +       When the commit log message begins with "squash! ..." ("fixup! ..."
> +       or "ack! ..."), and there is a commit whose title begins with
>         the same ..., automatically modify the todo list of rebase -i
>         so that the commit marked for squashing comes right after the
>         commit to be modified, and change the action of the moved
> -       commit from `pick` to `squash` (or `fixup`).  Ignores subsequent
> -       "fixup! " or "squash! " after the first, in case you referred to an
> +       commit from `pick` to `squash` (`fixup` or `ack`).  Ignores subsequent
> +       "ack! ", "fixup! " or "squash! " after the first, in case you referred to an
>         earlier fixup/squash with `git commit --fixup/--squash`.
>  +
>  This option is only valid when the '--interactive' option is used.
> @@ -624,6 +624,41 @@ consistent (they compile, pass the testsuite, etc.) you should use
>  'git stash' to stash away the not-yet-committed changes
>  after each commit, test, and amend the commit if fixes are necessary.
>
> +----------------
> +RECORDING ACKS
> +----------------
> +
> +Interactive mode with --autosquash can be used to concatenate
> +commit log for several commits, which is useful to record
> +extra information about the commit, such as ack signatures.
> +This allows, for example, the following workflow:
> +
> +1. receive patches by mail and commit
> +2. receive by mail ack signatures for the patches
> +3. prepare a series for submission
> +4. submit
> +
> +where point 2. consists of several instances of
> +       i) create a (possibly empty) commit with signature
> +         in the commit message
> +
> +Sometimes the ack signature added in i. cannot be amended to the
> +commit it acks, because that commit is buried deeply in a
> +patch series.  That is exactly what rebase --autosquash
> +option is for: use it
> +after plenty of "i"s, to automaticlly rearrange
> +commits, and squashing multiple sign-off commits into
> +the commit that is signed.
> +
> +Start it with the last commit you want to retain as-is:
> +
> +       git rebase --autosquash -i <after-this-commit>
> +
> +An editor will be fired up with all the commits in your current branch
> +which come after the given commit. Ack commits will be
> +re-arranged to come after the commit that is acked,
> +and the action will be utomticlly changed from `pick` to `ack`

s/utomticlly/automatically/

> +to cause them to be squashed into the acked commit.
>
>  RECOVERING FROM UPSTREAM REBASE
>  -------------------------------
> --
> MST
>

Re: [PATCH 3/4] rebase: test ack

[Junio C Hamano]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> test ack! handling
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Will queue with this squashed in.

4/4 seems to have some style issues as well, but I didn't look very
closely.

Thanks.

 t/t3415-rebase-autosquash.sh | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
index 9d7db13..dcdba6f 100755
--- a/t/t3415-rebase-autosquash.sh
+++ b/t/t3415-rebase-autosquash.sh
@@ -75,18 +75,18 @@ test_expect_success 'auto squash (option)' '
 '
 
 test_expect_success 'auto ack' '
-	ack="Acked-by: xyz"
-	msg=$(test_write_lines "ack! first commit" "" "$ack")
+	ack="Acked-by: xyz" &&
+	msg=$(test_write_lines "ack! first commit" "" "$ack") &&
 	git reset --hard base &&
 	git commit --allow-empty -m "$msg" -- &&
 	git tag ack &&
 	test_tick &&
 	git rebase --autosquash -i HEAD^^^ &&
 	git log --oneline >actual &&
-	git show -s first-commit | grep -v ^commit > expected-msg &&
-	echo "    $ack" >> expected-msg &&
-	git show -s HEAD^ | grep -v ^commit > actual-msg &&
-	diff actual-msg expected-msg
+	git show -s first-commit | grep -v ^commit >expected-msg &&
+	echo "    $ack" >>expected-msg &&
+	git show -s HEAD^ | grep -v ^commit >actual-msg &&
+	test_cmp actual-msg expected-msg
 '
 
 test_expect_success 'auto squash (config)' '

Re: [PATCH 3/4] rebase: test ack

[Michael S. Tsirkin]

On Mon, May 19, 2014 at 02:34:26PM -0700, Junio C Hamano wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > test ack! handling
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> Will queue with this squashed in.

Thanks! And sorry about the style issues.

> 4/4 seems to have some style issues as well, but I didn't look very
> closely.

I'll try to clean it for the next submission.
I'll be glad to hear about them as well.
Thanks!

> Thanks.
> 
>  t/t3415-rebase-autosquash.sh | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
> index 9d7db13..dcdba6f 100755
> --- a/t/t3415-rebase-autosquash.sh
> +++ b/t/t3415-rebase-autosquash.sh
> @@ -75,18 +75,18 @@ test_expect_success 'auto squash (option)' '
>  '
>  
>  test_expect_success 'auto ack' '
> -	ack="Acked-by: xyz"
> -	msg=$(test_write_lines "ack! first commit" "" "$ack")
> +	ack="Acked-by: xyz" &&
> +	msg=$(test_write_lines "ack! first commit" "" "$ack") &&
>  	git reset --hard base &&
>  	git commit --allow-empty -m "$msg" -- &&
>  	git tag ack &&
>  	test_tick &&
>  	git rebase --autosquash -i HEAD^^^ &&
>  	git log --oneline >actual &&
> -	git show -s first-commit | grep -v ^commit > expected-msg &&
> -	echo "    $ack" >> expected-msg &&
> -	git show -s HEAD^ | grep -v ^commit > actual-msg &&
> -	diff actual-msg expected-msg
> +	git show -s first-commit | grep -v ^commit >expected-msg &&
> +	echo "    $ack" >>expected-msg &&
> +	git show -s HEAD^ | grep -v ^commit >actual-msg &&
> +	test_cmp actual-msg expected-msg
>  '
>  
>  test_expect_success 'auto squash (config)' '

Re: [PATCH 3/4] rebase: test ack

[Michael S. Tsirkin]

On Mon, May 19, 2014 at 02:34:26PM -0700, Junio C Hamano wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > test ack! handling
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> Will queue with this squashed in.
> 
> 4/4 seems to have some style issues as well, but I didn't look very
> closely.
> 
> Thanks.

Just to clarify I can post v2 of 4/4 without reposting 1-3 since they
are queued?

>  t/t3415-rebase-autosquash.sh | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
> index 9d7db13..dcdba6f 100755
> --- a/t/t3415-rebase-autosquash.sh
> +++ b/t/t3415-rebase-autosquash.sh
> @@ -75,18 +75,18 @@ test_expect_success 'auto squash (option)' '
>  '
>  
>  test_expect_success 'auto ack' '
> -	ack="Acked-by: xyz"
> -	msg=$(test_write_lines "ack! first commit" "" "$ack")
> +	ack="Acked-by: xyz" &&
> +	msg=$(test_write_lines "ack! first commit" "" "$ack") &&
>  	git reset --hard base &&
>  	git commit --allow-empty -m "$msg" -- &&
>  	git tag ack &&
>  	test_tick &&
>  	git rebase --autosquash -i HEAD^^^ &&
>  	git log --oneline >actual &&
> -	git show -s first-commit | grep -v ^commit > expected-msg &&
> -	echo "    $ack" >> expected-msg &&
> -	git show -s HEAD^ | grep -v ^commit > actual-msg &&
> -	diff actual-msg expected-msg
> +	git show -s first-commit | grep -v ^commit >expected-msg &&
> +	echo "    $ack" >>expected-msg &&
> +	git show -s HEAD^ | grep -v ^commit >actual-msg &&
> +	test_cmp actual-msg expected-msg
>  '
>  
>  test_expect_success 'auto squash (config)' '

Re: [PATCH 3/4] rebase: test ack

[Junio C Hamano]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> Just to clarify I can post v2 of 4/4 without reposting 1-3 since they
> are queued?

If you need to update anything queued only on 'pu' but not yet in
'next', it is customary to resend the whole for everybody to see
(what is already in 'next' should only be built upon incrementally
and not updated with replacement patches), while noting which ones
are the same as before.  Christian Couder has been doing it nicely
in his recent rerolls (if the series were not in 'next', that is).

Thanks.

Re: [PATCH 3/4] rebase: test ack

[Michael S. Tsirkin]

On Tue, May 20, 2014 at 08:13:27AM -0700, Junio C Hamano wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > Just to clarify I can post v2 of 4/4 without reposting 1-3 since they
> > are queued?
> 
> If you need to update anything queued only on 'pu' but not yet in
> 'next', it is customary to resend the whole for everybody to see
> (what is already in 'next' should only be built upon incrementally
> and not updated with replacement patches), while noting which ones
> are the same as before.  Christian Couder has been doing it nicely
> in his recent rerolls (if the series were not in 'next', that is).
> 
> Thanks.

Actually I don't see anything like it in pu.
What I would like is for 1-3 to be in pu,
4/4 was for illustrative purposes it's not yet
ready for that, and 1-3 are useful by themselves.
I could then iterate on 4/4 without reposting 1-3.

Re: [PATCH 3/4] rebase: test ack

[Junio C Hamano]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Tue, May 20, 2014 at 08:13:27AM -0700, Junio C Hamano wrote:
>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> 
>> > Just to clarify I can post v2 of 4/4 without reposting 1-3 since they
>> > are queued?
>> 
>> If you need to update anything queued only on 'pu' but not yet in
>> 'next', it is customary to resend the whole for everybody to see
>> (what is already in 'next' should only be built upon incrementally
>> and not updated with replacement patches), while noting which ones
>> are the same as before.  Christian Couder has been doing it nicely
>> in his recent rerolls (if the series were not in 'next', that is).
>> 
>> Thanks.
>
> Actually I don't see anything like it in pu.

The way I usually work is to apply a non-fix (i.e. enhancement)
series on a topic branch forked from 'master' (or the last tagged
version contained in its tip) and see if it makes sense, and then
try-merge the result to 'next' to see if it is free of potential
funny interactions with other topics that are already in flight.
After that happens, the topic branch is merged to somewhere in 'pu'.

It is possible that I did not have time to go through all the steps
above (after all, I had to make another -rc release and there was an
unexpected last-minute change of plans in the morning that blew a
few hours of work).  Or there may have been some merge conflicts
that I didn't feel like resolving for various reasons (e.g. if I
knew the series would be rerolled anyway, it can wait; if the other
topic that interacts with your series has been cooking sufficiently
long in 'next' and if it is very close to the final release of this
cycle, it may be easier to wait for the other topic to graduate to
'master', which would happen soon after this cycle finishes, and ask
you to rebase your series).

I don't remember which ;-)

Re: [PATCH 3/4] rebase: test ack

[Michael S. Tsirkin]

On Wed, May 21, 2014 at 09:54:47AM -0700, Junio C Hamano wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Tue, May 20, 2014 at 08:13:27AM -0700, Junio C Hamano wrote:
> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> >> 
> >> > Just to clarify I can post v2 of 4/4 without reposting 1-3 since they
> >> > are queued?
> >> 
> >> If you need to update anything queued only on 'pu' but not yet in
> >> 'next', it is customary to resend the whole for everybody to see
> >> (what is already in 'next' should only be built upon incrementally
> >> and not updated with replacement patches), while noting which ones
> >> are the same as before.  Christian Couder has been doing it nicely
> >> in his recent rerolls (if the series were not in 'next', that is).
> >> 
> >> Thanks.
> >
> > Actually I don't see anything like it in pu.
> 
> The way I usually work is to apply a non-fix (i.e. enhancement)
> series on a topic branch forked from 'master' (or the last tagged
> version contained in its tip) and see if it makes sense, and then
> try-merge the result to 'next' to see if it is free of potential
> funny interactions with other topics that are already in flight.
> After that happens, the topic branch is merged to somewhere in 'pu'.
> 
> It is possible that I did not have time to go through all the steps
> above (after all, I had to make another -rc release and there was an
> unexpected last-minute change of plans in the morning that blew a
> few hours of work).  Or there may have been some merge conflicts
> that I didn't feel like resolving for various reasons (e.g. if I
> knew the series would be rerolled anyway, it can wait; if the other
> topic that interacts with your series has been cooking sufficiently
> long in 'next' and if it is very close to the final release of this
> cycle, it may be easier to wait for the other topic to graduate to
> 'master', which would happen soon after this cycle finishes, and ask
> you to rebase your series).
> 
> I don't remember which ;-)
> 

Oh sorry, didn't mean to try to pressure you. I was just surprised
not to see it there. I know this applies cleanly to next so I'll just
wait for 2.0 to be out.

-- 
MST

Re: [PATCH 3/4] rebase: test ack

[Junio C Hamano]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Wed, May 21, 2014 at 09:54:47AM -0700, Junio C Hamano wrote:
>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> 
>> > On Tue, May 20, 2014 at 08:13:27AM -0700, Junio C Hamano wrote:
>> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> >> 
>> >> > Just to clarify I can post v2 of 4/4 without reposting 1-3 since they
>> >> > are queued?
>> >> 
>> >> If you need to update anything queued only on 'pu' but not yet in
>> >> 'next', it is customary to ...
>> >
>> > Actually I don't see anything like it in pu.
>> ... 
> Oh sorry, didn't mean to try to pressure you. I was just surprised
> not to see it there. I know this applies cleanly to next so I'll just
> wait for 2.0 to be out.

Oh, no.  No pressure felt and no need to be sorry about anything.

I described a preferred procedure when the topic appeared in 'pu',
and I didn't answer your question for topics that are not even in
'pu' yet.

Being in 'pu' and not in 'next' is not much different from not being
in 'pu', so the preferred procedure is to send out the entire series
(unless it is a large 47-patch series ;-) to give everybody another
chance to comment, and it would be extra nice if you indicated which
ones are unchanged since the previous round to help those who did
already saw them.

Thanks.

Re: [PATCH 4/4] git-ack: record an ack

[Fabian Ruch]

Hi Michael,

I have some inline comments below. Also, some parts of the patch do not
adhere to the style rules

 - tabs for indentation
 - $(...) for command substitution
 - no space after redirection operators
 - double-quotes around redirection targets

for shell scripts (from the file `Documentation/CodingGuidelines`).

On 05/18/2014 11:17 PM, Michael S. Tsirkin wrote:
> diff --git a/contrib/git-ack b/contrib/git-ack
> new file mode 100755
> index 0000000..4aeb16a
> --- /dev/null
> +++ b/contrib/git-ack
> @@ -0,0 +1,84 @@
> +msg=`mktemp`
> +patch=`mktemp`
> +info=`git mailinfo $msg $patch`
> +subj=`echo "$info"|sed -n 's/^Subject: //p'`
> +author=`echo "$info"|sed -n 's/^Author: //p'`
> +email=`echo "$info"|sed -n 's/^Email: //p'`
> +auth="$author <$email>"
> +date=`echo "$info"|sed -n 's/^Date: //p'`
> +sign=`mktemp`
> +echo "ack! $subj" > $sign
> +echo "" >> $sign
> +if
> +    git diff --cached HEAD

If I am not mistaken, the exit code of `git-diff(1)` doesn't change
according to whether there are differences or not, unless the option
`--exit-code` is given.

> +then
> +    nop < /dev/null

Is it correct that this is a do-nothing operation? Is that a common
idiom? I found the null command (`:`, colon) to be used in many places
instead.

> +else
> +    echo "DIFF in cache. Not acked, reset or commit!"
> +    exit 1
> +fi
> +GIT_DIR=`pwd`/${GIT_DIR}

This seems incorrect to me. If `GIT_DIR` is already set, it might point
to an absolute path and not `.git`. If the environment variable is not
set, the state file `ACKS` ends up in the working directory.

Maybe `git-sh-setup(1)` can be of help. It uses

    git rev-parse --git-dir

to probe the path to the .git directory.

> +
> +usage () {
> +	echo "Usage: git ack " \
> +            "[-s|--save|-a|--append|-r|--restore |-c|--clear]\n" >& 2;
> +        exit 1;
> +}
> +
> +append=
> +save=
> +clear=

The restore flag seems to be missing from this list of declarations.

> +
> +while test $# != 0
> +do
> +	case "$1" in
> +	-a|--append)
> +		append="y"
> +		;;
> +	-s|--s)
> +		save="y"
> +		;;
> +	-r|--restore)
> +		restore="y"
> +		;;
> +	-c|--clear)
> +		clear="y"
> +                ;;
> +	*)
> +		usage ;;
> +	esac
> +	shift
> +done
> +
> +if
> +    test "$clear"
> +then
> +    rm -f "${GIT_DIR}/ACKS"
> +fi
> +
> +if
> +    test "$save"
> +then
> +    if
> +        test "$append"
> +    then
> +        cat $msg >> "${GIT_DIR}/ACKS"
> +    else
> +        cat $msg > "${GIT_DIR}/ACKS"
> +    fi
> +fi
> +
> +if
> +    test "$restore"
> +then
> +    msg = ${GIT_DIR}/ACKS
> +fi
> +
> +if
> +    grep '^[A-Z][A-Za-z-]*-by:' $msg >> $sign
> +then
> +    git commit --allow-empty -F $sign --author="$auth" --date="$date"
> +else
> +    echo "No signature found!"
> +    exit 2
> +fi

   Fabian

Re: [PATCH 0/4] ack recoding in commit log

[Fabian Ruch]

Hi Michael,

On 05/18/2014 11:17 PM, Michael S. Tsirkin wrote:
> As a maintainer, I often get patches by mail, then
> acked-by,reviewed-by etc responses are sent by separate
> mail.
> 
> I like making acks commits,
> this way they are easy to keep track of
> as part of git history.

In order to fully understand your additions, I think, I need some
clarification on the term "ack commit". What is an ack commit exactly?
Suppose our principal commit has the commit message

    Some changes

    The changes are...

    Signed-off-by: A U Thor <author@example.com>

and we receive an email from Somebody saying

    > Some changes
    >
    > The changes are...
    >
    > Signed-off-by: A U Thor <author@example.com>

    Reviewed-by: Somebody <somebody@example.com>

. Now, if I understand correctly, we create an empty commit on top of
the principal commit using the following commit message.

    Some changes

    Reviewed-by: Somebody <somebody@example.com>

Is this commit then called an ack commit?

Can an ack commit be non-empty?

Is a commit still an ack if its description mentions additional text
between the subject and the tag lines?

Maybe the ack command for todo lists and ack commits have little to do
with one another. If we stick to the term "ack commit", then the command
name suggests that it takes the tags from some commit b and appends them
to the list of tags in the previous commit's (a) message:

    pick a A commit
    ack  b The next commit

However, this obviously does not work by just appending messages. For
instance, there could be additional text before or after some tag line
in either commit message. If we treat the workflow you described as a
very specific use case of the ack command instead, it seems reasonable
to add such a todo list functionality for melding commits by silently
appending messages. However, we might consider parametrizing a single
squash command instead of defining just another name that one has to
keep in mind for melding commits:

    pick             a A commit
    squash --no-edit b The next commit

> Since response mail happens to have appropriate
> subject matching the patch, it's a natural fit to
> then use git rebase mechanics if we want to smash
> these acks into the original commit.
> 
> I have been using these patches without any problems
> for a while now, and find the approach very convenient.
> 
> Included:
> 	rebase: new ack! action to handle ack commits
> 		this part seems ready for merge to me,
> 		please review and comment
> 
> 	git-ack: new tool to record an ack
> 		this does not have proper documentation
> 		and tests yet, I definitely intend to
> 		do this but wanted to see whether people
> 		like the UI first.
> 		posting for early review and feedback
> 
> [..]

Thanks for your time,
   Fabian

Re: [PATCH 0/4] ack recoding in commit log

[Michael S. Tsirkin]

On Wed, Jun 11, 2014 at 10:05:46AM +0200, Fabian Ruch wrote:
> Hi Michael,
> 
> On 05/18/2014 11:17 PM, Michael S. Tsirkin wrote:
> > As a maintainer, I often get patches by mail, then
> > acked-by,reviewed-by etc responses are sent by separate
> > mail.
> > 
> > I like making acks commits,
> > this way they are easy to keep track of
> > as part of git history.
> 
> In order to fully understand your additions, I think, I need some
> clarification on the term "ack commit".

I think the two additions should be judged separately, they aren't
related.

So I am not sure we need to spend time discussing
what is an ack commit.

What I mean is that I prefer using git history for
recording activity, and git rebase for modifying
history.
I think I am not alone in this.

To record acks, I wrote this tool to
create empty commits, that only record log
changes. They are named specially such that
rebase knows not to discard them, and to
automate the editing stage.

I added some features to the tool since.
Did not yet address all comments I got here, but just in case
you are curious it's attached at the end.

> What is an ack commit exactly?
> Suppose our principal commit has the commit message
> 
>     Some changes
> 
>     The changes are...
> 
>     Signed-off-by: A U Thor <author@example.com>
> 
> and we receive an email from Somebody saying
> 
>     > Some changes
>     >
>     > The changes are...
>     >
>     > Signed-off-by: A U Thor <author@example.com>
> 
>     Reviewed-by: Somebody <somebody@example.com>

Other possible cases I handle:
    people reply with:

    For the series:
        Reviewed-by: Somebody <somebody@example.com>

This uses the save/restore flags.

or an informal
    Ack the series
(this one I handle by creating ACKS file manually,
 then applying it with git ack restore)

Someone sends a fixup! patch, and someone else
replies to the fixup with Reviewed-by: tag.
(this was not handled correctly in the version
you reviewed, I attach latest one below where it is
handled correctly).


> . Now, if I understand correctly, we create an empty commit on top of
> the principal commit using the following commit message.
> 
>     Some changes
> 
>     Reviewed-by: Somebody <somebody@example.com>
> 
> Is this commit then called an ack commit?

git ack creates commits like this:

----
ack! Some changes

Reviewed-by: Somebody <somebody@example.com>
---

Other work-flows


> Can an ack commit be non-empty?

At the moment git ack does not create non empty commits.

> Is a commit still an ack if its description mentions additional text
> between the subject and the tag lines?

At the moment git ack does not create such commits.

> Maybe the ack command for todo lists and ack commits have little to do
> with one another.

I use them together but ack command is more powerful than git ack.
Same as commit --fixup, that is not the only way to
use fixup!.


> If we stick to the term "ack commit", then the command
> name suggests that it takes the tags from some commit b and appends them
> to the list of tags in the previous commit's (a) message:
> 
>     pick a A commit
>     ack  b The next commit
> 
> However, this obviously does not work by just appending messages. For
> instance, there could be additional text before or after some tag line
> in either commit message. If we treat the workflow you described as a
> very specific use case of the ack command instead, it seems reasonable
> to add such a todo list functionality for melding commits by silently
> appending messages. However, we might consider parametrizing a single
> squash command instead of defining just another name that one has to
> keep in mind for melding commits:

Hmm, I don't see why is flag any easier to remember than a separate command
frankly, and there's help text at the end to remind you.

> 
>     pick             a A commit
>     squash --no-edit b The next commit

That's too much typing I think, I just want to use a
single letter.
You could argue the same thing for fixup right?
It's really squash but discards the log, and there
is no requirement that it fixes anything, it can
add functionality or whatever.
So why not
squash --discardlog b The next commit

I think that would be bad usability, and so would --no-edit.

--no-edit also does not convey the fact that it allows
empty commits.


I don't mind renaming the command to something else
but I could not come up with a better name so far.

> > Since response mail happens to have appropriate
> > subject matching the patch, it's a natural fit to
> > then use git rebase mechanics if we want to smash
> > these acks into the original commit.
> > 
> > I have been using these patches without any problems
> > for a while now, and find the approach very convenient.
> > 
> > Included:
> > 	rebase: new ack! action to handle ack commits
> > 		this part seems ready for merge to me,
> > 		please review and comment
> > 
> > 	git-ack: new tool to record an ack
> > 		this does not have proper documentation
> > 		and tests yet, I definitely intend to
> > 		do this but wanted to see whether people
> > 		like the UI first.
> > 		posting for early review and feedback
> > 
> > [..]
> 
> Thanks for your time,
>    Fabian


## latest git-ack
## changes: handle fixup!/squash!/ack! messages in mail

msg=`mktemp`
patch=`mktemp`
info=`git mailinfo $msg $patch`
subj=`echo "$info"|sed -n 's/^Subject: //p'`
#strip ack!/fixup!/squash! prefix
subj=`echo "$subj"|sed "s/^fixup![ 	]*//"`
subj=`echo "$subj"|sed "s/^squash![ 	]*//"`
subj=`echo "$subj"|sed "s/^ack![ 	]*//"`
author=`echo "$info"|sed -n 's/^Author: //p'`
email=`echo "$info"|sed -n 's/^Email: //p'`
auth="$author <$email>"
date=`echo "$info"|sed -n 's/^Date: //p'`
sign=`mktemp`
echo "ack! $subj" > $sign
echo "" >> $sign
if
    git diff --cached HEAD
then
    nop < /dev/null
else
    echo "DIFF in cache. Not acked, reset or commit!"
    exit 1
fi
GIT_DIR=`pwd`/.git

usage () {
	echo "Usage: git ack " \
            "[-s|--save|-a|--append|-r|--restore |-c|--clear]\n" >& 2;
        exit 1;
}

append=
save=
clear=

while test $# != 0
do
	case "$1" in
	-a|--append)
		append="y"
		;;
	-s|--s)
		save="y"
		;;
	-r|--restore)
		restore="y"
		;;
	-c|--clear)
		clear="y"
                ;;
	*)
		usage ;;
	esac
	shift
done

if
    test "$clear"
then
    rm -f "${GIT_DIR}/ACKS"
fi

if
    test "$save"
then
    if
        test "$append"
    then
        cat $msg >> "${GIT_DIR}/ACKS"
    else
        cat $msg > "${GIT_DIR}/ACKS"
    fi
fi

if
    test "$restore"
then
    msg=${GIT_DIR}/ACKS
fi

echo $msg > /dev/tty
if
    grep '^[A-Z][A-Za-z-]*-by:' $msg >> $sign
then
    git commit --allow-empty -F $sign --author="$auth" --date="$date"
else
    echo "No signature found!"
    exit 2
fi