Hi Pavel. There are still some issues with your patches (most of them reported below). No need for your to re-roll; I will fix them locally, and then send the amended patches out for further review before pushing. Not sure whether I can do that today though, so be patient.
On 02/11/2013 01:11 PM, Pavel Raiskup wrote: > Related to automake bug#13514. > > Every bootstrapping process which does not need to have the local > macro directory existing in version control system (because it does > not have any user-defined macros) would fail during 'autoreconf -vfi' > phase if the AC_CONFIG_MACRO_DIRS([m4]) is specified (to force tools > like 'autopoint' and 'libtoolize' to use 'm4' as the local directory > where to install definitions of their m4 macros, and to instruct > aclocal to look into it): > > autoreconf: Entering directory `.' > autoreconf: running: aclocal --force > aclocal: error: couldn't open directory 'm4': No such file or directory > autoreconf: aclocal failed with exit status: 1 > > The problem is that when the 'aclocal' is run for the first time > during 'autoreconf', the directory 'm4' does not exist yet. It will > be created by e.g. by 'libtoolize' later on. During the second run > (after 'libtoolize'), the 'm4' directory exists and aclocal does not > complain. > > For that reason, we degrade the error to a simple warning. The > warning is quite useful for running aclocal by hand - so we are not > removing completely. > > See: > <http://lists.gnu.org/archive/html/bug-automake/2013-01/msg00115.html> > <http://lists.gnu.org/archive/html/automake-patches/2010-02/msg00030.html> > > * aclocal.in (SCAN_M4_DIRS_SILENT, SCAN_M4_DIRS_WARN) > (SCAN_M4_DIRS_ERROR): New constants. > (scan_m4_dirs): Change the second parameter name to $ERR_LEVEL to > better reflect new semantic. Use new constants. > (scan_m4_files): Adjust to reflect the new 'scan_m4_dirs' semantics. > * t/aclocal-macrodir.tap: Check for proper return value of 'aclocal' > when AC_CONFIG_MACRO_DIR is specified. Check whether warning is (is > not) printed to stderr when the primary macro directory does not > exist. > > Suggested-by: Ben Pfaff <b...@cs.stanford.edu> > --- > aclocal.in | 31 +++++++++++++++++++++++-------- > t/aclocal-macrodir.tap | 12 ++++++++---- > 2 files changed, 31 insertions(+), 12 deletions(-) > > diff --git a/aclocal.in b/aclocal.in > index b51c09d..8bae156 100644 > --- a/aclocal.in > +++ b/aclocal.in > @@ -165,6 +165,11 @@ my @ac_config_macro_dirs; > # If set, names a temporary file that must be erased on abnormal exit. > my $erase_me; > > +# constants for scan_m4_dirs($ERR_LEVEL) parameter > +use constant SCAN_M4_DIRS_SILENT => 0; > +use constant SCAN_M4_DIRS_WARN => 1; > +use constant SCAN_M4_DIRS_ERROR => 2; > + > ################################################################ > > # Prototypes for all subroutines. > @@ -355,21 +360,29 @@ sub list_compare (\@\@) > > ################################################################ > > -# scan_m4_dirs($TYPE, $ERR_ON_NONEXISTING, @DIRS) > +# scan_m4_dirs($TYPE, $ERR_LEVEL, @DIRS) > # ----------------------------------------------- > # Scan all M4 files installed in @DIRS for new macro definitions. > # Register each file as of type $TYPE (one of the FT_* constants). > +# Fail without discussion on non-existing include directory when the > +# $ERR_LEVEL parameter equals to SCAN_M4_DIRS_ERROR, just print warning > +# when it equals to SCAN_M4_DIRS_WARN and don't complain at all when > +# it is set to SCAN_M4_DIRS_SILENT. > sub scan_m4_dirs ($$@) > { > - my ($type, $err_on_nonexisting, @dirlist) = @_; > + my ($type, $err_level, @dirlist) = @_; > > foreach my $m4dir (@dirlist) > { > if (! opendir (DIR, $m4dir)) > { > # TODO: maybe avoid complaining only if errno == ENONENT? > - next unless $err_on_nonexisting; > - fatal "couldn't open directory '$m4dir': $!"; > + my $message = "couldn't open directory '$m4dir': $!"; > + > + fatal $message if $err_level == SCAN_M4_DIRS_ERROR; > + msg ('unsupported', $message) if $err_level == SCAN_M4_DIRS_WARN; > + # don't complain if $err_level == SCAN_M4_DIRS_SILENT > + next > } > > # We reverse the directory contents so that foo2.m4 gets > @@ -408,11 +421,13 @@ sub scan_m4_files () > { > # Don't complain if the first user directory doesn't exist, in case > # we need to create it later (can happen if '--install' was given). > - scan_m4_dirs (FT_USER, !$install, $user_includes[0]); > - scan_m4_dirs (FT_USER, 1, @user_includes[1..$#user_includes]); > + my $first_dir_lvl = $install ? SCAN_M4_DIRS_SILENT : SCAN_M4_DIRS_WARN; > + scan_m4_dirs (FT_USER, $first_dir_lvl, $user_includes[0]); > + scan_m4_dirs (FT_USER, SCAN_M4_DIRS_ERROR, > + @user_includes[1..$#user_includes]); > } > - scan_m4_dirs (FT_AUTOMAKE, 1, @automake_includes); > - scan_m4_dirs (FT_SYSTEM, 1, @system_includes); > + scan_m4_dirs (FT_AUTOMAKE, SCAN_M4_DIRS_ERROR, @automake_includes); > + scan_m4_dirs (FT_SYSTEM, SCAN_M4_DIRS_ERROR, @system_includes); > > # Construct a new function that does the searching. We use a > # function (instead of just evaluating $search in the loop) so that > diff --git a/t/aclocal-macrodir.tap b/t/aclocal-macrodir.tap > index 3c66e53..4673d71 100755 > --- a/t/aclocal-macrodir.tap > +++ b/t/aclocal-macrodir.tap > @@ -105,7 +105,9 @@ mkdir sys-dir the-dir > echo 'AC_DEFUN([THE_MACRO], [:])' > sys-dir/my.m4 > > test ! -r the-dir/my.m4 \ > - && $ACLOCAL --install --system-acdir ./sys-dir \ > + && $ACLOCAL --install --system-acdir ./sys-dir 2>stderr \ > + && cat stderr >&2 \ > + && not grep "couldn't open directory" stderr \ > This check is not actually necessary; since aclocal is being run with the "-Wall -Werror" options, it will exit with error if any warning is triggered. So we can drop this "grep". Ditto for similar checks below. > && diff sys-dir/my.m4 the-dir/my.m4 \ > || r='not ok' > > @@ -149,7 +151,9 @@ mkdir acdir > echo 'AC_DEFUN([MY_MACRO], [:])' > acdir/bar.m4 > > test ! -d foo \ > - && $ACLOCAL --install --system-acdir ./acdir \ > + && $ACLOCAL --install --system-acdir ./acdir 2>stderr \ > + && cat stderr >&2 \ > + && not grep "couldn't open directory" stderr \ > && diff acdir/bar.m4 foo/bar.m4 \ > || r='not ok' > > @@ -157,14 +161,14 @@ test_end > > #--------------------------------------------------------------------------- > > -test_begin "AC_CONFIG_MACRO_DIR([non-existent]) errors out (1)" > +test_begin "AC_CONFIG_MACRO_DIR([non-existent]) warns but succeeds" > > cat > configure.ac << 'END' > AC_INIT([oops], [1.0]) > AC_CONFIG_MACRO_DIR([non-existent]) > END > > -not $ACLOCAL -Wnone 2>stderr \ > +$ACLOCAL -Wno-error 2>stderr \ > && cat stderr >&2 \ > && grep "couldn't open directory 'non-existent'" stderr \ > || r='not ok' > We should also check that we can disable the warning with the '-Wno-unsupported' option. Will do in my re-roll. In addition, the sister test case 't/aclocal-macrodirs.tap' should be adjusted similarly to what has been done for 't/aclocal-macrodir.tap'. I'll do that in my re-roll. Thanks, Stefano
