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

Re: [PATCH v2 0/3] Add regression tests for rectent rebase -i fixes

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Jun 16, 2017, 13:49 UTC
Message-ID
<alpine.DEB.2.21.1.1706161527140.4200@virtualbox>
In-Reply-To
<xmqqfuf0ygmu.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Thu, 15 Jun 2017, Junio C Hamano wrote:
Show 13 quoted lines
> diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh
> index 325ec75353..801bce25da 100755
> --- a/t/t3420-rebase-autostash.sh
> +++ b/t/t3420-rebase-autostash.sh
> @@ -45,7 +45,7 @@ create_expected_success_am() {
>  }
>  
>  create_expected_success_interactive() {
> -	cr=$'\r' &&
> +	cr=$(echo . | tr '.' '\015') &&
>  	cat >expected <<-EOF
>  	$(grep "^Created autostash: [0-9a-f][0-9a-f]*\$" actual)
>  	HEAD is now at $(git rev-parse --short feature-branch) third commit

This is still incorrect, as the \r\n (which $(echo . | tr \.\ '\015') would emit) is interpreted correctly as a line break on Windows, meaning that cr is now *empty*. Not what we want.

What I did is to replace the `cat` by `q_to_cr` (we have that lovely function, might just as well use it), replace `${cr}` by `Q` and skip the cr variable altogether.

Additionally, there is another huge problem: the "Applied autostash." is printed to stdout, not stderr, in line with how things were done in the shell version of rebase -i.

While this was just a minor bug previously, now we exercise that bug, by redirecting stderr to stdout and redirecting stdout to the file `actual`. Nothing says that stderr should be printed to that file before stdout, but that is exactly what the test case tries to verify.

There is only one slight problem: in my Git for Windows SDK, stdout is printed first.

The quick fix would be to redirect stderr and stdout independently.

However, I think it is time for that bug to be fixed: autostash messages should really go to stderr, just like the rest of them rebase messages.

I attached the patch, together with the two fixups to Phillip's patches, and a fixup for the autostash-messages-to-stderr patch that I think should be squashed in but I really ran out of time testing this.

Phillip, would you mind picking those changes up as you deem appropriate?

Ciao, Dscho

From c5a8319f0378d93be5aea05b833bb5e23c9f0b3d Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Fri, 16 Jun 2017 15:34:50 +0200
Subject: [PATCH 1/4] sequencer: print autostash messages to stderr

The rebase messages are printed to stderr traditionally. It was a bug introduced in 587947750bd (rebase: implement --[no-]autostash and rebase.autostash, 2013-05-12) and that bug has been faithfully copied when reimplementing parts of the interactive rebase in the sequencer.

It is time to fix that: let's print the autostash messages to stderr instead of stdout.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 sequencer.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index a59408a23a2..8713cc8d1d5 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1923,7 +1923,7 @@ static int apply_autostash(struct replay_opts *opts)
 	argv_array_push(&child.args, "apply");
 	argv_array_push(&child.args, stash_sha1.buf);
 	if (!run_command(&child))
-		printf(_("Applied autostash.\n"));
+		fprintf(stderr, _("Applied autostash.\n"));
 	else {
 		struct child_process store = CHILD_PROCESS_INIT;
 
@@ -1937,10 +1937,11 @@ static int apply_autostash(struct replay_opts *opts)
 		if (run_command(&store))
 			ret = error(_("cannot store %s"), stash_sha1.buf);
 		else
-			printf(_("Applying autostash resulted in conflicts.\n"
-				"Your changes are safe in the stash.\n"
-				"You can run \"git stash pop\" or"
-				" \"git stash drop\" at any time.\n"));
+			fprintf(stderr,
+				_("Applying autostash resulted in conflicts.\n"
+				  "Your changes are safe in the stash.\n"
+				  "You can run \"git stash pop\" or"
+				  " \"git stash drop\" at any time.\n"));
 	}
 
 	strbuf_release(&stash_sha1);

-- 
2.12.2.windows.1



From 3425f7c9bfb62c3d2d4adaff41a664dd4ae2efa9 Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Fri, 16 Jun 2017 15:39:20 +0200
Subject: [PATCH 2/4] fixup! rebase: add regression tests for console output

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t3420-rebase-autostash.sh | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh
index 801bce25da4..64904bef069 100755
--- a/t/t3420-rebase-autostash.sh
+++ b/t/t3420-rebase-autostash.sh
@@ -45,12 +45,11 @@ create_expected_success_am() {
 }
 
 create_expected_success_interactive() {
-	cr=$(echo . | tr '.' '\015') &&
-	cat >expected <<-EOF
+	q_to_cr >expected <<-EOF
 	$(grep "^Created autostash: [0-9a-f][0-9a-f]*\$" actual)
 	HEAD is now at $(git rev-parse --short feature-branch) third commit
-	Rebasing (1/2)${cr}Rebasing (2/2)${cr}Successfully rebased and updated refs/heads/rebased-feature-branch.
-	Applied autostash.
+	Rebasing (1/2)QRebasing (2/2)QApplied autostash.
+	Successfully rebased and updated refs/heads/rebased-feature-branch.
 	EOF
 }
 
-- 
2.12.2.windows.1



From e16ad989c85c55bdfcf45fe561911a699e962b44 Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Fri, 16 Jun 2017 15:39:35 +0200
Subject: [PATCH 3/4] fixup! rebase: add more regression tests for console
 output

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t3420-rebase-autostash.sh | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh
index 64904bef069..2c01ae6ad2a 100755
--- a/t/t3420-rebase-autostash.sh
+++ b/t/t3420-rebase-autostash.sh
@@ -102,14 +102,13 @@ create_expected_failure_am() {
 }
 
 create_expected_failure_interactive() {
-	cr=$(echo . | tr '.' '\015') &&
-	cat >expected <<-EOF
+	q_to_cr >expected <<-EOF
 	$(grep "^Created autostash: [0-9a-f][0-9a-f]*\$" actual)
 	HEAD is now at $(git rev-parse --short feature-branch) third commit
-	Rebasing (1/2)${cr}Rebasing (2/2)${cr}Successfully rebased and updated refs/heads/rebased-feature-branch.
-	Applying autostash resulted in conflicts.
+	Rebasing (1/2)QRebasing (2/2)QApplying autostash resulted in conflicts.
 	Your changes are safe in the stash.
 	You can run "git stash pop" or "git stash drop" at any time.
+	Successfully rebased and updated refs/heads/rebased-feature-branch.
 	EOF
 }
 
-- 
2.12.2.windows.1



From 128c4a66ab3413b2135264f17024e1f5e9250f03 Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Fri, 16 Jun 2017 15:40:46 +0200
Subject: [PATCH 4/4] fixup! sequencer: print autostash messages to stderr

This is the companion to the proposed patch, but I had no time to verify
that the test suite still passes (it may actually test for the buggy
behavior...)

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 git-rebase.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index db1deed8464..2cf73b88e8e 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -166,14 +166,14 @@ apply_autostash () {
 		stash_sha1=$(cat "$state_dir/autostash")
 		if git stash apply $stash_sha1 2>&1 >/dev/null
 		then
-			echo "$(gettext 'Applied autostash.')"
+			echo "$(gettext 'Applied autostash.')" >&2
 		else
 			git stash store -m "autostash" -q $stash_sha1 ||
 			die "$(eval_gettext "Cannot store \$stash_sha1")"
 			gettext 'Applying autostash resulted in conflicts.
 Your changes are safe in the stash.
 You can run "git stash pop" or "git stash drop" at any time.
-'
+' >&2
 		fi
 	fi
 }
-- 
2.12.2.windows.1
Previous: Junio C HamanoNext: Johannes Sixt
Message 39 of 59 in “Add regression tests for recent rebase -i fixes”
  1. 0/3 Add regression tests for recent rebase -i fixesPhillip Wood, May 31, 2017
  2. 3/3 rebase: Add tests for console output with conflicting stashPhillip Wood, May 31, 2017
  3. 1/3 rebase -i: Add test for reflog messagePhillip Wood, May 31, 2017
  4. Junio C HamanoJun 1, 2017
  5. 2/3 rebase: Add tests for console outputPhillip Wood, May 31, 2017
  6. Phillip WoodMay 31, 2017
  7. Junio C HamanoJun 1, 2017
  8. Johannes SchindelinJun 1, 2017
  9. Junio C HamanoJun 1, 2017
  10. Stefan BellerJun 1, 2017
  11. pushing for a new hash, was Re: [PATCH 2/3] rebase: Add tests for console outputJohannes Schindelin, Jun 2, 2017
  12. Jonathan NiederJun 2, 2017
  13. Jonathan NiederJun 2, 2017
  14. Ævar Arnfjörð BjarmasonJun 2, 2017
  15. Johannes SchindelinJun 15, 2017
  16. Junio C HamanoJun 3, 2017
  17. Johannes SchindelinJun 6, 2017
  18. Jonathan NiederJun 6, 2017
  19. Junio C HamanoJun 7, 2017
  20. t4005: modernize style and drop hard coded sha1Stefan Beller, Jun 7, 2017
  21. Brandon WilliamsJun 7, 2017
  22. Stefan BellerJun 6, 2017
  23. Jonathan NiederJun 6, 2017
  24. Samuel LijinJun 7, 2017
  25. Johannes SchindelinJun 7, 2017
  26. Stefan BellerJun 7, 2017
  27. Phillip WoodJun 7, 2017
  28. Junio C HamanoJun 9, 2017
  29. Phillip WoodJun 14, 2017
  30. Johannes SchindelinJun 14, 2017
  31. 0/3 Add regression tests for rectent rebase -i fixesPhillip Wood, Jun 14, 2017
  32. 1/3 rebase -i: Add test for reflog messagePhillip Wood, Jun 14, 2017
  33. 2/3 rebase: Add regression tests for console outputPhillip Wood, Jun 14, 2017
  34. 3/3 rebase: Add more regression tests for console outputPhillip Wood, Jun 14, 2017
  35. Johannes SchindelinJun 14, 2017
  36. Junio C HamanoJun 15, 2017
  37. Junio C HamanoJun 15, 2017
  38. Junio C HamanoJun 15, 2017
  39. Johannes SchindelinJun 16, 2017
  40. Johannes SixtJun 16, 2017
  41. Junio C HamanoJun 16, 2017
  42. Johannes SixtJun 19, 2017
  43. Junio C HamanoJun 19, 2017
  44. Phillip WoodJun 19, 2017
  45. Junio C HamanoJun 19, 2017
  46. Phillip WoodJun 19, 2017
  47. 0/4 Add regression tests for recent rebase -i fixesPhillip Wood, Jun 19, 2017
  48. 1/4 sequencer: print autostash messages to stderrPhillip Wood, Jun 19, 2017
  49. 2/4 rebase -i: Add test for reflog messagePhillip Wood, Jun 19, 2017
  50. 3/4 rebase: Add regression tests for console outputPhillip Wood, Jun 19, 2017
  51. 4/4 rebase: Add more regression tests for console outputPhillip Wood, Jun 19, 2017
  52. Junio C HamanoJun 23, 2017
  53. Junio C HamanoJun 23, 2017
  54. Phillip WoodJun 23, 2017
  55. Junio C HamanoJun 23, 2017
  56. Junio C HamanoJun 23, 2017
  57. Phillip WoodJun 26, 2017
  58. Junio C HamanoJun 23, 2017
  59. Phillip WoodJun 26, 2017

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.