Bug#940588: git-debpush: pipefail in get_file_from_ref
Hello, On Tue 17 Sep 2019 at 12:24PM +02, Andrej Shadura wrote: > In line 61, grep -Eq may cause a pipefail if grep exits before git > ls-tree concludes. With a debug print for $? I can see this: Here's a patch implementing the fix. Thank you for the report! -- Sean Whitton From 446b5b8c9c8179cd6f516fc5460ba70147c94e61 Mon Sep 17 00:00:00 2001 From: Sean Whitton Date: Sat, 19 Oct 2019 10:33:10 -0700 Subject: [PATCH] git-debpush: avoid a pipefail problem in get_file_from_ref `grep -q` exits as soon as it finds a matching line, potentially sending a SIGPIPE to git-ls-tree. We have pipefail turned on, so that can make the whole pipeline exit nonzero, which is wrong when grep did in fact find a match. This solution is more readable than disabling pipefail just for this line (as is done elsewhere in git-debpush). Reported-by: Andrej Shadura Signed-off-by: Sean Whitton --- git-debpush | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/git-debpush b/git-debpush index c3b067dc..2790560c 100755 --- a/git-debpush +++ b/git-debpush @@ -59,8 +59,10 @@ badusage () { get_file_from_ref () { local path=$1 +# redirect to /dev/null instead of using `grep -Eq` to avoid grep +# SIGPIPEing git-ls-tree if git ls-tree --name-only -r "$branch" \ -| grep -Eq "^$path$"; then +| grep -E "^$path$" >/dev/null; then git cat-file blob $branch:$path fi } -- 2.20.1 signature.asc Description: PGP signature
Bug#940588: git-debpush: pipefail in get_file_from_ref
Control: user d...@packages.debian.org Control: usertags -1 rsn Andrej Shadura writes ("Bug#940588: git-debpush: pipefail in get_file_from_ref"): > Package: git-debpush > Version: 9.9 > Severity: important ... > In line 61, grep -Eq may cause a pipefail if grep exits before git > ls-tree concludes. With a debug print for $? I can see this: > > ++ get_file_from_ref debian/source/format > ++ local path=debian/source/format > ++ git ls-tree --name-only -r refs/heads/debian/buster-security > ++ grep -Eq '^debian/source/format$' > ++ echo 141 > > It helped when I replaced it with a redirect: > > if git ls-tree --name-only -r "$branch" \ > | grep -E "^$path$" >/dev/null; then > git cat-file blob $branch:$path > fi > > ++ local path=debian/source/format > ++ git ls-tree --name-only -r refs/heads/debian/buster-security > ++ grep -E '^debian/source/format$' > ++ echo 0 I agree with the suggestion to redirect to >/dev/null Background from irc: 11:37 The thing with pipefail and SIGPIPE is very annoying. 11:37 IMO this is a bug in set -o pipefail 11:38 if ( git ls-tree --name-only -r "$branch" || test $? = 141 ) | ... 11:38 What even weirder this is all in an if 11:39 I’d just >/dev/null tbh 11:39 http://paste.debian.net/1101208/# ie: mariner:~> bash -xec 'set -o pipefail; if yes | head -1 ; then id; fi' + set -o pipefail + yes + head -1 y mariner:~> 11:39 that’s more readable to me at least 11:40 I guess the performance is not really a consideration since it's O(n) either way 11:40 (both because the worst case is O(n) and because the rest of it is O(n)) -- Ian JacksonThese opinions are my own. If I emailed you from an address @fyvzl.net or @evade.org.uk, that is a private address which bypasses my fierce spamfilter.
Bug#940588: git-debpush: pipefail in get_file_from_ref
Package: git-debpush Version: 9.9 Severity: important -BEGIN PGP SIGNED MESSAGE- Hash: SHA256 Dear Maintainer, In line 61, grep -Eq may cause a pipefail if grep exits before git ls-tree concludes. With a debug print for $? I can see this: ++ get_file_from_ref debian/source/format ++ local path=debian/source/format ++ git ls-tree --name-only -r refs/heads/debian/buster-security ++ grep -Eq '^debian/source/format$' ++ echo 141 It helped when I replaced it with a redirect: if git ls-tree --name-only -r "$branch" \ | grep -E "^$path$" >/dev/null; then git cat-file blob $branch:$path fi ++ local path=debian/source/format ++ git ls-tree --name-only -r refs/heads/debian/buster-security ++ grep -E '^debian/source/format$' ++ echo 0 - -- Cheers, Andrej -BEGIN PGP SIGNATURE- iQFIBAEBCAAyFiEEeuS9ZL8A0js0NGiOXkCM2RzYOdIFAl2AtGwUHGFuZHJld3No QGRlYmlhbi5vcmcACgkQXkCM2RzYOdJwAAgAlMAsuhQhpGGCWA5OXGfbhBIG3yT/ rwHjpo+qtiC2HJVOTkrS7NtP6CSDC+alxvBTfamLG2qtnbNR4ddk5z3AfHTPxoNL ZlE/3n3qqTHe4G0YGSY925aoR09okxRa/mjagIlKqDwg45xdE2W9ZdtkStipFno2 mPbvCzmzKo+XToXKpzOi4wRgbZcnFeum1U4qezHdKhSRApRmuq95nQ+uIJfbVrt4 Rn7ewarbvJFHsBXotVaX/rHWvxpzNlWBVQZYR5Bq7Y8wunEF6iY2Hxxq7MxAXkg4 DuUCtLbjpkHI1DGZKKm+6zcCKGaMAG3BGvuHqEy/IWwGbwkjDEoKtLo4kQ== =/dwb -END PGP SIGNATURE-