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 <luke...@parabola.nu>
>>>
>>> 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

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to