GNOME Bugzilla – Bug 728515
obsd-build: gst-autogen.sh export AUTOMAKE_VERSION which class with env used in automake wrapper
Last modified: 2014-05-03 08:04:19 UTC
Created attachment 274682 [details] [review] make it possible to set FLEX Hi. Here's a set of 3 patches that allows GStreamer to build out of the box on OpenBSD. *** 0001-build-AC_PROG_LEX-so-we-can-set-FLEX.patch OpenBSD base system bundles an old version of flex(1) which is not recent enough for GStreamer. Use AC_PROG_LEX so can can set the FLEX path (in this case "glex") to a more recent version. *** 0001-build-rename-FLEX_PATH-to-FLEX.patch Needed after the previous patch *** 0001-make-VARPREFIX-_VERSION-a-local-variable.patch Since the reason for this is quite long, here's the exact same issue that was fix in GNOME a few days ago: https://bugzilla.gnome.org/show_bug.cgi?id=728243 Thanks for any comments :-)
Created attachment 274684 [details] [review] FLEX_PATH -> FLEX
Created attachment 274685 [details] [review] don't leak AUTOTOOL_VERSION to the environment
Actually for FLEX, I can just use ac_cv_path_FLEX. But the AUTOCONF_VERSION env variable leaking to the environment is very much an issue. I would really appreciate if someone could review the patch. Thank you.
Hi guys. Any feedback? Thanks.
(In reply to comment #3) > Actually for FLEX, I can just use ac_cv_path_FLEX. > But the AUTOCONF_VERSION env variable leaking to the environment is very much > an issue. I would really appreciate if someone could review the patch. > Thank you. Would it be possible to explain here why this is an issue ?
(In reply to comment #5) > (In reply to comment #3) > > Actually for FLEX, I can just use ac_cv_path_FLEX. > > But the AUTOCONF_VERSION env variable leaking to the environment is very much > > an issue. I would really appreciate if someone could review the patch. > > Thank you. > > Would it be possible to explain here why this is an issue ? As mentioned in the original post: " *** 0001-make-VARPREFIX-_VERSION-a-local-variable.patch Since the reason for this is quite long, here's the exact same issue that was fix in GNOME a few days ago: https://bugzilla.gnome.org/show_bug.cgi?id=728243 " Here is a copy/paste from that bug: checking for automake >= 1.9... testing automake... found 1.14.1 checking for autoreconf >= 2.53... testing autoreconf... found 2.69 /usr/local/bin/aclocal[35]: /usr/local/bin/aclocal-1.14.1: not found Checking for forbidden M4 macros... Processing ./configure.ac Running autoreconf... autoreconf-2.69: Entering directory `.' autoreconf-2.69: configure.ac: not using Gettext autoreconf-2.69: running: aclocal --output=aclocal.m4t /usr/local/bin/aclocal[35]: /usr/local/bin/aclocal-1.14.1: not found autoreconf-2.69: aclocal failed with exit status: 127 Now, on OpenBSD, autoconf, automake, aclocal and friends are wrappers that call the actual command according to AUTOMAKE_VERSION and AUTOCONF_VERSION. The naming is program-MAJOR.MINOR which allows us to install several auto* versions and pick the one we want. We do not install automake X.Y.Z as automake-X.Y.Z because we only want one version of automake X.Y (it makes sense to have several major versions installed, but not minor ones). e.g: $ automake --version | head -1 Provide an AUTOMAKE_VERSION environment variable, please $ AUTOMAKE_VERSION=1.14 automake --version | head -1 automake (GNU automake) 1.14.1 Now what happens with gnome-common is since this commit: b6f099488526288ad0ab1b2061304441df0b2466 we are checking the automake version against $REQUIRED_AUTOMAKE_VERSION by using the version_check() function. Unfortunately, this function leaks AUTOMAKE_VERSION to the environment by using an eval: eval ${vc_variable}_VERSION=$vc_actual_version which in our case transforms as: eval AUTOMAKE_VERSION=1.14.1 overriding our own provided AUTOMAKE_VERSION. This diff make ${vc_variable}_VERSION a local variable so that it does not go out of the function.
Now, with this patch, the _VERSION will not be visible outside that function, is that right ? Why not simply removing that eval line, is it used somewhere ? This fixes one clash, but we have no control over what will be used next in the OpenBSD automake wrapper. What if they start using AUTOMAKE_MINOR ? I wonder if there is a better solution here.
(In reply to comment #7) > Now, with this patch, the _VERSION will not be visible outside that function, > is that right ? Why not simply removing that eval line, is it used somewhere ? Yes it's right, it isn't used outside of this function. But it is used within the function itself to check the full version (X.Y.Z). > This fixes one clash, but we have no control over what will be used next in the > OpenBSD automake wrapper. What if they start using AUTOMAKE_MINOR ? I wonder if > there is a better solution here. I don't see us using AUTOMAKE_MINOR for anything really :-) But that said, even if it wasn't for OpenBSD, I still think the patch is correct since this variable really should not get out of the function; it is only used there.
Please file a new bug if there is further cleanup necessary to not leak variables :) commit 9a39caf90f286183756bc8341d20fb466c45ff9d Author: Antoine Jacoutot <ajacoutot@gnome.org> Date: Fri Apr 18 17:54:02 2014 +0200 gst-autogen.sh: Make ${VARPREFIX}_VERSION a local variable Otherwise we may end up overriding what was manually set in the environment. https://bugzilla.gnome.org/show_bug.cgi?id=728515