We already ensure arch is an array but if arch is never defined this error never triggers. Add an explicit check for a missing arch. Signed-off-by: morganamilo --- scripts/libmakepkg/lint_pkgbuild/arch.sh.in | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/scripts/libmakepkg/lint_pkgbuild/arch.sh.in b/scripts/libmakepkg/lint_pkgbuild/arch.sh.in index ef1aac46..f2c80c73 100644 --- a/scripts/libmakepkg/lint_pkgbuild/arch.sh.in +++ b/scripts/libmakepkg/lint_pkgbuild/arch.sh.in @@ -33,6 +33,11 @@ lint_pkgbuild_functions+=('lint_arch') lint_arch() { local a name list ret=0 + if [[ -z $arch ]]; then + error "$(gettext "%s is not allowed to be empty.")" "arch" + return 1 + fi + if [[ $arch == 'any' ]]; then return 0 fi -- 2.17.1

makepkpg will now error if disallowed variables are overridden inside of the package function. Signed-off-by: morganamilo --- .../libmakepkg/lint_pkgbuild/variable.sh.in | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/scripts/libmakepkg/lint_pkgbuild/variable.sh.in b/scripts/libmakepkg/lint_pkgbuild/variable.sh.in index 3266cb5e..68512a62 100644 --- a/scripts/libmakepkg/lint_pkgbuild/variable.sh.in +++ b/scripts/libmakepkg/lint_pkgbuild/variable.sh.in @@ -37,6 +37,12 @@ lint_variable() { replaces sha1sums sha256sums sha384sums sha512sums source) local string=(changelog epoch install pkgdesc pkgrel pkgver url) + local no_overide_string=(pkgbase pkgver pkgrel epoch) + + local no_overide_array=(checkdepends makedepends + noextract source validpgpkeys md5suns sha1sums + sha224sums sha256sums sha384sums sha512sums) + local i a v pkg keys out bad ret=0 # global variables @@ -87,6 +93,22 @@ lint_variable() { for a in ${arch[@]}; do [[ $a == "any" ]] && continue + for i in ${no_overide_string[@]}; do + if extract_function_variable "package_$pkg" "${i}_${a}" 0 out; then + error "$(gettext "%s_%s can not be overridden in package function")" "$i" "$a" + ret=1 + fi + done + + for i in ${no_overide_array[@]}; do + if extract_function_variable "package_$pkg" "${i}_${a}" 1 out; then + error "$(gettext "%s_%s can not be overridden in package function")" "$i" "$a" + ret=1 + fi + + done + + for i in ${arch_array[@]}; do if extract_function_variable "package_$pkg" "${i}_${a}" 0 out; then error "$(gettext "%s_%s should be an array")" "$i" "$a" @@ -95,6 +117,20 @@ lint_variable() { done done + for i in ${no_overide_string[@]}; do + if extract_function_variable "package_$pkg" "$i" 0 out; then + error "$(gettext "%s can not be overridden in package function")" "$i" + ret=1 + fi + done + + for i in ${no_overide_string[@]}; do + if extract_function_variable "package_$pkg" "$i" 1 out; then + error "$(gettext "%s can not be overridden in package function")" "$i" + ret=1 + fi + done + for i in ${string[@]}; do if extract_function_variable "package_$pkg" $i 1 out; then error "$(gettext "%s should not be an array")" "$i" -- 2.17.1

Signed-off-by: morganamilo --- scripts/libmakepkg/lint_pkgbuild/variable.sh.in | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/scripts/libmakepkg/lint_pkgbuild/variable.sh.in b/scripts/libmakepkg/lint_pkgbuild/variable.sh.in index b9264768..6b7da4a2 100644 --- a/scripts/libmakepkg/lint_pkgbuild/variable.sh.in +++ b/scripts/libmakepkg/lint_pkgbuild/variable.sh.in @@ -57,12 +57,15 @@ lint_variable() { done for a in ${arch[@]}; do - [[ $a == "any" ]] && continue - for i in ${arch_array[@]}; do v="${i}_${a}" eval "keys=(\"\${!${v}[@]}\")" if (( ${#keys[*]} > 0 )); then + if [[ $a == "any" ]]; then + error "$(gettext "%s_%s any can not be used for an architecture specific variable")" "$i" "$a" + ret=1 + fi + if ! is_array $v; then error "$(gettext "%s_%s should be an array")" "$i" "$a" ret=1 @@ -111,6 +114,11 @@ lint_variable() { for i in ${array[@]}; do if extract_function_variable "package_$pkg" "${i}_${a}" 1 out; then + if [[ $a == "any" ]]; then + error "$(gettext "%s_%s any can not be used for an architecture specific variable")" "$i" "$a" + ret=1 + fi + error "$(gettext "%s_%s can not be architecture specific")" "$i" "$a" ret=1 fi -- 2.17.1

Error if the arch array contains any and any other values. This also fixes a bug where the check for `$arch == 'any'` which only evaluated the first value in the array, meaning the rest of the values would not be linted. Signed-off-by: morganamilo --- scripts/libmakepkg/lint_pkgbuild/arch.sh.in | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/scripts/libmakepkg/lint_pkgbuild/arch.sh.in b/scripts/libmakepkg/lint_pkgbuild/arch.sh.in index f2c80c73..98ae70af 100644 --- a/scripts/libmakepkg/lint_pkgbuild/arch.sh.in +++ b/scripts/libmakepkg/lint_pkgbuild/arch.sh.in @@ -38,11 +38,12 @@ lint_arch() { return 1 fi - if [[ $arch == 'any' ]]; then - return 0 - fi - for a in "${arch[@]}"; do + if [[ $a == 'any' ]]; then + error "$(gettext "any can not be used with other architectures")" + ret=1 + fi + if [[ $a = *[![:alnum:]_]* ]]; then error "$(gettext "%s contains invalid characters: '%s'")" \ 'arch' "${a//[[:alnum:]_]}" @@ -50,6 +51,10 @@ lint_arch() { fi done + if [[ $arch == 'any' ]]; then + return $ret + fi + if (( ! IGNOREARCH )) && ! in_array "$CARCH" "${arch[@]}"; then error "$(gettext "%s is not available for the '%s' architecture.")" "$pkgbase" "$CARCH" return 1 -- 2.17.1

On 06/08/2018 02:18 PM, morganamilo wrote:

When a split package overriddes an array using += and the array does not exist globally, makepkg --printsrcinfo will print the field with an empty vlaue before printing the acual values.

For exampple: having `depends+=(foo bar)` will generate: depends = depends = foo depends = bar

Explicity check for empty array values and only print the values that are not empty.

Signed-off-by: morganamilo --- scripts/libmakepkg/srcinfo.sh.in | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/scripts/libmakepkg/srcinfo.sh.in b/scripts/libmakepkg/srcinfo.sh.in index 509c4860..6a49be37 100644 --- a/scripts/libmakepkg/srcinfo.sh.in +++ b/scripts/libmakepkg/srcinfo.sh.in @@ -44,7 +44,11 @@ srcinfo_write_attr() { attrvalues=("${attrvalues[@]#[[:space:]]}") attrvalues=("${attrvalues[@]%[[:space:]]}")

- printf "\t$attrname = %s\n" "${attrvalues[@]}" + for val in "${attrvalues[@]}"; do + if [[ ! -z ${val// /} ]]; then + printf "\t$attrname = %s\n" "$val" + fi + done

This is odd, I wonder why get_pkgbuild_attribute is returning an array=('' foo bar) in this case? We should probably fix it more directly. -- Eli Schwartz Bug Wrangler and Trusted User

On Fri, 8 Jun 2018 at 20:33, Eli Schwartz wrote:

On 06/08/2018 02:18 PM, morganamilo wrote:

...

When a split package overriddes an array using += and the array does not exist globally, makepkg --printsrcinfo will print the field with an empty vlaue before printing the acual values.

For exampple: having `depends+=(foo bar)` will generate: depends = depends = foo depends = bar

Explicity check for empty array values and only print the values that are not empty.

Signed-off-by: morganamilo --- scripts/libmakepkg/srcinfo.sh.in | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/scripts/libmakepkg/srcinfo.sh.in b/scripts/libmakepkg/srcinfo.sh.in index 509c4860..6a49be37 100644 --- a/scripts/libmakepkg/srcinfo.sh.in +++ b/scripts/libmakepkg/srcinfo.sh.in @@ -44,7 +44,11 @@ srcinfo_write_attr() { attrvalues=("${attrvalues[@]#[[:space:]]}") attrvalues=("${attrvalues[@]%[[:space:]]}")

- printf "\t$attrname = %s\n" "${attrvalues[@]}" + for val in "${attrvalues[@]}"; do + if [[ ! -z ${val// /} ]]; then + printf "\t$attrname = %s\n" "$val" + fi + done

This is odd, I wonder why get_pkgbuild_attribute is returning an array=('' foo bar) in this case? We should probably fix it more directly.

-- Eli Schwartz Bug Wrangler and Trusted User

In my investigation it came down to lines like this https://git.archlinux.org/pacman.git/tree/scripts/libmakepkg/srcinfo.sh.in#n... It seems appending an array to a non array value generates an array with the first value being an empty string then the values appended. You can reproduce it with: foo= foo+=(bar) echo "${#foo[@]}" I did think about fixing it there, but that function is used all over and I didn't want to break something.

On Fri, Jun 08, 2018 at 08:42:11PM +0100, Morgan Adamiec wrote:

On Fri, 8 Jun 2018 at 20:33, Eli Schwartz wrote:

...

On 06/08/2018 02:18 PM, morganamilo wrote:

...

When a split package overriddes an array using += and the array does not exist globally, makepkg --printsrcinfo will print the field with an empty vlaue before printing the acual values.

For exampple: having `depends+=(foo bar)` will generate: depends = depends = foo depends = bar

Explicity check for empty array values and only print the values that are not empty.

Signed-off-by: morganamilo --- scripts/libmakepkg/srcinfo.sh.in | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/scripts/libmakepkg/srcinfo.sh.in b/scripts/libmakepkg/srcinfo.sh.in index 509c4860..6a49be37 100644 --- a/scripts/libmakepkg/srcinfo.sh.in +++ b/scripts/libmakepkg/srcinfo.sh.in @@ -44,7 +44,11 @@ srcinfo_write_attr() { attrvalues=("${attrvalues[@]#[[:space:]]}") attrvalues=("${attrvalues[@]%[[:space:]]}")

- printf "\t$attrname = %s\n" "${attrvalues[@]}" + for val in "${attrvalues[@]}"; do + if [[ ! -z ${val// /} ]]; then + printf "\t$attrname = %s\n" "$val" + fi + done

This is odd, I wonder why get_pkgbuild_attribute is returning an array=('' foo bar) in this case? We should probably fix it more directly.

-- Eli Schwartz Bug Wrangler and Trusted User

In my investigation it came down to lines like this https://git.archlinux.org/pacman.git/tree/scripts/libmakepkg/srcinfo.sh.in#n...

It seems appending an array to a non array value generates an array with the first value being an empty string then the values appended.

You can reproduce it with: foo= foo+=(bar) echo "${#foo[@]}"

I did think about fixing it there, but that function is used all over and I didn't want to break something.

That's just bad shell code. Using foo= to declare an array is the same as writing foo=(''). Please don't change this to paper over bad PKGBUILDs. If anything, the fix here is to leverage bash 4.4 in our lint rules when it's available and use @a expansion to detect if something is an array or a string, e.g. $ licenses='MIT' $ [[ ${licenses@a} = *a* ]] || echo "licenses must be an array"

On 06/11/2018 04:53 PM, morganamilo wrote:

Error if the arch array contains any and any other values. This also fixes a bug where the check for `$arch == 'any'` which only evaluated the first value in the array, meaning the rest of the values would not be linted.

Signed-off-by: morganamilo --- scripts/libmakepkg/lint_pkgbuild/arch.sh.in | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/scripts/libmakepkg/lint_pkgbuild/arch.sh.in b/scripts/libmakepkg/lint_pkgbuild/arch.sh.in index f2c80c73..8a1d2c11 100644 --- a/scripts/libmakepkg/lint_pkgbuild/arch.sh.in +++ b/scripts/libmakepkg/lint_pkgbuild/arch.sh.in @@ -38,11 +38,16 @@ lint_arch() { return 1 fi

- if [[ $arch == 'any' ]]; then + if [[ $arch == 'any' && ${#arch[@]} == 1 ]]; then

[[ ${#arch[@]} = 1 ]] is a string comparison (the test keyword or builtin uses -eq to handle numeric values). (( ${#arch[@]} == 1 )) is an integer comparison (shell arithmetic is generally superior when available). I specifically mentioned the latter in my previous email. -- Eli Schwartz Bug Wrangler and Trusted User

On 06/11/2018 06:57 PM, Morgan Adamiec wrote:

On Mon, 11 Jun 2018 at 22:27, Eli Schwartz wrote:

...

On 06/11/2018 04:53 PM, morganamilo wrote:

...

Error if the arch array contains any and any other values. This also fixes a bug where the check for `$arch == 'any'` which only evaluated the first value in the array, meaning the rest of the values would not be linted.

Signed-off-by: morganamilo --- scripts/libmakepkg/lint_pkgbuild/arch.sh.in | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/scripts/libmakepkg/lint_pkgbuild/arch.sh.in b/scripts/libmakepkg/lint_pkgbuild/arch.sh.in index f2c80c73..8a1d2c11 100644 --- a/scripts/libmakepkg/lint_pkgbuild/arch.sh.in +++ b/scripts/libmakepkg/lint_pkgbuild/arch.sh.in @@ -38,11 +38,16 @@ lint_arch() { return 1 fi

- if [[ $arch == 'any' ]]; then + if [[ $arch == 'any' && ${#arch[@]} == 1 ]]; then

[[ ${#arch[@]} = 1 ]] is a string comparison (the test keyword or builtin uses -eq to handle numeric values).

(( ${#arch[@]} == 1 )) is an integer comparison (shell arithmetic is generally superior when available).

I specifically mentioned the latter in my previous email.

-- Eli Schwartz Bug Wrangler and Trusted User

Yeah I see that now. Bash Isn't really my thing so I didn't really take note of the (( ))

Just to be sure before I send another patch it would be if [[ $arch == 'any' && (( ${#arch[@]} == 1 )) ]]; right? With the (( )) nested in the [[ ]].

Thanks for the feedback by the way, It helps a lot.

$ set -x $ [[ $arch = 'any' && (( ${#arch[@]} == 1 )) ]] + [[ any = \a\n\y ]] + [[ 1 == 1 ]] The () inside an [[ ]] results in logical grouping (twice), but it's still the test operator, not shell arithmetic. So just use: [[ $arch = any ]] && (( ${#arch[@]} == 1 )) -- Eli Schwartz Bug Wrangler and Trusted User

On Wed, 13 Jun 2018 at 02:31, Eli Schwartz wrote:

On 06/11/2018 06:57 PM, Morgan Adamiec wrote:

...

On Mon, 11 Jun 2018 at 22:27, Eli Schwartz wrote:

...

On 06/11/2018 04:53 PM, morganamilo wrote:

...

Error if the arch array contains any and any other values. This also fixes a bug where the check for `$arch == 'any'` which only evaluated the first value in the array, meaning the rest of the values would not be linted.

Signed-off-by: morganamilo --- scripts/libmakepkg/lint_pkgbuild/arch.sh.in | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/scripts/libmakepkg/lint_pkgbuild/arch.sh.in b/scripts/libmakepkg/lint_pkgbuild/arch.sh.in index f2c80c73..8a1d2c11 100644 --- a/scripts/libmakepkg/lint_pkgbuild/arch.sh.in +++ b/scripts/libmakepkg/lint_pkgbuild/arch.sh.in @@ -38,11 +38,16 @@ lint_arch() { return 1 fi

- if [[ $arch == 'any' ]]; then + if [[ $arch == 'any' && ${#arch[@]} == 1 ]]; then

[[ ${#arch[@]} = 1 ]] is a string comparison (the test keyword or builtin uses -eq to handle numeric values).

(( ${#arch[@]} == 1 )) is an integer comparison (shell arithmetic is generally superior when available).

I specifically mentioned the latter in my previous email.

-- Eli Schwartz Bug Wrangler and Trusted User

Yeah I see that now. Bash Isn't really my thing so I didn't really take note of the (( ))

Just to be sure before I send another patch it would be if [[ $arch == 'any' && (( ${#arch[@]} == 1 )) ]]; right? With the (( )) nested in the [[ ]].

Thanks for the feedback by the way, It helps a lot.

$ set -x $ [[ $arch = 'any' && (( ${#arch[@]} == 1 )) ]] + [[ any = \a\n\y ]] + [[ 1 == 1 ]]

The () inside an [[ ]] results in logical grouping (twice), but it's still the test operator, not shell arithmetic.

So just use:

[[ $arch = any ]] && (( ${#arch[@]} == 1 ))

-- Eli Schwartz Bug Wrangler and Trusted User

I tried that the first time, then got a syntax error. Probably just a dumb typo on my part. Anyway thanks for the help.

On 09/06/18 04:18, morganamilo wrote:

We already ensure arch is an array but if arch is never defined this error never triggers. Add an explicit check for a missing arch.

Signed-off-by: morganamilo ---

OK.

scripts/libmakepkg/lint_pkgbuild/arch.sh.in | 5 +++++ 1 file changed, 5 insertions(+)

diff --git a/scripts/libmakepkg/lint_pkgbuild/arch.sh.in b/scripts/libmakepkg/lint_pkgbuild/arch.sh.in index ef1aac46..f2c80c73 100644 --- a/scripts/libmakepkg/lint_pkgbuild/arch.sh.in +++ b/scripts/libmakepkg/lint_pkgbuild/arch.sh.in @@ -33,6 +33,11 @@ lint_pkgbuild_functions+=('lint_arch') lint_arch() { local a name list ret=0

+ if [[ -z $arch ]]; then + error "$(gettext "%s is not allowed to be empty.")" "arch" + return 1 + fi + if [[ $arch == 'any' ]]; then return 0 fi

On 13/06/18 11:40, morganamilo wrote:

Error if the arch array contains any and any other values. This also fixes a bug where the check for `$arch == 'any'` which only evaluated the first value in the array, meaning the rest of the values would not be linted.

Signed-off-by: morganamilo ---

Please append a version of you patch in the subject line. e.g. [PATCH 7/7 v3]

scripts/libmakepkg/lint_pkgbuild/arch.sh.in | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/scripts/libmakepkg/lint_pkgbuild/arch.sh.in b/scripts/libmakepkg/lint_pkgbuild/arch.sh.in index f2c80c73..f3dd8ee6 100644 --- a/scripts/libmakepkg/lint_pkgbuild/arch.sh.in +++ b/scripts/libmakepkg/lint_pkgbuild/arch.sh.in @@ -38,11 +38,16 @@ lint_arch() { return 1 fi

- if [[ $arch == 'any' ]]; then + if [[ $arch == 'any' ]] && (( ${#arch[@]} == 1 )); then return 0 fi

I'm still not happy with this patch. Why not just explicitly check that "any" is specified on its own, rather than a check here and one in the following loop. if in_array "any" "${arch[@]}"; then if (( ${#arch[@]} == 1 )); then return 0; else error "$(gettext "...")" return 1; fi fi Also, "any" should be %s in the error string as so it is not translated. A better error message is: $(gettext "Can not use '%s' architecture with other architectures")" "any"

for a in "${arch[@]}"; do + if [[ $a == 'any' ]]; then + error "$(gettext "any can not be used with other architectures")" + ret=1 + fi + if [[ $a = *[![:alnum:]_]* ]]; then error "$(gettext "%s contains invalid characters: '%s'")" \ 'arch' "${a//[[:alnum:]_]}"

makepkg will now error if disallowed variables are set inside of the package function. Disallowed variables are variables that do exist, like 'makedepends' and 'pkgver' but can not be set inside of a package function. Signed-off-by: morganamilo --- .../libmakepkg/lint_pkgbuild/variable.sh.in | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/scripts/libmakepkg/lint_pkgbuild/variable.sh.in b/scripts/libmakepkg/lint_pkgbuild/variable.sh.in index 1ee3c834..ad3ffd8e 100644 --- a/scripts/libmakepkg/lint_pkgbuild/variable.sh.in +++ b/scripts/libmakepkg/lint_pkgbuild/variable.sh.in @@ -38,6 +38,11 @@ lint_variable() { source) local string=(changelog epoch install pkgbase pkgdesc pkgrel pkgver url) + local no_package_string=(epoch pkgbase pkgname pkgrel pkgver) + + local no_package_array=(checkdepends makedepends md5sums noextract + sha1sums sha224sums sha256sums sha384sums sha512sums + source validpgpkeys) local i a pkg out bad ret=0 # global variables @@ -84,6 +89,21 @@ lint_variable() { for a in ${arch[@]}; do [[ $a == "any" ]] && continue + for i in ${no_package_string[@]}; do + if extract_function_variable "package_$pkg" "${i}_${a}" 0 out; then + error "$(gettext "%s_%s can not be set inside a package function")" "$i" "$a" + ret=1 + fi + done + + for i in ${no_package_array[@]}; do + if extract_function_variable "package_$pkg" "${i}_${a}" 1 out; then + error "$(gettext "%s_%s can not be set inside a package function")" "$i" "$a" + ret=1 + fi + + done + for i in ${arch_array[@]}; do if extract_function_variable "package_$pkg" "${i}_${a}" 0 out; then error "$(gettext "%s_%s should be an array")" "$i" "$a" @@ -92,6 +112,20 @@ lint_variable() { done done + for i in ${no_package_string[@]}; do + if extract_function_variable "package_$pkg" "$i" 0 out; then + error "$(gettext "%s can not be set inside a package function")" "$i" + ret=1 + fi + done + + for i in ${no_package_array[@]}; do + if extract_function_variable "package_$pkg" "$i" 1 out; then + error "$(gettext "%s can not be set inside a package function")" "$i" + ret=1 + fi + done + for i in ${string[@]}; do if extract_function_variable "package_$pkg" $i 1 out; then error "$(gettext "%s should not be an array")" "$i" -- 2.20.1

Signed-off-by: morganamilo --- scripts/libmakepkg/lint_pkgbuild/variable.sh.in | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/scripts/libmakepkg/lint_pkgbuild/variable.sh.in b/scripts/libmakepkg/lint_pkgbuild/variable.sh.in index b65c01f2..7420cdbc 100644 --- a/scripts/libmakepkg/lint_pkgbuild/variable.sh.in +++ b/scripts/libmakepkg/lint_pkgbuild/variable.sh.in @@ -56,7 +56,10 @@ lint_variable() { done for a in ${arch[@]}; do - [[ $a == "any" ]] && continue + if [[ $a == "any" ]]; then + error "$(gettext "Can not provide architecture specific variables for the '%s' architecture: %s_%s")" "any" "$i" "$a" + ret=1 + fi for i in ${arch_array[@]}; do if declare -p "${i}_${a}" > /dev/null 2>&1; then @@ -106,6 +109,11 @@ lint_variable() { for i in ${array[@]}; do if extract_function_variable "package_$pkg" "${i}_${a}" 1 out; then + if [[ $a == "any" ]]; then + error "$(gettext "Can not provide architecture specific variables for the '%s' architecture: %s_%s")" "any" "$i" "$a" + ret=1 + fi + error "$(gettext "%s can not be architecture specific: $s_$s")" "$i" ret=1 fi -- 2.20.1

On 16/1/19 8:34 am, morganamilo wrote:

Variables such as 'pkgdesc_x86_64' are invalid, instead of ignoring them raise an error.

Signed-off-by: morganamilo --- .../libmakepkg/lint_pkgbuild/variable.sh.in | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+)

diff --git a/scripts/libmakepkg/lint_pkgbuild/variable.sh.in b/scripts/libmakepkg/lint_pkgbuild/variable.sh.in index ad3ffd8e..b65c01f2 100644 --- a/scripts/libmakepkg/lint_pkgbuild/variable.sh.in +++ b/scripts/libmakepkg/lint_pkgbuild/variable.sh.in @@ -66,6 +66,14 @@ lint_variable() { fi fi done + + for i in ${array[@]} ${string[@]}; do + v="${i}_${a}" + if declare -p "$v" > /dev/null 2>&1; then + error "$(gettext "%s can not be architecture specific: $s_$s")" "$i" + ret=1 + fi + done done

for i in ${string[@]}; do @@ -88,6 +96,20 @@ lint_variable() {

for a in ${arch[@]}; do [[ $a == "any" ]] && continue + + for i in ${string[@]}; do + if extract_function_variable "package_$pkg" "${i}_${a}" 0 out; then + error "$(gettext "%s can not be architecture specific: $s_$s")" "$i"

Discussed on IRC, but for the record the $s_$s thing here needs fixed. A

Variables such as 'pkgdesc_x86_64' are invalid, instead of ignoring them raise an error. This also disallows using 'any' as an architecture specific variable Signed-off-by: morganamilo --- v5: "libmakepkg: disallow using any as an architecture specific variable" was squashed into this commit. Move this lint to its own file. scripts/Makefile.am | 1 + .../lint_pkgbuild/arch_specific.sh.in | 85 +++++++++++++++++++ scripts/libmakepkg/lint_pkgbuild/meson.build | 1 + 3 files changed, 87 insertions(+) create mode 100644 scripts/libmakepkg/lint_pkgbuild/arch_specific.sh.in diff --git a/scripts/Makefile.am b/scripts/Makefile.am index 6f9ada17..d230d54e 100644 --- a/scripts/Makefile.am +++ b/scripts/Makefile.am @@ -80,6 +80,7 @@ LIBMAKEPKG_IN = \ libmakepkg/lint_package/missing_backup.sh \ libmakepkg/lint_pkgbuild.sh \ libmakepkg/lint_pkgbuild/arch.sh \ + libmakepkg/lint_pkgbuild/arch_specific.sh \ libmakepkg/lint_pkgbuild/backup.sh \ libmakepkg/lint_pkgbuild/changelog.sh \ libmakepkg/lint_pkgbuild/checkdepends.sh \ diff --git a/scripts/libmakepkg/lint_pkgbuild/arch_specific.sh.in b/scripts/libmakepkg/lint_pkgbuild/arch_specific.sh.in new file mode 100644 index 00000000..7dba0e84 --- /dev/null +++ b/scripts/libmakepkg/lint_pkgbuild/arch_specific.sh.in @@ -0,0 +1,85 @@ +#!/bin/bash +# +# arch_specific.sh - Check that arch specific variables can be arch specific. +# +# Copyright (c) 2014-2018 Pacman Development Team +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see http://www.gnu.org/licenses/. +# + +[[ -n "$LIBMAKEPKG_LINT_PKGBUILD_ARCH_SPECIFIC_SH" ]] && return +LIBMAKEPKG_LINT_PKGBUILD_ARCH_SPECIFIC_SH=1 + +LIBRARY=${LIBRARY:-'@libmakepkgdir@'} + +source "$LIBRARY/util/message.sh" +source "$LIBRARY/util/pkgbuild.sh" + + +lint_pkgbuild_functions+=('lint_arch_specific') + + +lint_arch_specific() { + # TODO: refactor - similar arrays are used elsewhere + local array=(arch backup groups license noextract options validpgpkeys) + local arch_array=(checkdepends conflicts depends makedepends md5sums + optdepends provides replaces sha1sums sha224sums + sha256sums sha384sums sha512sums source) + local string=(changelog epoch install pkgbase pkgdesc pkgrel pkgver url) + + local i a pkg ret=0 + + # global variables + for a in ${arch[@]}; do + if [[ $a == "any" ]]; then + for i in ${arch_array[@]}; do + if declare -p "${i}_${a}" > /dev/null 2>&1; then + error "$(gettext "Can not provide architecture specific variables for the '%s' architecture: %s")" "any" "${i}_${a}" + ret=1 + fi + done + fi + + for i in ${array[@]} ${string[@]}; do + v="${i}_${a}" + if declare -p "$v" > /dev/null 2>&1; then + error "$(gettext "%s can not be architecture specific: %s")" "$i" "${i}_${a}" + ret=1 + fi + done + done + + # package function variables + for pkg in ${pkgname[@]}; do + for a in ${arch[@]}; do + if [[ $a == "any" ]]; then + for i in ${arch_array[@]}; do + if exists_function_variable "package_$pkg" "${i}_${a}"; then + error "$(gettext "Can not provide architecture specific variables for the '%s' architecture: %s")" "any" "${i}_${a}" + ret=1 + fi + done + fi + + for i in ${array[@]} ${string[@]}; do + if exists_function_variable "package_$pkg" "${i}_${a}"; then + error "$(gettext "%s can not be architecture specific: %s")" "$i" "${i}_${a}" + ret=1 + fi + done + done + done + + return $ret +} diff --git a/scripts/libmakepkg/lint_pkgbuild/meson.build b/scripts/libmakepkg/lint_pkgbuild/meson.build index f699a8e2..6050df2f 100644 --- a/scripts/libmakepkg/lint_pkgbuild/meson.build +++ b/scripts/libmakepkg/lint_pkgbuild/meson.build @@ -2,6 +2,7 @@ libmakepkg_module = 'lint_pkgbuild' sources = [ 'arch.sh.in', + 'arch_specific.sh.in', 'backup.sh.in', 'changelog.sh.in', 'checkdepends.sh.in', -- 2.20.1

makepkg will now error if disallowed variables are set inside of the package function. Disallowed variables are variables that do exist, like 'makedepends' and 'pkgver' but can not be set inside of a package function. Signed-off-by: morganamilo --- v5: Move this lint to its own file. v6: "libmakepkg: add exists_function_variable helper" was squashed into this commit. Fixups and rebase against master scripts/Makefile.am | 1 + scripts/libmakepkg/lint_pkgbuild/meson.build | 1 + .../package_function_variable.sh.in | 62 +++++++++++++++++++ scripts/libmakepkg/util/pkgbuild.sh.in | 9 +++ 4 files changed, 73 insertions(+) create mode 100644 scripts/libmakepkg/lint_pkgbuild/package_function_variable.sh.in diff --git a/scripts/Makefile.am b/scripts/Makefile.am index 0e5619bd..f84fda1d 100644 --- a/scripts/Makefile.am +++ b/scripts/Makefile.am @@ -92,6 +92,7 @@ LIBMAKEPKG_IN = \ libmakepkg/lint_pkgbuild/optdepends.sh \ libmakepkg/lint_pkgbuild/options.sh \ libmakepkg/lint_pkgbuild/package_function.sh \ + libmakepkg/lint_pkgbuild/package_function_variable.sh \ libmakepkg/lint_pkgbuild/pkgbase.sh \ libmakepkg/lint_pkgbuild/pkglist.sh \ libmakepkg/lint_pkgbuild/pkgname.sh \ diff --git a/scripts/libmakepkg/lint_pkgbuild/meson.build b/scripts/libmakepkg/lint_pkgbuild/meson.build index 4ca414e4..f699a8e2 100644 --- a/scripts/libmakepkg/lint_pkgbuild/meson.build +++ b/scripts/libmakepkg/lint_pkgbuild/meson.build @@ -14,6 +14,7 @@ sources = [ 'optdepends.sh.in', 'options.sh.in', 'package_function.sh.in', + 'package_function_variable.sh.in', 'pkgbase.sh.in', 'pkglist.sh.in', 'pkgname.sh.in', diff --git a/scripts/libmakepkg/lint_pkgbuild/package_function_variable.sh.in b/scripts/libmakepkg/lint_pkgbuild/package_function_variable.sh.in new file mode 100644 index 00000000..172e0a08 --- /dev/null +++ b/scripts/libmakepkg/lint_pkgbuild/package_function_variable.sh.in @@ -0,0 +1,62 @@ +#!/bin/bash +# +# package_function_variable.sh - Check variables inside the package function. +# +# Copyright (c) 2014-2018 Pacman Development Team +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see http://www.gnu.org/licenses/. +# + +[[ -n "$LIBMAKEPKG_LINT_PKGBUILD_PACKAGE_FUNCTION_VARIABLE_SH" ]] && return +LIBMAKEPKG_LINT_PKGBUILD_PACKAGE_FUNCTION_VARIABLE_SH=1 + +LIBRARY=${LIBRARY:-'@libmakepkgdir@'} + +source "$LIBRARY/util/message.sh" +source "$LIBRARY/util/pkgbuild.sh" +source "$LIBRARY/util/schema.sh" +source "$LIBRARY/util/util.sh" + + +lint_pkgbuild_functions+=('lint_package_function_variable') + + +lint_package_function_variable() { + local i a pkg ret=0 + + # package function variables + for pkg in ${pkgname[@]}; do + for a in ${arch[@]}; do + [[ $a == "any" ]] && continue + + for i in ${pkgbuild_schema_arrays[@]} ${pkgbuild_schema_strings[@]}; do + in_array "$i" ${pkgbuild_schema_package_overrides[@]} && continue + if exists_function_variable "package_$pkg" "${i}_${a}"; then + error "$(gettext "%s can not be set inside a package function")" "${i}_${a}" + ret=1 + fi + done + done + + for i in ${pkgbuild_schema_arrays[@]} ${pkgbuild_schema_strings[@]}; do + in_array "$i" ${pkgbuild_schema_package_overrides[@]} && continue + if exists_function_variable "package_$pkg" "$i"; then + error "$(gettext "%s can not be set inside a package function")" "$i" + ret=1 + fi + done + done + + return $ret +} diff --git a/scripts/libmakepkg/util/pkgbuild.sh.in b/scripts/libmakepkg/util/pkgbuild.sh.in index 0dc239d1..298275a8 100644 --- a/scripts/libmakepkg/util/pkgbuild.sh.in +++ b/scripts/libmakepkg/util/pkgbuild.sh.in @@ -100,6 +100,15 @@ extract_function_variable() { return $r } +exists_function_variable() { + # $1: function name + # $2: variable name + + local funcname=$1 attr=$2 out + extract_function_variable "$funcname" "$attr" 0 out || + extract_function_variable "$funcname" "$attr" 1 out +} + get_pkgbuild_attribute() { # $1: package name # $2: attribute name -- 2.20.1