First of all, it ran `is_btrfs "$chrootdir"` to decide if it was on btrfs, but $chrootdir wasn't defined locally; it just happens to work because $chrootdir was defined in main(). (I noticed this because in Parabola, it is called differently, so $chrootdir was empty). So I was tempted to just change it to `is_btrfs "$copydir"`, but if $copydir is just a regular directory on a btrfs filesystem, then it It would leave much of $copydir intact. What we really care about is if $copydir is a btrfs subvolume; which we can check by combining the is_btrfs check with inspecting the inum of the directory. I put this combined check in lib/archroot.sh:is_subvolume. https://lists.archlinux.org/pipermail/arch-projects/2013-September/003901.ht... --- lib/archroot.sh | 11 ++++++++++- makechrootpkg.in | 2 +- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/archroot.sh b/lib/archroot.sh index 46d4963..87c28a2 100644 --- a/lib/archroot.sh +++ b/lib/archroot.sh @@ -25,6 +25,15 @@ is_btrfs() { } ## +# usage : is_subvolume( $path ) +# return : whether $path is a the root of a btrfs subvolume (including +# the top-level subvolume). +## +is_subvolume() { + [[ -e "$1" && "$(stat -f -c %T "$1")" == btrfs && "$(stat -c %i "$1")" == 256 ]] +} + +## # usage : subvolume_delete_recursive( $path ) # # Find all btrfs subvolumes under and including $path and delete them. @@ -32,7 +41,7 @@ is_btrfs() { subvolume_delete_recursive() { local subvol - is_btrfs "$1" || return 0 + is_subvolume "$1" || return 0 while IFS= read -d $'\0' -r subvol; do if ! btrfs subvolume delete "$subvol" &>/dev/null; then diff --git a/makechrootpkg.in b/makechrootpkg.in index 72b7eb7..88c2cdc 100644 --- a/makechrootpkg.in +++ b/makechrootpkg.in @@ -144,7 +144,7 @@ delete_chroot() { local copy=${1:-$2} stat_busy "Removing chroot copy [%s]" "$copy" - if is_btrfs "$chrootdir" && ! mountpoint -q "$copydir"; then + if is_subvolume "$copydir" && ! mountpoint -q "$copydir"; then subvolume_delete_recursive "$copydir" || die "Unable to delete subvolume %s" "$copydir" else -- 2.12.2

This is inspired by the thought that went in to the delete_chroot is_subvolume commit. sync_chroot($chrootdir, $copydir) copies `$chrootdir/root` to `$copydir`. That seems a little silly; why do we care about "$chrootdir"? Have it just be sync_chroot(source, destination) like every other sync/copy command. Where this becomes tricky is check to decide if we are going to use btrfs subvolumes or not. We don't care if "$source/.." is on btrfs; the root could be a directly-mounted subvolume, but and the destination could be another subvolume of the same btrfs mounted somewhere else. The things we do care about are: - The source is a btrfs subvolume (so that we can snapshot it) - The source is on the same filesystem as the directory that the copy will be created in. - If the destination exists: * that it is not a mountpoint (so that we can delete and recreate it) * that it is a btrfs subvolume (so that we can quickly delete it) On the last point, it isn't necessary for creating the new snapshot, just for quick deletion. That can be a separate check, where we use regular `rm` for deleting the existing copy, but use subvolume snapshots for creating the new one. --- lib/archroot.sh | 8 ++++++++ makechrootpkg.in | 32 +++++++++++++++++++------------- 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/lib/archroot.sh b/lib/archroot.sh index 87c28a2..6b1b52e 100644 --- a/lib/archroot.sh +++ b/lib/archroot.sh @@ -34,6 +34,14 @@ is_subvolume() { } ## +# usage : is_same_fs( $path_a, $path_b ) +# return : whether $path_a and $path_b are on the same filesystem +## +is_same_fs() { + [[ "$(stat -c %d "$1")" == "$(stat -c %d "$1")" ]] +} + +## # usage : subvolume_delete_recursive( $path ) # # Find all btrfs subvolumes under and including $path and delete them. diff --git a/makechrootpkg.in b/makechrootpkg.in index d92d6ab..cc3c738 100644 --- a/makechrootpkg.in +++ b/makechrootpkg.in @@ -98,31 +98,37 @@ load_vars() { return 0 } -# Usage: sync_chroot $chrootdir $copydir [$copy] +# Usage: sync_chroot $rootdir $copydir [$copy] sync_chroot() { - local chrootdir=$1 + local rootdir=$1 local copydir=$2 local copy=${3:-$2} - if [[ "$chrootdir/root" -ef "$copydir" ]]; then + if [[ "$rootdir" -ef "$copydir" ]]; then error 'Cannot sync copy with itself: %s' "$copydir" return 1 fi # 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 [%s]" "$chrootdir/root" - - stat_busy "Synchronizing chroot copy [%s] -> [%s]" "$chrootdir/root" "$copy" - if is_btrfs "$chrootdir" && ! mountpoint -q "$copydir"; then - subvolume_delete_recursive "$copydir" || - die "Unable to delete subvolume %s" "$copydir" - btrfs subvolume snapshot "$chrootdir/root" "$copydir" >/dev/null || + slock 8 "$rootdir.lock" \ + "Locking clean chroot [%s]" "$rootdir" + + stat_busy "Synchronizing chroot copy [%s] -> [%s]" "$rootdir" "$copy" + if is_subvolume "$rootdir" && is_same_fs "$rootdir" "$(dirname -- "$copydir")" && ! mountpoint -q "$copydir"; then + if is_subvolume "$copydir"; then + subvolume_delete_recursive "$copydir" || + die "Unable to delete subvolume %s" "$copydir" + else + # avoid change of filesystem in case of an umount failure + rm --recursive --force --one-file-system "$copydir" || + die "Unable to delete %s" "$copydir" + fi + btrfs subvolume snapshot "$rootdir" "$copydir" >/dev/null || die "Unable to create subvolume %s" "$copydir" else mkdir -p "$copydir" - rsync -a --delete -q -W -x "$chrootdir/root/" "$copydir" + rsync -a --delete -q -W -x "$rootdir/" "$copydir" fi stat_done @@ -374,7 +380,7 @@ main() { lock 9 "$copydir.lock" "Locking chroot copy [%s]" "$copy" if [[ ! -d $copydir ]] || $clean_first; then - sync_chroot "$chrootdir" "$copydir" "$copy" + sync_chroot "$chrootdir/root" "$copydir" "$copy" fi $update_first && arch-nspawn "$copydir" \ -- 2.12.2

