Re: [bug-gettext] AC_CONFIG_MACRO_DIR{,S} & ACLOCAL_AMFLAGS

[Pavel Raiskup]

On Wed, 2013-02-27 at 17:40 +0900, Daiki Ueno wrote:
> Hi Pavel,
>
> Pavel Raiskup <address@hidden> writes:
>
>> Many projects started to use AC_CONFIG_MACRO_DIR inside configure.ac but
>> it is then missed by gettextize.  When there is specified something like
>>
>>   AC_CONFIG_MACRO_DIRS([some_different_place_than_m4])
>>
>> (and the ACLOCAL_AMFLAGS unspecified) gettextize still uses the 'm4'
>> directory as place to put macros.  The problem is that aclocal then does
>> not look for m4s files 'm4' (because the name 'm4' is not defined as
>> something like default place where copy macros into).
>
> Thanks for the report.  Could you check the attached patch?
> Probably autopoint also needs similar treatment.

Of course — the main bottleneck of actual implementation is autopoint.

> ---
>  gettext-tools/misc/ChangeLog     |  6 ++++
>  gettext-tools/misc/gettextize.in | 72 
> ++++++++++++++++++++--------------------
>  2 files changed, 42 insertions(+), 36 deletions(-)
>
> diff --git a/gettext-tools/misc/ChangeLog b/gettext-tools/misc/ChangeLog
> index 6351ace..9d9d261 100644
> --- a/gettext-tools/misc/ChangeLog
> +++ b/gettext-tools/misc/ChangeLog
> @@ -1,3 +1,9 @@
> +2013-02-27  Daiki Ueno  <address@hidden>
> +
> +     * gettextize.in: Handle macro directories specified in configure.ac.
> +     Reported by Pavel Raiskup in
> +     <http://lists.gnu.org/archive/html/bug-gettext/2013-02/msg00017.html>.
> +
>  2013-01-07  Daiki Ueno  <address@hidden>
>
>       * autopoint.in: Extract version number from configure.ac in a more
> diff --git a/gettext-tools/misc/gettextize.in 
> b/gettext-tools/misc/gettextize.in
> index c5ee610..6031016 100644
> --- a/gettext-tools/misc/gettextize.in
> +++ b/gettext-tools/misc/gettextize.in
> @@ -325,6 +325,13 @@ if test -n "$auxdir"; then
>    auxdir="$auxdir/"
>  fi
>
> +# Check in which directory gettext.m4 etc. belong.
> +macrodirs=`cat "$configure_in" | grep '^AC_CONFIG_MACRO_DIR' | sed -n -e 
> 's/AC_CONFIG_MACRO_DIRS\{,1\}(\([^()]*\))/\1/p' | sed -e 's/^\[\(.*\)\]$/\1/'`
> +for arg in $macrodirs; do
> +  m4dir="$arg"
> +  break
> +done
> +

Hmms, somehow unreadable :) and duplicating code but it should work.  I
don't like it -- not sure whether there are reasons to not to use
'autom4te' for this purpuse.. but it could be more error prone IMO (and
even more readable) as autom4te is designed for such tasks:

  autom4te --language Autoconf-without-aclocal-m4 --trace=AC_CONFIG_MACRO_DIR 
configure.ac

  // and for the up2date autoconf (not yet released) should be used (See
  // autoconf's manual from git)
  autom4te --language Autoconf-without-aclocal-m4 
--trace=AC_CONFIG_MACRO_DIR_TRACE configure.ac

Are there any problems to be dependant on autoconf because of the
'autom4te' utility?  There is a lot of tracing done in gettext.
(Note that this is not a blocker.)

