Cosmetic git-am interactive bug

Jeff Garzik· Dec 4, 2007, 19:19 UTC · lore

Just now, I was exploring git-am's interactive mode, so as to comply with Linus's urgings of a cleaner changelog <grin>. I had a raw email changelog with the subject

	Re: 2.6.24-rc3-git6: Reported regressions from 2.6.23

I used the [e]dit feature of interactive mode to change this first-line/one-line summary to

	pata_amd/pata_via: de-couple programming of PIO/MWDMA and UDMA
	timings

When I hit [y]es to apply the patch, git reported

	Applying 2.6.24-rc3-git6: Reported regressions from 2.6.23

	Wrote tree 34cebd48e3b4e90fe3e6a6c6c03154ae1ed0c827
	Committed: 943547abdfe9b4e27e36a25987909619908dffbf

The use of the older one-line summary led me to believe that it had not committed my changelog edits. Looking at the result, however, proved that the commit changelog was my new, corrected version.

Thus, I concluded that the printing of the old-and-no-longer-valid changelog during the commit-to-tree phase was a bug.

Please see attached file for full example.

Thanks,

	Jeff

[jgarzik@pretzel libata-dev]$ git-am --utf8 --signoff -i /g/tmp/mbox Commit Body is: -------------------------- 2.6.24-rc3-git6: Reported regressions from 2.6.23

On Saturday 01 December 2007, Rafael J. Wysocki wrote:

Show 7 quoted lines

> Subject               : 2.6.24-rc1: pata_amd fails to detect 80-pin wire
> Submitter     : "Thomas Lindroth" <thomas.lindroth@gmail.com>
> References    : http://lkml.org/lkml/2007/11/7/152
>                 http://bugzilla.kernel.org/show_bug.cgi?id=9322
> Handled-By    : Tejun Heo <htejun@gmail.com>
>                 Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> Patch         : http://lkml.org/lkml/2007/11/11/115

Tejun's rework of cable detection code which fixes the problem has been just applied into "upstream" (not "upstream-fixes") so it is destined for 2.6.25 (I wasn't on cc: BTW) and since I got no feedback on my patch (below) which also happens to fix the regression, was acked by Alan, tested by Thomas and has been in -mm for 3 weeks now I assume that everybody is happy with it (Jeff/Tejun: you were also on cc: when the patch was merged into -mm)...

Linus, please apply.

[PATCH] pata_amd/pata_via: de-couple programming of PIO/MWDMA and UDMA timings

* Don't program UDMA timings when programming PIO or MWDMA modes.

  This has also a nice side-effect of fixing regression added by commit
  681c80b5d96076f447e8101ac4325c82d8dce508 ("libata: correct handling of
  SRST reset sequences") (->set_piomode method for PIO0 is called before
  ->cable_detect method which checks UDMA timings to get the cable type).

* Bump driver version.

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Tested-by: "Thomas Lindroth" <thomas.lindroth@gmail.com>
Acked-by: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: Jeff Garzik <jeff@garzik.org>
Cc: Tejun Heo <htejun@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Signed-off-by: Jeff Garzik <jeff@garzik.org>
--------------------------
Apply? [y]es/[n]o/[e]dit/[v]iew patch/[a]ccept all e
Commit Body is:
--------------------------
pata_amd/pata_via: de-couple programming of PIO/MWDMA and UDMA timings

* Don't program UDMA timings when programming PIO or MWDMA modes.

  This has also a nice side-effect of fixing regression added by commit
  681c80b5d96076f447e8101ac4325c82d8dce508 ("libata: correct handling of
  SRST reset sequences") (->set_piomode method for PIO0 is called before
  ->cable_detect method which checks UDMA timings to get the cable type).

* Bump driver version.

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Tested-by: "Thomas Lindroth" <thomas.lindroth@gmail.com>
Acked-by: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: Tejun Heo <htejun@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Signed-off-by: Jeff Garzik <jeff@garzik.org>
--------------------------
Apply? [y]es/[n]o/[e]dit/[v]iew patch/[a]ccept all y

Applying 2.6.24-rc3-git6: Reported regressions from 2.6.23

Wrote tree 34cebd48e3b4e90fe3e6a6c6c03154ae1ed0c827
Committed: 943547abdfe9b4e27e36a25987909619908dffbf

Junio C Hamano· Dec 5, 2007, 07:01 UTC · re: Jeff Garzik · lore

Re: Cosmetic git-am interactive bug

Jeff Garzik <jeff@garzik.org> writes:

> The use of the older one-line summary led me to believe that it had
> not committed my changelog edits.  Looking at the result, however,
> proved that the commit changelog was my new, corrected version.

I knew about this for quite some time but it was a very low priority for me. This should fix it.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

diff --git a/git-am.sh b/git-am.sh
index 2e40708..339d863 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -117,6 +117,10 @@ It does not apply to blobs recorded in its index."
     unset GITHEAD_$his_tree
 }
 
+reread_subject () {
+	git stripspace <"$1" | sed -e 1q
+}
+
 prec=4
 dotest=.dotest sign= utf8=t keep= skip= interactive= resolved= binary=
 resolvemsg= resume=
@@ -376,6 +380,7 @@ do
 		[aA]*) action=yes interactive= ;;
 		[nN]*) action=skip ;;
 		[eE]*) git_editor "$dotest/final-commit"
+		       SUBJECT=$(reread_subject "$dotest/final-commit")
 		       action=again ;;
 		[vV]*) action=again
 		       LESS=-S ${PAGER:-less} "$dotest/patch" ;;

Jeff Garzik· Dec 5, 2007, 23:58 UTC · re: Junio C Hamano · lore

Re: Cosmetic git-am interactive bug

Junio C Hamano wrote:

Show 10 quoted lines

> Jeff Garzik <jeff@garzik.org> writes:
> 
>> The use of the older one-line summary led me to believe that it had
>> not committed my changelog edits.  Looking at the result, however,
>> proved that the commit changelog was my new, corrected version.
> 
> I knew about this for quite some time but it was a very low priority for
> me.  This should fix it.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Thanks!  :)