What this is really doing is fixing a conflict that I had incorrectly resolved when rebasing what became 2fd5931 onto cda9cf4. Of course, because of dynamic scoping, everything worked out, and everything worked as intended. Before cda9cf4, it was appropriate for download_sources to take src_owner as an argument, but after cda9cf4, it is now appropriate to take makepkg_user as an argument. However, it still takes src_owner as an argument, but pays 0 attention to it; instead looking at makepkg_user which it happily inherited because of dynamic scoping. So change it to take makepkg_user as the argument. --- makechrootpkg.in | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/makechrootpkg.in b/makechrootpkg.in index cc3c738..d922fa0 100644 --- a/makechrootpkg.in +++ b/makechrootpkg.in @@ -252,20 +252,19 @@ _chrootnamcap() { done } -# Usage: download_sources $copydir $src_owner +# Usage: download_sources $copydir $makepkg_user # Globals: # - SRCDEST # - USER download_sources() { local copydir=$1 - local src_owner=$2 + local makepkg_user=$2 local builddir="$(mktemp -d)" chmod 1777 "$builddir" # Ensure sources are downloaded - makepkg_user=${makepkg_user:-$SUDO_USER} - if [[ -n $makepkg_user ]]; then + if [[ "$(id -u "$makepkg_user")" != 0 ]]; then sudo -u "$makepkg_user" env SRCDEST="$SRCDEST" BUILDDIR="$builddir" \ makepkg --config="$copydir/etc/makepkg.conf" --verifysource -o else @@ -332,6 +331,7 @@ main() { [[ ! -f PKGBUILD && -z "${install_pkgs[*]}" ]] && die 'This must be run in a directory containing a PKGBUILD.' [[ -n $makepkg_user && -z $(id -u "$makepkg_user") ]] && die 'Invalid makepkg user.' + makepkg_user=${makepkg_user:-${SUDO_USER:-$USER}} check_root @@ -394,7 +394,7 @@ main() { [[ -f PKGBUILD ]] || return $ret fi - download_sources "$copydir" "$src_owner" + download_sources "$copydir" "$makepkg_user" prepare_chroot "$copydir" "$USER_HOME" "$repack" -- 2.12.2

The default m4 quote characters: `QUOTE' are troublesome, because ` is fairly likely to pop up in a shell script (if not for a subshell, because it is a useful character in comments and user-facing messages). So, this changes it to [[[QUOTE]]], as it is unlikely to see three braces together like that, let alone in unbalanced sets. --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index f4a00ba..d5f6ad7 100644 --- a/Makefile +++ b/Makefile @@ -74,7 +74,7 @@ edit = sed -e "s|@pkgdatadir[@]|$(DESTDIR)$(PREFIX)/share/devtools|g" %: %.in Makefile lib/common.sh @echo "GEN $@" @$(RM) "$@" - @m4 -P $@.in | $(edit) >$@ + @{ echo -n 'm4_changequote([[[,]]])'; cat $@.in; } | m4 -P | $(edit) >$@ @chmod a-w "$@" @chmod +x "$@" @bash -O extglob -n "$@" -- 2.12.2

--- Makefile | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index d5f6ad7..39614a4 100644 --- a/Makefile +++ b/Makefile @@ -114,5 +114,8 @@ dist: upload: scp devtools-$(V).tar.gz devtools-$(V).tar.gz.sig repos.archlinux.org:/srv/ftp/other/devtools/ -.PHONY: all clean install uninstall dist upload +check: $(BINPROGS) bash_completion makepkg-x86_64.conf + shellcheck $^ + +.PHONY: all clean install uninstall dist upload check .DELETE_ON_ERROR: -- 2.12.2

