[arch-projects] [dbscripts] [PATCH v3] test: db-update: @test "update same any package to same repository fails": change PKGEXT
From: Luke Shumaker
On 02/19/2018 06:31 PM, 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...
v2: Follow Eli's suggestion to simplify it using the check in __buildPackage v3: Simplify further by assuming __buildPackage checks PKGEXT, not PKGEXTS --- This is written againt Eli's v2 patchset (my concerns there don't affect this). You can verify--applying this patch first makes the tests fail, then applying Eli's patches make the tests pass again.
Dave's objections to the __isGlobfile name and comment apply to this patch as well. As far as the testsuite is concerned, you can just use "Fix overloading PKGEXT to mean two things." as a base. This means that all you need to do is check that if you releasePackage the same package twice using a new $PKGEXT it is still rejected.
... We're not testing whether or not globs work, we're testing whether or not check_pkgrepos properly detects pre-existing packages (which it does via globs). Using __isGlobfile() here will no longer be useful information once $PKGEXT is only ever something from makepkg. So it doesn't make sense to add code that will be almost immediately removed. -- Eli Schwartz Bug Wrangler and Trusted User
On 02/19/2018 08:47 PM, Eli Schwartz wrote:
On 02/19/2018 06:31 PM, 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...
v2: Follow Eli's suggestion to simplify it using the check in __buildPackage v3: Simplify further by assuming __buildPackage checks PKGEXT, not PKGEXTS --- This is written againt Eli's v2 patchset (my concerns there don't affect this). You can verify--applying this patch first makes the tests fail, then applying Eli's patches make the tests pass again.
Dave's objections to the __isGlobfile name and comment apply to this patch as well. As far as the testsuite is concerned, you can just use "Fix overloading PKGEXT to mean two things." as a base. This means that all you need to do is check that if you releasePackage the same package twice using a new $PKGEXT it is still rejected.
...
We're not testing whether or not globs work, we're testing whether or not check_pkgrepos properly detects pre-existing packages (which it does via globs). Using __isGlobfile() here will no longer be useful information once $PKGEXT is only ever something from makepkg. So it doesn't make sense to add code that will be almost immediately removed.
Actually, it would tend to help if we had the actual candidate filenames here. Hmm... -- Eli Schwartz Bug Wrangler and Trusted User
On 02/19/2018 09:12 PM, Eli Schwartz wrote:
On 02/19/2018 08:47 PM, Eli Schwartz wrote:
On 02/19/2018 06:31 PM, 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...
v2: Follow Eli's suggestion to simplify it using the check in __buildPackage v3: Simplify further by assuming __buildPackage checks PKGEXT, not PKGEXTS --- This is written againt Eli's v2 patchset (my concerns there don't affect this). You can verify--applying this patch first makes the tests fail, then applying Eli's patches make the tests pass again.
Dave's objections to the __isGlobfile name and comment apply to this patch as well. As far as the testsuite is concerned, you can just use "Fix overloading PKGEXT to mean two things." as a base. This means that all you need to do is check that if you releasePackage the same package twice using a new $PKGEXT it is still rejected.
...
We're not testing whether or not globs work, we're testing whether or not check_pkgrepos properly detects pre-existing packages (which it does via globs). Using __isGlobfile() here will no longer be useful information once $PKGEXT is only ever something from makepkg. So it doesn't make sense to add code that will be almost immediately removed.
Actually, it would tend to help if we had the actual candidate filenames here. Hmm...
Maybe:
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
could use if cp -Lv ${cache}/*${PKGEXT}{,.sig} ${pkgdest} 2>/dev/null; then return 0 else This would avoid the need for __isGlobfile function altogether. I'd also like a more descriptive commit message. Don't tell me what you changed, tell me why you changed it. :p -- Eli Schwartz Bug Wrangler and Trusted User
Eli Schwartz wrote:
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
We're not testing whether or not globs work, we're testing whether or not check_pkgrepos properly detects pre-existing packages (which it does via globs). Using __isGlobfile() here will no longer be useful information once $PKGEXT is only ever something from makepkg. So it doesn't make sense to add code that will be almost immediately removed.
The glob I was using it for wasn't PKGEXT(s), it was the '*' that's right there in the argument!
Maybe: ... if cp -Lv ${cache}/*${PKGEXT}{,.sig} ${pkgdest} 2>/dev/null; then return 0 else
This would avoid the need for __isGlobfile function altogether.
I like that! Good idea!
I'd also like a more descriptive commit message. Don't tell me what you changed, tell me why you changed it. :p
Ok, I'll go in to that more. -- Happy hacking, ~ Luke Shumaker
On 02/19/2018 10:24 PM, Luke Shumaker wrote:
The glob I was using it for wasn't PKGEXT(s), it was the '*' that's right there in the argument!
Right, that's why I replied to myself with "Actually, it would tend to help if we had the actual candidate filenames here. Hmm..." Context: I wrote a makepkg patch whose sole purpose is to ensure that people can extract a newline-separated list of absolute filepaths for each package that a PKGBUILD would/did create. No globs needed. :) I'm hoping this will simplify some rather hackish and fragile logic in *many* projects that interface with makepkg.
Maybe: ... if cp -Lv ${cache}/*${PKGEXT}{,.sig} ${pkgdest} 2>/dev/null; then return 0 else
This would avoid the need for __isGlobfile function altogether.
I like that! Good idea!
I'd also like a more descriptive commit message. Don't tell me what you changed, tell me why you changed it. :p
Ok, I'll go in to that more.
The changes look great, thanks. -- Eli Schwartz Bug Wrangler and Trusted User
participants (2)
-
Eli Schwartz
-
Luke Shumaker