[arch-projects] [devtools] [PATCH 0/9] Changes I've been maintaining in Parabola's fork
Hi, I maintain Parabola's "fork" of devtools. I've always had a set of "generic" changes to contribute back upstream, but until now never got around to pulling them into nice changesets. I also had one to allow makechrootpkg to handle arguments with spaces, but I just sent that as a reply in another thread since Dave Reisner seems to have just written the same thing! These are arranged roughly by how "controversial" I expect them to be; I expect that no one will object to the first couple commits, while someone might question the utility of the changes to lock/slock. Happy hacking, ~ Luke Shumaker Luke Shumaker (9): arch-nspawn: Fix a grammar mistake (a/an) in an error message. archbuild: Fix a variable name mistake. ($copydir -> $copy) mkarchroot: Correct "Usage:" text. makechrootpkg: Use the btrfs mountpoint/subvolume check consistently. arch-nspawn: Avoid errors where $working_dir is longer than HOST_NAME_MAX. lib/common.sh: Add a lock_close function. lib/common.sh:stat_busy(): Accept printf-style arguments. lib/common.sh: Improve lock() and slock(). Replace multiple uses of string interpolation with printf-type arguments. arch-nspawn.in | 10 ++++++++-- archbuild.in | 10 +++++----- archrelease.in | 2 +- checkpkg.in | 4 ++-- crossrepomove.in | 8 ++++---- lddd.in | 4 ++-- lib/common.sh | 48 +++++++++++++++++++++++++++++++++++++----------- makechrootpkg.in | 10 +++++----- mkarchroot.in | 4 ++-- rebuildpkgs.in | 14 +++++++------- 10 files changed, 73 insertions(+), 41 deletions(-) -- 1.9.2
--- arch-nspawn.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch-nspawn.in b/arch-nspawn.in index 4436a0e..15db2e9 100644 --- a/arch-nspawn.in +++ b/arch-nspawn.in @@ -84,7 +84,7 @@ umask 0022 # Sanity check if [[ ! -f "$working_dir/.arch-chroot" ]]; then - die "'%s' does not appear to be a Arch chroot." "$working_dir" + die "'%s' does not appear to be an Arch chroot." "$working_dir" elif [[ $(cat "$working_dir/.arch-chroot") != $CHROOT_VERSION ]]; then die "chroot '%s' is not at version %s. Please rebuild." "$working_dir" "$CHROOT_VERSION" fi -- 1.9.2
It tried to lock `$copydir.lock`, which was the ONLY mention of $copydir in the entire file. Surely it meant `$copy.lock`; the line was probably originally copy/pasted from makechrootpkg or similar, where $copydir is used. --- archbuild.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/archbuild.in b/archbuild.in index ae2f511..dc45c7f 100644 --- a/archbuild.in +++ b/archbuild.in @@ -49,7 +49,7 @@ if ${clean_first} || [[ ! -d "${chroots}/${repo}-${arch}" ]]; then [[ -d $copy ]] || continue msg2 "Deleting chroot copy '$(basename "${copy}")'..." - lock 9 "$copydir.lock" "Locking chroot copy '$copy'" + lock 9 "$copy.lock" "Locking chroot copy '$copy'" if [[ "$(stat -f -c %T "${copy}")" == btrfs ]]; then { type -P btrfs && btrfs subvolume delete "${copy}"; } &>/dev/null -- 1.9.2
The "app" hasn't been an option since arch-nspawn was created. --- mkarchroot.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mkarchroot.in b/mkarchroot.in index fb472bc..5f3d6fe 100644 --- a/mkarchroot.in +++ b/mkarchroot.in @@ -15,7 +15,7 @@ CHROOT_VERSION='v3' working_dir='' usage() { - echo "Usage: ${0##*/} [options] working-dir [package-list | app]" + echo "Usage: ${0##*/} [options] working-dir package-list..." echo ' options:' echo ' -C <file> Location of a pacman config file' echo ' -M <file> Location of a makepkg config file' -- 1.9.2
Commit 59e348fc3c5dd086331d884a6dd76fb43a92b7eb added a btrfs subvolume check, but only used it in create_chroot(); it missed clean_temporary(). --- makechrootpkg.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/makechrootpkg.in b/makechrootpkg.in index 97c7780..c3edc72 100644 --- a/makechrootpkg.in +++ b/makechrootpkg.in @@ -164,7 +164,7 @@ create_chroot() { clean_temporary() { stat_busy "Removing temporary copy [$copy]" - if [[ "$chroottype" == btrfs ]]; then + if [[ "$chroottype" == btrfs ]] && ! mountpoint -q "$copydir"; then btrfs subvolume delete "$copydir" >/dev/null || die "Unable to delete subvolume %s" "$copydir" else -- 1.9.2
Currently, if absolute path of the chroot directory is more than 65 bytes (HOST_NAME_MAX+1) long, arch-nspawn will fail (mysteriously too, since stderr is directed to /dev/null). This patch makes it so that if $machine_name is more than 64 (HOST_NAME_MAX) bytes long, it is stripped to use just the last 64 bytes. I left in a comment that would have it find HOST_NAME_MAX dynamically, but have it just hard-coded as '64'; the script already isn't portable away from systemd/Linux, and it likely won't change on Linux anytime soon. --- arch-nspawn.in | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/arch-nspawn.in b/arch-nspawn.in index 15db2e9..df3a08f 100644 --- a/arch-nspawn.in +++ b/arch-nspawn.in @@ -96,6 +96,12 @@ eval $(grep '^CARCH=' "$working_dir/etc/makepkg.conf") machine_name="${working_dir//[![:alnum:]_-]/-}" machine_name="${machine_name#-}" +#HOST_NAME_MAX="$(printf '%s\n' '#include <limits.h>' 'HOST_NAME_MAX'|cpp -|sed -n '$p')" +HOST_NAME_MAX=64 +if [[ ${#machine_name} -gt "$HOST_NAME_MAX" ]]; then + machine_name="${machine_name:(-${HOST_NAME_MAX})}" + machine_name="${machine_name#-}" +fi exec ${CARCH:+setarch "$CARCH"} systemd-nspawn 2>/dev/null \ -D "$working_dir" \ -- 1.9.2
On Sat, May 10, 2014 at 10:22:07PM -0400, Luke Shumaker wrote:
Currently, if absolute path of the chroot directory is more than 65 bytes (HOST_NAME_MAX+1) long, arch-nspawn will fail (mysteriously too, since stderr is directed to /dev/null).
This patch makes it so that if $machine_name is more than 64 (HOST_NAME_MAX) bytes long, it is stripped to use just the last 64 bytes.
I left in a comment that would have it find HOST_NAME_MAX dynamically, but have it just hard-coded as '64'; the script already isn't portable away from systemd/Linux, and it likely won't change on Linux anytime soon. --- arch-nspawn.in | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/arch-nspawn.in b/arch-nspawn.in index 15db2e9..df3a08f 100644 --- a/arch-nspawn.in +++ b/arch-nspawn.in @@ -96,6 +96,12 @@ eval $(grep '^CARCH=' "$working_dir/etc/makepkg.conf")
machine_name="${working_dir//[![:alnum:]_-]/-}" machine_name="${machine_name#-}" +#HOST_NAME_MAX="$(printf '%s\n' '#include <limits.h>' 'HOST_NAME_MAX'|cpp -|sed -n '$p')"
No need to write the include as if this were a source file, just tell cpp about it directly: cpp -I limits.h <<<HOST_NAME_MAX | ...
+HOST_NAME_MAX=64 +if [[ ${#machine_name} -gt "$HOST_NAME_MAX" ]]; then + machine_name="${machine_name:(-${HOST_NAME_MAX})}" + machine_name="${machine_name#-}"
I think this is worth warning about, since it might result in name clashes.
+fi
exec ${CARCH:+setarch "$CARCH"} systemd-nspawn 2>/dev/null \ -D "$working_dir" \ -- 1.9.2
At Sat, 10 May 2014 22:36:44 -0400, Dave Reisner wrote:
On Sat, May 10, 2014 at 10:22:07PM -0400, Luke Shumaker wrote:
+#HOST_NAME_MAX="$(printf '%s\n' '#include <limits.h>' 'HOST_NAME_MAX'|cpp -|sed -n '$p')"
No need to write the include as if this were a source file, just tell cpp about it directly:
cpp -I limits.h <<<HOST_NAME_MAX | ...
That doesn't work, but this does: cpp -include limits.h <<<HOST_NAME_MAX | ... I'd never had occasion to use that flag before, so thank you!
+HOST_NAME_MAX=64 +if [[ ${#machine_name} -gt "$HOST_NAME_MAX" ]]; then + machine_name="${machine_name:(-${HOST_NAME_MAX})}" + machine_name="${machine_name#-}"
I think this is worth warning about, since it might result in name clashes.
Is it any more likely to cause clashes than the existing machine name munging? 64 characters is a lot, and I figure that the end is the part that is most likely to change. What about passing '-q' to systemd-nspawn and not directing its stderr to /dev/null? That way the user gets a reasonably friendly "Failed to register machine: Machine '$machine_name' already exits", as well as a message for any other errors that may occur. IIRC, the reason stderr was directed to /dev/null was that at the time it was written, systemd-nspawn didn't have a '-q' flag. Happy hacking, ~ Luke Shumaker
On Sun, May 11, 2014 at 12:04:31AM -0400, Luke Shumaker wrote:
At Sat, 10 May 2014 22:36:44 -0400, Dave Reisner wrote:
On Sat, May 10, 2014 at 10:22:07PM -0400, Luke Shumaker wrote:
+#HOST_NAME_MAX="$(printf '%s\n' '#include <limits.h>' 'HOST_NAME_MAX'|cpp -|sed -n '$p')"
No need to write the include as if this were a source file, just tell cpp about it directly:
cpp -I limits.h <<<HOST_NAME_MAX | ...
That doesn't work, but this does:
cpp -include limits.h <<<HOST_NAME_MAX | ...
I'd never had occasion to use that flag before, so thank you!
Ah, right. -I is for directories.
+HOST_NAME_MAX=64 +if [[ ${#machine_name} -gt "$HOST_NAME_MAX" ]]; then + machine_name="${machine_name:(-${HOST_NAME_MAX})}" + machine_name="${machine_name#-}"
I think this is worth warning about, since it might result in name clashes.
Is it any more likely to cause clashes than the existing machine name munging? 64 characters is a lot, and I figure that the end is the part that is most likely to change.
Oh, I see. You're trimming leading characters, not truncating. Alternatively, just hash the full path? Is the machine name really useful in any way other than to define a unique name for the build container?
What about passing '-q' to systemd-nspawn and not directing its stderr to /dev/null? That way the user gets a reasonably friendly "Failed to register machine: Machine '$machine_name' already exits", as well as a message for any other errors that may occur. IIRC, the reason stderr was directed to /dev/null was that at the time it was written, systemd-nspawn didn't have a '-q' flag.
Sounds good to me. The --quiet flag was added to systemd in v209. d
--- archbuild.in | 2 +- lib/common.sh | 8 ++++++++ makechrootpkg.in | 2 +- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/archbuild.in b/archbuild.in index dc45c7f..618d0f0 100644 --- a/archbuild.in +++ b/archbuild.in @@ -56,7 +56,7 @@ if ${clean_first} || [[ ! -d "${chroots}/${repo}-${arch}" ]]; then fi rm -rf --one-file-system "${copy}" done - exec 9>&- + lock_close 9 rm -rf --one-file-system "${chroots}/${repo}-${arch}" mkdir -p "${chroots}/${repo}-${arch}" diff --git a/lib/common.sh b/lib/common.sh index b885080..38e31a2 100644 --- a/lib/common.sh +++ b/lib/common.sh @@ -158,6 +158,14 @@ slock() { } ## +# usage : lock_close( $fd ) +## +lock_close() { + local fd=$1 + eval "exec $fd>&-" +} + +## # usage: pkgver_equal( $pkgver1, $pkgver2 ) ## pkgver_equal() { diff --git a/makechrootpkg.in b/makechrootpkg.in index c3edc72..731b560 100644 --- a/makechrootpkg.in +++ b/makechrootpkg.in @@ -155,7 +155,7 @@ create_chroot() { stat_done # Drop the read lock again - exec 8>&- + lock_close 8 fi # Update mtime -- 1.9.2
On Sat, May 10, 2014 at 10:22:08PM -0400, Luke Shumaker wrote:
--- archbuild.in | 2 +- lib/common.sh | 8 ++++++++ makechrootpkg.in | 2 +- 3 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/archbuild.in b/archbuild.in index dc45c7f..618d0f0 100644 --- a/archbuild.in +++ b/archbuild.in @@ -56,7 +56,7 @@ if ${clean_first} || [[ ! -d "${chroots}/${repo}-${arch}" ]]; then fi rm -rf --one-file-system "${copy}" done - exec 9>&- + lock_close 9
rm -rf --one-file-system "${chroots}/${repo}-${arch}" mkdir -p "${chroots}/${repo}-${arch}" diff --git a/lib/common.sh b/lib/common.sh index b885080..38e31a2 100644 --- a/lib/common.sh +++ b/lib/common.sh @@ -158,6 +158,14 @@ slock() { }
## +# usage : lock_close( $fd ) +## +lock_close() { + local fd=$1 + eval "exec $fd>&-"
eval isn't needed here, you can use: exec {fd}>&-
+} + +## # usage: pkgver_equal( $pkgver1, $pkgver2 ) ## pkgver_equal() { diff --git a/makechrootpkg.in b/makechrootpkg.in index c3edc72..731b560 100644 --- a/makechrootpkg.in +++ b/makechrootpkg.in @@ -155,7 +155,7 @@ create_chroot() { stat_done
# Drop the read lock again - exec 8>&- + lock_close 8 fi
# Update mtime -- 1.9.2
At Sat, 10 May 2014 22:40:02 -0400, Dave Reisner wrote:
+ eval "exec $fd>&-"
eval isn't needed here, you can use:
exec {fd}>&-
Whoa! I should have taken a closer look at the Bash 4.1 CHANGES file. That's cool new bit of syntax. I suppose neither of us is concerned about supporting Bash 4.0 or earlier? Happy hacking, ~ Luke Shumaker
On Sun, May 11, 2014 at 12:16:33AM -0400, Luke Shumaker wrote:
At Sat, 10 May 2014 22:40:02 -0400, Dave Reisner wrote:
+ eval "exec $fd>&-"
eval isn't needed here, you can use:
exec {fd}>&-
Whoa! I should have taken a closer look at the Bash 4.1 CHANGES file. That's cool new bit of syntax. I suppose neither of us is concerned about supporting Bash 4.0 or earlier?
pacman adopted bash4 quite some time ago, and tends to be a little on the conservative side. I don't see any problem with devtools adopting bash4 features, since they're expected to be run on an up to date Arch install.
At Sun, 11 May 2014 11:49:57 -0400, Dave Reisner wrote:
On Sun, May 11, 2014 at 12:16:33AM -0400, Luke Shumaker wrote:
At Sat, 10 May 2014 22:40:02 -0400, Dave Reisner wrote:
+ eval "exec $fd>&-"
eval isn't needed here, you can use:
exec {fd}>&-
Whoa! I should have taken a closer look at the Bash 4.1 CHANGES file. That's cool new bit of syntax. I suppose neither of us is concerned about supporting Bash 4.0 or earlier?
pacman adopted bash4 quite some time ago, and tends to be a little on the conservative side. I don't see any problem with devtools adopting bash4 features, since they're expected to be run on an up to date Arch install.
That syntax wasn't in 4.0, it's new to 4.1. But yeah, I agree.
This seems to be logical to fit in with the other message handling functions, and it only takes a couple more characters. --- lib/common.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/common.sh b/lib/common.sh index 38e31a2..d6da1ab 100644 --- a/lib/common.sh +++ b/lib/common.sh @@ -52,7 +52,7 @@ error() { stat_busy() { local mesg=$1; shift - printf "${GREEN}==>${ALL_OFF}${BOLD} ${mesg}...${ALL_OFF}" >&2 + printf "${GREEN}==>${ALL_OFF}${BOLD} ${mesg}...${ALL_OFF}" "$@" >&2 } stat_done() { -- 1.9.2
There are 3 things of note here: - Pass extra arguments to stat_busy, to allow it to take printf-type messages. - Check to make sure that process doesn't already own the lock. The code for this is mostly taken from mkarchroot; the logic had been dropped when the functions were pulled into common.sh - The old mkarchroot code used `readlink -f` on the file in /dev/fd to resolve symlinks. This didn't work correctly if there is a symlink in the path of the lock file. One solution would have been to use `readlink -f` on both files, but it is simpler to use `[[ a -ef b ]]`. --- lib/common.sh | 38 ++++++++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/lib/common.sh b/lib/common.sh index d6da1ab..9cc4e1d 100644 --- a/lib/common.sh +++ b/lib/common.sh @@ -134,25 +134,43 @@ get_full_version() { } ## -# usage : lock( $fd, $file, $message ) +# usage : lock( $fd, $file, $message, [ $message_arguments... ] ) ## lock() { - eval "exec $1>"'"$2"' - if ! flock -n $1; then - stat_busy "$3" - flock $1 + local fd=$1 + local file=$2 + local mesg=("${@:3}") + + # Only reopen the FD if it wasn't handed to us + if ! [[ "/dev/fd/$fd" -ef "$file" ]]; then + mkdir -p "${file%/*}" + eval "exec $fd>"'"$file"' + fi + + if ! flock -n $fd; then + stat_busy "${mesg[@]}" + flock $fd stat_done fi } ## -# usage : slock( $fd, $file, $message ) +# usage : slock( $fd, $file, $message, [ $message_arguments... ] ) ## slock() { - eval "exec $1>"'"$2"' - if ! flock -sn $1; then - stat_busy "$3" - flock -s $1 + local fd=$1 + local file=$2 + local mesg=("${@:3}") + + # Only reopen the FD if it wasn't handed to us + if ! [[ "/dev/fd/$fd" -ef "$file" ]]; then + mkdir -p "${file%/*}" + eval "exec $fd>"'"$file"' + fi + + if ! flock -sn $fd; then + stat_busy "${mesg[@]}" + flock -s $fd stat_done fi } -- 1.9.2
In some places this wasn't possible until now, brecause `stat_busy`, `lock`, and `slock` didn't accept printf-style arguments. --- arch-nspawn.in | 2 +- archbuild.in | 8 ++++---- archrelease.in | 2 +- checkpkg.in | 4 ++-- crossrepomove.in | 8 ++++---- lddd.in | 4 ++-- makechrootpkg.in | 6 +++--- mkarchroot.in | 2 +- rebuildpkgs.in | 14 +++++++------- 9 files changed, 25 insertions(+), 25 deletions(-) diff --git a/arch-nspawn.in b/arch-nspawn.in index df3a08f..ca7edbe 100644 --- a/arch-nspawn.in +++ b/arch-nspawn.in @@ -32,7 +32,7 @@ while getopts 'hC:M:c:' arg; do M) makepkg_conf="$OPTARG" ;; c) cache_dir="$OPTARG" ;; h|?) usage ;; - *) error "invalid argument '$arg'"; usage ;; + *) error "invalid argument '%s'" "$arg"; usage ;; esac done shift $(($OPTIND - 1)) diff --git a/archbuild.in b/archbuild.in index 618d0f0..64e0904 100644 --- a/archbuild.in +++ b/archbuild.in @@ -43,13 +43,13 @@ makechrootpkg_args+=("${@:$OPTIND}") check_root "$0" "$@" if ${clean_first} || [[ ! -d "${chroots}/${repo}-${arch}" ]]; then - msg "Creating chroot for [${repo}] (${arch})..." + msg "Creating chroot for [%s] (%s)..." "${repo}" "${arch}" for copy in "${chroots}/${repo}-${arch}"/*; do [[ -d $copy ]] || continue - msg2 "Deleting chroot copy '$(basename "${copy}")'..." + msg2 "Deleting chroot copy '%s'..." "$(basename "${copy}")" - lock 9 "$copy.lock" "Locking chroot copy '$copy'" + lock 9 "$copy.lock" "Locking chroot copy '%s'" "$copy" if [[ "$(stat -f -c %T "${copy}")" == btrfs ]]; then { type -P btrfs && btrfs subvolume delete "${copy}"; } &>/dev/null @@ -74,5 +74,5 @@ else pacman -Syu --noconfirm || abort fi -msg "Building in chroot for [${repo}] (${arch})..." +msg "Building in chroot for [%s] (%s)..." "${repo}" "${arch}" exec makechrootpkg -r "${chroots}/${repo}-${arch}" "${makechrootpkg_args[@]}" diff --git a/archrelease.in b/archrelease.in index 6f52dbc..4ac55db 100644 --- a/archrelease.in +++ b/archrelease.in @@ -58,7 +58,7 @@ done known_files=("${known_files[@]/%/@}") for tag in "$@"; do - stat_busy "Copying ${trunk} to ${tag}" + stat_busy "Copying %s to %s" "${trunk}" "${tag}" if [[ -d repos/$tag ]]; then declare -a trash diff --git a/checkpkg.in b/checkpkg.in index ccbbecd..81e7184 100644 --- a/checkpkg.in +++ b/checkpkg.in @@ -70,8 +70,8 @@ for _pkgname in "${pkgname[@]}"; do echo "${i}: " "$(objdump -p "$TEMPDIR/$i" | grep SONAME)" done else - msg "No soname differences for $_pkgname." + msg "No soname differences for %s." "$_pkgname" fi done -msg "Files saved to $TEMPDIR" +msg "Files saved to %s" "$TEMPDIR" diff --git a/crossrepomove.in b/crossrepomove.in index 912504f..ac08c67 100644 --- a/crossrepomove.in +++ b/crossrepomove.in @@ -39,13 +39,13 @@ setup_workdir pushd $WORKDIR >/dev/null -msg "Downloading sources for ${pkgbase}" +msg "Downloading sources for %s" "${pkgbase}" svn -q checkout -N "${target_svn}" target_checkout mkdir -p "target_checkout/${pkgbase}/repos" svn -q export "${source_svn}/${pkgbase}/trunk" "target_checkout/${pkgbase}/trunk" || die . "target_checkout/${pkgbase}/trunk/PKGBUILD" -msg "Downloading packages for ${pkgbase}" +msg "Downloading packages for %s" "${pkgbase}" for _arch in ${arch[@]}; do if [[ "${_arch[*]}" == 'any' ]]; then repo_arch='x86_64' @@ -59,7 +59,7 @@ for _arch in ${arch[@]}; do done done -msg "Adding ${pkgbase} to ${target_repo}" +msg "Adding %s to %s" "${pkgbase}" "${target_repo}" svn -q add "target_checkout/${pkgbase}" svn -q propset svn:keywords 'Id' "target_checkout/${pkgbase}/trunk/PKGBUILD" svn -q commit -m"${scriptname}: Moving ${pkgbase} from ${source_repo} to ${target_repo}" target_checkout @@ -69,7 +69,7 @@ popd >/dev/null ssh "${server}" "${target_dbscripts}/db-update" || die -msg "Removing ${pkgbase} from ${source_repo}" +msg "Removing %s from %s" "${pkgbase}" "${source_repo}" for _arch in ${arch[@]}; do ssh "${server}" "${source_dbscripts}/db-remove ${source_repo} ${_arch} ${pkgbase}" done diff --git a/lddd.in b/lddd.in index 43aa8c1..f111d67 100644 --- a/lddd.in +++ b/lddd.in @@ -16,7 +16,7 @@ TEMPDIR=$(mktemp -d --tmpdir lddd-script.XXXX) msg 'Go out and drink some tea, this will take a while :) ...' # Check ELF binaries in the PATH and specified dir trees. for tree in $PATH $libdirs $extras; do - msg2 "DIR $tree" + msg2 "DIR %s" "$tree" # Get list of files in tree. files=$(find $tree -type f ! -name '*.a' ! -name '*.la' ! -name '*.py*' ! -name '*.txt' ! -name '*.h' ! -name '*.ttf' ! \ @@ -45,4 +45,4 @@ done # clean list sort -u $TEMPDIR/pacman.txt >> $TEMPDIR/possible-rebuilds.txt -msg "Files saved to $TEMPDIR" +msg "Files saved to %s" "$TEMPDIR" diff --git a/makechrootpkg.in b/makechrootpkg.in index 731b560..6a99408 100644 --- a/makechrootpkg.in +++ b/makechrootpkg.in @@ -133,14 +133,14 @@ load_vars() { create_chroot() { # Lock the chroot we want to use. We'll keep this lock until we exit. - lock 9 "$copydir.lock" "Locking chroot copy [$copy]" + lock 9 "$copydir.lock" "Locking chroot copy [%s]" "$copy" if [[ ! -d $copydir ]] || $clean_first; then # Get a read lock on the root chroot to make # sure we don't clone a half-updated chroot slock 8 "$chrootdir/root.lock" "Locking clean chroot" - stat_busy "Creating clean working copy [$copy]" + stat_busy "Creating clean working copy [%s]" "$copy" if [[ "$chroottype" == btrfs ]] && ! mountpoint -q "$copydir"; then if [[ -d $copydir ]]; then btrfs subvolume delete "$copydir" >/dev/null || @@ -163,7 +163,7 @@ create_chroot() { } clean_temporary() { - stat_busy "Removing temporary copy [$copy]" + stat_busy "Removing temporary copy [%s]" "$copy" if [[ "$chroottype" == btrfs ]] && ! mountpoint -q "$copydir"; then btrfs subvolume delete "$copydir" >/dev/null || die "Unable to delete subvolume %s" "$copydir" diff --git a/mkarchroot.in b/mkarchroot.in index 5f3d6fe..09ba64f 100644 --- a/mkarchroot.in +++ b/mkarchroot.in @@ -30,7 +30,7 @@ while getopts 'hC:M:c:' arg; do M) makepkg_conf="$OPTARG" ;; c) cache_dir="$OPTARG" ;; h|?) usage ;; - *) error "invalid argument '$arg'"; usage ;; + *) error "invalid argument '%s'" "$arg"; usage ;; esac done shift $(($OPTIND - 1)) diff --git a/rebuildpkgs.in b/rebuildpkgs.in index 2f71c40..1a02954 100644 --- a/rebuildpkgs.in +++ b/rebuildpkgs.in @@ -49,7 +49,7 @@ pkgs="$@" SVNPATH='svn+ssh://nymeria.archlinux.org/srv/repos/svn-packages/svn' -msg "Work will be done in $(pwd)/rebuilds" +msg "Work will be done in %s" "$(pwd)/rebuilds" REBUILD_ROOT="$(pwd)/rebuilds" mkdir -p "$REBUILD_ROOT" @@ -61,11 +61,11 @@ FAILED="" for pkg in $pkgs; do cd "$REBUILD_ROOT/svn-packages" - msg2 "Building '$pkg'" + msg2 "Building '%s'" "$pkg" /usr/bin/svn update "$pkg" if [[ ! -d "$pkg/trunk" ]]; then FAILED="$FAILED $pkg" - warning "$pkg does not exist in SVN" + warning "%s does not exist in SVN" "$pkg" continue fi cd "$pkg/trunk/" @@ -74,14 +74,14 @@ for pkg in $pkgs; do if ! sudo makechrootpkg -u -d -r "$chrootdir" -- --noconfirm; then FAILED="$FAILED $pkg" - error "$pkg Failed!" + error "%s Failed!" "$pkg" else pkgfile=$(pkg_from_pkgbuild) if [[ -e $pkgfile ]]; then - msg2 "$pkg Complete" + msg2 "%s Complete" "$pkg" else FAILED="$FAILED $pkg" - error "$pkg Failed, no package built!" + error "%s Failed, no package built!" "$pkg" fi fi done @@ -90,7 +90,7 @@ cd "$REBUILD_ROOT" if [[ -n $FAILED ]]; then msg 'Packages failed:' for pkg in $FAILED; do - msg2 "$pkg" + msg2 "%s" "$pkg" done fi -- 1.9.2
On Sat, May 10, 2014 at 10:22:02PM -0400, Luke Shumaker wrote:
Hi, I maintain Parabola's "fork" of devtools. I've always had a set of "generic" changes to contribute back upstream, but until now never got around to pulling them into nice changesets.
I also had one to allow makechrootpkg to handle arguments with spaces, but I just sent that as a reply in another thread since Dave Reisner seems to have just written the same thing!
These are arranged roughly by how "controversial" I expect them to be; I expect that no one will object to the first couple commits, while someone might question the utility of the changes to lock/slock.
Happy hacking, ~ Luke Shumaker
Modulo the comments I've made in 2 of the patches, this whole set gets an ACK from me. Thanks for contributing back upstream!
Luke Shumaker (9): arch-nspawn: Fix a grammar mistake (a/an) in an error message. archbuild: Fix a variable name mistake. ($copydir -> $copy) mkarchroot: Correct "Usage:" text. makechrootpkg: Use the btrfs mountpoint/subvolume check consistently. arch-nspawn: Avoid errors where $working_dir is longer than HOST_NAME_MAX. lib/common.sh: Add a lock_close function. lib/common.sh:stat_busy(): Accept printf-style arguments. lib/common.sh: Improve lock() and slock(). Replace multiple uses of string interpolation with printf-type arguments.
arch-nspawn.in | 10 ++++++++-- archbuild.in | 10 +++++----- archrelease.in | 2 +- checkpkg.in | 4 ++-- crossrepomove.in | 8 ++++---- lddd.in | 4 ++-- lib/common.sh | 48 +++++++++++++++++++++++++++++++++++++----------- makechrootpkg.in | 10 +++++----- mkarchroot.in | 4 ++-- rebuildpkgs.in | 14 +++++++------- 10 files changed, 73 insertions(+), 41 deletions(-)
-- 1.9.2
At Sun, 11 May 2014 11:54:55 -0400, Dave Reisner wrote:
Modulo the comments I've made in 2 of the patches, this whole set gets an ACK from me. Thanks for contributing back upstream!
What's the process here? Should I resubmit those two patches, or...? Happy hacking, ~ Luke Shumaker
participants (3)
-
Dave Reisner
-
Luke Shumaker
-
Luke Shumaker