| Message ID | 20180219201145.14057-4-eschwartz@archlinux.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fix ambiguous uses of $PKGEXT | expand |
On Mon, 19 Feb 2018 15:11:43 -0500, Eli Schwartz via arch-projects wrote: > --- a/db-update > +++ b/db-update > @@ -9,9 +9,14 @@ if (( $# >= 1 )); then > fi > > # Find repos with packages to release > -if ! staging_repos=($(find "${STAGING}" -mindepth 1 -type f -name "*${PKGEXTS}" -printf '%h\n' | sort -u)); then > - die "Could not read %s" "$STAGING" > -fi > +mapfile -t -d '' staging_repos < <( > + for f in "${STAGING}"/**/*${PKGEXTS}; do > + f="${f%/*}" > + if [[ -d $f ]]; then > + printf '%s\0' "$f" > + fi > + done | sort -uz > +) > > repos=() > for staging_repo in ${staging_repos[@]##*/}; do Isn't [[ -d ]] there redundant? If globbing gave us $dir/file, of course $dir is a directory! Meanwhile, this dropped the `-type f` check, though I'm not sure how important that was. Shouldn't this be written as: mapfile -t -d '' staging_repos < <( for f in "${STAGING}"/**/*${PKGEXTS}; do if [[ -f $f && ! -h $f ]]; then printf '%s\0' "${f/*}" fi done | sort -uz ) The original `find` command rejected symlinks; I don't know if that's an important property; but that's what the `&& ! -h $f` bit is for.
On 02/19/2018 04:53 PM, Luke Shumaker wrote: > Isn't [[ -d ]] there redundant? If globbing gave us $dir/file, of > course $dir is a directory! True. I think I still had that in from some point where I hadn't enabled nullglob yet. > Meanwhile, this dropped the `-type f` check, though I'm not sure how > important that was. > > Shouldn't this be written as: > > mapfile -t -d '' staging_repos < <( > for f in "${STAGING}"/**/*${PKGEXTS}; do > if [[ -f $f && ! -h $f ]]; then > printf '%s\0' "${f/*}" > fi > done | sort -uz > ) > > The original `find` command rejected symlinks; I don't know if that's > an important property; but that's what the `&& ! -h $f` bit is for. It is not important, the find command only checked if the file itself was a symlink but if there is another package file in the same directory then we still add those staging repos. Meanwhile, we check later on for `die "Package %s is a symbolic link"`. So I guess technically it would make more sense to stage the package and then utilize the explicit error message rather than silently dropping the package altogether (but only sometimes) simply because we didn't think to use -xtype. At this stage in the game, we're just trying to assemble a list of the packages that the uploader is asserting they want to db-update. We perform all actual validation later on.
diff --git a/db-functions b/db-functions index e8eb2bc..394c7a2 100644 --- a/db-functions +++ b/db-functions @@ -2,6 +2,10 @@ . /usr/share/makepkg/util.sh +# global shell options for enhanced bash scripting +shopt -s globstar nullglob + + # Some PKGBUILDs need CARCH to be set CARCH="x86_64" diff --git a/db-update b/db-update index a8d885a..db12df8 100755 --- a/db-update +++ b/db-update @@ -9,9 +9,14 @@ if (( $# >= 1 )); then fi # Find repos with packages to release -if ! staging_repos=($(find "${STAGING}" -mindepth 1 -type f -name "*${PKGEXTS}" -printf '%h\n' | sort -u)); then - die "Could not read %s" "$STAGING" -fi +mapfile -t -d '' staging_repos < <( + for f in "${STAGING}"/**/*${PKGEXTS}; do + f="${f%/*}" + if [[ -d $f ]]; then + printf '%s\0' "$f" + fi + done | sort -uz +) repos=() for staging_repo in ${staging_repos[@]##*/}; do
Don't bother emitting errors. bash doesn't show globbing errors if it cannot read a directory to try globbing there. And the former code never aborted on errors anyway, as without `set -o pipefail` the sort command swallowed the return code. Signed-off-by: Eli Schwartz <eschwartz@archlinux.org> --- v2: use mapfile as suggested by Luke, rather than running in_array in a loop. db-functions | 4 ++++ db-update | 11 ++++++++--- 2 files changed, 12 insertions(+), 3 deletions(-)