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 736710 - remove unnecessary executions of libtool from configure
remove unnecessary executions of libtool from configure
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: build
unspecified
Other All
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2014-09-16 02:02 UTC by Patrick Welche
Modified: 2017-08-22 08:02 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
remove unnecessary ./libtool calls (1.36 KB, patch)
2014-09-16 02:06 UTC, Patrick Welche
none Details | Review
remove unnessary executions of libtool from configure (1.42 KB, patch)
2014-10-06 16:57 UTC, Patrick Welche
accepted-commit_now Details | Review

Description Patrick Welche 2014-09-16 02:02:03 UTC
We require libtool 2.2, and from the libtool 1.9b: 2004-08-29 change log:

* libtool script is now created by config.status.  Instead of interrogating
  './libtool' from configure.ac after calling AC_PROG_LIBTOOL, use the
  variable names directly.

We seem to have 3 of those ./libtool invocations in configure.ac.
Comment 1 Patrick Welche 2014-09-16 02:06:44 UTC
Created attachment 286248 [details] [review]
remove unnecessary ./libtool calls

export_dynamic_flag_spec often contains ${wl}, so needs the eval
objdir is defined anyway
shrext_cmds seems to be eval'd very often...
Comment 2 Patrick Welche 2014-10-06 16:57:51 UTC
Created attachment 287864 [details] [review]
remove unnessary executions of libtool from configure

Remove one more line - shrext_cmds is already defined (by libtool) inside configure.ac.

(Just a reminder, the interesting case is darwin:

$cat foo
shrext_cmds='`test .$module = .yes && echo .so || echo .dylib`'
module=yes eval std_shrext=$shrext_cmds
echo "darwin yes: $std_shrext"
module=no eval std_shrext=$shrext_cmds
echo "darwin no: $std_shrext"
shrext_cmds=".so"
module=yes eval std_shrext=$shrext_cmds
echo "unix: $std_shrext"
$ sh foo
darwin yes: .so
darwin no: .dylib
unix: .so
)
Comment 3 Patrick Welche 2016-11-22 16:29:46 UTC
ping? (Two years on)
Comment 4 Colin Walters 2016-11-23 02:00:17 UTC
Review of attachment 287864 [details] [review]:

I'd like to see some explanation of why you're making this change.  You're saying it helps
something on Darwin?  Just a one liner would be ok.

The change itself looks OK to me.
Comment 5 Patrick Welche 2016-11-23 23:34:14 UTC
I'm making the change to not make unnecessary execs of external programs from within a script, and simply to use "modern" libtool as intended.

The mention of Darwin, is just to say that it is the more complicated case, but the change is still correct. (Patch has been in pkgsrc.org for 2 years, and no complaints from Darwin users or anyone else for the matter.)

I happen to want to use a libtool which isn't ./libtool, but libtool from $PATH, which is a personal (pkgsrc.org) motivation for reducing the number of calls.

Thanks for the review.
Comment 6 Colin Walters 2016-11-24 03:05:18 UTC
But this is autoconf we're talking about here, execution of external binaries happens...(goes to look at generated configure script)...actually, not as much as I thought, I guess autoconf goes to some effort to use shell builtins.
Comment 7 Colin Walters 2016-11-24 03:06:11 UTC
Review of attachment 287864 [details] [review]:

OK.  Can you fold your rationale into the commit message too for posterity?  (I trust git more than the centralized relational database backing bugzilla)
Comment 8 Patrick Welche 2017-08-22 08:02:04 UTC
My turn to drop the ball. Finally committed (now that you're changing to meson I patch configure.ac).

https://git.gnome.org/browse/glib/commit/?id=2237bb451df037467c0dda400d290dc050f95df9