Re: [pacman-dev] [PATCH v3 1/2] Make get_pkg_arch treat arch as an array
On 21/03/15 10:19, joyfulg...@archlinux.us wrote:
From: Ivy Foster joyfulg...@archlinux.us
Signed-off-by: Ivy Foster joyfulg...@archlinux.us
---
v3: Separate change to get_pkg_arch into separate commit
scripts/makepkg.sh.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
mode change 100644 = 100755 scripts/makepkg.sh.in
This took me a whole lot of reviewing for a single character change! I
only knew that this could potentially break something on the basis of
skimming IRC chat a while ago. So I tested...
pkgname=f
arch=('any')
package_f() {
arch='x86_64' #1
arch=('x86_64') #2
arch=('i686 'x86_64') #3
}
for the --packagelist patch, currently #1 works and #2 and #3 fail.
After the patch, #2 and #3 succeed and #1 fails.
All our documentation says that arch should be an array, so the patch is
correct. In fact, the .SRCINFO generating code uses the array version
of pkgbuild_get_attribute, and so fails when the arch variable is not
an array.
This issue was never exposed because we don't use get_pkg_arch with a
parameter very often, and I guess no-one has looked at a .SRCINFO file
from a bad PKGBUILD before.
TODO: We should probably add a PKGBUILD lint function that confirms the
fields we expect to be arrays are in fact arrays. And that those that
are not arrays are not arrays.
tl;dr - Ack. I'll expand the commit message.
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
old mode 100644
new mode 100755
index 168f334..5b3bffd
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -879,7 +879,7 @@ get_pkg_arch() {
fi
else
local arch_override
- pkgbuild_get_attribute $1 arch 0 arch_override
+ pkgbuild_get_attribute $1 arch 1 arch_override
(( ${#arch_override[@]} == 0 )) arch_override=(${arch[@]})
if [[ $arch_override = any ]]; then
printf %s\n any
Re: [pacman-dev] [PATCH v3 1/2] Make get_pkg_arch treat arch as an array
On 25/03/15 14:39, Allan McRae wrote:
On 21/03/15 10:19, joyfulg...@archlinux.us wrote:
From: Ivy Foster joyfulg...@archlinux.us
Signed-off-by: Ivy Foster joyfulg...@archlinux.us
---
v3: Separate change to get_pkg_arch into separate commit
scripts/makepkg.sh.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
mode change 100644 = 100755 scripts/makepkg.sh.in
Also - BAD! file permission change.
This took me a whole lot of reviewing for a single character change! I
only knew that this could potentially break something on the basis of
skimming IRC chat a while ago. So I tested...
pkgname=f
arch=('any')
package_f() {
arch='x86_64' #1
arch=('x86_64') #2
arch=('i686 'x86_64') #3
}
for the --packagelist patch, currently #1 works and #2 and #3 fail.
After the patch, #2 and #3 succeed and #1 fails.
All our documentation says that arch should be an array, so the patch is
correct. In fact, the .SRCINFO generating code uses the array version
of pkgbuild_get_attribute, and so fails when the arch variable is not
an array.
This issue was never exposed because we don't use get_pkg_arch with a
parameter very often, and I guess no-one has looked at a .SRCINFO file
from a bad PKGBUILD before.
TODO: We should probably add a PKGBUILD lint function that confirms the
fields we expect to be arrays are in fact arrays. And that those that
are not arrays are not arrays.
tl;dr - Ack. I'll expand the commit message.
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
old mode 100644
new mode 100755
index 168f334..5b3bffd
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -879,7 +879,7 @@ get_pkg_arch() {
fi
else
local arch_override
-pkgbuild_get_attribute $1 arch 0 arch_override
+pkgbuild_get_attribute $1 arch 1 arch_override
(( ${#arch_override[@]} == 0 )) arch_override=(${arch[@]})
if [[ $arch_override = any ]]; then
printf %s\n any
Re: [pacman-dev] [PATCH v3 1/2] Make get_pkg_arch treat arch as an array
On 25 Mar 2015, at 2:39 pm +1000, Allan McRae wrote: On 21/03/15 10:19, joyfulg...@archlinux.us wrote: From: Ivy Foster joyfulg...@archlinux.us This took me a whole lot of reviewing for a single character change! Yeah, I was afraid it might. I did some grepping of my own to make sure nothing in the repos was using a singleton $arch, but I'm reassured that you found the same thing. tl;dr - Ack. I'll expand the commit message. Thanks! Ivy
[pacman-dev] [PATCH v3 1/2] Make get_pkg_arch treat arch as an array
From: Ivy Foster joyfulg...@archlinux.us
Signed-off-by: Ivy Foster joyfulg...@archlinux.us
---
v3: Separate change to get_pkg_arch into separate commit
scripts/makepkg.sh.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
mode change 100644 = 100755 scripts/makepkg.sh.in
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
old mode 100644
new mode 100755
index 168f334..5b3bffd
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -879,7 +879,7 @@ get_pkg_arch() {
fi
else
local arch_override
- pkgbuild_get_attribute $1 arch 0 arch_override
+ pkgbuild_get_attribute $1 arch 1 arch_override
(( ${#arch_override[@]} == 0 )) arch_override=(${arch[@]})
if [[ $arch_override = any ]]; then
printf %s\n any
--
2.3.3
