[pacman-dev] pacman 5.1.1 and the "final" makepkg bug

[Allan McRae]

On 21/06/18 14:38, Eli Schwartz wrote:
> On 06/20/2018 09:41 PM, Allan McRae wrote:
>> Hi all,
>>
>> I think we are near the point for a pacman-5.1.1 release.   My intention
>> is for it to be basically what on master current + the couple of patches
>> in my patchqueue branch.
>>
>> The final thing that needs addressing is:
>> https://bugs.archlinux.org/task/58776
>>
>> The gist of this bug is makepkg --printsrcinfo was giving entries like
>> "depends = perl>=".  We caught these entries because they could be a
>> genuine mistake.  It turns out this also happens when the version used
>> requires some bash that calls a function or variable that is not at a
>> global scope.
>>
>> While this is fairly easily worked around, it breaks all perl PKGBUILDs
>> that use the makepkg-template in Arch Linux, and removes the "anything
>> correct bash is OK" mantra of PKGBUILDs.
>>
>>
>> I see some choices here:
>>
>> 1) back out the change
>> 2) partially back out the change
>> 3) "hide" the change
>> 4) document the change
>>
>> For 2), I could see changing the error to a warning, documenting the
>> change and including it for next release.
>>
>> For 3), we could print a warning, then strip these empty "<=" at the
>> end.  Note libprovides and libdepends have dynamcially generated
>> versions that are not in --sourceinfo so this would align with them.  We
>> should document that limitation.
>>
>> Opinions?   (I'm leaning towards #3)
> 
> libprovides/libdepends are only sort of similar. They're the ideal state
> of what the perl template really should be. :p
> 
> I dislike option 1, because it results in broken .SRCINFO at the very
> least and depending on implementation will result in genuinely allowing
> bad PKGBUILDs to pass linting.
> 
> option 2 I see no real point in. If we consider these PKGBUILDs to be
> broken, then giving people extra time to fix them may be nice but it's
> also hardly makepkg's problem.

The problem is that these PKGBUILDs are not broken.  They are fully
functional in creating the package that the author intended, apart from
restrictions that were placed without warning in pacman-5.1.  Now
PKGBUILDs are limited in the range of bash constructs they can use.

> option 3 is an interesting proposition, but where should we do this? We
> cannot modify function variables in the lint stage. We could hack
> something into otherwise unrelated code in write_srcinfo and
> write_pkginfo, though, slightly repetitively.

No need to touch write_pkginfo - everything works at that stage because
the package function has been fully evaluated.

> In all honesty, I'd much prefer to just explicitly document that
> "metadata in split package functions cannot be multi-statement bash, but
> otherwise anything bash goes". This is not the only issue with
> extracting metadata from the package function, e.g.
> https://bugs.archlinux.org/task/55004
> 
> And sure, as soon as the internal context of the function is run, we
> suddenly get the intended behavior asserted. But up until then, it just
> doesn't work.
> 
> It caused issues for printsrcinfo, and now that we're trying to do
> better linting it's causing issues for that too.

The whole joy I get from PKGBUILDs is they are bash.  You just wrap
everything you have down to build your package in a bash function and
add metadata.  Now because the need for accurate print of source info
(i.e. nothing to do with actually building a package), we are being
restricted to a more limited subset of bash, and a subset that is not
well defined.

I was OK restricting variables to be arrays/strings to ensure we could
extract these from the PKGBUILD with --printsrcinfo, because that could
be attributed to "incorrect" bash usage.  But I am not happy about
putting restrictions on correct bash usage within a PKGBUILD in order
for a auxiliary (to package building) function to work.


After spending the last hour thinking about this, I think the pacman
codebase should:
1) documente that --printsrcinfo is fragile
2) remove this check
3) potentially add a much more limited version of the check that will
detect only genuine "depends=('foo>=')" examples.

Distros wanting more specific bash restrictions can add whatever they
want into libmakepkg.  That is the whole point of it being expandable.

Allan