>  # For simplicity we change to the gettext source directory.
>  cd "$gettext_dir" ||
>    func_fatal_error "gettext source directory '${gettext_dir}' doesn't exist"
> @@ -796,31 +803,35 @@ if test -f "$srcdir/Makefile.am"; then
>      esac
>    fi
>
> -  # Extract the macro directory name from Makefile.am.
> -  aclocal_amflags=`grep '^ACLOCAL_AMFLAGS[   ]*=' "$srcdir/Makefile.am" | 
> sed -e 's/^ACLOCAL_AMFLAGS[        ]*=\(.*\)$/\1/'`
> -  m4dir=m4
> -  m4dir_defaulted=yes
> -  m4dir_is_next=
> -  for arg in $aclocal_amflags; do
> -    if test -n "$m4dir_is_next"; then
> -      # Ignore absolute directory pathnames, like /usr/local/share/aclocal.
> -      case "$arg" in
> -        /*) ;;
> -        *)
> -          m4dir="$arg"
> -          m4dir_defaulted=
> -          break
> -          ;;
> -      esac
> -      m4dir_is_next=
> -    else
> -      if test "X$arg" = "X-I"; then
> -        m4dir_is_next=yes
> -      else
> +  if test -z "$m4dir"; then
> +    # Extract the macro directory name from Makefile.am.
> +    aclocal_amflags=`grep '^ACLOCAL_AMFLAGS[         ]*=' 
> "$srcdir/Makefile.am" | sed -e 's/^ACLOCAL_AMFLAGS[        ]*=\(.*\)$/\1/'`
> +    m4dir_is_next=
> +    for arg in $aclocal_amflags; do
> +      if test -n "$m4dir_is_next"; then
> +        # Ignore absolute directory pathnames, like /usr/local/share/aclocal.
> +        case "$arg" in
> +          /*) ;;
> +          *)
> +            test -z "$m4dir" || m4dir="$arg"
> +            macrodirs="$macrodirs $arg"
> +            ;;
> +        esac
>          m4dir_is_next=
> +      else
> +        if test "X$arg" = "X-I"; then
> +          m4dir_is_next=yes
> +        else
> +          m4dir_is_next=
> +        fi
>        fi
> -    fi
> -  done
> +    done
> +  fi
> +
> +  if test -z "$m4dir"; then
> +    m4dir=m4
> +    m4dir_defaulted=yes
> +  fi

Seems to be OK to me.  Ignoring of ACLOCAL_AMFLAGS should be ok in a case
that AC_CONFIG_MACRO_DIR{,S} is specified.  Especially when this code is
just about the sentence what is going to be suggested to gettextize's
user.

>    # Decide whether to use $m4dir/ChangeLog, or to use ChangeLog instead.
>    if test -d "$srcdir/$m4dir" && test -f "$srcdir/ChangeLog" && test ! -f 
> "$srcdir/$m4dir/ChangeLog"; then
> @@ -1026,20 +1037,9 @@ if test -f "$srcdir/Makefile.am"; then
>      fi
>    fi
>    # Extract the aclocal options name from Makefile.am.
> -  aclocal_amflags=`grep '^ACLOCAL_AMFLAGS[   ]*=' "$srcdir/Makefile.am" | 
> sed -e 's/^ACLOCAL_AMFLAGS[        ]*=\(.*\)$/\1/'`

This ^^^ seems to be like nice de-duplication.

>    aclocal_options=
> -  m4dir_is_next=
> -  for arg in $aclocal_amflags; do
> -    if test -n "$m4dir_is_next"; then
> -      aclocal_options="$aclocal_options -I $arg"
> -      m4dir_is_next=
> -    else
> -      if test "X$arg" = "X-I"; then
> -        m4dir_is_next=yes
> -      else
> -        m4dir_is_next=
> -      fi
> -    fi
> +  for arg in $macrodirs; do
> +    aclocal_options="$aclocal_options -I $arg"
>    done
>    please="$please
>  Please run 'aclocal$aclocal_options' to regenerate the aclocal.m4 file.

Thanks for working on this - for me: ACK for this change.  And please keep
your radar on 'autopoint' if possible.

Btw. I tried to test this and it seems to work - but building of gettext
from git is quite painful :(
  ~> http://lists.gnu.org/archive/html/bug-gettext/2012-12/msg00058.html

Pavel