[pacman-dev] [PATCH] updpkgsums: omit downloader output when capturing checksums
Download the source files with an invocation of makepkg before capturing the output of makepkg -g. Without this, data printed to STDOUT by the downloader is included in the PKGBUILD. --- contrib/updpkgsums.sh.in | 3 +++ 1 file changed, 3 insertions(+) diff --git a/contrib/updpkgsums.sh.in b/contrib/updpkgsums.sh.in index ffea96b..760c414 100644 --- a/contrib/updpkgsums.sh.in +++ b/contrib/updpkgsums.sh.in @@ -72,6 +72,9 @@ if [[ ! -w . ]]; then fi { + # Download sources first to avoid downloader output in newsums array below. + makepkg --verifysource -dp "$buildfile" >/dev/null 2>&1 + # Generate the new sums and try to unlink the file before writing stdin back # into it. This final precaution shouldn't fail based on the previous checks, # but it's better to be extra careful before unlinking files. -- 1.8.4
Xyne wrote:
{ + # Download sources first to avoid downloader output in newsums array below. + makepkg --verifysource -dp "$buildfile" >/dev/null 2>&1 +
I could not find a combination of makepkg options that would omit nonsensical warnings and irrelevant output, so I have redirected everything to /dev/null. This is not ideal because the user may be left wondering what's happening as the sources are downloaded, but the output is confusing otherwise. Using "-g" leads to the least output but that will lead to redundant calculations of the checksums. The best approach would probably be to redirect downloader output to stderr in makepkg proper when invoked with -g, which would avoid this issue entirely. Thoughts?
On 18/09/13 00:02, Xyne wrote:
Download the source files with an invocation of makepkg before capturing the output of makepkg -g. Without this, data printed to STDOUT by the downloader is included in the PKGBUILD. --- contrib/updpkgsums.sh.in | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/contrib/updpkgsums.sh.in b/contrib/updpkgsums.sh.in index ffea96b..760c414 100644 --- a/contrib/updpkgsums.sh.in +++ b/contrib/updpkgsums.sh.in @@ -72,6 +72,9 @@ if [[ ! -w . ]]; then fi
{ + # Download sources first to avoid downloader output in newsums array below. + makepkg --verifysource -dp "$buildfile" >/dev/null 2>&1 +
This does a lot more checks of the PKGBUILD that "makepkg -g" does. And will fail for a VCS package when the needed VCS software is in the makedepends array and not installed on the system. (makepkg -g does not download VCS sources, makepkg --verifysource does). Is the solution just to extract the checksum output first?
# Generate the new sums and try to unlink the file before writing stdin back # into it. This final precaution shouldn't fail based on the previous checks, # but it's better to be extra careful before unlinking files.
--- scripts/makepkg.sh.in | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index d0951df..406b2c9 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -2825,7 +2825,8 @@ if (( GENINTEG )); then mkdir -p "$srcdir" chmod a-s "$srcdir" cd_safe "$srcdir" - download_sources fast + # Redirect downloader output so that scripts can capture the checksum output. + download_sources fast 1>&2 generate_checksums exit 0 # $E_OK fi -- 1.8.4
On Tue, Sep 17, 2013 at 02:02:00PM +0000, Xyne wrote:
Download the source files with an invocation of makepkg before capturing the output of makepkg -g. Without this, data printed to STDOUT by the downloader is included in the PKGBUILD. ---
This works for me... $ rm /mnt/Haven/sources/fuse-2.9.3.tar.gz $ makepkg -g 2>stderr sha1sums=('94bd1974a9f2173ac3c2cf122f9fa3c35996b88e' '3b42e37a741d4651099225987dc40e7f02a716ad') $ cat stderr ==> Retrieving sources... -> Downloading fuse-2.9.3.tar.gz... % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0 0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0 100 558k 100 558k 0 0 480k 0 0:00:01 0:00:01 --:--:-- 5130k -> Found fuse.conf ==> Generating checksums for source files... Can you explain more about how you ran into this?
contrib/updpkgsums.sh.in | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/contrib/updpkgsums.sh.in b/contrib/updpkgsums.sh.in index ffea96b..760c414 100644 --- a/contrib/updpkgsums.sh.in +++ b/contrib/updpkgsums.sh.in @@ -72,6 +72,9 @@ if [[ ! -w . ]]; then fi
{ + # Download sources first to avoid downloader output in newsums array below. + makepkg --verifysource -dp "$buildfile" >/dev/null 2>&1 + # Generate the new sums and try to unlink the file before writing stdin back # into it. This final precaution shouldn't fail based on the previous checks, # but it's better to be extra careful before unlinking files. -- 1.8.4
Dave Reisner wrote:
Can you explain more about how you ran into this?
I use aria2c as a downloader. If the sources are not already present then I end up with the full output of aria2c in the PKGBUILD above the checksums.
On 18/09/13 00:31, Xyne wrote:
Dave Reisner wrote:
Can you explain more about how you ran into this?
I use aria2c as a downloader. If the sources are not already present then I end up with the full output of aria2c in the PKGBUILD above the checksums.
I'd guess that "makepkg -g >> PKGBUILD" does bad things too if the source is not there. Try this change in download_file in makepkg: - eval "$dlcmd || ret=\$?" + eval "$dlcmd >&2 || ret=\$?" If that is good, send a patch. Allan
Allan McRae wrote:
Try this change in download_file in makepkg:
- eval "$dlcmd || ret=\$?" + eval "$dlcmd >&2 || ret=\$?"
If that is good, send a patch.
I sent another patch just before getting this. I used the same approach, but I redirected the output of the call to download_sources in the conditional GENINTEG block. Let me know if you still want me to move it to the download_file function and I'll resubmit. Incidentally, when I build makepkg locally and run "makepkg -g" it only outputs the first checksum array if multiple are present (e.g. if the PKGBUILD contains sha256sums and md5sums, in that order, it only outputs sha256sums). At first I thought it was due to something I had done, but it happens even with the current master branch. I've triple-checked this but I'm still expecting it to be my fault. I'm building with ./autogen.sh # From pacman PKGBUILD ./configure --prefix=/usr --sysconfdir=/etc \ --localstatedir=/var --enable-doc \ --with-scriptlet-shell=/usr/bin/bash \ --with-ldconfig=/usr/bin/ldconfig make make -C contrib Can someone else check this? Using the powerpill PKGBUILD [1] I only get this: ==> Retrieving sources... -> Found powerpill-2013.7.25.tar.xz -> Found powerpill-2013.7.25.tar.xz.sig ==> Generating checksums for source files... md5sums=('280b69b951a6e309a18897d7bdb1394c' 'SKIP') [1] http://xyne.archlinux.ca/projects/powerpill/pkgbuild/PKGBUILD
On 2013-09-17 14:56 +0000
Incidentally, when I build makepkg locally and run "makepkg -g" it only outputs the first checksum array if multiple are present (e.g. if the PKGBUILD contains sha256sums and md5sums, in that order, it only outputs sha256sums).
Make that "it only seems to output the first checksum array after sorting them".
--- scripts/makepkg.sh.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index d0951df..6b5a193 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1133,7 +1133,7 @@ generate_checksums() { local integlist if (( $# == 0 )); then - IFS=$'\n' read -ra integlist < <(get_integlist) + IFS=$'\n' integlist=($(get_integlist)) else integlist=("$@") fi -- 1.8.4
On 2013-09-17 15:18 +0000 Xyne wrote:
--- scripts/makepkg.sh.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index d0951df..6b5a193 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1133,7 +1133,7 @@ generate_checksums() {
local integlist if (( $# == 0 )); then - IFS=$'\n' read -ra integlist < <(get_integlist) + IFS=$'\n' integlist=($(get_integlist)) else integlist=("$@") fi -- 1.8.4
Without this only the first checksum is included in the output of makepkg -g.
On Tue, Sep 17, 2013 at 03:18:05PM +0000, Xyne wrote:
--- scripts/makepkg.sh.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index d0951df..6b5a193 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1133,7 +1133,7 @@ generate_checksums() {
local integlist if (( $# == 0 )); then - IFS=$'\n' read -ra integlist < <(get_integlist) + IFS=$'\n' integlist=($(get_integlist))
This isn't the right fix. It should be: IFS=$'\n' read -rd '' -a integlist < <(get_integlist)
else integlist=("$@") fi -- 1.8.4
Dave Reisner wrote:
This isn't the right fix. It should be:
IFS=$'\n' read -rd '' -a integlist < <(get_integlist)
else integlist=("$@") fi
Out of curiosity, what difference does it make? Is subshell invocation more expensive than file substitution?
On Tue, Sep 17, 2013 at 03:28:31PM +0000, Xyne wrote:
Dave Reisner wrote:
This isn't the right fix. It should be:
IFS=$'\n' read -rd '' -a integlist < <(get_integlist)
else integlist=("$@") fi
Out of curiosity, what difference does it make? Is subshell invocation more expensive than file substitution?
It's a reliance on word expansion (and the side effects that potentially come with it) versus just reading the data as is. Using read is about being correct, not about cost.
On 2013-09-17 11:31 -0400 Dave Reisner wrote:
On Tue, Sep 17, 2013 at 03:28:31PM +0000, Xyne wrote:
Dave Reisner wrote:
This isn't the right fix. It should be:
IFS=$'\n' read -rd '' -a integlist < <(get_integlist)
else integlist=("$@") fi
Out of curiosity, what difference does it make? Is subshell invocation more expensive than file substitution?
It's a reliance on word expansion (and the side effects that potentially come with it) versus just reading the data as is. Using read is about being correct, not about cost.
With IFS set to newline, there will be no unintentional word expansion in either case. The results are the same. I do not see how one approach is more correct than the other. Note that I have submitted a patch with your recommended fix (it's your code after all).
On Tue, Sep 17, 2013 at 03:35:57PM +0000, Xyne wrote:
On 2013-09-17 11:31 -0400 Dave Reisner wrote:
On Tue, Sep 17, 2013 at 03:28:31PM +0000, Xyne wrote:
Dave Reisner wrote:
This isn't the right fix. It should be:
IFS=$'\n' read -rd '' -a integlist < <(get_integlist)
else integlist=("$@") fi
Out of curiosity, what difference does it make? Is subshell invocation more expensive than file substitution?
It's a reliance on word expansion (and the side effects that potentially come with it) versus just reading the data as is. Using read is about being correct, not about cost.
With IFS set to newline, there will be no unintentional word expansion in either case. The results are the same. I do not see how one approach is more correct than the other.
No, it isn't the same. The cases I'm referring to probably aren't interesting for the purpose of reading checksum names, but glob expansion happens when using direct assignment, and not when using read.
Note that I have submitted a patch with your recommended fix (it's your code after all).
--- scripts/makepkg.sh.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index d0951df..0d72794 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1133,7 +1133,7 @@ generate_checksums() { local integlist if (( $# == 0 )); then - IFS=$'\n' read -ra integlist < <(get_integlist) + IFS=$'\n' read -rd '' -a integlist < <(get_integlist) else integlist=("$@") fi -- 1.8.4
On 18/09/13 00:56, Xyne wrote:
Allan McRae wrote:
Try this change in download_file in makepkg:
- eval "$dlcmd || ret=\$?" + eval "$dlcmd >&2 || ret=\$?"
If that is good, send a patch.
I sent another patch just before getting this. I used the same approach, but I redirected the output of the call to download_sources in the conditional GENINTEG block.
Let me know if you still want me to move it to the download_file function and I'll resubmit.
I'd prefer to limit the redirection as much as possible. A
This allows scripts to safely capture the output of "makepkg -g". --- scripts/makepkg.sh.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index d0951df..b0086a3 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -372,7 +372,7 @@ download_file() { fi local ret=0 - eval "$dlcmd || ret=\$?" + eval "$dlcmd >&2 || ret=\$?" if (( ret )); then [[ ! -s $dlfile ]] && rm -f -- "$dlfile" error "$(gettext "Failure while downloading %s")" "$filename" -- 1.8.4
participants (3)
-
Allan McRae
-
Dave Reisner
-
Xyne