These changes are all strictly "slap some double-quotes in there". Anything more than that is not included in this commit. --- arch-nspawn.in | 12 ++++++------ archrelease.in | 2 +- archrm.in | 2 +- bash_completion.in | 4 ++-- checkpkg.in | 2 +- commitpkg.in | 30 +++++++++++++++--------------- crossrepomove.in | 10 +++++----- find-libdeps.in | 4 ++-- finddeps.in | 6 +++--- lddd.in | 16 ++++++++-------- lib/common.sh | 16 +++++++++------- makechrootpkg.in | 6 +++--- mkarchroot.in | 4 ++-- 13 files changed, 58 insertions(+), 56 deletions(-) diff --git a/arch-nspawn.in b/arch-nspawn.in index 548ebec..2d42e3a 100644 --- a/arch-nspawn.in +++ b/arch-nspawn.in @@ -71,7 +71,7 @@ build_mount_args() { mount_args+=("--bind=${cache_dirs[0]}") - for cache_dir in ${cache_dirs[@]:1}; do + for cache_dir in "${cache_dirs[@]:1}"; do mount_args+=("--bind-ro=$cache_dir") done } @@ -80,8 +80,8 @@ copy_hostconf () { cp -a /etc/pacman.d/gnupg "$working_dir/etc/pacman.d" echo "Server = $host_mirror" >"$working_dir/etc/pacman.d/mirrorlist" - [[ -n $pac_conf ]] && cp $pac_conf "$working_dir/etc/pacman.conf" - [[ -n $makepkg_conf ]] && cp $makepkg_conf "$working_dir/etc/makepkg.conf" + [[ -n $pac_conf ]] && cp "$pac_conf" "$working_dir/etc/pacman.conf" + [[ -n $makepkg_conf ]] && cp "$makepkg_conf" "$working_dir/etc/makepkg.conf" local file for file in "${files[@]}"; do @@ -89,7 +89,7 @@ copy_hostconf () { cp -T "$file" "$working_dir$file" done - sed -r "s|^#?\\s*CacheDir.+|CacheDir = $(echo -n ${cache_dirs[@]})|g" -i "$working_dir/etc/pacman.conf" + sed -r "s|^#?\\s*CacheDir.+|CacheDir = $(echo -n "${cache_dirs[@]}")|g" -i "$working_dir/etc/pacman.conf" } # }}} @@ -98,14 +98,14 @@ umask 0022 # Sanity check if [[ ! -f "$working_dir/.arch-chroot" ]]; then die "'%s' does not appear to be an Arch chroot." "$working_dir" -elif [[ $(cat "$working_dir/.arch-chroot") != $CHROOT_VERSION ]]; then +elif [[ $(cat "$working_dir/.arch-chroot") != "$CHROOT_VERSION" ]]; then die "chroot '%s' is not at version %s. Please rebuild." "$working_dir" "$CHROOT_VERSION" fi build_mount_args copy_hostconf -eval $(grep '^CARCH=' "$working_dir/etc/makepkg.conf") +eval "$(grep '^CARCH=' "$working_dir/etc/makepkg.conf")" [[ -z $nosetarch ]] || unset CARCH diff --git a/archrelease.in b/archrelease.in index 3b11652..2ba9d48 100644 --- a/archrelease.in +++ b/archrelease.in @@ -74,7 +74,7 @@ for tag in "$@"; do fi # copy all files at once from trunk to the subdirectory in repos/ - svn copy -q -r HEAD ${known_files[@]/#/$trunk/} "repos/$tag/" + svn copy -q -r HEAD "${known_files[@]/#/$trunk/}" "repos/$tag/" stat_done done diff --git a/archrm.in b/archrm.in index 3173131..6f4dfc3 100644 --- a/archrm.in +++ b/archrm.in @@ -13,4 +13,4 @@ fi # #popd -rm -rf $1 +rm -rf "$1" diff --git a/bash_completion.in b/bash_completion.in index f0a6bd0..9feef74 100644 --- a/bash_completion.in +++ b/bash_completion.in @@ -5,8 +5,8 @@ _devtools_compgen() { local i r COMPREPLY=($(compgen -W '$*' -- "$cur")) for ((i=1; i < ${#COMP_WORDS[@]}-1; i++)); do - for r in ${!COMPREPLY[@]}; do - if [[ ${COMP_WORDS[i]} = ${COMPREPLY[r]} ]]; then + for r in "${!COMPREPLY[@]}"; do + if [[ ${COMP_WORDS[i]} = "${COMPREPLY[r]}" ]]; then unset 'COMPREPLY[r]'; break fi done diff --git a/checkpkg.in b/checkpkg.in index 03e29f7..20920be 100644 --- a/checkpkg.in +++ b/checkpkg.in @@ -44,7 +44,7 @@ for _pkgname in "${pkgname[@]}"; do oldpkg=${pkgurl##*://*/} - if [[ ${oldpkg##*/} = ${pkgfile##*/} ]]; then + if [[ ${oldpkg##*/} = "${pkgfile##*/}" ]]; then die "The built package (%s) is the one in the repo right now!" "$_pkgname" fi diff --git a/commitpkg.in b/commitpkg.in index 87f5b8b..0482170 100644 --- a/commitpkg.in +++ b/commitpkg.in @@ -50,7 +50,7 @@ done for i in 'changelog' 'install'; do while read -r file; do # evaluate any bash variables used - eval file=\"$(sed "s/^\(['\"]\)\(.*\)\1\$/\2/" <<< "$file")\" + eval "file=\"$(sed "s/^\(['\"]\)\(.*\)\1\$/\2/" <<< "$file")\"" needsversioning+=("$file") done < <(sed -n "s/^[[:space:]]*$i=//p" PKGBUILD) done @@ -79,12 +79,12 @@ done shift $(( OPTIND - 1 )) # check packages have the packager field set -for _arch in ${arch[@]}; do +for _arch in "${arch[@]}"; do if [[ -n $commit_arch && ${_arch} != "$commit_arch" ]]; then continue fi - for _pkgname in ${pkgname[@]}; do - fullver=$(get_full_version $_pkgname) + for _pkgname in "${pkgname[@]}"; do + fullver=$(get_full_version "$_pkgname") if pkgfile=$(find_cached_package "$_pkgname" "$_arch" "$fullver"); then if grep -q "packager = Unknown Packager" <(bsdtar -xOqf "$pkgfile" .PKGINFO); then @@ -128,18 +128,18 @@ declare -a uploads declare -a commit_arches declare -a skip_arches -for _arch in ${arch[@]}; do +for _arch in "${arch[@]}"; do if [[ -n $commit_arch && ${_arch} != "$commit_arch" ]]; then - skip_arches+=($_arch) + skip_arches+=("$_arch") continue fi - for _pkgname in ${pkgname[@]}; do - fullver=$(get_full_version $_pkgname) + for _pkgname in "${pkgname[@]}"; do + fullver=$(get_full_version "$_pkgname") if ! pkgfile=$(find_cached_package "$_pkgname" "$fullver" "${_arch}"); then warning "Skipping %s: failed to locate package file" "$_pkgname-$fullver-$_arch" - skip_arches+=($_arch) + skip_arches+=("$_arch") continue 2 fi uploads+=("$pkgfile") @@ -159,9 +159,9 @@ for _arch in ${arch[@]}; do done done -for _arch in ${arch[@]}; do - if ! in_array $_arch ${skip_arches[@]}; then - commit_arches+=($_arch) +for _arch in "${arch[@]}"; do + if ! in_array "$_arch" "${skip_arches[@]}"; then + commit_arches+=("$_arch") fi done @@ -187,8 +187,8 @@ if [[ "${arch[*]}" == 'any' ]]; then if [[ -d ../repos/$repo-i686 && -d ../repos/$repo-x86_64 ]]; then pushd ../repos/ >/dev/null stat_busy "Removing %s and %s" "$repo-i686" "$repo-x86_64" - svn rm -q $repo-i686 - svn rm -q $repo-x86_64 + svn rm -q "$repo-i686" + svn rm -q "$repo-x86_64" svn commit -q -m "Removed $repo-i686 and $repo-x86_64 for $pkgname" stat_done popd >/dev/null @@ -197,7 +197,7 @@ else if [[ -d ../repos/$repo-any ]]; then pushd ../repos/ >/dev/null stat_busy "Removing %s" "$repo-any" - svn rm -q $repo-any + svn rm -q "$repo-any" svn commit -q -m "Removed $repo-any for $pkgname" stat_done popd >/dev/null diff --git a/crossrepomove.in b/crossrepomove.in index 14c264e..ffc4507 100644 --- a/crossrepomove.in +++ b/crossrepomove.in @@ -38,7 +38,7 @@ target_dbscripts="/srv/repos/svn-${target_name}/dbscripts" setup_workdir -pushd $WORKDIR >/dev/null +pushd "$WORKDIR" >/dev/null msg "Downloading sources for %s" "${pkgbase}" svn -q checkout -N "${target_svn}" target_checkout @@ -47,14 +47,14 @@ svn -q export "${source_svn}/${pkgbase}/trunk" "target_checkout/${pkgbase}/trunk . "target_checkout/${pkgbase}/trunk/PKGBUILD" msg "Downloading packages for %s" "${pkgbase}" -for _arch in ${arch[@]}; do +for _arch in "${arch[@]}"; do if [[ "${_arch[*]}" == 'any' ]]; then repo_arch='x86_64' else repo_arch=${_arch} fi - for _pkgname in ${pkgname[@]}; do - fullver=$(get_full_version $_pkgname) + for _pkgname in "${pkgname[@]}"; do + fullver=$(get_full_version "$_pkgname") pkgpath="/srv/ftp/$source_repo/os/$repo_arch/$_pkgname-$fullver-${_arch}.pkg.tar.*" ssh "$server" "cp $pkgpath staging/$target_repo" || die done @@ -71,7 +71,7 @@ popd >/dev/null ssh "${server}" "${target_dbscripts}/db-update" || die msg "Removing %s from %s" "${pkgbase}" "${source_repo}" -for _arch in ${arch[@]}; do +for _arch in "${arch[@]}"; do ssh "${server}" "${source_dbscripts}/db-remove ${source_repo} ${_arch} ${pkgbase}" done svn -q checkout -N "${source_svn}" source_checkout diff --git a/find-libdeps.in b/find-libdeps.in index c596f48..04adebf 100644 --- a/find-libdeps.in +++ b/find-libdeps.in @@ -28,7 +28,7 @@ if [[ -z $1 ]]; then fi if [[ -d $1 ]]; then - pushd $1 >/dev/null + pushd "$1" >/dev/null else setup_workdir @@ -48,7 +48,7 @@ process_sofile() { if [[ "$soversion" = "$sofile" ]] && ((IGNORE_INTERNAL)); then continue fi - if ! in_array "${soname}=${soversion}-${soarch}" ${soobjects[@]}; then + if ! in_array "${soname}=${soversion}-${soarch}" "${soobjects[@]}"; then # libfoo.so=1-64 echo "${soname}=${soversion}-${soarch}" soobjects+=("${soname}=${soversion}-${soarch}") diff --git a/finddeps.in b/finddeps.in index 03e5501..c71f151 100644 --- a/finddeps.in +++ b/finddeps.in @@ -24,17 +24,17 @@ find . -type d | while read d; do for dep in "${depends[@]}"; do # lose the version comparator, if any depname=${dep%%[<>=]*} - [[ $depname = $match ]] && echo "$d (depends)" + [[ $depname = "$match" ]] && echo "$d (depends)" done for dep in "${makedepends[@]}"; do # lose the version comparator, if any depname=${dep%%[<>=]*} - [[ $depname = $match ]] && echo "$d (makedepends)" + [[ $depname = "$match" ]] && echo "$d (makedepends)" done for dep in "${optdepends[@]/:*}"; do # lose the version comaparator, if any depname=${dep%%[<>=]*} - [[ $depname = $match ]] && echo "$d (optdepends)" + [[ $depname = "$match" ]] && echo "$d (optdepends)" done fi done diff --git a/lddd.in b/lddd.in index f01ebf9..d83c3e6 100644 --- a/lddd.in +++ b/lddd.in @@ -20,7 +20,7 @@ for tree in $PATH $libdirs $extras; do 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' ! \ + files=$(find "$tree" -type f ! -name '*.a' ! -name '*.la' ! -name '*.py*' ! -name '*.txt' ! -name '*.h' ! -name '*.ttf' ! \ -name '*.rb' ! -name '*.ko' ! -name '*.pc' ! -name '*.enc' ! -name '*.cf' ! -name '*.def' ! -name '*.rules' ! -name \ '*.cmi' ! -name '*.mli' ! -name '*.ml' ! -name '*.cma' ! -name '*.cmx' ! -name '*.cmxa' ! -name '*.pod' ! -name '*.pm' \ ! -name '*.pl' ! -name '*.al' ! -name '*.tcl' ! -name '*.bs' ! -name '*.o' ! -name '*.png' ! -name '*.gif' ! -name '*.cmo' \ @@ -28,22 +28,22 @@ for tree in $PATH $libdirs $extras; do -name '*.mcopclass' ! -name '*.mcoptype') IFS=$ifs for i in $files; do - if (( $(file $i | grep -c 'ELF') != 0 )); then + if (( $(file "$i" | grep -c 'ELF') != 0 )); then # Is an ELF binary. - if (( $(ldd $i 2>/dev/null | grep -c 'not found') != 0 )); then + if (( $(ldd "$i" 2>/dev/null | grep -c 'not found') != 0 )); then # Missing lib. - echo "$i:" >> $TEMPDIR/raw.txt - ldd $i 2>/dev/null | grep 'not found' >> $TEMPDIR/raw.txt + echo "$i:" >> "$TEMPDIR/raw.txt" + ldd "$i" 2>/dev/null | grep 'not found' >> "$TEMPDIR/raw.txt" fi fi done done -grep '^/' $TEMPDIR/raw.txt | sed -e 's/://g' >> $TEMPDIR/affected-files.txt +grep '^/' "$TEMPDIR/raw.txt" | sed -e 's/://g' >> "$TEMPDIR/affected-files.txt" # invoke pacman for i in $(cat $TEMPDIR/affected-files.txt); do - pacman -Qo $i | awk '{print $4,$5}' >> $TEMPDIR/pacman.txt + pacman -Qo "$i" | awk '{print $4,$5}' >> "$TEMPDIR/pacman.txt" done # clean list -sort -u $TEMPDIR/pacman.txt >> $TEMPDIR/possible-rebuilds.txt +sort -u "$TEMPDIR/pacman.txt" >> "$TEMPDIR/possible-rebuilds.txt" msg "Files saved to %s" "$TEMPDIR" diff --git a/lib/common.sh b/lib/common.sh index 8f043e8..4a15d9a 100644 --- a/lib/common.sh +++ b/lib/common.sh @@ -79,7 +79,7 @@ cleanup() { if [[ -n ${WORKDIR:-} ]] && $_setup_workdir; then rm -rf "$WORKDIR" fi - exit ${1:-0} + exit "${1:-0}" } abort() { @@ -112,7 +112,7 @@ in_array() { local needle=$1; shift local item for item in "$@"; do - [[ $item = $needle ]] && return 0 # Found + [[ $item = "$needle" ]] && return 0 # Found done return 1 # Not Found } @@ -134,7 +134,7 @@ get_full_version() { else for i in pkgver pkgrel epoch; do local indirect="${i}_override" - eval $(declare -f package_$1 | sed -n "s/\(^[[:space:]]*$i=\)/${i}_override=/p") + eval "$(declare -f "package_$1" | sed -n "s/\(^[[:space:]]*$i=\)/${i}_override=/p")" [[ -z ${!indirect} ]] && eval ${indirect}=\"${!i}\" done if (( ! epoch_override )); then @@ -155,9 +155,9 @@ lock() { eval "exec $1>"'"$2"' fi - if ! flock -n $1; then + if ! flock -n "$1"; then stat_busy "${@:3}" - flock $1 + flock "$1" stat_done fi } @@ -172,9 +172,9 @@ slock() { eval "exec $1>"'"$2"' fi - if ! flock -sn $1; then + if ! flock -sn "$1"; then stat_busy "${@:3}" - flock -s $1 + flock -s "$1" stat_done fi } @@ -184,6 +184,8 @@ slock() { ## lock_close() { local fd=$1 + # https://github.com/koalaman/shellcheck/issues/862 + # shellcheck disable=2034 exec {fd}>&- } diff --git a/makechrootpkg.in b/makechrootpkg.in index f8e8505..2dd160a 100644 --- a/makechrootpkg.in +++ b/makechrootpkg.in @@ -92,7 +92,7 @@ load_vars() { [[ -f $makepkg_conf ]] || return 1 for var in {SRC,SRCPKG,PKG,LOG}DEST MAKEFLAGS PACKAGER; do - [[ -z ${!var:-} ]] && eval $(grep "^${var}=" "$makepkg_conf") + [[ -z ${!var:-} ]] && eval "$(grep "^${var}=" "$makepkg_conf")" done return 0 @@ -200,8 +200,8 @@ prepare_chroot() { # which we might not be able to load (i.e. when building i686 packages on # an x86_64 host). sed -e '/^builduser:/d' -i "$copydir"/etc/{passwd,group} - printf >>"$copydir/etc/group" 'builduser:x:%d:\n' $builduser_gid - printf >>"$copydir/etc/passwd" 'builduser:x:%d:%d:builduser:/build:/bin/bash\n' $builduser_uid $builduser_gid + printf >>"$copydir/etc/group" 'builduser:x:%d:\n' "$builduser_gid" + printf >>"$copydir/etc/passwd" 'builduser:x:%d:%d:builduser:/build:/bin/bash\n' "$builduser_uid" "$builduser_gid" $install -d "$copydir"/{build,build/.gnupg,startdir,{pkg,srcpkg,src,log}dest} diff --git a/mkarchroot.in b/mkarchroot.in index 152d323..52e363f 100644 --- a/mkarchroot.in +++ b/mkarchroot.in @@ -46,13 +46,13 @@ shift $((OPTIND - 1)) check_root -working_dir="$(readlink -f $1)" +working_dir="$(readlink -f "$1")" shift 1 [[ -z $working_dir ]] && die 'Please specify a working directory.' if [[ -z $cache_dir ]]; then - cache_dirs=($(pacman -v $cache_conf 2>&1 | grep '^Cache Dirs:' | sed 's/Cache Dirs:\s*//g')) + cache_dirs=($(pacman -v "$cache_conf" 2>&1 | grep '^Cache Dirs:' | sed 's/Cache Dirs:\s*//g')) else cache_dirs=(${cache_dir}) fi -- 2.12.2

The added PKGBUILD.proto file is so that shellcheck can know know what to expect that a PKGBUILD sets. --- Makefile | 2 +- PKGBUILD.proto | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ arch-nspawn.in | 2 ++ checkpkg.in | 4 ++++ commitpkg.in | 5 +++++ crossrepomove.in | 4 ++++ finddeps.in | 1 + lib/archroot.sh | 2 ++ lib/common.sh | 7 +++++++ lib/valid-tags.sh | 3 +++ makechrootpkg.in | 1 + makepkg-i686.conf | 3 +++ makepkg-x86_64.conf | 3 +++ rebuildpkgs.in | 4 ++++ 14 files changed, 88 insertions(+), 1 deletion(-) create mode 100644 PKGBUILD.proto diff --git a/Makefile b/Makefile index 39614a4..9549569 100644 --- a/Makefile +++ b/Makefile @@ -114,7 +114,7 @@ dist: upload: scp devtools-$(V).tar.gz devtools-$(V).tar.gz.sig repos.archlinux.org:/srv/ftp/other/devtools/ -check: $(BINPROGS) bash_completion makepkg-x86_64.conf +check: $(BINPROGS) bash_completion makepkg-x86_64.conf PKGBUILD.proto shellcheck $^ .PHONY: all clean install uninstall dist upload check diff --git a/PKGBUILD.proto b/PKGBUILD.proto new file mode 100644 index 0000000..e8690e4 --- /dev/null +++ b/PKGBUILD.proto @@ -0,0 +1,48 @@ +#!/hint/bash +# shellcheck disable=2034 + +# This is an example PKGBUILD file, so that shellcheck can know what +# variables to expect be set after including a PKGBUILD. + +# Maintainer: Your Name <youremail@domain.com> +pkgname=NAME +pkgver=VERSION +pkgrel=1 +epoch= +pkgdesc="" +arch=() +url="" +license=('GPL') +groups=() +depends=() +makedepends=() +checkdepends=() +optdepends=() +provides=() +conflicts=() +replaces=() +backup=() +options=() +install= +changelog= +source=("$pkgname-$pkgver.tar.gz" + "$pkgname-$pkgver.patch") +noextract=() +md5sums=() +validpgpkeys=() + +prepare() { + : +} + +build() { + : +} + +check() { + : +} + +package() { + : +} diff --git a/arch-nspawn.in b/arch-nspawn.in index 2d42e3a..c21b2ce 100644 --- a/arch-nspawn.in +++ b/arch-nspawn.in @@ -58,7 +58,9 @@ else cache_dirs=("$cache_dir") fi +# shellcheck disable=2016 host_mirror=$(pacman --cachedir /doesnt/exist -Sddp extra/devtools 2>/dev/null | sed -r 's#(.*/)extra/os/.*#\1$repo/os/$arch#') +# shellcheck disable=2016 [[ $host_mirror == *file://* ]] && host_mirror_path=$(echo "$host_mirror" | sed -r 's#file://(/.*)/\$repo/os/\$arch#\1#g') # {{{ functions diff --git a/checkpkg.in b/checkpkg.in index 20920be..e0e1f83 100644 --- a/checkpkg.in +++ b/checkpkg.in @@ -7,6 +7,7 @@ m4_include(lib/common.sh) # Source makepkg.conf; fail if it is not found if [[ -r '/etc/makepkg.conf' ]]; then + # shellcheck source=makepkg-x86_64.conf source '/etc/makepkg.conf' else die '/etc/makepkg.conf not found!' @@ -14,8 +15,10 @@ fi # Source user-specific makepkg.conf overrides if [[ -r "${XDG_CONFIG_HOME:-$HOME/.config}/pacman/makepkg.conf" ]]; then + # shellcheck source=/dev/null source "${XDG_CONFIG_HOME:-$HOME/.config}/pacman/makepkg.conf" elif [[ -r "$HOME/.makepkg.conf" ]]; then + # shellcheck source=/dev/null source "$HOME/.makepkg.conf" fi @@ -23,6 +26,7 @@ if [[ ! -f PKGBUILD ]]; then die 'This must be run in the directory of a built package.' fi +# shellcheck source=PKGBUILD.proto . ./PKGBUILD if [[ ${arch[0]} == 'any' ]]; then CARCH='any' diff --git a/commitpkg.in b/commitpkg.in index 90210e5..53b6612 100644 --- a/commitpkg.in +++ b/commitpkg.in @@ -5,6 +5,7 @@ m4_include(lib/common.sh) # Source makepkg.conf; fail if it is not found if [[ -r '/etc/makepkg.conf' ]]; then + # shellcheck source=makepkg-x86_64.conf source '/etc/makepkg.conf' else die '/etc/makepkg.conf not found!' @@ -12,8 +13,10 @@ fi # Source user-specific makepkg.conf overrides if [[ -r "${XDG_CONFIG_HOME:-$HOME/.config}/pacman/makepkg.conf" ]]; then + # shellcheck source=/dev/null source "${XDG_CONFIG_HOME:-$HOME/.config}/pacman/makepkg.conf" elif [[ -r "$HOME/.makepkg.conf" ]]; then + # shellcheck source=/dev/null source "$HOME/.makepkg.conf" fi @@ -23,6 +26,8 @@ if [[ ! -f PKGBUILD ]]; then die 'No PKGBUILD file' fi +source=() +# shellcheck source=PKGBUILD.proto . ./PKGBUILD pkgbase=${pkgbase:-$pkgname} diff --git a/crossrepomove.in b/crossrepomove.in index ffc4507..b45b8ae 100644 --- a/crossrepomove.in +++ b/crossrepomove.in @@ -44,6 +44,7 @@ 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 +# shellcheck source=PKGBUILD.proto . "target_checkout/${pkgbase}/trunk/PKGBUILD" msg "Downloading packages for %s" "${pkgbase}" @@ -56,6 +57,7 @@ for _arch in "${arch[@]}"; do for _pkgname in "${pkgname[@]}"; do fullver=$(get_full_version "$_pkgname") pkgpath="/srv/ftp/$source_repo/os/$repo_arch/$_pkgname-$fullver-${_arch}.pkg.tar.*" + # shellcheck disable=2029 ssh "$server" "cp $pkgpath staging/$target_repo" || die done done @@ -68,10 +70,12 @@ pushd "target_checkout/${pkgbase}/trunk" >/dev/null archrelease "${arch[@]/#/$target_repo-}" || die popd >/dev/null +# shellcheck disable=2029 ssh "${server}" "${target_dbscripts}/db-update" || die msg "Removing %s from %s" "${pkgbase}" "${source_repo}" for _arch in "${arch[@]}"; do + # shellcheck disable=2029 ssh "${server}" "${source_dbscripts}/db-remove ${source_repo} ${_arch} ${pkgbase}" done svn -q checkout -N "${source_svn}" source_checkout diff --git a/finddeps.in b/finddeps.in index 80774bb..2a085e5 100644 --- a/finddeps.in +++ b/finddeps.in @@ -20,6 +20,7 @@ fi find . -type d | while read -r d; do if [[ -f "$d/PKGBUILD" ]]; then pkgname=() depends=() makedepends=() optdepends=() + # shellcheck source=PKGBUILD.proto . "$d/PKGBUILD" for dep in "${depends[@]}"; do # lose the version comparator, if any diff --git a/lib/archroot.sh b/lib/archroot.sh index 3a1023e..98fd2cf 100644 --- a/lib/archroot.sh +++ b/lib/archroot.sh @@ -1,6 +1,8 @@ #!/hint/bash # License: Unspecified +: +# shellcheck disable=2034 CHROOT_VERSION='v4' ## diff --git a/lib/common.sh b/lib/common.sh index ad6194d..0fb93d9 100644 --- a/lib/common.sh +++ b/lib/common.sh @@ -35,35 +35,42 @@ readonly ALL_OFF BOLD BLUE GREEN RED YELLOW plain() { local mesg=$1; shift + # shellcheck disable=2059 printf "${BOLD} ${mesg}${ALL_OFF}\n" "$@" >&2 } msg() { local mesg=$1; shift + # shellcheck disable=2059 printf "${GREEN}==>${ALL_OFF}${BOLD} ${mesg}${ALL_OFF}\n" "$@" >&2 } msg2() { local mesg=$1; shift + # shellcheck disable=2059 printf "${BLUE} ->${ALL_OFF}${BOLD} ${mesg}${ALL_OFF}\n" "$@" >&2 } warning() { local mesg=$1; shift + # shellcheck disable=2059 printf "${YELLOW}==> WARNING:${ALL_OFF}${BOLD} ${mesg}${ALL_OFF}\n" "$@" >&2 } error() { local mesg=$1; shift + # shellcheck disable=2059 printf "${RED}==> ERROR:${ALL_OFF}${BOLD} ${mesg}${ALL_OFF}\n" "$@" >&2 } stat_busy() { local mesg=$1; shift + # shellcheck disable=2059 printf "${GREEN}==>${ALL_OFF}${BOLD} ${mesg}...${ALL_OFF}" "$@" >&2 } stat_done() { + # shellcheck disable=2059 printf "${BOLD}done${ALL_OFF}\n" >&2 } diff --git a/lib/valid-tags.sh b/lib/valid-tags.sh index e0a3b7c..2916dc7 100644 --- a/lib/valid-tags.sh +++ b/lib/valid-tags.sh @@ -1,12 +1,15 @@ #!/hint/bash # License: Unspecified +: +# shellcheck disable=2034 _arch=( i686 x86_64 any ) +# shellcheck disable=2034 _tags=( core-i686 core-x86_64 core-any extra-i686 extra-x86_64 extra-any diff --git a/makechrootpkg.in b/makechrootpkg.in index 2dd160a..ffe6e42 100644 --- a/makechrootpkg.in +++ b/makechrootpkg.in @@ -241,6 +241,7 @@ EOF # These functions aren't run in makechrootpkg, # so no global variables _chrootbuild() { + # shellcheck source=/dev/null . /etc/profile # Beware, there are some stupid arbitrary rules on how you can # use "$" in arguments to commands with "sudo -i". ${foo} or diff --git a/makepkg-i686.conf b/makepkg-i686.conf index c565795..4cbc28b 100644 --- a/makepkg-i686.conf +++ b/makepkg-i686.conf @@ -1,3 +1,6 @@ +#!/hint/bash +# shellcheck disable=2034 + # # /etc/makepkg.conf # diff --git a/makepkg-x86_64.conf b/makepkg-x86_64.conf index 058da9b..79858a7 100644 --- a/makepkg-x86_64.conf +++ b/makepkg-x86_64.conf @@ -1,3 +1,6 @@ +#!/hint/bash +# shellcheck disable=2034 + # # /etc/makepkg.conf # diff --git a/rebuildpkgs.in b/rebuildpkgs.in index a0e8250..4f4f98b 100644 --- a/rebuildpkgs.in +++ b/rebuildpkgs.in @@ -21,6 +21,7 @@ fi # Source makepkg.conf; fail if it is not found if [[ -r '/etc/makepkg.conf' ]]; then + # shellcheck source=makepkg-x86_64.conf source '/etc/makepkg.conf' else die '/etc/makepkg.conf not found!' @@ -28,8 +29,10 @@ fi # Source user-specific makepkg.conf overrides if [[ -r "${XDG_CONFIG_HOME:-$HOME/.config}/pacman/makepkg.conf" ]]; then + # shellcheck source=/dev/null source "${XDG_CONFIG_HOME:-$HOME/.config}/pacman/makepkg.conf" elif [[ -r "$HOME/.makepkg.conf" ]]; then + # shellcheck source=/dev/null source "$HOME/.makepkg.conf" fi @@ -50,6 +53,7 @@ bump_pkgrel() { pkg_from_pkgbuild() { # we want the sourcing to be done in a subshell so we don't pollute our current namespace export CARCH PKGEXT + # shellcheck source=PKGBUILD.proto (source PKGBUILD; echo "$pkgname-$pkgver-$pkgrel-$CARCH$PKGEXT") } -- 2.12.2