[pacman-dev] pacman 5.1.1 and the "final" makepkg bug
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) Allan
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. 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. 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. -- Eli Schwartz Bug Wrangler and Trusted User
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
participants (2)
-
Allan McRae
-
Eli Schwartz