[+cc automake-patches, since patches should be discussed there]
On 01/11/2013 05:07 AM, Mike Frysinger wrote:
> i can't imagine this is a big runtime penalty, so why does configure check
> for
> the perl's thread settings and then hardcode that in the generated automake ?
>
I don't know, I wasn't around when the change was introduced. You'll have
to dig out the archives; if we don't find any compelling reason for having
the check at configure time rather than runtime, I agree your change is a
nice simplification ...
> it means if you change your perl config or deploy an automake package on a
> system that has threads disabled, you get errors when trying to run automake.
>
... and also a fixlet for this corner-case situations.
> patch below turns it into a dynamic check using the same logic that's in
> configure.ac already.
>
Not really, unfortunately. See below.
> -mike
>
> [SNIP]
>
> --- a/configure.ac
> +++ b/configure.ac
> @@ -77,32 +77,6 @@ installed, select the one Automake should use using
> ./configure PERL=/path/to/perl])
> }
>
> -# We require ithreads support, and version 5.7.2 for CLONE.
>
Here, the comments say (and the code agree) that we should support
ithreads only from perl 5.7.2 onwards ...
> -AC_CACHE_CHECK([whether $PERL supports ithreads], [am_cv_prog_PERL_ithreads],
> -[if $PERL -e '
> - require 5.007_002;
> - use Config;
> - if ($Config{useithreads})
> - {
> - require threads;
> - import threads;
> - require Thread::Queue;
> - import Thread::Queue;
> - exit 0;
> - }
> - exit 1;' >&AS_MESSAGE_LOG_FD 2>&1
> -then
> - am_cv_prog_PERL_ithreads=yes
> -else
> - am_cv_prog_PERL_ithreads=no
> -fi])
> -if test $am_cv_prog_PERL_ithreads = yes; then
> - PERL_THREADS=1;
> -else
> - PERL_THREADS=0;
> -fi
> -AC_SUBST([PERL_THREADS])
> -
> # The test suite will skip some tests if tex is absent.
> AC_CHECK_PROG([TEX], [tex], [tex])
> # Save details about the selected TeX program in config.log.
>
> --- a/lib/Automake/Config.in
> +++ b/lib/Automake/Config.in
> @@ -32,7 +32,9 @@ our $PACKAGE = '@PACKAGE@';
> our $PACKAGE_BUGREPORT = '@PACKAGE_BUGREPORT@';
> our $VERSION = '@VERSION@';
> our $libdir = '@datadir@/@PACKAGE@-@APIVERSION@';
> -our $perl_threads = @PERL_THREADS@;
> +
> +use Config;
> +our $perl_threads = $Config{useithreads};
>
... but there is no such check here. And yes, so far we still support
perl 5.6 so far (albeit we might want to start requiring 5.8 in Automake
1.14, but that's for another thread).
> 1;
>
> --- a/lib/Automake/Makefile.am
> +++ b/lib/Automake/Makefile.am
> @@ -50,7 +50,6 @@ do_subst = in=`echo $@ | sed 's/\.[^.]*$$//'`; sed \
> -e 's,[@]PACKAGE[@],$(PACKAGE),g' \
> -e 's,[@]PACKAGE_BUGREPORT[@],$(PACKAGE_BUGREPORT),g' \
> -e 's,[@]PERL[@],$(PERL),g' \
> - -e 's,[@]PERL_THREADS[@],$(PERL_THREADS),g' \
> -e 's,[@]SHELL[@],$(SHELL),g' \
> -e 's,[@]VERSION[@],$(VERSION),g' \
> -e "s,[@]configure_input[@],Generated from $$in.in; do not edit by
> hand.,g"
> \
Also, might I ask you to format your patches with "git am"? That will make
it super-easy for me to apply them.
Thanks,
Stefano
