Re: patch to build glib with automake 1.6/1.7

[James Henstridge <james daa com au>]

Owen Taylor wrote:

> >    * use AC_HELP_STRING() to format help messages for --with-* and
> >      --enable-* arguments.
> >    
>
> For completeness, it should be noted that Art Haas sent in a patch
> to do this some time ago:
>
> http://mail.gnome.org/archives/gtk-devel-list/2002-June/msg00175.html
>
> Though it was never put in bugzilla. Maybe you want to check over
> that and make sure that your patch completely subsumes the changes
> there.
It looks like my patch does.  (the AC_HELP_STRING calls it makes that my 
patch doesn't is for the gtk-doc related ones, which are now handled by 
gtk-doc's m4 macro).

> >    * Add an AC_CONFIG_FILES() call in an "if false" block.  This lists
> >      things like the makefile.mingw files, etc.  This way they don't
> >      get built when you run config.status with no arguments (as
> >      before), but Automake will automatically generate rules to rebuild
> >      them.  This allowed me to remove a fair number of Makefile.am
> >      rules which simplified things.
> >    
>
> Well, we'll hope this continues working in the future ... 
The AC_CONFIG_FILES() inside an if block idiom has been suggested on the 
autoconf list a number of times (I could give you a few archive links if 
it would help).  Puting the AC_CONFIG_FILES() call inside a block that 
will never be called by the shell (but is evaluated by M4) is pretty 
much the same thing.

> Some other questions about your configure.in changes:
>
> - Any particular reason feature that you changed the AC_PREREQ
>   from 2.53 => 2.54. I don't mind the newer prereq ... just
>   curious.
That change probably wasn't strictly needed (I don't think anything in 
configure.in itself requires 2.54), but some of automake's macros (that 
are included by configure.in) require 2.54.  This should help make the 
requirements more obvious.

> - Is there some particular principle behind where you added 
>   extra quoting? Tool you were using to decide where to
>   add them?
I was originally planning to do a more extensive review of the quoting 
in the configure script, but didn't get round to it.  It is probably 
better to leave any more of that kind of work til after committing this 
patch (it is large enough as is).

> - For the change of echo glibconfig.h is unchanged to 
>   AC_MSG_NOTICE() .. is that legitimate since this is 
>   run inside config.status rather than configure?
That is how the other messages config.status prints are produced, so I 
assume it is correct.  It has the benefit that the message will be 
appended to config.log as well as printed to the screen (which can be 
useful when diagnosing problems).

> The one other thing that I noticed is that gmarshal.c should
> depend on gmarshal.list/glib-genmarshal rather than gmarshal.h ..
> gmarshal.h could remain the same and gmarshal.c have to change.
Oops.  That looks like a mistake from when I was converting those files 
over to using BUILT_SOURCES.  I'll fix it before checking in.

> In general, the patch looks good, as much as I can tell from
> a patch of this size. Why don't you go ahead and commit.. 
> I don't really want to go through it all again :-)
>  
Okay.  I will check it in tonight.  There are a few other cleanups I 
would like to do, but they should be much smaller and easier to review.

James.

--
Email: james daa com au
WWW:   http://www.daa.com.au/~james/