Skip to contentSkip to conversation

Reintegrate: send "Huh?" warnings to stderr, not stdout

PATCH Superseded Erik Cervin-Edin 2 participants 4 messages Apr 27 – May 4

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Reintegrate: send "Huh?" warnings to stderr, not stdout

The "Huh?: $msg" warning in show_merge(), emitted when a first-parent merge subject does not match either "Merge branch '...'" or "Merge remote branch '...'", uses

	echo 2>&1 "Huh?: $msg"

The "2>&1" redirect dupes stderr onto stdout's destination; it does not change where stdout itself points. Since echo writes to stdout, the "Huh?:" message lands on stdout regardless -- as would any command's normal output. The intent appears to have been ">&2", which dupes stdout onto stderr.

In the common Reintegrate invocation that captures stdout, e.g.

Meta/Reintegrate next..seen >Meta/redo-seen.sh

this means the warning is silently embedded in the generated heredoc body instead of being printed to the maintainer's terminal. The resulting redo-* script is corrupted with a "Huh?:..." line and the maintainer has no diagnostic that something went wrong.

Every other diagnostic in this script already uses ">&2"; this line is the lone outlier.

Use ">&2" so the warning reaches stderr as intended.

---
 Reintegrate | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Reintegrate +1 -1
index a1e67a0330..6fdc7c5f41 100755
@@ -327,7 +327,7 @@ show_merge () {
 		merge_hier=
 		;;
 	*)
-		echo 2>&1 "Huh?: $msg"
+		echo >&2 "Huh?: $msg"
 		return
 		;;
 	esac &&
Signature · 1 line
2.53.0
#1 lore raw
Quoted 5 lines · Erik Cervin-Edin

The "Huh?: $msg" warning in show_merge(), emitted when a first-parent merge subject does not match either "Merge branch '...'" or "Merge remote branch '...'", uses

	echo 2>&1 "Huh?: $msg"

Yes, this is clearly wrong.

Quoted 6 lines

The "2>&1" redirect dupes stderr onto stdout's destination; it does not change where stdout itself points. Since echo writes to stdout, the "Huh?:" message lands on stdout regardless -- as would any command's normal output.

The intent appears to have been ">&2", which dupes stdout onto stderr.

It would be better for me if this sentence (i.e., what was the code originally *intended* to do before the patch?) were placed right at the beginning. Something similar to:

In show_merge(), the warning "Huh?: $msg" is emitted to stdout because
it uses the erroneous redirect echo 2>&1. The intent was clearly to use >&2 to print to stderr...

Of course, it’s up to you ;)

Quoted 10 lines

In the common Reintegrate invocation that captures stdout, e.g.

Meta/Reintegrate next..seen >Meta/redo-seen.sh

this means the warning is silently embedded in the generated heredoc body instead of being printed to the maintainer's terminal. The resulting redo-* script is corrupted with a "Huh?:..." line and the maintainer has no diagnostic that something went wrong.

Quoted 4 lines

Every other diagnostic in this script already uses ">&2"; this line is the lone outlier.

Use ">&2" so the warning reaches stderr as intended.

Overall, the reasoning is nice :-)

Quoted 10 lines
---
  Reintegrate | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
Reintegrate +0 -0
index a1e67a0330..6fdc7c5f41 100755

Reintegrate show_merge () { · line 333

Quoted 8 lines
Reintegrate +1 -1
@@ -327,7 +327,7 @@ show_merge () {
  		merge_hier=
  		;;
  	*)
-		echo 2>&1 "Huh?: $msg"
+		echo >&2 "Huh?: $msg"
  		return
  		;;
  	esac &&

Thanks, Yuchen

#2 lore raw
Quoted 9 lines · Tian Yuchen

It would be better for me if this sentence (i.e., what was the code originally *intended* to do before the patch?) were placed right at the beginning. Something similar to:

In show_merge(), the warning "Huh?: $msg" is emitted to stdout because it
uses the erroneous redirect echo 2>&1. The intent was clearly to use >&2 to print to stderr...

Of course, it’s up to you ;)

Good point! I hoisted the intent up to the opening paragraph; took the spirit of your wording rather than copying it verbatim, and tightened the rest while I was at it.

#3 lore raw

PATCH v2 1/1 Reintegrate: send "Huh?" warnings to stderr, not stdout

In reply to Erik Cervin-Edin

From: Erik Cervin-Edin <[email protected]>

In show_merge(), the "Huh?: $msg" warning -- emitted when a first-parent merge subject does not match either "Merge branch '...'" or "Merge remote branch '...'" -- is meant to go to stderr, but the redirect

	echo 2>&1 "Huh?: $msg"

goes the wrong way and sends the message to stdout instead.

In the common Reintegrate invocation that captures stdout, e.g.

Meta/Reintegrate next..seen >Meta/redo-seen.sh

this means the warning is silently embedded in the generated heredoc body instead of being printed to the maintainer's terminal, with no diagnostic that something went wrong. Worse, the resulting redo-* script is corrupted with a "Huh?:..." line.

Switch the redirection to ">&2", matching every other diagnostic in this script.

---
 Reintegrate | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Reintegrate +1 -1
index a1e67a0330..6fdc7c5f41 100755
@@ -327,7 +327,7 @@ show_merge () {
 		merge_hier=
 		;;
 	*)
-		echo 2>&1 "Huh?: $msg"
+		echo >&2 "Huh?: $msg"
 		return
 		;;
 	esac &&
Signature · 1 line
2.53.0