[arch-projects] [dbscripts] [PATCH 2/3] ftpdir-cleanup, sourceballs: replace external find command with bash globbing

[Eli Schwartz]

On 02/17/2018 02:29 PM, Luke Shumaker wrote:
> On Thu, 15 Feb 2018 22:45:03 -0500,
> Eli Schwartz via arch-projects wrote:
>> diff --git a/cron-jobs/ftpdir-cleanup b/cron-jobs/ftpdir-cleanup
>> index 2f3d5aa..2d33047 100755
>> --- a/cron-jobs/ftpdir-cleanup
>> +++ b/cron-jobs/ftpdir-cleanup
> ...
>> -if [ ${#old_pkgs[@]} -ge 1 ]; then
>> +if (( ${#old_pkgs[@]} > 1 )); then
> 
> That should either be >= 1 or > 0.
> 
>> diff --git a/cron-jobs/sourceballs b/cron-jobs/sourceballs
>> index 9ab4e98..5844817 100755
>> --- a/cron-jobs/sourceballs
>> +++ b/cron-jobs/sourceballs
> ...
>> -if [ ${#old_pkgs[@]} -ge 1 ]; then
>> +if (( ${#old_pkgs[@]} > 1 )); then
> 
> Likewise.

Thanks. I've also noticed this whole patchset terribly breaks the
testsuite. Which is sort of expected.

We are overloading PKGEXT to mean something dbscripts specific, but that
then (I think?) gets imported into makepkg during the testsuite builds.
I'm going to rename it to PKGEXTS as that serves a number of purposes:
it avoids clashing with makepkg, it is more descriptive of its actual
purpose, and it provides a free semantic warning to future readers of
the code that this variable is meant to be more than one thing, and
extra care *must* be taken when using it.

But the real issue is that we then use this variable to complete
${pkgnames[@]/%/${PKGEXT}} which works, sort of, as it coincidentally
globs okay with ? but is technically quite wrong for the above mentioned
reasons.

Really, once pacman 5.1 is released containing my fix that makes
--packagelist finally useful for the first time ever, this will
automatically be fixed, as the use of print_all_package_names will
simply return full filename paths and there will be no need to glob
something that matches both filenames from your patch "Update tests to
check for glob regression".

The testsuite keeps looking for files that match some random dbscripts
glob which has nothing to do with the hardcoded .pkg.tar.xz in a stock
makepkg.conf, and the testsuite seems to be subtly buggy. Looks like
PKGEXT is also used for complicated things in checkPackageDB, with more
unquoted  [ ] paths as well as grep -q "${pkgfile%${PKGEXT}}" which
actually *breaks* with extglob. Because extended globs don't fall back
on being a string literal -- which is behavior I approve of. So this
needs to use ${pkgname}-${pkgver}-${pkgarch}.

We can either have $pkgfile not include $PKGEXT, use more is_globfile
(which is not actually available in the testsuite as db-functions is not
sourced and will break everything if you try since it runs mktemp with
the bats TMPDIR or something), or rename PKGEXT, and have the testsuite
use the PKGEXT from makepkg.conf since that is what it will use anyway
when running makepkg...

-- 
Eli Schwartz
Bug Wrangler and Trusted User

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.archlinux.org/pipermail/arch-projects/attachments/20180217/9ed83e6a/attachment.asc>