[pacman-dev] We've broken the bash
I think this was alluded to a while back, but we are starting to show some problems with our naming scheme and bash variable/function naming rules: /usr/bin/makepkg: line 1831: declare: `package_python-xlwt': not a valid identifier With latest makepkg built from git. -Dan
On 14/10/10 00:11, Dan McGee wrote:
I think this was alluded to a while back, but we are starting to show some problems with our naming scheme and bash variable/function naming rules:
/usr/bin/makepkg: line 1831: declare: `package_python-xlwt': not a valid identifier
With latest makepkg built from git.
Any chance of some more info on how to replicate? At least the pkgname line or even the whole PKGBUILD? Allan
On Wed, Oct 13, 2010 at 9:54 AM, Allan McRae <allan@archlinux.org> wrote:
On 14/10/10 00:11, Dan McGee wrote:
I think this was alluded to a while back, but we are starting to show some problems with our naming scheme and bash variable/function naming rules:
/usr/bin/makepkg: line 1831: declare: `package_python-xlwt': not a valid identifier
With latest makepkg built from git.
Any chance of some more info on how to replicate? At least the pkgname line or even the whole PKGBUILD?
Haha, my bad. wget http://aur.archlinux.org/packages/python-xlwt/python-xlwt.tar.gz bsdtar xf python-xlwt.tar.gz cd python-xlwt makepkg
On 14/10/10 00:59, Dan McGee wrote:
On Wed, Oct 13, 2010 at 9:54 AM, Allan McRae<allan@archlinux.org> wrote:
On 14/10/10 00:11, Dan McGee wrote:
I think this was alluded to a while back, but we are starting to show some problems with our naming scheme and bash variable/function naming rules:
/usr/bin/makepkg: line 1831: declare: `package_python-xlwt': not a valid identifier
With latest makepkg built from git.
Any chance of some more info on how to replicate? At least the pkgname line or even the whole PKGBUILD?
Haha, my bad.
wget http://aur.archlinux.org/packages/python-xlwt/python-xlwt.tar.gz bsdtar xf python-xlwt.tar.gz cd python-xlwt makepkg
I can not replicate with 4.1.x or 3.2.x. Allan
On 14/10/10 01:11, Allan McRae wrote:
On 14/10/10 00:59, Dan McGee wrote:
On Wed, Oct 13, 2010 at 9:54 AM, Allan McRae<allan@archlinux.org> wrote:
On 14/10/10 00:11, Dan McGee wrote:
I think this was alluded to a while back, but we are starting to show some problems with our naming scheme and bash variable/function naming rules:
/usr/bin/makepkg: line 1831: declare: `package_python-xlwt': not a valid identifier
With latest makepkg built from git.
Any chance of some more info on how to replicate? At least the pkgname line or even the whole PKGBUILD?
Haha, my bad.
wget http://aur.archlinux.org/packages/python-xlwt/python-xlwt.tar.gz bsdtar xf python-xlwt.tar.gz cd python-xlwt makepkg
I can not replicate with 4.1.x or 3.2.x.
^ these are bash version....
On Wed, Oct 13, 2010 at 10:12 AM, Allan McRae <allan@archlinux.org> wrote:
On 14/10/10 01:11, Allan McRae wrote:
On 14/10/10 00:59, Dan McGee wrote:
On Wed, Oct 13, 2010 at 9:54 AM, Allan McRae<allan@archlinux.org> wrote:
On 14/10/10 00:11, Dan McGee wrote:
I think this was alluded to a while back, but we are starting to show some problems with our naming scheme and bash variable/function naming rules:
/usr/bin/makepkg: line 1831: declare: `package_python-xlwt': not a valid identifier
With latest makepkg built from git.
Any chance of some more info on how to replicate? At least the pkgname line or even the whole PKGBUILD?
Haha, my bad.
wget http://aur.archlinux.org/packages/python-xlwt/python-xlwt.tar.gz bsdtar xf python-xlwt.tar.gz cd python-xlwt makepkg
I can not replicate with 4.1.x or 3.2.x.
^ these are bash version....
Hmmmmm. Build output attached, and here is the relevant part of this particular version of makepkg: 1825 # test for available PKGBUILD functions 1826 if declare -f build >/dev/null; then 1827 BUILDFUNC=1 1828 fi 1829 if declare -f package >/dev/null; then 1830 PKGFUNC=1 1831 elif [[ $SPLITPKG -eq 0 ]] && declare -f package_${pkgname}
/dev/null; then 1832 SPLITPKG=1 1833 fi
-Dan
On 14/10/10 01:36, Dan McGee wrote:
On Wed, Oct 13, 2010 at 10:12 AM, Allan McRae<allan@archlinux.org> wrote:
On 14/10/10 01:11, Allan McRae wrote:
On 14/10/10 00:59, Dan McGee wrote:
On Wed, Oct 13, 2010 at 9:54 AM, Allan McRae<allan@archlinux.org> wrote:
On 14/10/10 00:11, Dan McGee wrote:
I think this was alluded to a while back, but we are starting to show some problems with our naming scheme and bash variable/function naming rules:
/usr/bin/makepkg: line 1831: declare: `package_python-xlwt': not a valid identifier
With latest makepkg built from git.
Any chance of some more info on how to replicate? At least the pkgname line or even the whole PKGBUILD?
Haha, my bad.
wget http://aur.archlinux.org/packages/python-xlwt/python-xlwt.tar.gz bsdtar xf python-xlwt.tar.gz cd python-xlwt makepkg
I can not replicate with 4.1.x or 3.2.x.
^ these are bash version....
Hmmmmm. Build output attached, and here is the relevant part of this particular version of makepkg:
1825 # test for available PKGBUILD functions 1826 if declare -f build>/dev/null; then 1827 BUILDFUNC=1 1828 fi 1829 if declare -f package>/dev/null; then 1830 PKGFUNC=1 1831 elif [[ $SPLITPKG -eq 0 ]]&& declare -f package_${pkgname}
/dev/null; then 1832 SPLITPKG=1 1833 fi
OK - I can replicate. I forgot my makepkg-git package builds from my working branch which had not been rebased to master after the last couple of pushes. But it turns out that was a good thing as there were only a couple of patches that could cause this issue: 05f0a28932c1acab7a9ddb58435d69626dad54da is the first bad commit commit 05f0a28932c1acab7a9ddb58435d69626dad54da Author: Nezmer <git@nezmer.info> Date: Tue Oct 12 02:23:16 2010 +0300 Use sysconfdir, localstatedir, BASH instead of hardcoded values This applies to contrib/ files, our scripts, and the documentation. Dan: fix 'make clean' in contrib/ directory. Signed-off-by: Nezmer <git@nezmer.info> Signed-off-by: Dan McGee <dan@archlinux.org> .... which does this to the top of makepkg: #!/bin/sh -e which is bad... even when we symlink that to bash as it starts in standards compliance mode. So you have not broken bash, you have broken configure! So looking at configure output: checking for bash... /bin/sh and the configure check: AC_CHECK_PROGS([BASH], [bash bash4 bash3], [false]) So... WTF? It turns out the issues is that the BASH variable seems to be used elsewhere and we have a naming conflict... Allan
On Wed, Oct 13, 2010 at 5:32 PM, Allan McRae <allan@archlinux.org> wrote:
On 14/10/10 01:36, Dan McGee wrote:
On Wed, Oct 13, 2010 at 10:12 AM, Allan McRae<allan@archlinux.org> wrote:
On 14/10/10 01:11, Allan McRae wrote:
On 14/10/10 00:59, Dan McGee wrote:
On Wed, Oct 13, 2010 at 9:54 AM, Allan McRae<allan@archlinux.org> wrote:
On 14/10/10 00:11, Dan McGee wrote: > > I think this was alluded to a while back, but we are starting to show > some problems with our naming scheme and bash variable/function > naming > rules: > > /usr/bin/makepkg: line 1831: declare: `package_python-xlwt': not a > valid identifier > > With latest makepkg built from git. >
Any chance of some more info on how to replicate? At least the pkgname line or even the whole PKGBUILD?
Haha, my bad.
wget http://aur.archlinux.org/packages/python-xlwt/python-xlwt.tar.gz bsdtar xf python-xlwt.tar.gz cd python-xlwt makepkg
I can not replicate with 4.1.x or 3.2.x.
^ these are bash version....
Hmmmmm. Build output attached, and here is the relevant part of this particular version of makepkg:
1825 # test for available PKGBUILD functions 1826 if declare -f build>/dev/null; then 1827 BUILDFUNC=1 1828 fi 1829 if declare -f package>/dev/null; then 1830 PKGFUNC=1 1831 elif [[ $SPLITPKG -eq 0 ]]&& declare -f package_${pkgname}
/dev/null; then
1832 SPLITPKG=1 1833 fi
OK - I can replicate. I forgot my makepkg-git package builds from my working branch which had not been rebased to master after the last couple of pushes. But it turns out that was a good thing as there were only a couple of patches that could cause this issue:
05f0a28932c1acab7a9ddb58435d69626dad54da is the first bad commit
commit 05f0a28932c1acab7a9ddb58435d69626dad54da Author: Nezmer <git@nezmer.info> Date: Tue Oct 12 02:23:16 2010 +0300
Use sysconfdir, localstatedir, BASH instead of hardcoded values
This applies to contrib/ files, our scripts, and the documentation.
Dan: fix 'make clean' in contrib/ directory.
Signed-off-by: Nezmer <git@nezmer.info> Signed-off-by: Dan McGee <dan@archlinux.org>
....
which does this to the top of makepkg:
#!/bin/sh -e
which is bad... even when we symlink that to bash as it starts in standards compliance mode. So you have not broken bash, you have broken configure!
So looking at configure output:
checking for bash... /bin/sh
and the configure check:
AC_CHECK_PROGS([BASH], [bash bash4 bash3], [false])
So... WTF? It turns out the issues is that the BASH variable seems to be used elsewhere and we have a naming conflict...
Oh man, this makes so much more sense. WTF, configure? The shell itself sets this, try the following (which is what ./configure ends up doing): #!/bin/sh echo $BASH I think I have a patch on the way... -Dan
BASH is defined when you are actually using bash during configure, which sucks because it ends up being '/bin/sh', messing up all of our scripts. Change the name of the variable we use in configure, and also ensure we get a full path to the executable by using AC_PATH_PROGS rather than AC_CHECK_PROGS. Finally, change the variable name everywhere we use it. Signed-off-by: Dan McGee <dan@archlinux.org> --- configure.ac | 2 +- contrib/Makefile.am | 2 +- contrib/bacman.in | 2 +- contrib/pacdiff.in | 2 +- contrib/pacscripts.in | 2 +- contrib/pactree.in | 2 +- contrib/wget-xdelta.sh.in | 2 +- scripts/Makefile.am | 2 +- scripts/makepkg.sh.in | 2 +- scripts/pacman-optimize.sh.in | 2 +- scripts/pkgdelta.sh.in | 2 +- scripts/rankmirrors.sh.in | 2 +- scripts/repo-add.sh.in | 2 +- 13 files changed, 13 insertions(+), 13 deletions(-) diff --git a/configure.ac b/configure.ac index 115aeb1..6f601f4 100644 --- a/configure.ac +++ b/configure.ac @@ -127,7 +127,7 @@ AC_PROG_LN_S AC_PROG_MAKE_SET AC_PROG_LIBTOOL AC_CHECK_PROGS([PYTHON], [python2.7 python2.6 python2.5 python2 python], [false]) -AC_CHECK_PROGS([BASH], [bash bash4 bash3], [false]) +AC_PATH_PROGS([BASH_SHELL], [bash bash4 bash3], [false]) # find installed gettext AM_GNU_GETTEXT([external]) diff --git a/contrib/Makefile.am b/contrib/Makefile.am index 0199763..55366b4 100644 --- a/contrib/Makefile.am +++ b/contrib/Makefile.am @@ -29,7 +29,7 @@ MOSTLYCLEANFILES = $(OURFILES) *.tmp edit = sed \ -e 's|@sysconfdir[@]|$(sysconfdir)|g' \ -e 's|@localstatedir[@]|$(localstatedir)|g' \ - -e 's|@BASH[@]|$(BASH)|g' + -e 's|@BASH_SHELL[@]|$(BASH_SHELL)|g' $(OURFILES): Makefile @echo ' ' GEN $@; diff --git a/contrib/bacman.in b/contrib/bacman.in index 2a93f8b..991acb0 100755 --- a/contrib/bacman.in +++ b/contrib/bacman.in @@ -1,4 +1,4 @@ -#!@BASH@ +#!@BASH_SHELL@ # # bacman: recreate a package from a running system # This script rebuilds an already installed package using metadata diff --git a/contrib/pacdiff.in b/contrib/pacdiff.in index 716333a..ac4ce89 100755 --- a/contrib/pacdiff.in +++ b/contrib/pacdiff.in @@ -1,4 +1,4 @@ -#!@BASH@ +#!@BASH_SHELL@ # pacdiff : a simple pacnew/pacorig/pacsave updater # # Copyright (c) 2007 Aaron Griffin <aaronmgriffin@gmail.com> diff --git a/contrib/pacscripts.in b/contrib/pacscripts.in index 123c79d..d366409 100755 --- a/contrib/pacscripts.in +++ b/contrib/pacscripts.in @@ -1,4 +1,4 @@ -#!@BASH@ +#!@BASH_SHELL@ # # pacscripts : tries to print out the {pre,post}_{install,remove,upgrade} # scripts of a given package diff --git a/contrib/pactree.in b/contrib/pactree.in index 29c6af6..b43005a 100755 --- a/contrib/pactree.in +++ b/contrib/pactree.in @@ -1,4 +1,4 @@ -#!@BASH@ +#!@BASH_SHELL@ # pactree : a simple dependency tree viewer # # Copyright (C) 2008 Carlo "carlocci" Bersani <carlocci@gmail.com> diff --git a/contrib/wget-xdelta.sh.in b/contrib/wget-xdelta.sh.in index caf0171..f2ac1c8 100755 --- a/contrib/wget-xdelta.sh.in +++ b/contrib/wget-xdelta.sh.in @@ -1,4 +1,4 @@ -#!@BASH@ +#!@BASH_SHELL@ if [ -r "@sysconfdir@/makepkg.conf" ]; then source @sysconfdir@/makepkg.conf diff --git a/scripts/Makefile.am b/scripts/Makefile.am index b9e3038..78deb0b 100644 --- a/scripts/Makefile.am +++ b/scripts/Makefile.am @@ -35,7 +35,7 @@ edit = sed \ -e 's|@sysconfdir[@]|$(sysconfdir)|g' \ -e 's|@localstatedir[@]|$(localstatedir)|g' \ -e 's|@prefix[@]|$(prefix)|g' \ - -e 's|@BASH[@]|$(BASH)|g' \ + -e 's|@BASH_SHELL[@]|$(BASH_SHELL)|g' \ -e 's|@PACKAGE_VERSION[@]|$(REAL_PACKAGE_VERSION)|g' \ -e 's|@PACKAGE_BUGREPORT[@]|$(PACKAGE_BUGREPORT)|g' \ -e 's|@PACKAGE_NAME[@]|$(PACKAGE_NAME)|g' \ diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index afedc31..9220032 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -1,4 +1,4 @@ -#!@BASH@ -e +#!@BASH_SHELL@ -e # # makepkg - make packages compatible for use with pacman # @configure_input@ diff --git a/scripts/pacman-optimize.sh.in b/scripts/pacman-optimize.sh.in index eb6691c..f4642ab 100644 --- a/scripts/pacman-optimize.sh.in +++ b/scripts/pacman-optimize.sh.in @@ -1,4 +1,4 @@ -#!@BASH@ +#!@BASH_SHELL@ # # pacman-optimize # @configure_input@ diff --git a/scripts/pkgdelta.sh.in b/scripts/pkgdelta.sh.in index d45678d..6bc3f5d 100644 --- a/scripts/pkgdelta.sh.in +++ b/scripts/pkgdelta.sh.in @@ -1,4 +1,4 @@ -#!@BASH@ +#!@BASH_SHELL@ # # pkgdelta - create delta files for use with pacman and repo-add # @configure_input@ diff --git a/scripts/rankmirrors.sh.in b/scripts/rankmirrors.sh.in index 3851b77..64d5a73 100644 --- a/scripts/rankmirrors.sh.in +++ b/scripts/rankmirrors.sh.in @@ -1,4 +1,4 @@ -#!@BASH@ +#!@BASH_SHELL@ # # rankmirrors - read a list of mirrors from a file and rank them by speed # @configure_input@ diff --git a/scripts/repo-add.sh.in b/scripts/repo-add.sh.in index 9bc7069..d3e3bc8 100644 --- a/scripts/repo-add.sh.in +++ b/scripts/repo-add.sh.in @@ -1,4 +1,4 @@ -#!@BASH@ +#!@BASH_SHELL@ # # repo-add - add a package to a given repo database file # repo-remove - remove a package entry from a given repo database file -- 1.7.3.1
On 14/10/10 08:52, Dan McGee wrote:
BASH is defined when you are actually using bash during configure, which sucks because it ends up being '/bin/sh', messing up all of our scripts. Change the name of the variable we use in configure, and also ensure we get a full path to the executable by using AC_PATH_PROGS rather than AC_CHECK_PROGS. Finally, change the variable name everywhere we use it.
Signed-off-by: Dan McGee<dan@archlinux.org>
I was going to that this for a spin but the patch to contrib/Makefile.am does not apply on top of master. Have you another patch to that in the pipeline? Allan
On Wed, Oct 13, 2010 at 8:11 PM, Allan McRae <allan@archlinux.org> wrote:
On 14/10/10 08:52, Dan McGee wrote:
BASH is defined when you are actually using bash during configure, which sucks because it ends up being '/bin/sh', messing up all of our scripts. Change the name of the variable we use in configure, and also ensure we get a full path to the executable by using AC_PATH_PROGS rather than AC_CHECK_PROGS. Finally, change the variable name everywhere we use it.
Signed-off-by: Dan McGee<dan@archlinux.org>
I was going to that this for a spin but the patch to contrib/Makefile.am does not apply on top of master. Have you another patch to that in the pipeline?
Sí: http://code.toofishes.net/cgit/dan/pacman.git/commit/?id=a7dc3875f15f9600e01...
On Wed, Oct 13, 2010 at 05:40:44PM -0500, Dan McGee wrote:
On Wed, Oct 13, 2010 at 5:32 PM, Allan McRae <allan@archlinux.org> wrote:
On 14/10/10 01:36, Dan McGee wrote:
On Wed, Oct 13, 2010 at 10:12 AM, Allan McRae<allan@archlinux.org> wrote:
On 14/10/10 01:11, Allan McRae wrote:
On 14/10/10 00:59, Dan McGee wrote:
On Wed, Oct 13, 2010 at 9:54 AM, Allan McRae<allan@archlinux.org> wrote: > > On 14/10/10 00:11, Dan McGee wrote: >> >> I think this was alluded to a while back, but we are starting to show >> some problems with our naming scheme and bash variable/function >> naming >> rules: >> >> /usr/bin/makepkg: line 1831: declare: `package_python-xlwt': not a >> valid identifier >> >> With latest makepkg built from git. >> > > Any chance of some more info on how to replicate? At least the > pkgname line > or even the whole PKGBUILD?
Haha, my bad.
wget http://aur.archlinux.org/packages/python-xlwt/python-xlwt.tar.gz bsdtar xf python-xlwt.tar.gz cd python-xlwt makepkg
I can not replicate with 4.1.x or 3.2.x.
^ these are bash version....
Hmmmmm. Build output attached, and here is the relevant part of this particular version of makepkg:
1825 # test for available PKGBUILD functions 1826 if declare -f build>/dev/null; then 1827 BUILDFUNC=1 1828 fi 1829 if declare -f package>/dev/null; then 1830 PKGFUNC=1 1831 elif [[ $SPLITPKG -eq 0 ]]&& declare -f package_${pkgname}
/dev/null; then
1832 SPLITPKG=1 1833 fi
OK - I can replicate. I forgot my makepkg-git package builds from my working branch which had not been rebased to master after the last couple of pushes. But it turns out that was a good thing as there were only a couple of patches that could cause this issue:
05f0a28932c1acab7a9ddb58435d69626dad54da is the first bad commit
commit 05f0a28932c1acab7a9ddb58435d69626dad54da Author: Nezmer <git@nezmer.info> Date: Tue Oct 12 02:23:16 2010 +0300
Use sysconfdir, localstatedir, BASH instead of hardcoded values
This applies to contrib/ files, our scripts, and the documentation.
Dan: fix 'make clean' in contrib/ directory.
Signed-off-by: Nezmer <git@nezmer.info> Signed-off-by: Dan McGee <dan@archlinux.org>
....
which does this to the top of makepkg:
#!/bin/sh -e
which is bad... even when we symlink that to bash as it starts in standards compliance mode. So you have not broken bash, you have broken configure!
So looking at configure output:
checking for bash... /bin/sh
and the configure check:
AC_CHECK_PROGS([BASH], [bash bash4 bash3], [false])
So... WTF? It turns out the issues is that the BASH variable seems to be used elsewhere and we have a naming conflict...
Oh man, this makes so much more sense. WTF, configure?
The shell itself sets this, try the following (which is what ./configure ends up doing): #!/bin/sh echo $BASH
I think I have a patch on the way...
-Dan
My bad. I would have caught this If I tested with Arch. This does not happen in FreeBSD. What is setting BASH exactly?
Oh man, this makes so much more sense. WTF, configure?
The shell itself sets this, try the following (which is what ./configure ends up doing): #!/bin/sh echo $BASH
I think I have a patch on the way...
-Dan
My bad.
I would have caught this If I tested with Arch. This does not happen in FreeBSD.
What is setting BASH exactly?
Just saw the patch. Sorry again for the breakage.
On Wed, Oct 13, 2010 at 6:00 PM, Nezmer <git@nezmer.info> wrote:
Oh man, this makes so much more sense. WTF, configure?
The shell itself sets this, try the following (which is what ./configure ends up doing): #!/bin/sh echo $BASH
I think I have a patch on the way...
-Dan
My bad.
I would have caught this If I tested with Arch. This does not happen in FreeBSD.
What is setting BASH exactly?
Just saw the patch. Sorry again for the breakage.
Once I decide to take the patch it is as much my fault, so have no worries at all here. That's why we don't release straight from git without some testing first. :) -Dan
participants (4)
-
Allan McRae
-
Dan McGee
-
Dan McGee
-
Nezmer