Thread (14 messages) 14 messages, 3 authors, 2020-01-18

Re: [PATCH 2/3] t6025: replace pipe with redirection operator

From: Eric Sunshine <hidden>
Date: 2020-01-17 21:24:50

Possibly related (same subject, not in this thread)

On Fri, Jan 17, 2020 at 3:45 PM Shourya Shukla
[off-list ref] wrote:
The exit code of pipes(|) are always ignored, which will create
errors in subsequent statements. Let's handle it by redirecting
its output to a file and capturing return values. Replace pipe
with redirect(>) operator.
This is not an accurate description. The proper way to explain this is
that exit code of a command upstream of a pipe is lost; the exit code
of a command downstream is not lost. We don't want to lose the exit
code of a git command, so a git command should not be upstream. (We
don't care about non-git commands being upstream when that command's
exit code is not relevant.)
quoted hunk ↗ jump to hunk
Signed-off-by: Shourya Shukla <redacted>
---
diff --git a/t/t6025-merge-symlinks.sh b/t/t6025-merge-symlinks.sh
@@ -17,14 +17,13 @@ test_expect_success 'setup' '
        git commit -m initial &&
        git branch b-symlink &&
        git branch b-file &&
-       l=$(printf file | git hash-object -t blob -w --stdin) &&
This command is fine as-is. We are interested in the exit code of the
git command (which is correctly downstream), and we don't care about
the exit code of 'printf' (which is upstream), so there is no reason
to rewrite this to use temporary files instead.
+       printf file >file &&
+       l=$(git hash-object -t blob -w --stdin) &&
Sorry, but this just doesn't make sense. You're telling "git
hash-object" to take its input from the standard input stream, yet you
don't feed anything to it on that stream. If anything, it should have
been written like this:

     l=$(git hash-object -t blob -w --stdin <file) &&

however, as noted above, there is no reason to avoid pipes in this
case, so this rewrite is unnecessary.

By the way, it's hard to imagine that this test passed once this
change was made (and, if it did pass, then that would likely indicate
that the test is somehow flawed.)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help