git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 23/23] rebase -i: enable options --signoff, --reset-author for pick, reword

From
Fabian Ruch <bafain@gmail.com>
Date
Aug 12, 2014, 21:04 UTC
Message-ID
<53EA814C.2050802@gmail.com>
In-Reply-To
<87sil6ajbx.fsf@thomasrast.ch>
Hi Thomas,
Thomas Rast writes:
Show 12 quoted lines
> Fabian Ruch <bafain@gmail.com> writes:
>> @@ -634,21 +644,24 @@ do_replay () {
>>  		comment_for_reflog pick
>>  
>>  		mark_action_done
>> -		do_pick $sha1 || die_with_patch $sha1 "Could not apply $sha1... $rest"
>> +		eval do_pick $opts $sha1 \
>> +			|| die_with_patch $sha1 "Could not apply $sha1... $rest"
> 
> You had me a little puzzled at the switch to 'eval' here.  That is
> necessary to match the quoting added in 20/23, not for any change in
> this commit.  This commit is simply the first one to trigger this.

This patch switches to 'eval' here because it is the first time 'opts' occurs. However, I agree that it might be confusing that 'opts' wasn't already added to the 'do_pick' lines by 20/23. By "trigger" you mean that this commit is the first to actually fill 'opts' with contents? I will move these changes to 20/23 then.

> Also, are you sure $sha1 does not require quoting through an eval?

At least if we can assume that it is really a SHA-1 object name. As such it does not contain characters interpreted by the shell, like backslashes, quotes or whitespace.

> Please add tests to this patch.

The ones I had in mind are attached below the scissors line. The current reroll fails the authorship checks and the 'git rebase --continue' test cases. As the necessary changes would obfuscate this sub-thread, they will be included in the next reroll.

   Fabian
-- >8 --
diff --git a/t/t3427-rebase-line-options.sh b/t/t3427-rebase-line-options.sh
index 5881162..a5a9e66 100755
--- a/t/t3427-rebase-line-options.sh
+++ b/t/t3427-rebase-line-options.sh
@@ -6,10 +6,32 @@ test_description='git rebase -i with line options'
 
 . "$TEST_DIRECTORY"/lib-rebase.sh
 
+commit_message () {
+	git cat-file commit "$1" | sed '1,/^$/d'
+}
+
+commit_authorship () {
+	git cat-file commit "$1" | sed -n '/^$/q;/^author /p'
+}
+
+authorship () {
+	echo "author $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL> $GIT_AUTHOR_DATE"
+}
+
+test_diff_file () {
+	if cmp "$1" "$2" >/dev/null
+	then
+		echo "'$1' and '$2' are the same"
+		return 1
+	fi
+}
+
 test_expect_success 'Set up repository' '
 	test_commit Initial &&
 	test_commit Commit1 &&
-	test_commit Commit2
+	test_commit Commit2 &&
+	git checkout -b branch Commit1 &&
+	test_commit Commit2_ Commit2.t
 '
 
 test_expect_success 'Unknown option' '
@@ -23,4 +45,137 @@ test_expect_success 'Unknown option' '
 	git rebase --continue
 '
 
+test_msg_author () {
+	set_fake_editor &&
+	FAKE_LINES="1 $1 2" git rebase -i HEAD~2 &&
+	commit_message HEAD >actual.msg &&
+	commit_authorship HEAD >actual.author &&
+	test_cmp expected.msg actual.msg &&
+	test_cmp expected.author actual.author
+}
+
+test_msg_author_misspelled () {
+	set_cat_todo_editor &&
+	test_must_fail git rebase -i HEAD^ >todo &&
+	set_fake_editor &&
+	test_must_fail env FAKE_LINES="1 $1-misspelled 2" git rebase -i HEAD~2 &&
+	set_fixed_todo_editor "$(pwd)"/todo &&
+	FAKE_LINES="$1 1" git rebase --edit-todo &&
+	git rebase --continue &&
+	commit_message HEAD >actual.msg &&
+	commit_authorship HEAD >actual.author &&
+	test_cmp expected.msg actual.msg &&
+	test_cmp expected.author actual.author
+}
+
+test_msg_author_conflicted () {
+	set_fake_editor &&
+	test_must_fail env FAKE_LINES="$1 1" git rebase -i master &&
+	git checkout --theirs Commit2.t &&
+	git add Commit2.t &&
+	git rebase --continue &&
+	commit_message HEAD >actual.msg &&
+	commit_authorship HEAD >actual.author &&
+	test_cmp expected.msg actual.msg &&
+	test_cmp expected.author actual.author
+}
+
+test_expect_success 'Misspelled pick --signoff' '
+	git checkout -b misspelled-pick--signoff master &&
+	cat >expected.msg <<-EOF &&
+	$(commit_message HEAD)
+
+	Signed-off-by: C O Mitter <committer@example.com>
+	EOF
+	commit_authorship HEAD >expected.author &&
+	test_msg_author_misspelled pick_--signoff
+'
+
+test_expect_success 'Conflicted pick --signoff' '
+	git checkout -b conflicted-pick--signoff branch &&
+	cat >expected.msg <<-EOF &&
+	$(commit_message HEAD)
+
+	Signed-off-by: C O Mitter <committer@example.com>
+	EOF
+	commit_authorship HEAD >expected.author &&
+	test_msg_author_conflicted pick_--signoff
+'
+
+test_expect_success 'pick --signoff' '
+	git checkout -b pick--signoff master &&
+	cat >expected.msg <<-EOF &&
+	$(commit_message HEAD)
+
+	Signed-off-by: C O Mitter <committer@example.com>
+	EOF
+	commit_authorship HEAD >expected.author &&
+	test_msg_author pick_--signoff
+'
+
+test_expect_success 'reword --signoff' '
+	git checkout -b reword--signoff master &&
+	cat >expected.msg <<-EOF &&
+	$(commit_message HEAD)
+
+	Signed-off-by: C O Mitter <committer@example.com>
+	EOF
+	commit_authorship HEAD >expected.author &&
+	test_msg_author reword_--signoff
+'
+
+test_expect_success 'Misspelled pick --reset-author' '
+	git checkout -b misspelled-pick--reset-author master &&
+	commit_message HEAD >expected.msg &&
+	test_tick &&
+	authorship >expected.author &&
+	commit_authorship HEAD >original.author &&
+	test_diff_file expected.author original.author &&
+	test_msg_author_misspelled pick_--reset-author
+'
+
+test_expect_success 'Conflicted pick --reset-author' '
+	git checkout -b conflicted-pick--reset-author branch &&
+	commit_message HEAD >expected.msg &&
+	test_tick &&
+	authorship >expected.author &&
+	commit_authorship HEAD >original.author &&
+	test_diff_file expected.author original.author &&
+	test_msg_author_conflicted pick_--reset-author
+'
+
+test_expect_success 'pick --reset-author' '
+	git checkout -b pick--reset-author master &&
+	commit_message HEAD >expected.msg &&
+	test_tick &&
+	authorship >expected.author &&
+	commit_authorship HEAD >original.author &&
+	test_diff_file expected.author original.author &&
+	test_msg_author pick_--reset-author
+'
+
+test_expect_success 'pick --reset-author --signoff' '
+	git checkout -b pick--reset-author--signoff master &&
+	cat >expected.msg <<-EOF &&
+	$(commit_message HEAD)
+
+	Signed-off-by: C O Mitter <committer@example.com>
+	EOF
+	test_tick &&
+	authorship >expected.author &&
+	commit_authorship HEAD >original.author &&
+	test_diff_file expected.author original.author &&
+	test_msg_author pick_--reset-author_--signoff
+'
+
+test_expect_success 'reword --reset-author' '
+	git checkout -b reword--reset-author master &&
+	commit_message HEAD >expected.msg &&
+	test_tick &&
+	authorship >expected.author &&
+	commit_authorship HEAD >original.author &&
+	test_diff_file expected.author original.author &&
+	test_msg_author reword_--reset-author
+'
+
 test_done
Previous: Thomas RastNext: Michael Haggerty
Message 117 of 148 in “rebase -i: Implement `reword` and `squash` in terms of `do_pick`”
  1. 0/7 rebase -i: Implement `reword` and `squash` in terms of `do_pick`Fabian Ruch, Jun 19, 2014
  2. 00/19 Enable options --signoff, --reset-author for pick, rewordFabian Ruch, Jul 2, 2014
  3. 01/19 rebase -i: Failed reword prints redundant error messageFabian Ruch, Jul 2, 2014
  4. Junio C HamanoJul 8, 2014
  5. Andrew WongJul 10, 2014
  6. Fabian RuchJul 10, 2014
  7. Andrew WongJul 10, 2014
  8. 02/19 rebase -i: reword complains about empty commit despite --keep-emptyFabian Ruch, Jul 2, 2014
  9. Junio C HamanoJul 8, 2014
  10. Fabian RuchJul 9, 2014
  11. 03/19 rebase -i: reword executes pre-commit hook on interim commitFabian Ruch, Jul 2, 2014
  12. Junio C HamanoJul 8, 2014
  13. Fabian RuchJul 13, 2014
  14. 04/19 rebase -i: Teach do_pick the option --editFabian Ruch, Jul 2, 2014
  15. 05/19 rebase -i: Implement reword in terms of do_pickFabian Ruch, Jul 2, 2014
  16. Matthieu MoyAug 4, 2014
  17. Fabian RuchAug 4, 2014
  18. 06/19 rebase -i: Stop on root commits with empty log messagesFabian Ruch, Jul 2, 2014
  19. Junio C HamanoJul 8, 2014
  20. Fabian RuchJul 10, 2014
  21. Junio C HamanoJul 10, 2014
  22. Junio C HamanoJul 10, 2014
  23. 07/19 rebase -i: The replay of root commits is not shown with --verboseFabian Ruch, Jul 2, 2014
  24. Junio C HamanoJul 8, 2014
  25. Fabian RuchJul 11, 2014
  26. Chris WebbJul 15, 2014
  27. 08/19 rebase -i: Root commits are replayed with an unnecessary optionFabian Ruch, Jul 2, 2014
  28. Junio C HamanoJul 8, 2014
  29. Fabian RuchJul 18, 2014
  30. Junio C HamanoJul 18, 2014
  31. Fabian RuchJul 19, 2014
  32. 09/19 rebase -i: Commit only once when rewriting picksFabian Ruch, Jul 2, 2014
  33. 10/19 rebase -i: Do not die in do_pickFabian Ruch, Jul 2, 2014
  34. 11/19 rebase -i: Teach do_pick the option --amendFabian Ruch, Jul 2, 2014
  35. 12/19 rebase -i: Teach do_pick the option --fileFabian Ruch, Jul 2, 2014
  36. 13/19 rebase -i: Prepare for squash in terms of do_pick --amendFabian Ruch, Jul 2, 2014
  37. 14/19 rebase -i: Implement squash in terms of do_pickFabian Ruch, Jul 2, 2014
  38. 15/19 rebase -i: Explicitly distinguish replay commands and exec tasksFabian Ruch, Jul 2, 2014
  39. Junio C HamanoJul 10, 2014
  40. 16/19 rebase -i: Parse to-do list command line optionsFabian Ruch, Jul 2, 2014
  41. 17/19 rebase -i: Teach do_pick the option --reset-authorFabian Ruch, Jul 2, 2014
  42. 18/19 rebase -i: Teach do_pick the option --signoffFabian Ruch, Jul 2, 2014
  43. 19/19 rebase -i: Enable options --signoff, --reset-author for pick, rewordFabian Ruch, Jul 2, 2014
  44. Michael HaggertyJul 3, 2014
  45. Junio C HamanoJul 8, 2014
  46. Fabian RuchJul 9, 2014
  47. Thomas RastJul 18, 2014
  48. 00/19 Enable options --signoff, --reset-author for pick, rewordFabian Ruch, Jul 28, 2014
  49. 01/19 rebase -i: failed reword prints redundant error messageFabian Ruch, Jul 28, 2014
  50. 02/19 rebase -i: allow rewording an empty commit without complaintsFabian Ruch, Jul 28, 2014
  51. 03/19 rebase -i: reword executes pre-commit hook on interim commitFabian Ruch, Jul 28, 2014
  52. Jeff KingAug 1, 2014
  53. Fabian RuchAug 4, 2014
  54. Jeff KingAug 6, 2014
  55. 04/19 rebase -i: teach do_pick the option --editFabian Ruch, Jul 28, 2014
  56. 05/19 rebase -i: implement reword in terms of do_pickFabian Ruch, Jul 28, 2014
  57. 06/19 rebase -i: allow replaying commits with empty log messagesFabian Ruch, Jul 28, 2014
  58. 07/19 rebase -i: log the replay of root commitsFabian Ruch, Jul 28, 2014
  59. Jeff KingAug 2, 2014
  60. Fabian RuchAug 4, 2014
  61. Jeff KingAug 6, 2014
  62. 08/19 rebase -i: root commits are replayed with an unnecessary optionFabian Ruch, Jul 28, 2014
  63. Jeff KingAug 2, 2014
  64. Fabian RuchAug 4, 2014
  65. 09/19 rebase -i: commit only once when rewriting picksFabian Ruch, Jul 28, 2014
  66. Jeff KingAug 2, 2014
  67. Fabian RuchAug 7, 2014
  68. 10/19 rebase -i: do not die in do_pickFabian Ruch, Jul 28, 2014
  69. 11/19 rebase -i: teach do_pick the option --amendFabian Ruch, Jul 28, 2014
  70. 12/19 rebase -i: teach do_pick the option --fileFabian Ruch, Jul 28, 2014
  71. 13/19 rebase -i: prepare for squash in terms of do_pick --amendFabian Ruch, Jul 28, 2014
  72. 14/19 rebase -i: implement squash in terms of do_pickFabian Ruch, Jul 28, 2014
  73. 15/19 rebase -i: explicitly distinguish replay commands and exec tasksFabian Ruch, Jul 28, 2014
  74. 16/19 rebase -i: parse to-do list command line optionsFabian Ruch, Jul 28, 2014
  75. 17/19 rebase -i: teach do_pick the option --reset-authorFabian Ruch, Jul 28, 2014
  76. 18/19 rebase -i: teach do_pick the option --signoffFabian Ruch, Jul 28, 2014
  77. 19/19 rebase -i: enable options --signoff, --reset-author for pick, rewordFabian Ruch, Jul 28, 2014
  78. Jeff KingAug 2, 2014
  79. Fabian RuchAug 4, 2014
  80. 00/23 Enable options --signoff, --reset-author for pick, rewordFabian Ruch, Aug 6, 2014
  81. 01/23 rebase -i: allow replaying commits with empty log messagesFabian Ruch, Aug 6, 2014
  82. 02/23 rebase -i: allow squashing empty commits without complaintsFabian Ruch, Aug 6, 2014
  83. Peter KreftingAug 7, 2014
  84. Eric SunshineAug 7, 2014
  85. Fabian RuchAug 11, 2014
  86. Phil HordAug 13, 2014
  87. 03/23 rebase -i: allow rewording empty commits without complaintsFabian Ruch, Aug 6, 2014
  88. 04/23 rebase -i: hide interactive command messages in verbose modeFabian Ruch, Aug 6, 2014
  89. Thomas RastAug 8, 2014
  90. Fabian RuchAug 11, 2014
  91. Thomas RastAug 11, 2014
  92. 05/23 rebase -i: failed reword prints redundant error messageFabian Ruch, Aug 6, 2014
  93. 06/23 commit: allow disabling pre-commit and commit-msg separatelyFabian Ruch, Aug 6, 2014
  94. 07/23 rebase -i: squash skips commit-msg hookFabian Ruch, Aug 6, 2014
  95. 08/23 rebase -i: reword executes pre-commit hook on interim commitFabian Ruch, Aug 6, 2014
  96. Thomas RastAug 8, 2014
  97. Fabian RuchAug 11, 2014
  98. Thomas RastAug 11, 2014
  99. 09/23 rebase -i: teach do_pick the option --editFabian Ruch, Aug 6, 2014
  100. 10/23 rebase -i: implement reword in terms of do_pickFabian Ruch, Aug 6, 2014
  101. 11/23 rebase -i: log the replay of root commitsFabian Ruch, Aug 6, 2014
  102. 12/23 rebase -i: root commits are replayed with an unnecessary optionFabian Ruch, Aug 6, 2014
  103. 13/23 rebase -i: commit only once when rewriting picksFabian Ruch, Aug 6, 2014
  104. 14/23 rebase -i: do not die in do_pickFabian Ruch, Aug 6, 2014
  105. 15/23 rebase -i: teach do_pick the option --amendFabian Ruch, Aug 6, 2014
  106. 16/23 rebase -i: teach do_pick the option --fileFabian Ruch, Aug 6, 2014
  107. 17/23 rebase -i: prepare for squash in terms of do_pick --amendFabian Ruch, Aug 6, 2014
  108. 18/23 rebase -i: implement squash in terms of do_pickFabian Ruch, Aug 6, 2014
  109. 19/23 rebase -i: explicitly distinguish replay commands and exec tasksFabian Ruch, Aug 6, 2014
  110. 20/23 rebase -i: parse to-do list command line optionsFabian Ruch, Aug 6, 2014
  111. Thomas RastAug 8, 2014
  112. Fabian RuchAug 11, 2014
  113. 21/23 rebase -i: teach do_pick the option --reset-authorFabian Ruch, Aug 6, 2014
  114. 22/23 rebase -i: teach do_pick the option --signoffFabian Ruch, Aug 6, 2014
  115. 23/23 rebase -i: enable options --signoff, --reset-author for pick, rewordFabian Ruch, Aug 6, 2014
  116. Thomas RastAug 8, 2014
  117. Fabian RuchAug 12, 2014
  118. Michael HaggertyAug 13, 2014
  119. Fabian RuchAug 14, 2014
  120. Fabian RuchSep 21, 2014
  121. 00/27 Enable options --signoff, --reset-author for pick, reword, editFabian Ruch, Aug 18, 2014
  122. 01/27 rebase -i: allow replaying commits with empty log messagesFabian Ruch, Aug 18, 2014
  123. 02/27 rebase -i: allow squashing empty commits without complaintsFabian Ruch, Aug 18, 2014
  124. 03/27 rebase -i: allow rewording empty commits without complaintsFabian Ruch, Aug 18, 2014
  125. 04/27 fake_editor: leave standard output unchangedFabian Ruch, Aug 18, 2014
  126. 05/27 rebase -i: hide interactive command messages in verbose modeFabian Ruch, Aug 18, 2014
  127. 06/27 rebase -i: discard redundant message when rewording failsFabian Ruch, Aug 18, 2014
  128. 07/27 commit: allow disabling pre-commit and commit-msg separatelyFabian Ruch, Aug 18, 2014
  129. 08/27 rebase -i: verify squash messages using commit-msgFabian Ruch, Aug 18, 2014
  130. 09/27 rebase -i: do not verify reworded patches using pre-commitFabian Ruch, Aug 18, 2014
  131. 10/27 rebase -i: teach do_pick the option --editFabian Ruch, Aug 18, 2014
  132. 11/27 rebase -i: implement reword in terms of do_pickFabian Ruch, Aug 18, 2014
  133. 12/27 rebase -i: log the replay of root commitsFabian Ruch, Aug 18, 2014
  134. 13/27 rebase -i: do not use -C when --no-edit is sufficientFabian Ruch, Aug 18, 2014
  135. 14/27 rebase -i: commit only once when rewriting picksFabian Ruch, Aug 18, 2014
  136. 15/27 rebase -i: do not die in do_pickFabian Ruch, Aug 18, 2014
  137. 16/27 rebase -i: teach do_pick the option --amendFabian Ruch, Aug 18, 2014
  138. 17/27 rebase -i: teach do_pick the option --fileFabian Ruch, Aug 18, 2014
  139. 18/27 rebase -i: remove no-op do_with_author git commit --amendFabian Ruch, Aug 18, 2014
  140. 19/27 rebase -i: prepare for squash in terms of do_pick --amendFabian Ruch, Aug 18, 2014
  141. 20/27 rebase -i: implement squash in terms of do_pickFabian Ruch, Aug 18, 2014
  142. 21/27 rebase -i: explicitly distinguish replay commands and exec tasksFabian Ruch, Aug 18, 2014
  143. 22/27 rebase -i: parse to-do list command line optionsFabian Ruch, Aug 18, 2014
  144. 23/27 rebase -i: teach do_pick the option --reset-authorFabian Ruch, Aug 18, 2014
  145. 24/27 rebase -i: teach do_pick the option --signoffFabian Ruch, Aug 18, 2014
  146. 25/27 rebase -i: do not overwrite user author informationFabian Ruch, Aug 18, 2014
  147. 26/27 rebase -i: refuse to commit when resuming with updated headFabian Ruch, Aug 18, 2014
  148. 27/27 rebase -i: enable --signoff, --reset-author for pick, reword, editFabian Ruch, Aug 18, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.