[pacman-dev] [PATCH v2] libmakepkg: lint all arrays for empty values
Empty values break pacman's db format, as an empty line indicates an end
of section.
---
I've seen this out in the wild:
https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=vim-coc-highlight-git=3063e1a6d3e72a35528
diff --git a/scripts/libmakepkg/lint_pkgbuild/variable.sh.in
b/scripts/libmakepkg/lint_pkgbuild/variable.sh.in
index 1bc49722..22f5fbbb 100644
--- a/scripts/libmakepkg/lint_pkgbuild/variable.sh.in
+++ b/scripts/libmakepkg/lint_pkgbuild/variable.sh.in
@@ -31,7 +31,7 @@ lint_pkgbuild_functions+=('lint_variable')
lint_variable() {
- local i a pkg out bad ret=0
+ local i a pkg out bad array var ret=0
# global variables
for i in ${pkgbuild_schema_arrays[@]}; do
@@ -93,5 +93,62 @@ lint_variable() {
done
done
+ # ensure lists don't contain empty values
+ for i in ${pkgbuild_schema_arrays[@]}; do
+ if declare -p $i > /dev/null 2>&1; then
+ array_build "array" "$i"
+ for var in "${array[@]}"; do
+ if [[ -z "$var" ]]; then
+ error "$(gettext "%s is not allowed to
be empty")" "$i"
+ ret=1
+ fi
+ done
+ fi
+ done
+
+ for a in ${arch[@]}; do
+ [[ $a == "any" ]] && continue
+
+ for i in ${pkgbuild_schema_arch_arrays[@]}; do
+ if declare -p "${i}_${a}" > /dev/null 2>&1; then
+ array_build "array" "${i}_${a}"
+ for var in "${array[@]}"; do
+ if [[ -z "$var" ]]; then
+ error "$(gettext "%s is not
allowed to be empty")" "${i}_${a}"
+ ret=1
+ fi
+ done
+ fi
+ done
+ done
+
+ for pkg in ${pkgname[@]}; do
+ for i in ${pkgbuild_schema_arrays[@]}; do
+ if extract_function_variable "package_$pkg" $i 1 out;
then
+ for val in "${out[@]}" ;do
+ if [[ -z "$val" ]]; then
+ error "$(gettext "%s is not
allowed to be empty")" "$i"
+ ret=1
+ fi
+ done
+ fi
+ done
+
+ for a in ${arch[@]}; do
+ [[ $a == "any" ]] && continue
+
+ for i in ${pkgbuild_schema_arch_arrays[@]}; do
+ if extract_function_variable "package_$pkg"
"${i}_${a}" 1 out; then
+ for val in "${out[@]}" ;do
+ if [[ -z "$val" ]]; then
+ error "$(gettext "%s is
not allowed to be empty")" "${i}_${a}"
+ ret=1
+ fi
+ done
+ fi
+ done
+ done
+ done
+
return $ret
}
--
2.29.1
Re: [pacman-dev] [PATCH] libmakepkg: lint all arrays for empty values
On 10/28/20 at 01:17am, Morgan Adamiec wrote:
>
>
> On 28/10/2020 01:13, Eli Schwartz wrote:
> > On 10/27/20 8:59 PM, Morgan Adamiec wrote:
> >> On 28/10/2020 00:46, Eli Schwartz wrote:
> >>> It remains unclear to me, why the ambiguity matters.
> >>>
> >>
> >> I guess if you were to officially declare `Key =` means to set the key
> >> to none then that would solve the issue of it being ambiguous
> >>
> >> Then again that would make `license=('a' '' 'b')` parse as only as
> >> licence=(b) seems as it was cleared before.
> >>
> >> But funnily enough, if you build the package then pacman -Qi reports
> >> only license a. Fun!
> >
> > I just checked this, but -Qip reported all the licenses, both before and
> > after the '' license. Is this an observation about the current state of
> > pacman, or about what the proposed semantics would be?
> >
>
> So as it turns out -Qip shows the licenses. Installing the package and
> doing -Qi only shows a.
The joys of having two different package info file formats. Whatever
it does with .SRCINFO, an empty string is not a valid value in the db
package info file, so makepkg should not be writing .PKGINFO files
with them.
Re: [pacman-dev] [PATCH] libmakepkg: lint all arrays for empty values
On 10/27/20 9:17 PM, Morgan Adamiec wrote: > So as it turns out -Qip shows the licenses. Installing the package and > doing -Qi only shows a. Confirmed... In /var/lib/pacman/local/e-1-1/desc I do see: ``` %LICENSE% 1 all the extra licenses even in the middle ``` $ pacman -Qi e Licenses: 1 all the extra licenses This makes a definite case for linting this by accepting your patch. OTOH maybe we should fix this... this /whatever/. -- Eli Schwartz Bug Wrangler and Trusted User signature.asc Description: OpenPGP digital signature
Re: [pacman-dev] [PATCH] libmakepkg: lint all arrays for empty values
On 28/10/2020 01:13, Eli Schwartz wrote:
> On 10/27/20 8:59 PM, Morgan Adamiec wrote:
>> On 28/10/2020 00:46, Eli Schwartz wrote:
>>> It remains unclear to me, why the ambiguity matters.
>>>
>>
>> I guess if you were to officially declare `Key =` means to set the key
>> to none then that would solve the issue of it being ambiguous
>>
>> Then again that would make `license=('a' '' 'b')` parse as only as
>> licence=(b) seems as it was cleared before.
>>
>> But funnily enough, if you build the package then pacman -Qi reports
>> only license a. Fun!
>
> I just checked this, but -Qip reported all the licenses, both before and
> after the '' license. Is this an observation about the current state of
> pacman, or about what the proposed semantics would be?
>
So as it turns out -Qip shows the licenses. Installing the package and
doing -Qi only shows a.
signature.asc
Description: OpenPGP digital signature
Re: [pacman-dev] [PATCH] libmakepkg: lint all arrays for empty values
On 10/27/20 8:59 PM, Morgan Adamiec wrote:
> On 28/10/2020 00:46, Eli Schwartz wrote:
>> It remains unclear to me, why the ambiguity matters.
>>
>
> I guess if you were to officially declare `Key =` means to set the key
> to none then that would solve the issue of it being ambiguous
>
> Then again that would make `license=('a' '' 'b')` parse as only as
> licence=(b) seems as it was cleared before.
>
> But funnily enough, if you build the package then pacman -Qi reports
> only license a. Fun!
I just checked this, but -Qip reported all the licenses, both before and
after the '' license. Is this an observation about the current state of
pacman, or about what the proposed semantics would be?
--
Eli Schwartz
Bug Wrangler and Trusted User
signature.asc
Description: OpenPGP digital signature
Re: [pacman-dev] [PATCH] libmakepkg: lint all arrays for empty values
On 28/10/2020 00:46, Eli Schwartz wrote:
> On 10/27/20 8:25 PM, Morgan Adamiec wrote:
>> On 28/10/2020 00:04, Allan McRae wrote:
>>
>>> pkgnames/depends/etc where it may be an issue. So I'm not sure this
>>> check finds anything in the "break makepkg/pacman" category.
>>
>> I disagree, it actually does break something, the srcinfio file
>>
>> Consider the following pkgbuild:
>>
>> pkgbase=foo
>> pkgname=(a b)
>> pkgver=1
>> pkgrel=1
>> arch=(any)
>>
>> license=(1)
>>
>> package_a() {
>> license=()
>>
>> }
>>
>> package_b() {
>> license=('')
>> }
>>
>> And the srcinfo file:
>>
>> pkgbase = foo
>> pkgver = 1
>> pkgrel = 1
>> arch = any
>> license = 1
>>
>> pkgname = a
>> license =
>>
>> pkgname = b
>> license =
>>
>>
>> Now package `a` overrides license to an empty array. The srcinfo
>> expresses this by putting an empty license entry.
>>
>> Now package b defines a license of `empty string`, yet it generates the
>> same output. So it's impossible to tell what the original pkgbuild
>> actually meant.
>
> In both cases, it overrides the license=('1') into non-existence.
>
> The question is if "There is no license" or "the license is a
> zero-length string of emptiness" is semantically meaningful. Especially
> given the author of the PKGBUILD probably intended to put the former.
>
> Given it's a purely display-oriented field, you can absolutely stuff it
> with whatever you want, and "a zero-length string of emptiness" is not
> technically invalid even if it is stupid, whereas it's useful to catch
> makedepends=('') before makepkg -s fails to find a satisfier, root and all.
>
>> This example may seen a little contrived, but i assure you it's not.
>> Because stuff like this is done in the wild [1] and as a maintainer of a
>> srcinfo parser it's annoying that it creates this ambiguity.
>
> It remains unclear to me, why the ambiguity matters.
>
I guess if you were to officially declare `Key =` means to set the key
to none then that would solve the issue of it being ambiguous
Then again that would make `license=('a' '' 'b')` parse as only as
licence=(b) seems as it was cleared before.
But funnily enough, if you build the package then pacman -Qi reports
only license a. Fun!
signature.asc
Description: OpenPGP digital signature
Re: [pacman-dev] [PATCH] libmakepkg: lint all arrays for empty values
On 10/27/20 8:25 PM, Morgan Adamiec wrote:
> On 28/10/2020 00:04, Allan McRae wrote:
>
>> pkgnames/depends/etc where it may be an issue. So I'm not sure this
>> check finds anything in the "break makepkg/pacman" category.
>
> I disagree, it actually does break something, the srcinfio file
>
> Consider the following pkgbuild:
>
> pkgbase=foo
> pkgname=(a b)
> pkgver=1
> pkgrel=1
> arch=(any)
>
> license=(1)
>
> package_a() {
> license=()
>
> }
>
> package_b() {
> license=('')
> }
>
> And the srcinfo file:
>
> pkgbase = foo
> pkgver = 1
> pkgrel = 1
> arch = any
> license = 1
>
> pkgname = a
> license =
>
> pkgname = b
> license =
>
>
> Now package `a` overrides license to an empty array. The srcinfo
> expresses this by putting an empty license entry.
>
> Now package b defines a license of `empty string`, yet it generates the
> same output. So it's impossible to tell what the original pkgbuild
> actually meant.
In both cases, it overrides the license=('1') into non-existence.
The question is if "There is no license" or "the license is a
zero-length string of emptiness" is semantically meaningful. Especially
given the author of the PKGBUILD probably intended to put the former.
Given it's a purely display-oriented field, you can absolutely stuff it
with whatever you want, and "a zero-length string of emptiness" is not
technically invalid even if it is stupid, whereas it's useful to catch
makedepends=('') before makepkg -s fails to find a satisfier, root and all.
> This example may seen a little contrived, but i assure you it's not.
> Because stuff like this is done in the wild [1] and as a maintainer of a
> srcinfo parser it's annoying that it creates this ambiguity.
It remains unclear to me, why the ambiguity matters.
--
Eli Schwartz
Bug Wrangler and Trusted User
signature.asc
Description: OpenPGP digital signature
Re: [pacman-dev] [PATCH] libmakepkg: lint all arrays for empty values
On 28/10/2020 00:04, Allan McRae wrote:
> pkgnames/depends/etc where it may be an issue. So I'm not sure this
> check finds anything in the "break makepkg/pacman" category.
I disagree, it actually does break something, the srcinfio file
Consider the following pkgbuild:
pkgbase=foo
pkgname=(a b)
pkgver=1
pkgrel=1
arch=(any)
license=(1)
package_a() {
license=()
}
package_b() {
license=('')
}
And the srcinfo file:
pkgbase = foo
pkgver = 1
pkgrel = 1
arch = any
license = 1
pkgname = a
license =
pkgname = b
license =
Now package `a` overrides license to an empty array. The srcinfo
expresses this by putting an empty license entry.
Now package b defines a license of `empty string`, yet it generates the
same output. So it's impossible to tell what the original pkgbuild
actually meant.
This example may seen a little contrived, but i assure you it's not.
Because stuff like this is done in the wild [1] and as a maintainer of a
srcinfo parser it's annoying that it creates this ambiguity.
signature.asc
Description: OpenPGP digital signature
Re: [pacman-dev] [PATCH] libmakepkg: lint all arrays for empty values
On 28/10/20 9:39 am, Morgan Adamiec wrote: > Accidentally sent off list. resending: > > Because there's no reason for a pkgbuild to ever do this so may as well > make it a hard error. AUR packages can't be trusted to get it right. > > We already make an effort to link pkgbuilds to make sure they're right. > I don't how this is different to the other lints. We lint PKGBUILDs to make sure they do not break assumptions needed by makepkg or pacman. I don't think this check falls in that category - without going through the code, I beleive we do check this for pkgnames/depends/etc where it may be an issue. So I'm not sure this check finds anything in the "break makepkg/pacman" category. My opinion is this is something for namcap or the like. e.g. https://github.com/allanmcrae/pkglint Allan
Re: [pacman-dev] [PATCH] libmakepkg: lint all arrays for empty values
Accidentally sent off list. resending: Because there's no reason for a pkgbuild to ever do this so may as well make it a hard error. AUR packages can't be trusted to get it right. We already make an effort to link pkgbuilds to make sure they're right. I don't how this is different to the other lints. On 27/10/2020 22:48, Allan McRae wrote: > On 28/10/20 7:44 am, morganamilo wrote: >> --- >> >> I've seen this out in the wild: >> https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=vim-coc-highlight-git=3063e1a6d3e72a35528 >> > > Why would this be an error?I think it would be a warning at best. > > Also, I think PKGBUILD linting done by makepkg (as in the lints > distributed with the pacman source) should be limited to things that > cause issues while packaging. An empty value for the license is "fine" > as far as makepkg is concerned. > > Allan > signature.asc Description: OpenPGP digital signature
Re: [pacman-dev] [PATCH] libmakepkg: lint all arrays for empty values
On 28/10/20 7:44 am, morganamilo wrote: > --- > > I've seen this out in the wild: > https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=vim-coc-highlight-git=3063e1a6d3e72a35528 > Why would this be an error?I think it would be a warning at best. Also, I think PKGBUILD linting done by makepkg (as in the lints distributed with the pacman source) should be limited to things that cause issues while packaging. An empty value for the license is "fine" as far as makepkg is concerned. Allan
[pacman-dev] [PATCH] libmakepkg: lint all arrays for empty values
---
I've seen this out in the wild:
https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=vim-coc-highlight-git=3063e1a6d3e72a35528
diff --git a/scripts/libmakepkg/lint_pkgbuild/variable.sh.in
b/scripts/libmakepkg/lint_pkgbuild/variable.sh.in
index 1bc49722..22f5fbbb 100644
--- a/scripts/libmakepkg/lint_pkgbuild/variable.sh.in
+++ b/scripts/libmakepkg/lint_pkgbuild/variable.sh.in
@@ -31,7 +31,7 @@ lint_pkgbuild_functions+=('lint_variable')
lint_variable() {
- local i a pkg out bad ret=0
+ local i a pkg out bad array var ret=0
# global variables
for i in ${pkgbuild_schema_arrays[@]}; do
@@ -93,5 +93,62 @@ lint_variable() {
done
done
+ # ensure lists don't contain empty values
+ for i in ${pkgbuild_schema_arrays[@]}; do
+ if declare -p $i > /dev/null 2>&1; then
+ array_build "array" "$i"
+ for var in "${array[@]}"; do
+ if [[ -z "$var" ]]; then
+ error "$(gettext "%s is not allowed to
be empty")" "$i"
+ ret=1
+ fi
+ done
+ fi
+ done
+
+ for a in ${arch[@]}; do
+ [[ $a == "any" ]] && continue
+
+ for i in ${pkgbuild_schema_arch_arrays[@]}; do
+ if declare -p "${i}_${a}" > /dev/null 2>&1; then
+ array_build "array" "${i}_${a}"
+ for var in "${array[@]}"; do
+ if [[ -z "$var" ]]; then
+ error "$(gettext "%s is not
allowed to be empty")" "${i}_${a}"
+ ret=1
+ fi
+ done
+ fi
+ done
+ done
+
+ for pkg in ${pkgname[@]}; do
+ for i in ${pkgbuild_schema_arrays[@]}; do
+ if extract_function_variable "package_$pkg" $i 1 out;
then
+ for val in "${out[@]}" ;do
+ if [[ -z "$val" ]]; then
+ error "$(gettext "%s is not
allowed to be empty")" "$i"
+ ret=1
+ fi
+ done
+ fi
+ done
+
+ for a in ${arch[@]}; do
+ [[ $a == "any" ]] && continue
+
+ for i in ${pkgbuild_schema_arch_arrays[@]}; do
+ if extract_function_variable "package_$pkg"
"${i}_${a}" 1 out; then
+ for val in "${out[@]}" ;do
+ if [[ -z "$val" ]]; then
+ error "$(gettext "%s is
not allowed to be empty")" "${i}_${a}"
+ ret=1
+ fi
+ done
+ fi
+ done
+ done
+ done
+
return $ret
}
--
2.29.1
