After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 706719 - Quote autogen.sh arguments when passing to autogen_options
Quote autogen.sh arguments when passing to autogen_options
Status: RESOLVED WONTFIX
Product: GStreamer
Classification: Platform
Component: common
git master
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-08-24 19:21 UTC by Kerrick Staley
Modified: 2013-09-04 07:47 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch fixing the issue (990 bytes, patch)
2013-08-24 19:21 UTC, Kerrick Staley
reviewed Details | Review
Patch fixing the issue (4.48 KB, patch)
2013-08-26 04:50 UTC, Kerrick Staley
none Details | Review
Patch fixing the issue (4.48 KB, patch)
2013-08-26 04:53 UTC, Kerrick Staley
reviewed Details | Review

Description Kerrick Staley 2013-08-24 19:21:33 UTC
Created attachment 253022 [details] [review]
Patch fixing the issue

More build problems (and fixes) \o/

To fix an unrelated package, I had to append liblua_LIBS='-llua5.1 -lm' to autogenargs in my jhbuildrc. gstreamer's autogen.sh should just pass this flag on to configure (which will disregard it, because gstreamer doesn't have anything to do with Lua), but instead it passes liblua_LIBS=-llua5.1 and -lm separately, causing configure to choke.
Comment 1 Kerrick Staley 2013-08-24 19:22:51 UTC
N.B. run

./autogen.sh liblua_LIBS='-llua5.1 -lm'

from some gstreamer repository to reproduce this issue.
Comment 2 Tim-Philipp Müller 2013-08-24 19:52:26 UTC
Does this really make it work for you?

Quoting the "$@" makes it pass the first argument properly (and as a whole) to the autogen_options subroutine in gst-autogen.sh, but then the argument chunking is still lost when passing the arguments to configure. I made some minimal test program, and I get this:


$ /tmp/autogen.sh liblua_LIBS='-llua5.1 -lm' foo bar
OPTION: liblua_LIBS=-llua5.1 -lm
+ passing argument liblua_LIBS=-llua5.1 -lm to configure
OPTION: foo
+ passing argument foo to configure
OPTION: bar
+ passing argument bar to configure
+ options passed to configure:  liblua_LIBS=-llua5.1 -lm foo bar
configure: ARG 1: liblua_LIBS=-llua5.1
configure: ARG 2: -lm
configure: ARG 3: foo
configure: ARG 4: bar
Comment 3 Kerrick Staley 2013-08-24 21:21:41 UTC
Crap, you're right. Is turning CONFIGURE_DEF_OPT and CONFIGURE_EXT_OPT into Bash arrays an option? Or does it need to be sh-compliant?
Comment 4 Tim-Philipp Müller 2013-08-24 21:34:56 UTC
I think it should be sh-compliant, but if you have a bash-specific solution I'd be interested to see it in any case.
Comment 5 Kerrick Staley 2013-08-26 04:50:52 UTC
Created attachment 253091 [details] [review]
Patch fixing the issue

OK, I've updated the patch; the new version, though ugly, is POSIX-compliant and fixes the issue for me.
Comment 6 Kerrick Staley 2013-08-26 04:53:20 UTC
Created attachment 253092 [details] [review]
Patch fixing the issue

Previous patch had a formatting issue; this one's fixed.
Comment 7 Sebastian Dröge (slomo) 2013-08-26 07:42:57 UTC
Now we just need to find a non-bash solution to this :)
Comment 8 Kerrick Staley 2013-08-26 08:15:11 UTC
This should work in the regular sh; I deliberately avoided arrays, which would be more straightforward and less verbose, but are bash-specific.
Comment 9 Sebastian Dröge (slomo) 2013-08-26 08:38:36 UTC
I thought that $IFS was a bash-only feature
Comment 10 Kerrick Staley 2013-08-26 16:26:42 UTC
It's part of the POSIX sh spec:
http://pubs.opengroup.org/onlinepubs/009695399/utilities/sh.html
Comment 11 Sebastian Dröge (slomo) 2013-08-27 08:21:28 UTC
Comment on attachment 253092 [details] [review]
Patch fixing the issue

Yeah, it seems like all you use there is indeed POSIX shell compatible. I'm just not sure yet if it won't break other things :)
Comment 12 Tim-Philipp Müller 2013-09-03 14:50:08 UTC
Comment on attachment 253092 [details] [review]
Patch fixing the issue

>+test ! -z "$CONFIGURE_DEF_OPT" && echo "  default flags:  ${CONFIGURE_DEF_OPT//$nl/ }"
>+test ! -z "$CONFIGURE_EXT_OPT" && echo "  external flags: ${CONFIGURE_EXT_OPT//$nl/ }"

Isn't this substitution a bash-ism?

If I use it in a script with #/bin/sh I get 'Bad substitution' (from /bin/sh -> dash)
>
>+IFS="$nl"
> "$srcdir/configure" $CONFIGURE_DEF_OPT $CONFIGURE_EXT_OPT || {
>         echo "  configure failed"
>         exit 1
> }
>+unset IFS

The IFS change does not affect the configure script, does it? Because it's run as a separate executable and not just included?

Is separating the DEF_OPT and EXT_OPT with a space in order here when passing the args to configure, or should it be a newline as well?

Sorry if these are noobish questions.
Comment 13 Kerrick Staley 2013-09-03 17:18:13 UTC
I realized that JHBuild allows you to specify per-module autogen.sh flags (and furthermore libquvi just fixed their Lua 5.2 compatibility problem, meaning the flags aren't even necessary any more), so I'd be fine with abandoning or de-prioritizing this issue.
Comment 14 Tim-Philipp Müller 2013-09-03 21:41:43 UTC
I'd like to either fix it, or WONTFIX it :)
Comment 15 Kerrick Staley 2013-09-04 07:47:20 UTC
OK, marking it as WONTFIX. I found a workaround, and it's an edge case in the first place, so I'm guessing the number of people affected by this issue is approximately 0 (and it seems like a fix would make the code substantially more cumbersome, increasing maintenance burden). If someone encounters this issue in the future, the comments on this bug will serve as a good starting point towards a solution.