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 728515 - obsd-build: gst-autogen.sh export AUTOMAKE_VERSION which class with env used in automake wrapper
obsd-build: gst-autogen.sh export AUTOMAKE_VERSION which class with env used ...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: common
1.2.4
Other OpenBSD
: Normal normal
: 1.3.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-04-18 16:06 UTC by Antoine Jacoutot
Modified: 2014-05-03 08:04 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
make it possible to set FLEX (1.38 KB, patch)
2014-04-18 16:06 UTC, Antoine Jacoutot
none Details | Review
FLEX_PATH -> FLEX (987 bytes, patch)
2014-04-18 16:06 UTC, Antoine Jacoutot
none Details | Review
don't leak AUTOTOOL_VERSION to the environment (755 bytes, patch)
2014-04-18 16:07 UTC, Antoine Jacoutot
committed Details | Review

Description Antoine Jacoutot 2014-04-18 16:06:07 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 :-)
Comment 1 Antoine Jacoutot 2014-04-18 16:06:31 UTC
Created attachment 274684 [details] [review]
FLEX_PATH -> FLEX
Comment 2 Antoine Jacoutot 2014-04-18 16:07:08 UTC
Created attachment 274685 [details] [review]
don't leak AUTOTOOL_VERSION to the environment
Comment 3 Antoine Jacoutot 2014-04-21 09:06:07 UTC
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.
Comment 4 Antoine Jacoutot 2014-04-27 06:41:14 UTC
Hi guys. Any feedback?
Thanks.
Comment 5 Nicolas Dufresne (ndufresne) 2014-04-27 13:58:18 UTC
(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 ?
Comment 6 Antoine Jacoutot 2014-04-27 15:13:15 UTC
(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.
Comment 7 Nicolas Dufresne (ndufresne) 2014-04-27 16:07:44 UTC
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.
Comment 8 Antoine Jacoutot 2014-04-27 16:45:47 UTC
(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.
Comment 9 Sebastian Dröge (slomo) 2014-05-03 08:04:17 UTC
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