[arch-projects] [devtools] Minor cleanups of common.sh
I'm the maintainer of the equivalent of devtools in Parabola, and I realized that most of the changes we apply have never been pushed upstream. So I'm doing that now. This is a set of minor improvements to common.sh that I hope are fairly uncontroversial; even if they aren't directly taken advantage of right now in devtools. In the coming days, I will be sending more patches as I isolate out many years of `git merge`s into sets of atomic commits that I think you might welcome. -- Happy hacking, ~ Luke Shumaker
`lock_close FD` is easier to remember than 'exec FD>&-`; and is especially easier if FD is a variable (though that isn't actually taken advantage of here). This uses Bash 4.1+ `exec {var}>&-`, rather than the clunkier `eval exec "$var>&-"` that was necessary in older versions of Bash. Thanks to Dave Reisner for pointing this new bit of syntax out to me the last time I submitted this (back in 2014, 4.1 had just come out). --- 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 a78353c..4d1b351 100644 --- a/archbuild.in +++ b/archbuild.in @@ -55,7 +55,7 @@ if ${clean_first} || [[ ! -d "${chroots}/${repo}-${arch}" ]]; then subvolume_delete_recursive "${copy}" 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 599be54..689772f 100644 --- a/lib/common.sh +++ b/lib/common.sh @@ -159,6 +159,14 @@ slock() { } ## +# usage : lock_close( $fd ) +## +lock_close() { + local fd=$1 + exec {fd}>&- +} + +## # usage: pkgver_equal( $pkgver1, $pkgver2 ) ## pkgver_equal() { diff --git a/makechrootpkg.in b/makechrootpkg.in index dc598f7..ad99334 100644 --- a/makechrootpkg.in +++ b/makechrootpkg.in @@ -108,7 +108,7 @@ create_chroot() { stat_done # Drop the read lock again - exec 8>&- + lock_close 8 fi # Update mtime -- 2.12.0
Allow for locks to be inherited. Inheriting the lock is something that mkarchroot could do previously, but has since lost the ability to do. This allows for the programs to be more compos-able. Do this by instead of unconditionally opening $file on $fd, first check if $file is already open on $fd; and go ahead use it if it is. The naive way of doing this would be to `$(readlink /dev/fd/$fd)` and compare that to `$file`. However, if `$file` is itself a symlink; or there is a symlink somewhere in the path to `$file`, then this could easily fail. Instead, check `[[ "/dev/fd/$fd" -ef "$file" ]]`. Even though the Bash documentation (`help test`) says that `-ef` checks for if the two files are hard links to eachother, because it uses stat(3) (which resolves symlinks) to do this check, it also works with the /dev/fd/ soft links. --- lib/common.sh | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/common.sh b/lib/common.sh index 689772f..63b7795 100644 --- a/lib/common.sh +++ b/lib/common.sh @@ -138,7 +138,11 @@ get_full_version() { # usage : lock( $fd, $file, $message ) ## lock() { - eval "exec $1>"'"$2"' + # Only reopen the FD if it wasn't handed to us + if ! [[ "/dev/fd/$1" -ef "$2" ]]; then + eval "exec $1>"'"$2"' + fi + if ! flock -n $1; then stat_busy "$3" flock $1 @@ -150,7 +154,11 @@ lock() { # usage : slock( $fd, $file, $message ) ## slock() { - eval "exec $1>"'"$2"' + # Only reopen the FD if it wasn't handed to us + if ! [[ "/dev/fd/$1" -ef "$2" ]]; then + eval "exec $1>"'"$2"' + fi + if ! flock -sn $1; then stat_busy "$3" flock -s $1 -- 2.12.0
--- lib/common.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/common.sh b/lib/common.sh index 63b7795..6d873ed 100644 --- a/lib/common.sh +++ b/lib/common.sh @@ -140,6 +140,7 @@ get_full_version() { lock() { # Only reopen the FD if it wasn't handed to us if ! [[ "/dev/fd/$1" -ef "$2" ]]; then + mkdir -p -- "$(dirname -- "$2")" eval "exec $1>"'"$2"' fi @@ -156,6 +157,7 @@ lock() { slock() { # Only reopen the FD if it wasn't handed to us if ! [[ "/dev/fd/$1" -ef "$2" ]]; then + mkdir -p -- "$(dirname -- "$2")" eval "exec $1>"'"$2"' fi -- 2.12.0
--- lib/common.sh | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/common.sh b/lib/common.sh index 6d873ed..5ef9f97 100644 --- a/lib/common.sh +++ b/lib/common.sh @@ -59,12 +59,18 @@ stat_done() { printf "${BOLD}done${ALL_OFF}\n" >&2 } +_setup_workdir=false setup_workdir() { [[ -z $WORKDIR ]] && WORKDIR=$(mktemp -d --tmpdir "${0##*/}.XXXXXXXXXX") + _setup_workdir=true + trap 'trap_abort' INT QUIT TERM HUP + trap 'trap_exit' EXIT } cleanup() { - [[ -n $WORKDIR ]] && rm -rf "$WORKDIR" + if [[ -n $WORKDIR ]] && $_setup_workdir; then + rm -rf "$WORKDIR" + fi exit ${1:-0} } @@ -89,9 +95,6 @@ die() { cleanup 255 } -trap 'trap_abort' INT QUIT TERM HUP -trap 'trap_exit' EXIT - ## # usage : in_array( $needle, $haystack ) # return : 0 - found -- 2.12.0
On Sat, Mar 25, 2017 at 5:35 PM Luke Shumaker <lukeshu@parabola.nu> wrote:
I'm the maintainer of the equivalent of devtools in Parabola, and I realized that most of the changes we apply have never been pushed upstream. So I'm doing that now.
This is a set of minor improvements to common.sh that I hope are fairly uncontroversial; even if they aren't directly taken advantage of right now in devtools.
In the coming days, I will be sending more patches as I isolate out many years of `git merge`s into sets of atomic commits that I think you might welcome.
Yes, they look good. Thanks a lot.
participants (2)
-
Jan Alexander Steffens
-
Luke Shumaker