Re: [arch-projects] [dbscripts] [PATCH v3] test: db-update: @test "update same any package to same repository fails": change PKGEXT
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
signature.asc
Description: OpenPGP digital signature
Re: [arch-projects] [dbscripts] [PATCH v3] test: db-update: @test "update same any package to same repository fails": change PKGEXT
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
Re: [arch-projects] [dbscripts] [PATCH v3] test: db-update: @test "update same any package to same repository fails": change PKGEXT
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.html
>>>
>>> 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
signature.asc
Description: OpenPGP digital signature
Re: [arch-projects] [dbscripts] [PATCH v3] test: db-update: @test "update same any package to same repository fails": change PKGEXT
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.html >> >> 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 signature.asc Description: OpenPGP digital signature
Re: [arch-projects] [dbscripts] [PATCH v3] test: db-update: @test "update same any package to same repository fails": change PKGEXT
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.html > > 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 signature.asc Description: OpenPGP digital signature
