Re: [pacman-dev] [PATCH 3/7] libmakepkg: lint disallowed variables in package()
On Sun, 24 Jun 2018 at 03:26, Eli Schwartz wrote: > But Allan's point seems to be that we shouldn't try to stop people from > using variables that makepkg does not utilize, just because they look > vaguely like something makepkg might have used. The thing is makepkg does use these variables to some extent. Sure pkgname_i686 is ignored. But things like makedepends_i686 do get picked up. So the section would stay, I would instead just loop over a different array of non overridable and non architecture specific fields. I'm all for stricter linting, It doesn't make sense to write pkgname_i686, banning these kind of things would help catch mistakes. > At a bare minimum, if we are going to ban them, the error message and > entire handling logic should be merged into your next patch. Sure I'll squash those two commits if you like. > This error message would have the benefit of being the actual, true > reason we're aborting. (You could check inside the package functions > too, and extend the error message to refer to where they come from.) Better error messages is something I like too. I'll leave it for either after v2 or a different patch set altogether though. I'd like to get the current issues reviewed and out the way first. On Sun, 24 Jun 2018 at 03:26, Eli Schwartz wrote: > > On 06/23/2018 10:01 PM, Morgan Adamiec wrote: > > Silly mistakes aside. > > > >> These are not variables being overridden... pkgname_i686 is just not a > >> thing as far as makepkg is concerned. > > > > The point of this is specifically disallow things like 'pkgname_i686' > > rather than just ignore them. > > But Allan's point seems to be that we shouldn't try to stop people from > using variables that makepkg does not utilize, just because they look > vaguely like something makepkg might have used. > > At a bare minimum, if we are going to ban them, the error message and > entire handling logic should be merged into your next patch. > > This error message would have the benefit of being the actual, true > reason we're aborting. (You could check inside the package functions > too, and extend the error message to refer to where they come from.) > > > -- > Eli Schwartz > Bug Wrangler and Trusted User >
Re: [pacman-dev] [PATCH 3/7] libmakepkg: lint disallowed variables in package()
On 06/23/2018 10:01 PM, Morgan Adamiec wrote: > Silly mistakes aside. > >> These are not variables being overridden... pkgname_i686 is just not a >> thing as far as makepkg is concerned. > > The point of this is specifically disallow things like 'pkgname_i686' > rather than just ignore them. But Allan's point seems to be that we shouldn't try to stop people from using variables that makepkg does not utilize, just because they look vaguely like something makepkg might have used. At a bare minimum, if we are going to ban them, the error message and entire handling logic should be merged into your next patch. This error message would have the benefit of being the actual, true reason we're aborting. (You could check inside the package functions too, and extend the error message to refer to where they come from.) -- Eli Schwartz Bug Wrangler and Trusted User signature.asc Description: OpenPGP digital signature
Re: [pacman-dev] [PATCH 3/7] libmakepkg: lint disallowed variables in package()
Silly mistakes aside. > These are not variables being overridden... pkgname_i686 is just not a > thing as far as makepkg is concerned. The point of this is specifically disallow things like 'pkgname_i686' rather than just ignore them. > Also, this is nothing to do with > overriding in a package function. Get rid of this section. This section disallows overriding foo_arch, without it they are not checked. The bellow section lints the non architecture specific variables. If there is a better way to do it or you think it would be a good idea moving this to a separate loop/file I would be glad to get your feedback. But just removing this section does stop the foo_arch linting.
Re: [pacman-dev] [PATCH 3/7] libmakepkg: lint disallowed variables in package()
On 09/06/18 04:18, morganamilo wrote:
> 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)
override
Note the arrays above this have all variables in alphabetical order.
> +
> + local no_overide_array=(checkdepends makedepends
> + noextract source validpgpkeys md5suns sha1sums
md5sums
> + 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
These are not variables being overridden... pkgname_i686 is just not a
thing as far as makepkg is concerned. Also, this is nothing to do with
overridding in a package function. Get rid of this section.
> +
> + 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
> +
As above.
> +
> 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
s/string/array
> + 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"
>
[pacman-dev] [PATCH 3/7] libmakepkg: lint disallowed variables in package()
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
