GNOME Bugzilla – Bug 169960
gnome-autogen.sh doesn't find some aclocal dirs
Last modified: 2014-03-13 14:21:31 UTC
The Automake manual teached me to use /usr/share/aclocal/dirlist for global macro directories. And for project specific macros, one usually uses ACLOCAL_AMFLAGS variable in the top level Makefile.am. Neither of these had enough support in gnome-autogen.sh.
Created attachment 38558 [details] [review] a patch This patch adds support for dirlist and ACLOCAL_AMFLAGS. The error message was modified so that it suggest all three alternatives. The patch is not tested yet, but can I commit it as soon as I test it?
Comment on attachment 38558 [details] [review] a patch >+add_to_cm_macrodirs() { >+ case $cm_macrodirs in >+ "$1 "* | *" $1 "* | *" $1") ;; >+ *) cm_macrodirs="$cm_macrodirs $1";; >+ esac >+} You can simplify the case statement here by writing it like so: case " $cm_macrodirs " in *\ $1\ *) ;; ... >+ # Search the dirlist file: >+ if [ -f $cm_macrodirs/dirlist ]; then >+ while read cm_dir; do >+ add_to_cm_macrodirs "$cm_dir" >+ done <$cm_macrodirs/dirlist >+ fi I'd suggest writing "cat .../dirlist | while read cm_dir ...", so it is obvious what is happening. A few other points: * can the dirlist file contain comments? If so, they shouldn't be added to the list * these directories should probably only be added for new versions of aclocal. >@@ -193,14 +214,35 @@ > fi > if [ "$cm_status" != 0 ]; then > printerr "***Error***: some autoconf macros required to build $PKG_NAME" >- printerr " were not found in your aclocal path, or some forbidden" >- printerr " macros were found. Perhaps you need to adjust your" >- printerr " ACLOCAL_FLAGS?" >+ printerr ' were not found in your aclocal path, or some forbidden' >+ printerr ' macros were found. Perhaps you need to adjust one of these:' >+ printerr ' - the contents of the file $prefix/share/aclocal/dirlist, or' >+ printerr ' - variable ACLOCAL_AMFLAGS in the top level Makefile.am, or' >+ printerr ' - the environment variable ACLOCAL_FLAGS.' > printerr > fi > return $cm_status > } I'd leave the error message as is. Not all users can write to dirlist, and it won't necessarily make any difference for the version of automake being used (also, $prefix here is a bit misleading). Editing the Makefile.am to work around a local configuration problem is not something we should be advocating either. This leaves the one option which is going to work reliably, so why mention the others at all? >+# Sed command to extract AC_CONFIG_MACRO_DIR: >+sed_ac_config_macro_dir=' >+s/^AC_CONFIG_MACRO_DIR(// >+t ok >+d >+:ok >+s/).*// >+s/^ *[[] *// >+s/ *]$ *// >+q' Where does this sed rule come from, and why is it better than what is currently in place? I took the current rule from libtoolize (2.0 branch), so if this is not fixing an actual bug I'd prefer to leave it as is. > if [ -n "$m4dir" ]; then >- m4dir="-I $m4dir" >+ ACLOCAL_FLAGS="-I $m4dir $ACLOCAL_FLAGS" > fi Don't alter ACLOCAL_FLAGS in autogen.sh. There were a bunch of rebuild related bugs that were due to differences in the environment when configure is run by autogen.sh compared to when it is run by hand or from "make". I'd prefer not to start introducing those bugs again. >+ # And likewise for ACLOCAL_AMFLAGS in the corresponding Makefile.am. >+ ACLOCAL_FLAGS=`sed "$sed_aclocal_amflags" "$basename"`" $ACLOCAL_FLAGS" Does this rule fix a problem in any package using the gnome-common autogen.sh? If not, then I'd leave it out. If a package wants to include a local macro directory they can do so with AC_CONFIG_MACRO_DIR(), which they should be using to work correctly with libtool-2.0, etc.
> case " $cm_macrodirs " in *\ $1\ *) Could we settle with this? case " $cm_macrodirs " in *" $1 "*) > * can the dirlist file contain comments? The aclocal script has a code to strip comments, so I should do it, too. Thanks. > * these directories should probably only be added for new versions of aclocal. I'll look at it. > I'd leave the error message as is. OK. > sed_ac_config_macro_dir I just wanted to replace an ineffective pipe of five commands by one command. I'll try to push this change to libtoolize, too. ;-) > Don't alter ACLOCAL_FLAGS in autogen.sh. OK. >+ # And likewise for ACLOCAL_AMFLAGS in the corresponding Makefile.am. OK, I'll leave this code out. I'll submit another version of the patch on Monday.
This is interesting. Where I wrote: >+ if [ -f $cm_macrodirs/dirlist ]; then >+ while read cm_dir; do >+ add_to_cm_macrodirs "$cm_dir" >+ done <$cm_macrodirs/dirlist >+ fi you suggested: I'd suggest writing "cat .../dirlist | while read cm_dir ..." When debugging my patch, I noticed I cannot do that, because the pipe means that the while loop is executed in a subshell, so we cannot modify $cm_macrodirs there. I'll use `for'.
Created attachment 38755 [details] [review] set AUTOMAKE_VERSION and such This patch implements the following enhancement: version_check autobus AUTOBUS 'omnibus VLB PCI' not only sets $AUTOBUS, but also stores the actual version to $AUTOBUS_VERSION.
Created attachment 38756 [details] [review] Read /usr/share/aclocal/dirlist This patch enhances gnome-autogen.sh to recognize the dirlist file. Requires patch 38755: set AUTOMAKE_VERSION and such. I accepted (almost) all the proposals from James' comment to the previous patch.
Created attachment 38758 [details] [review] Some optimizations to gnome-autogen.sh This patch contains some changes which make the code prettier in my eyes. James you mentioned libtoolize. I looked at the CVS HEAD of libttool and found out that they now use a very similar approach to what I propose. Search for my_sed_serial in libtoolize.m4sh. I did some quick testing of all three patches.
Comment on attachment 38756 [details] [review] Read /usr/share/aclocal/dirlist I'd like to explain the following change: >- cm_macrodirs="`$ACLOCAL --print-ac-dir`" >+ cm_macrodirs=`$ACLOCAL --print-ac-dir` The Autoconf manual explains the issue here: http://www.gnu.org/software/autoconf/manual/autoconf-2.57/html_node/autoconf_11 9.html 1) The double quotes are not needed. 2) In certain situations, it is better to omit them, though this is not the case. 3) So it think it's good practice to omit them in all cases.
Comment on attachment 38756 [details] [review] Read /usr/share/aclocal/dirlist The link should have been http://www.gnu.org/software/autoconf/manual/autoconf-2.57/html_node/autoconf_11 9
Comment on attachment 38756 [details] [review] Read /usr/share/aclocal/dirlist Third attempt: http://gnu.org/software/autoconf/manual/autoconf-2.57/html_node/autoconf_119
The patch doesn't apply cleanly against svn trunk anymore.
I found that the gnome-autogen.sh should also honor ACLOCAL_PATH. written against git hash 1fed4ee7015b. http://mawercer.de/~marc/0001-make-gnome-autogen.sh-look-in-ACLOCAL_PATH-for-macro.patch I'm not sure whether it is worth creating a new bug report - all entries here are pretty old - but its related to the topic
I think newest autoconf handles ACLOCAL_PATH itself, right? So this should be conditional on an older autoconf being used, I think.
Its not about autoconf, autoconf should work because it reads ACLOCAL_PATH. the gnome-autogen.sh script checks for macros to provide a nicer error message. (That's my understanding). And that fails for the usage of nixos because it stores all libraries in its own directory thus relies on env vars heavily to tell apps where things are. So its not about making autoconf work - its about fixing the check providing nicer error messages than missing m4 macros. And that check is implemented in the gnome-autogen.sh file. So its about making gnome-autogen.sh look at the same locations as autoconf/automake/... tools do.
(In reply to comment #12) > I found that the gnome-autogen.sh should also honor ACLOCAL_PATH. > > http://mawercer.de/~marc/0001-make-gnome-autogen.sh-look-in-ACLOCAL_PATH-for-macro.patch This gives me a 404 error. Could you attach the patch here?
*** This bug has been marked as a duplicate of bug 726208 ***