[arch-projects] [dbscripts] [PATCH v2 0/8] PKGEXT fixup
From: Luke Shumaker
From: Luke Shumaker
From: Luke Shumaker
On Sun, Feb 18, 2018 at 12:17:30PM -0500, Luke Shumaker wrote:
From: Luke Shumaker
This has the test change PKGEXT the second time it tries to release the package. Currently, this causes the tests to fail. That's a good thing; it's checking for the regression where db-functions:check_pkgrepos isn't treating PKGEXT as a glob.
Without this, that regression didn't cause test failure because the checks right after it were tripping anyway.
https://lists.archlinux.org/pipermail/arch-projects/2018-February/004742.htm... --- test/cases/db-update.bats | 3 ++- test/lib/common.bash | 7 ++++++- 2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/test/cases/db-update.bats b/test/cases/db-update.bats index 1da7eef..36511c9 100644 --- a/test/cases/db-update.bats +++ b/test/cases/db-update.bats @@ -92,8 +92,9 @@ load ../lib/common db-update checkPackage extra pkg-any-a
- releasePackage extra pkg-any-a + PKGEXT=.pkg.tar.gz releasePackage extra pkg-any-a run db-update + [[ -z $BUILDDIR ]] || rm -f "${BUILDDIR}/$(__getCheckSum "${TMP}/svn-packages-copy/pkg-any-a/trunk/PKGBUILD")"/*.pkg.tar.gz{,.sig} [ "$status" -ne 0 ] }
diff --git a/test/lib/common.bash b/test/lib/common.bash index cad4e13..d34af8a 100644 --- a/test/lib/common.bash +++ b/test/lib/common.bash @@ -14,6 +14,11 @@ __getCheckSum() { echo "${result%% *}" }
+# Check if a file exists, even if the file uses wildcards +__isGlobfile() {
Frankly, this function name and comment sucks, because it says nothing about quoting. As I read the comment, I'm lead to believe that given a file "foobar" existing, I can call: __isGlobfile "foo*", and this will succeed. To the naive reader, you might even believe this claim based on the unquotedness of $1 within the -f test.
+ [[ -f $1 ]] +}
+ __buildPackage() { local pkgdest=${1:-.} local p @@ -24,7 +29,7 @@ __buildPackage() {
if [[ -n ${BUILDDIR} ]]; then cache=${BUILDDIR}/$(__getCheckSum PKGBUILD) - if [[ -d ${cache} ]]; then + if __isGlobfile "${cache}"/*${PKGEXT}; then cp -Lv ${cache}/*${PKGEXT}{,.sig} ${pkgdest} return 0 else -- 2.16.1
From: Luke Shumaker
From: Luke Shumaker
On Sun, 18 Feb 2018 12:17:32 -0500, Luke Shumaker wrote:
diff --git a/test/cases/ftpdir-cleanup.bats b/test/cases/ftpdir-cleanup.bats index 7dfad4a..efc18a8 100644 --- a/test/cases/ftpdir-cleanup.bats +++ b/test/cases/ftpdir-cleanup.bats @@ -13,8 +13,8 @@ __checkRepoRemovedPackage() { local pkgname
for pkgname in $(__getPackageNamesFromPackageBase ${pkgbase}); do - [[ ! -f ${FTP_BASE}/${PKGPOOL}/${pkgname}-*${PKGEXT_glob} ]] - [[ ! -f ${FTP_BASE}/${repo}/os/${repoarch}/${pkgname}-*${PKGEXT_glob} ]] + ! is_globfile "${FTP_BASE}/${PKGPOOL}/${pkgname}"-*${PKGEXT_glob} + ! is_globfile "${FTP_BASE}/${repo}/os/${repoarch}/${pkgname}"-*${PKGEXT_glob} done
Shoot, that needs to be __isGlobfile; we don't load db-functions here; is_globfile is undefined. It's basically doing "! false". No mater how many times you look over it before hitting send... -- Happy hacking, ~ Luke Shumaker
From: Luke Shumaker
From: Luke Shumaker
From: Luke Shumaker
From: Luke Shumaker
On 02/18/2018 12:17 PM, Luke Shumaker wrote:
From: Luke Shumaker
This incorporates and improves on work from 3 previously submitted patch sets:
1. My testcase patch, but - take Eli's suggestion to simplify db-update.bats - add another commit so we don't hit a bug in BATS
2. Renaming PKGEXT->PKGEXT_glob from my first fix patchset
3. Eli's fix/extglob patchset, but - re-order things between commits - also accept ".tar" (with no compression suffix) - use `old_pkgs=()` instead of `unset old_pkgs` - replace `[ -ge 1]` with `(( > 0 ))` instead of `(( > 1 ))` - db-update: use `readarray < <(... | sort -u)` instead of the O(n^2) op of using in_array in a for loop and appending to the array - common.bash, sourceballs.bats: update to work with shopt -s extglob nullglob globstar; the tests needed updated too, not just the main code
The last 3 commits aren't really related to the goal of the patchset, but I wanted to include all of the work from Eli's fix patchset.
I don't mean to take credit away from Eli by re-working his patches (I credit him in the commit messages); I just wanted to make it clearer what is accomplished by each change, and how each of the changes relate to our goals; as well as actually testing each of them against the test suite. No need to resubmit someone else's amended patches, or resubmit not-amended patches with the statement "I've tested this" (you did that for devtools). Please just leave comments on the submitted patch.
I can resubmit the patch with a "v2: fix blahblahblah" comment after the commit message detailing what changed. I just won't have time until Monday. This also makes it easier to keep track of *which* patches are rerolls of previous patches. -- Eli Schwartz Bug Wrangler and Trusted User
On Sun, 18 Feb 2018 12:29:31 -0500, Eli Schwartz wrote:
On 02/18/2018 12:17 PM, Luke Shumaker wrote:
I don't mean to take credit away from Eli by re-working his patches (I credit him in the commit messages); I just wanted to make it clearer what is accomplished by each change, and how each of the changes relate to our goals; as well as actually testing each of them against the test suite. No need to resubmit someone else's amended patches, or resubmit not-amended patches with the statement "I've tested this" (you did that for devtools). Please just leave comments on the submitted patch.
There were also rebase/merge conflicts. I wanted to have a commit that fixed glob support (is_globfile), to verify that that worked before adding extglob support; re-ordering that/splitting those commits created rebase conflicts. I verified that the test status of each of the commits is as expected. I put the PKGEXT->PKGEXT_glob commit before the fix because it turns out that it's easy to accidentally effectively revert the test during that rename; and I wanted to ensure that it was still being tested by the time the fix is applied. This, of course, meant that your patches didn't apply cleanly. Noting the 3 patchsets that this incorporates: 1. luke1: Renaming PKGEXT->PKGEXT_glob from my first fix patchset 2. luke2: My testcase patch 3. eli : Eli's fix/extglob patchset The origin of the changes in each, and it's testsuite status: # origin test subject [1/8] new PASS test: common.bash:__getCheckSum: Don't rely on IFS [2/8] luke2+reroll FAIL test: db-update: @test "update same any package to same repository fails": change PKGEXT [3/8] luke1+reroll FAIL config: Rename PKGEXT to PKGEXT_glob [4/8] eli+reroll PASS Correctly treat PKGEXT_glob as a glob [5/8] eli+reroll PASS config: let PKGEXT_glob be an extglob; have its value match makepkg [6/8] eli PASS ftpdir-cleanup: fix typo in a comment ("pacakge") [7/8] eli PASS Replace all instances of `find` command with bash globbing [8/8] eli+reroll PASS ftpdir-cleanup, sourceballs: swap out [ -ge 1 ] for (( > 0 )) I don't believe the changes that are in commits 6 or 7 changed from your work, except that they got split in to separate commits. -- Happy hacking, ~ Luke Shumaker
participants (3)
-
Dave Reisner
-
Eli Schwartz
-
Luke Shumaker