Discussion:
[arch-projects] [dbscripts] [PATCH 2/2] fixup: fix potential bsdtar stream close error by grep
Levente Polyak via arch-projects
2018-09-03 11:50:16 UTC
Permalink
From: anthraxx <***@archlinux.org>

bsdtar doesn't like it when the stream gets closed before it finishes
which may be the case when grep found its match on potentially huge
archives. Instead of suppressing the whole strerr , we just pipe
the output through cat which ensures the stream remains open for bsdtar
but we may still catch and see useful messages on stderr.
---
db-functions | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/db-functions b/db-functions
index 0491c22..f0a6453 100644
--- a/db-functions
+++ b/db-functions
@@ -174,7 +174,7 @@ repo_unlock () { #repo_unlock <repo-name> <arch>
_grep_pkginfo() {
local _ret

- _ret="$(/usr/bin/bsdtar -xOqf "$1" .PKGINFO | grep -m 1 "^${2} = ")"
+ _ret="$(/usr/bin/bsdtar -xOqf "$1" .PKGINFO | cat | grep -m 1 "^${2} = ")"
echo "${_ret#${2} = }"
}

@@ -182,7 +182,7 @@ _grep_pkginfo() {
_grep_buildinfo() {
local _ret

- _ret="$(/usr/bin/bsdtar -xOqf "$1" .BUILDINFO | grep -m 1 "^${2} = ")"
+ _ret="$(/usr/bin/bsdtar -xOqf "$1" .BUILDINFO | cat | grep -m 1 "^${2} = ")"
echo "${_ret#${2} = }"
}
--
2.18.0
Levente Polyak via arch-projects
2018-09-03 20:01:16 UTC
Permalink
From: anthraxx <***@archlinux.org>

The old travis-ci.org is deprecated and this project was migrated.
---
README.md | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/README.md b/README.md
index c672998..bdd958b 100644
--- a/README.md
+++ b/README.md
@@ -1,4 +1,4 @@
-# Arch Linux repository management scripts [![Build Status](https://travis-ci.org/archlinux/dbscripts.svg?branch=master)](https://travis-ci.org/archlinux/dbscripts)
+# Arch Linux repository management scripts [![Build Status](https://travis-ci.com/archlinux/dbscripts.svg?branch=master)](https://travis-ci.com/archlinux/dbscripts)
## Configuration
* The default configuration can be found in `config`.
* An optional `config.local` may override the default configuration.
--
2.18.0
Eli Schwartz via arch-projects
2018-09-09 15:20:51 UTC
Permalink
Post by Levente Polyak via arch-projects
bsdtar doesn't like it when the stream gets closed before it finishes
which may be the case when grep found its match on potentially huge
archives. Instead of suppressing the whole strerr , we just pipe
the output through cat which ensures the stream remains open for bsdtar
but we may still catch and see useful messages on stderr.
This is functionally 23c2b82c336bf19b7a29a90d19bca4423d8b8839 again, but
for more locations. I'm never going to understand why some people get
this SIGPIPE but I don't, but I guess it makes sense to do this change
-- especially as we do the same elsewhere.

(We need to buffer it somehow with some extra command, grep doesn't have
a way to only output the first result but still avoid propagating
SIGPIPE. Why does bsdtar care about this anyway...)

Accepted.
--
Eli Schwartz
Bug Wrangler and Trusted User
Eli Schwartz via arch-projects
2018-09-09 16:20:41 UTC
Permalink
Post by Eli Schwartz via arch-projects
Post by Levente Polyak via arch-projects
bsdtar doesn't like it when the stream gets closed before it finishes
which may be the case when grep found its match on potentially huge
archives. Instead of suppressing the whole strerr , we just pipe
the output through cat which ensures the stream remains open for bsdtar
but we may still catch and see useful messages on stderr.
This is functionally 23c2b82c336bf19b7a29a90d19bca4423d8b8839 again, but
for more locations. I'm never going to understand why some people get
this SIGPIPE but I don't, but I guess it makes sense to do this change
-- especially as we do the same elsewhere.
(We need to buffer it somehow with some extra command, grep doesn't have
a way to only output the first result but still avoid propagating
SIGPIPE. Why does bsdtar care about this anyway...)
As discussed on IRC, I can finally reproduce this, e.g.
bsdtar xOf /path/to/file | grep --binary-files=text a
(many matches for the string "a")

With grep -q or grep -m1, it errors

with cat | grep -q/-m1 it still errors due to the buffer going *into*
grep, having insufficient room :(

with grep | head -1 it only errors, if the matching buffer going *out*
of grep is too large. For our uses it should only ever match exactly
once. So this is what we should do.

(Or complain to libarchive. :p)
--
Eli Schwartz
Bug Wrangler and Trusted User
Eli Schwartz via arch-projects
2018-09-09 16:31:46 UTC
Permalink
Post by Eli Schwartz via arch-projects
Post by Eli Schwartz via arch-projects
Post by Levente Polyak via arch-projects
bsdtar doesn't like it when the stream gets closed before it finishes
which may be the case when grep found its match on potentially huge
archives. Instead of suppressing the whole strerr , we just pipe
the output through cat which ensures the stream remains open for bsdtar
but we may still catch and see useful messages on stderr.
This is functionally 23c2b82c336bf19b7a29a90d19bca4423d8b8839 again, but
for more locations. I'm never going to understand why some people get
this SIGPIPE but I don't, but I guess it makes sense to do this change
-- especially as we do the same elsewhere.
(We need to buffer it somehow with some extra command, grep doesn't have
a way to only output the first result but still avoid propagating
SIGPIPE. Why does bsdtar care about this anyway...)
As discussed on IRC, I can finally reproduce this, e.g.
bsdtar xOf /path/to/file | grep --binary-files=text a
(many matches for the string "a")
With grep -q or grep -m1, it errors
with cat | grep -q/-m1 it still errors due to the buffer going *into*
grep, having insufficient room :(
with grep | head -1 it only errors, if the matching buffer going *out*
of grep is too large. For our uses it should only ever match exactly
once. So this is what we should do.
(Or complain to libarchive. :p)
As discussed on IRC, applying modified version of the patch (with
amended commit message) which uses tail (since that should never close
early at all). Thanks.
--
Eli Schwartz
Bug Wrangler and Trusted User
Eli Schwartz via arch-projects
2018-09-09 16:33:55 UTC
Permalink
From: anthraxx <***@archlinux.org>

This silences a useless error message that confuses the user.

bsdtar doesn't like it when the stream gets closed before it finishes
which may be the case when grep found its match on potentially huge
archives. Instead of suppressing the whole stderr , we find all matches
with grep, then use a second pass with `tail` to find only the last
match, which ensures the stream remains open for bsdtar but we may still
catch and see useful messages on stderr.

This works because tail has the useful property of not closing early.

Signed-off-by: Eli Schwartz <***@archlinux.org>
---
db-functions | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/db-functions b/db-functions
index 0491c22d..6d6084a9 100644
--- a/db-functions
+++ b/db-functions
@@ -174,7 +174,7 @@ repo_unlock () { #repo_unlock <repo-name> <arch>
_grep_pkginfo() {
local _ret

- _ret="$(/usr/bin/bsdtar -xOqf "$1" .PKGINFO | grep -m 1 "^${2} = ")"
+ _ret="$(/usr/bin/bsdtar -xOqf "$1" .PKGINFO | grep "^${2} = " | tail -1)"
echo "${_ret#${2} = }"
}

@@ -182,7 +182,7 @@ _grep_pkginfo() {
_grep_buildinfo() {
local _ret

- _ret="$(/usr/bin/bsdtar -xOqf "$1" .BUILDINFO | grep -m 1 "^${2} = ")"
+ _ret="$(/usr/bin/bsdtar -xOqf "$1" .BUILDINFO | grep "^${2} = " | tail -1)"
echo "${_ret#${2} = }"
}
--
2.18.0
Loading...