GNU bug report logs - #8168
macros directory not created automatically

Previous Next

Package: automake;

Reported by: Javier Jard�n <jjardon <at> gnome.org>

Date: Thu, 3 Mar 2011 20:38:02 UTC

Severity: wishlist

Tags: patch

Merged with 10816

Done: Stefano Lattarini <stefano.lattarini <at> gmail.com>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 8168 in the body.
You can then email your comments to 8168 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox

Report forwarded to owner <at> debbugs.gnu.org, bug-automake <at> gnu.org:
bug#8168; Package automake. (Thu, 03 Mar 2011 20:38:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Stefano Lattarini <stefano.lattarini <at> gmail.com>:
New bug report received and forwarded. Copy sent to bug-automake <at> gnu.org. (Thu, 03 Mar 2011 20:38:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: automake <at> gnu.org
Cc: bug-automake <at> gnu.org
Subject: Re: macros directory not created automatically
Date: Thu, 3 Mar 2011 21:36:51 +0100

[adding bug-automake]

On Thursday 03 March 2011, Javier Jardón wrote:
> Hi,
>
Hi Javier, and thanks for the report.

> I'm using
> 
> ACLOCAL_AMFLAGS = -I m4 ${ACLOCAL_FLAGS}
> 
> in my main Makefile.am, but I have to add the m4 directory to the git
> tree, or create it in my autogen script (and this doesnt work if I do
> not use a autogen file and only autoreconf instead)
> 
> Shouldnt be automake smart enough to create that directory automatically?
>
Probably yes -- but IMHO it should then also be smart enough to create
the directory *only* when it makes sense, i.e. only when the `--install'
flag is used too.

Also, it might be useful if aclocal would not complain (or at least would
not error out) in case it is passed a non-existent or unreadable directory
through the `-I' option.  This "laxer" behaviour is implemented in many
other gnu tools already (e.g., make, m4. gcc).

I might take a look at this issues in the next(ish) days.

Thanks,
  Stefano

Changed bug submitter to 'Javier Jard�n <jjardon <at> gnome.org>' from 'Stefano Lattarini <stefano.lattarini <at> gmail.com>' Request was from Stefano Lattarini <stefano.lattarini <at> gmail.com> to control <at> debbugs.gnu.org. (Mon, 14 Mar 2011 18:46:02 GMT) Full text and rfc822 format available.

Information forwarded to owner <at> debbugs.gnu.org, bug-automake <at> gnu.org:
bug#8168; Package automake. (Mon, 14 Mar 2011 19:45:02 GMT) Full text and rfc822 format available.

Message #10 received at 8168 <at> debbugs.gnu.org (full text, mbox):

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: 8168 <at> debbugs.gnu.org
Cc: Javier Jardón <jjardon <at> gnome.org>,
	automake-patches <at> gnu.org
Subject: Re: bug#8168: macros directory not created automatically
Date: Mon, 14 Mar 2011 20:44:03 +0100

[Message part 1 (text/plain, inline)]

[dropping automake list, adding automake-patches]

References:
  <http://lists.gnu.org/archive/html/automake/2011-03/msg00000.html>
  <http://lists.gnu.org/archive/html/bug-automake/2011-03/msg00004.html>

-*-*-

Hello Javier and all automakes, and sorry for the delay.

On Thursday 03 March 2011, Stefano Lattarini wrote:
> 
> On Thursday 03 March 2011, Javier Jardón wrote:
> > Hi,
> >
> Hi Javier, and thanks for the report.
> 
> > I'm using
> > 
> > ACLOCAL_AMFLAGS = -I m4 ${ACLOCAL_FLAGS}
> > 
> > in my main Makefile.am, but I have to add the m4 directory to the git
> > tree, or create it in my autogen script (and this doesnt work if I do
> > not use a autogen file and only autoreconf instead)
> > 
> > Shouldnt be automake smart enough to create that directory automatically?
> >
> Probably yes -- but IMHO it should then also be smart enough to create
> the directory *only* when it makes sense, i.e. only when the `--install'
> flag is used too.
> 
> Also, it might be useful if aclocal would not complain (or at least would
> not error out) in case it is passed a non-existent or unreadable directory
> through the `-I' option.  This "laxer" behaviour is implemented in many
> other gnu tools already (e.g., make, m4. gcc).
> 
> I might take a look at this issues in the next(ish) days.
> 
> Thanks,
>   Stefano
> 
Attached is a patch series (2 patches) that should address the issue.
The series is based off of maint -- as I'm not yet sure whether it
would be better to merge it to master only, or to maint too.

Javier, are you ok with your name being added to THANKS?

Ralf, OK to apply?  If yes, where (maint, or master only)?

Regards,
  Stefano

-*-*-

PATCH 1/2] aclocal: `-I' does not bail out on invalid directories.

Related to automake bug#8168.

* aclocal.in (scan_m4_dirs): If a user-specified "include
directory" is unreadable or non-existent, do not issue a
fatal error anymore, but simply issue a warning, and only
when running in verbose mode.
* NEWS: Update.
* tests/aclocal-bad-dirlist-include-dir.test: New test.
* tests/aclocal-bad-local-include-dir.test: Likewise.
* tests/aclocal-bad-system-include-dir.test: Likewise.
* tests/Makefile.am (TESTS): Update.
* tests/.gitignore: Update.
---
 ChangeLog                                  |   15 +++++
 NEWS                                       |    5 ++
 aclocal.in                                 |   10 +++-
 tests/.gitignore                           |    4 +-
 tests/Makefile.am                          |    3 +
 tests/Makefile.in                          |    3 +
 tests/aclocal-bad-dirlist-include-dir.test |   36 +++++++++++
 tests/aclocal-bad-local-include-dir.test   |   90 ++++++++++++++++++++++++++++
 tests/aclocal-bad-system-include-dir.test  |   36 +++++++++++
 9 files changed, 199 insertions(+), 3 deletions(-)
 create mode 100755 tests/aclocal-bad-dirlist-include-dir.test
 create mode 100755 tests/aclocal-bad-local-include-dir.test
 create mode 100755 tests/aclocal-bad-system-include-dir.test

-*-*-

[PATCH 2/2] aclocal: create local directory where to install m4 files

Before this change, a call like `aclocal -I m4 --install' would
fail if the `m4' directory wasn't pre-existing.  This could be
particularly annoying when running in a checked-out version
from a VCS like git, which doesn't allow empty directories to
be tracked.

Closes automake bug#8168.

* aclocal.in (install_file): Change signature: take as second
argument the destination directory rather than the destination
file.  Crate the destination directory if it doesn't already
exist.  In verbose mode, tell what is being copied where.
(write_aclocal: Update.
* NEWS: Update.
* THANKS: Update.
* tests/aclocal-install-fail.test: New test.
* tests/aclocal-install-mkdir.test: Likewise.
* tests/aclocal-no-install-no-mkdir.test: Likewise.
* tests/aclocal-verbose-install.test: Likewise.
* tests/Makefile.am (TESTS): Update.

From a report by Javier Jardón.
---
 ChangeLog                              |   23 ++++++++++++
 NEWS                                   |    4 ++
 THANKS                                 |    1 +
 aclocal.in                             |   13 ++++---
 tests/Makefile.am                      |    4 ++
 tests/Makefile.in                      |    4 ++
 tests/aclocal-install-fail.test        |   52 ++++++++++++++++++++++++++
 tests/aclocal-install-mkdir.test       |   62 ++++++++++++++++++++++++++++++++
 tests/aclocal-no-install-no-mkdir.test |   39 ++++++++++++++++++++
 tests/aclocal-verbose-install.test     |   53 +++++++++++++++++++++++++++
 10 files changed, 250 insertions(+), 5 deletions(-)
 create mode 100755 tests/aclocal-install-fail.test
 create mode 100755 tests/aclocal-install-mkdir.test
 create mode 100755 tests/aclocal-no-install-no-mkdir.test
 create mode 100755 tests/aclocal-verbose-install.test

[0001-aclocal-I-does-not-bail-out-on-invalid-directories.patch (text/x-patch, inline)]

From 7e00010b25459b62c62f011b0d4909313c131217 Mon Sep 17 00:00:00 2001
From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
Date: Mon, 14 Mar 2011 17:06:49 +0100
Subject: [PATCH 1/2] aclocal: `-I' does not bail out on invalid directories.

Related to automake bug#8168.

* aclocal.in (scan_m4_dirs): If a user-specified "include
directory" is unreadable or non-existent, do not issue a
fatal error anymore, but simply issue a warning, and only
when running in verbose mode.
* NEWS: Update.
* tests/aclocal-bad-dirlist-include-dir.test: New test.
* tests/aclocal-bad-local-include-dir.test: Likewise.
* tests/aclocal-bad-system-include-dir.test: Likewise.
* tests/Makefile.am (TESTS): Update.
* tests/.gitignore: Update.
---
 ChangeLog                                  |   15 +++++
 NEWS                                       |    5 ++
 aclocal.in                                 |   10 +++-
 tests/.gitignore                           |    4 +-
 tests/Makefile.am                          |    3 +
 tests/Makefile.in                          |    3 +
 tests/aclocal-bad-dirlist-include-dir.test |   36 +++++++++++
 tests/aclocal-bad-local-include-dir.test   |   90 ++++++++++++++++++++++++++++
 tests/aclocal-bad-system-include-dir.test  |   36 +++++++++++
 9 files changed, 199 insertions(+), 3 deletions(-)
 create mode 100755 tests/aclocal-bad-dirlist-include-dir.test
 create mode 100755 tests/aclocal-bad-local-include-dir.test
 create mode 100755 tests/aclocal-bad-system-include-dir.test

diff --git a/ChangeLog b/ChangeLog
index 804fae6..ed9f35b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,18 @@
+2011-03-14  Stefano Lattarini  <stefano.lattarini <at> gmail.com>
+
+	aclocal: `-I' does not bail out on invalid directories.
+	Related to automake bug#8168.
+	* aclocal.in (scan_m4_dirs): If a user-specified "include
+	directory" is unreadable or non-existent, do not issue a
+	fatal error anymore, but simply issue a warning, and only
+	when running in verbose mode.
+	* NEWS: Update.
+	* tests/aclocal-bad-dirlist-include-dir.test: New test.
+	* tests/aclocal-bad-local-include-dir.test: Likewise.
+	* tests/aclocal-bad-system-include-dir.test: Likewise.
+	* tests/Makefile.am (TESTS): Update.
+	* tests/.gitignore: Update.
+
 2011-03-04  Stefano Lattarini  <stefano.lattarini <at> gmail.com>
 
 	tests: fix bug in alloca*.test
diff --git a/NEWS b/NEWS
index 3132b16..953e62a 100644
--- a/NEWS
+++ b/NEWS
@@ -5,6 +5,11 @@ New in 1.11.0a:
   - The `lzma' compression scheme and associated automake option `dist-lzma'
     is obsoleted by `xz' and `dist-xz' due to upstream changes.
 
+* Changes to aclocal:
+
+  - aclocal does not issue a fatal error anymore if one of the directories
+    specified with `-I' does not exist, or is not readable.
+
 Bugs fixed in 1.11.0a:
 
 * Bugs introduced by 1.11:
diff --git a/aclocal.in b/aclocal.in
index 2210fe3..3b9beab 100644
--- a/aclocal.in
+++ b/aclocal.in
@@ -312,7 +312,15 @@ sub scan_m4_dirs ($@)
     {
       if (! opendir (DIR, $m4dir))
 	{
-	  fatal "couldn't open directory `$m4dir': $!";
+	  if ($type == FT_USER)
+	    {
+	      verb "cannot open directory `$m4dir': $!";
+	      next;
+	    }
+	  else
+	    {
+	      fatal "couldn't open directory `$m4dir': $!";
+	    }
 	}
 
       # We reverse the directory contents so that foo2.m4 gets
diff --git a/tests/.gitignore b/tests/.gitignore
index 3c1f990..5e97c89 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -1,5 +1,5 @@
-aclocal-*
-automake-*
+aclocal-1.*
+automake-1.*
 defs
 parallel-tests.am
 *.dir
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 3fdb90a..da4a037 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -59,6 +59,9 @@ acloca19.test \
 acloca20.test \
 acloca21.test \
 acloca22.test \
+aclocal-bad-dirlist-include-dir.test \
+aclocal-bad-local-include-dir.test \
+aclocal-bad-system-include-dir.test \
 acoutnoq.test \
 acoutpt.test \
 acoutpt2.test \
diff --git a/tests/Makefile.in b/tests/Makefile.in
index cd00833..6467f1a 100644
--- a/tests/Makefile.in
+++ b/tests/Makefile.in
@@ -329,6 +329,9 @@ acloca19.test \
 acloca20.test \
 acloca21.test \
 acloca22.test \
+aclocal-bad-dirlist-include-dir.test \
+aclocal-bad-local-include-dir.test \
+aclocal-bad-system-include-dir.test \
 acoutnoq.test \
 acoutpt.test \
 acoutpt2.test \
diff --git a/tests/aclocal-bad-dirlist-include-dir.test b/tests/aclocal-bad-dirlist-include-dir.test
new file mode 100755
index 0000000..1b8df7d
--- /dev/null
+++ b/tests/aclocal-bad-dirlist-include-dir.test
@@ -0,0 +1,36 @@
+#! /bin/sh
+# Copyright (C) 2011 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that `aclocal' errors out when passed a non-readable directory
+# with the `dirlist' file.
+
+. ./defs || Exit 1
+
+set -e
+
+cat > configure.in <<END
+AC_INIT([$me], [1.0])
+END
+
+mkdir dirlist-test
+chmod a-r dirlist-test
+ls -l disrlist-test && Exit 77
+
+$ACLOCAL 2>stderr && { cat stderr >&2; Exit 1; }
+cat stderr >&2
+grep " couldn't open directory .*dirlist-test" stderr
+
+:
diff --git a/tests/aclocal-bad-local-include-dir.test b/tests/aclocal-bad-local-include-dir.test
new file mode 100755
index 0000000..680c754
--- /dev/null
+++ b/tests/aclocal-bad-local-include-dir.test
@@ -0,0 +1,90 @@
+#! /bin/sh
+# Copyright (C) 2011 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that `aclocal' does not bail out when passed a non-existent
+# or non-readable directory with the `-I' option.  Also check that
+# warns appropriately when `--verbose' is used.
+
+. ./defs || Exit 1
+
+set -e
+
+cat > configure.in <<END
+AC_INIT([$me], [1.0])
+MY_MACRO
+NOT_A_MACRO
+END
+
+mkdir m4
+cat > m4/my-defs.m4 <<END
+AC_DEFUN([MY_MACRO], [my--macro--expansion])
+END
+
+mkdir unreadable unopenable unopenable/sub private
+
+cat > unreadable/not-defs.m4 <<END
+AC_DEFUN([NOT_A_MACRO], [invalid--expansion])
+END
+cp unreadable/not-defs.m4 unopenable/sub
+cp unreadable/not-defs.m4 private
+
+chmod a-r unreadable
+chmod a-x unopenable
+chmod a-rwx private
+
+ls -l unreadable && Exit 77
+ls -l unopenable/sub && Exit 77
+ls -l private && Exit 77
+
+aclocal_call ()
+{
+  $ACLOCAL -I m4 \
+           -I non-existent \
+           -I "$cwd"/non-existent \
+           -I unreadable \
+           -I "$cwd"/unreadable \
+           -I unopenable/sub \
+           -I "$cwd"/unopenable/sub \
+           -I private \
+           -I "$cwd"/private \
+           ${1+"$@"} >stdout 2>stderr
+}
+
+cwd=`pwd` || Exit 99
+
+aclocal_call && test ! -s stdout && test ! -s stderr \
+  || { cat stdout; cat stderr >&2; Exit 1; }
+
+$EGREP '(MY_MACRO|my-defs\.m4)' aclocal.m4
+$EGREP '(NOT_A_MACRO|not-defs\.m4)' aclocal.m4 && Exit 1
+
+$AUTOCONF
+
+$FGREP MY_MACRO configure && Exit 1
+$FGREP my--macro--expansion configure
+$FGREP NOT_A_MACRO configure
+$FGREP invalid--expansion configure && Exit 1
+
+aclocal_call --verbose || { cat stdout; cat stderr >&2; Exit 1; }
+cat stdout
+cat stderr >&2
+
+for d in non-existent unreadable unopenable/sub private; do
+  grep " cannot open .*$d" stderr
+  grep " cannot open .*$cwd/$d" stderr
+done
+
+:
diff --git a/tests/aclocal-bad-system-include-dir.test b/tests/aclocal-bad-system-include-dir.test
new file mode 100755
index 0000000..f497d9e
--- /dev/null
+++ b/tests/aclocal-bad-system-include-dir.test
@@ -0,0 +1,36 @@
+#! /bin/sh
+# Copyright (C) 2011 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that `aclocal' errors out when passed a non-existent or
+# non-readable directory with the `dirlist' file.
+
+. ./defs || Exit 1
+
+set -e
+
+cat > configure.in <<END
+AC_INIT([$me], [1.0])
+END
+
+mkdir fake-acdir
+chmod a-r fake-acdir
+ls -l fake-acdir && Exit 77
+
+$ACLOCAL --acdir fake-acdir 2>stderr && { cat stderr >&2; Exit 1; }
+cat stderr >&2
+grep " couldn't open directory .*fake-acdir" stderr
+
+:
-- 
1.7.2.3

[0002-aclocal-create-local-directory-where-to-install-m4-f.patch (text/x-patch, inline)]

From 8e31da6cb5712bc43181861c715cc2d4e1d8def7 Mon Sep 17 00:00:00 2001
From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
Date: Mon, 14 Mar 2011 19:41:55 +0100
Subject: [PATCH 2/2] aclocal: create local directory where to install m4 files
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Before this change, a call like `aclocal -I m4 --install' would
fail if the `m4' directory wasn't pre-existing.  This could be
particularly annoying when running in a checked-out version
from a VCS like git, which doesn't allow empty directories to
be tracked.

Closes automake bug#8168.

* aclocal.in (install_file): Change signature: take as second
argument the destination directory rather than the destination
file.  Crate the destination directory if it doesn't already
exist.  In verbose mode, tell what is being copied where.
(write_aclocal: Update.
* NEWS: Update.
* THANKS: Update.
* tests/aclocal-install-fail.test: New test.
* tests/aclocal-install-mkdir.test: Likewise.
* tests/aclocal-no-install-no-mkdir.test: Likewise.
* tests/aclocal-verbose-install.test: Likewise.
* tests/Makefile.am (TESTS): Update.

From a report by Javier Jard��n.
---
 ChangeLog                              |   23 ++++++++++++
 NEWS                                   |    4 ++
 THANKS                                 |    1 +
 aclocal.in                             |   13 ++++---
 tests/Makefile.am                      |    4 ++
 tests/Makefile.in                      |    4 ++
 tests/aclocal-install-fail.test        |   52 ++++++++++++++++++++++++++
 tests/aclocal-install-mkdir.test       |   62 ++++++++++++++++++++++++++++++++
 tests/aclocal-no-install-no-mkdir.test |   39 ++++++++++++++++++++
 tests/aclocal-verbose-install.test     |   53 +++++++++++++++++++++++++++
 10 files changed, 250 insertions(+), 5 deletions(-)
 create mode 100755 tests/aclocal-install-fail.test
 create mode 100755 tests/aclocal-install-mkdir.test
 create mode 100755 tests/aclocal-no-install-no-mkdir.test
 create mode 100755 tests/aclocal-verbose-install.test

diff --git a/ChangeLog b/ChangeLog
index ed9f35b..ae47a54 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,28 @@
 2011-03-14  Stefano Lattarini  <stefano.lattarini <at> gmail.com>
 
+	aclocal: create local directory where to install m4 files
+	Before this change, a call like `aclocal -I m4 --install' would
+	fail if the `m4' directory wasn't pre-existing.  This could be
+	particularly annoying when running in a checked-out version
+	from a VCS like git, which doesn't allow empty directories to
+	be tracked.
+	Closes automake bug#8168.
+	* aclocal.in (install_file): Change signature: take as second
+	argument the destination directory rather than the destination
+	file.  Crate the destination directory if it doesn't already
+	exist.  In verbose mode, tell what is being copied where.
+	(write_aclocal: Update.
+	* NEWS: Update.
+	* THANKS: Update.
+	* tests/aclocal-install-fail.test: New test.
+	* tests/aclocal-install-mkdir.test: Likewise.
+	* tests/aclocal-no-install-no-mkdir.test: Likewise.
+	* tests/aclocal-verbose-install.test: Likewise.
+	* tests/Makefile.am (TESTS): Update.
+	From a report by Javier Jard��n.
+
+2011-03-14  Stefano Lattarini  <stefano.lattarini <at> gmail.com>
+
 	aclocal: `-I' does not bail out on invalid directories.
 	Related to automake bug#8168.
 	* aclocal.in (scan_m4_dirs): If a user-specified "include
diff --git a/NEWS b/NEWS
index 953e62a..a1383b2 100644
--- a/NEWS
+++ b/NEWS
@@ -10,6 +10,10 @@ New in 1.11.0a:
   - aclocal does not issue a fatal error anymore if one of the directories
     specified with `-I' does not exist, or is not readable.
 
+  - If `aclocal --install' is used, and the first directory specified with
+    `-I' is non-existent, aclocal will now create it before trying to copy
+    files in it.
+
 Bugs fixed in 1.11.0a:
 
 * Bugs introduced by 1.11:
diff --git a/THANKS b/THANKS
index 60af1ee..fdf01b2 100644
--- a/THANKS
+++ b/THANKS
@@ -144,6 +144,7 @@ Janos Farkas		chexum <at> shadow.banki.hu
 Jared Davis		abiword <at> aiksaurus.com
 Jason Duell		jcduell <at> lbl.gov
 Jason Molenda		crash <at> cygnus.co.jp
+Javier Jard��n		jjardon <at> gnome.org
 Jeff Bailey		Jbailey <at> phn.ca
 Jeff Garzik		jgarzik <at> pobox.com
 Jeff Squyres		jsquyres <at> lam-mpi.org
diff --git a/aclocal.in b/aclocal.in
index 3b9beab..bc582d6 100644
--- a/aclocal.in
+++ b/aclocal.in
@@ -207,12 +207,15 @@ sub reset_maps ()
   undef &search;
 }
 
-# install_file ($SRC, $DEST)
+# install_file ($SRC, $DESTDIR)
 sub install_file ($$)
 {
-  my ($src, $dest) = @_;
+  my ($src, $destdir) = @_;
+  my $dest = $destdir . "/" . basename $src;
   my $diff_dest;
 
+  verb "installing $src to $dest";
+
   if ($force_output
       || !exists $file_contents{$dest}
       || $file_contents{$src} ne $file_contents{$dest})
@@ -265,6 +268,8 @@ sub install_file ($$)
 	}
       elsif (!$dry_run)
 	{
+	  xsystem ('mkdir', $destdir)
+            unless -d $destdir;
 	  xsystem ('cp', $src, $dest);
 	}
     }
@@ -778,9 +783,7 @@ sub write_aclocal ($@)
 	      my $dest;
 	      for my $ifile (@{$file_includes{$file}}, $file)
 		{
-		  $dest = "$user_includes[0]/" . basename $ifile;
-		  verb "installing $ifile to $dest";
-		  install_file ($ifile, $dest);
+		  install_file ($ifile, $user_includes[0]);
 		}
 	      $installed = 1;
 	    }
diff --git a/tests/Makefile.am b/tests/Makefile.am
index da4a037..f3a1fb1 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -62,6 +62,10 @@ acloca22.test \
 aclocal-bad-dirlist-include-dir.test \
 aclocal-bad-local-include-dir.test \
 aclocal-bad-system-include-dir.test \
+aclocal-install-fail.test \
+aclocal-install-mkdir.test \
+aclocal-no-install-no-mkdir.test \
+aclocal-verbose-install.test \
 acoutnoq.test \
 acoutpt.test \
 acoutpt2.test \
diff --git a/tests/Makefile.in b/tests/Makefile.in
index 6467f1a..40db068 100644
--- a/tests/Makefile.in
+++ b/tests/Makefile.in
@@ -332,6 +332,10 @@ acloca22.test \
 aclocal-bad-dirlist-include-dir.test \
 aclocal-bad-local-include-dir.test \
 aclocal-bad-system-include-dir.test \
+aclocal-install-fail.test \
+aclocal-install-mkdir.test \
+aclocal-no-install-no-mkdir.test \
+aclocal-verbose-install.test \
 acoutnoq.test \
 acoutpt.test \
 acoutpt2.test \
diff --git a/tests/aclocal-install-fail.test b/tests/aclocal-install-fail.test
new file mode 100755
index 0000000..7f672c0
--- /dev/null
+++ b/tests/aclocal-install-fail.test
@@ -0,0 +1,52 @@
+#! /bin/sh
+# Copyright (C) 2011 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that `aclocal --install' fails when it should.
+
+. ./defs || Exit 1
+
+set -e
+
+cat > configure.in <<END
+AC_INIT([$me], [1.0])
+MY_MACRO
+END
+
+mkdir dirlist-test
+cat > dirlist-test/my-defs.m4 <<END
+AC_DEFUN([MY_MACRO], [:])
+END
+
+: > a-regular-file
+mkdir unwritable-dir
+chmod a-w unwritable-dir
+
+ACLOCAL_TESTSUITE_FLAGS='-I a-regular-file' $ACLOCAL --install 2>stderr \
+  && { cat stderr >&2; Exit 1; }
+cat stderr >&2
+grep 'mkdir:.*a-regular-file' stderr
+
+ACLOCAL_TESTSUITE_FLAGS='-I unwritable-dir/sub' $ACLOCAL --install \
+  2>stderr && { cat stderr >&2; Exit 1; }
+cat stderr >&2
+grep 'mkdir:.*unwritable-dir/sub' stderr
+
+ACLOCAL_TESTSUITE_FLAGS='-I unwritable-dir' $ACLOCAL --install 2>stderr \
+  && { cat stderr >&2; Exit 1; }
+cat stderr >&2
+grep 'cp:.*unwritable-dir' stderr
+
+:
diff --git a/tests/aclocal-install-mkdir.test b/tests/aclocal-install-mkdir.test
new file mode 100755
index 0000000..e727f59
--- /dev/null
+++ b/tests/aclocal-install-mkdir.test
@@ -0,0 +1,62 @@
+#! /bin/sh
+# Copyright (C) 2011 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that `aclocal --install' create the local m4 directory if
+# necessary.
+
+. ./defs || Exit 1
+
+set -e
+
+cat > configure.in <<END
+AC_INIT([$me], [1.0])
+MY_MACRO
+END
+
+mkdir dirlist-test
+cat > dirlist-test/my-defs.m4 <<END
+AC_DEFUN([MY_MACRO], [:])
+END
+
+ACLOCAL_TESTSUITE_FLAGS='-I foo' $ACLOCAL --install
+ls -l . foo
+test -f foo/my-defs.m4
+
+cwd=`pwd`
+case $pwd in
+  *$sp*|*$tab*)
+    : cannot run this check
+    ;;
+  *)
+    ACLOCAL_TESTSUITE_FLAGS="-I $cwd/bar" $ACLOCAL --install
+    ls -l . bar
+    test -f bar/my-defs.m4
+    ;;
+esac
+
+mkdir zardoz
+# What should happen:
+#  * quux should be created, and required m4 files copied into there.
+#  * zardoz shouldn't be preferred to quux, if if the former exists while
+#    the latter does not.
+#  * blah shouldn't be uselessly created.
+ACLOCAL_TESTSUITE_FLAGS='-I quux -I zardoz -I blah' $ACLOCAL --install
+ls -l . quux zardoz
+grep MY_MACRO quux/my-defs.m4
+ls zardoz | grep . && Exit 1
+test -d blah || test -r blah && Exit 1
+
+:
diff --git a/tests/aclocal-no-install-no-mkdir.test b/tests/aclocal-no-install-no-mkdir.test
new file mode 100755
index 0000000..c826c5e
--- /dev/null
+++ b/tests/aclocal-no-install-no-mkdir.test
@@ -0,0 +1,39 @@
+#! /bin/sh
+# Copyright (C) 2011 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that `aclocal' do not create non-existent local m4 directory
+# if the `--install' option is not given.
+
+. ./defs || Exit 1
+
+set -e
+
+cat > configure.in <<END
+AC_INIT([$me], [1.0])
+MY_MACRO
+END
+
+mkdir dirlist-test
+cat > dirlist-test/my-defs.m4 <<END
+AC_DEFUN([MY_MACRO], [:])
+END
+
+# Ignore the exit status of aclocal; that is checked in other tests.
+ACLOCAL_TESTSUITE_FLAGS='-I foo' $ACLOCAL -I bar || :
+test ! -d foo && test ! -r foo
+test ! -d bar && test ! -r bar
+
+:
diff --git a/tests/aclocal-verbose-install.test b/tests/aclocal-verbose-install.test
new file mode 100755
index 0000000..5479ed4
--- /dev/null
+++ b/tests/aclocal-verbose-install.test
@@ -0,0 +1,53 @@
+#! /bin/sh
+# Copyright (C) 2011 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check verbose messages by `aclocal --install'.
+
+. ./defs || Exit 1
+
+set -e
+
+cat > configure.in <<END
+AC_INIT([$me], [1.0])
+MY_MACRO_BAR
+MY_MACRO_QUUX
+END
+
+mkdir dirlist-test
+cat > dirlist-test/bar.m4 <<END
+AC_DEFUN([MY_MACRO_BAR], [:])
+END
+cat > dirlist-test/quux.m4 <<END
+AC_DEFUN([MY_MACRO_QUUX], [:])
+END
+
+mkdir foodir
+: > foodir/bar.m4
+
+ACLOCAL_TESTSUITE_FLAGS='-I foodir' $ACLOCAL --install --verbose 2>stderr \
+ || { cat stderr >&2; Exit 1; }
+cat stderr >&2
+grep ' installing .*dirlist-test/bar\.m4.* to .*foodir/bar\.m4' stderr
+grep ' installing .*dirlist-test/quux\.m4.* to .*foodir/quux\.m4' stderr
+grep ' overwriting .*foodir/bar\.m4.* with .*dirlist-test/bar\.m4' stderr
+grep ' installing .*foodir/quux\.m4.* from .*dirlist-test/quux\.m4' stderr
+
+# Sanity checks.
+ls -l foodir
+grep MY_MACRO_BAR foodir/bar.m4
+grep MY_MACRO_QUUX foodir/quux.m4
+
+:
-- 
1.7.2.3

Added tag(s) patch. Request was from Stefano Lattarini <stefano.lattarini <at> gmail.com> to control <at> debbugs.gnu.org. (Mon, 14 Mar 2011 19:51:02 GMT) Full text and rfc822 format available.

Information forwarded to owner <at> debbugs.gnu.org, bug-automake <at> gnu.org:
bug#8168; Package automake. (Tue, 15 Mar 2011 14:41:01 GMT) Full text and rfc822 format available.

Message #15 received at 8168 <at> debbugs.gnu.org (full text, mbox):

From: Javier Jardón <jjardon <at> gnome.org>
To: Stefano Lattarini <stefano.lattarini <at> gmail.com>
Cc: 8168 <at> debbugs.gnu.org, automake-patches <at> gnu.org
Subject: Re: bug#8168: macros directory not created automatically
Date: Tue, 15 Mar 2011 11:30:55 +0100

On 14 March 2011 20:44, Stefano Lattarini <stefano.lattarini <at> gmail.com> wrote:
> [dropping automake list, adding automake-patches]
>
> References:
>  <http://lists.gnu.org/archive/html/automake/2011-03/msg00000.html>
>  <http://lists.gnu.org/archive/html/bug-automake/2011-03/msg00004.html>
>
> -*-*-
>
> Hello Javier and all automakes, and sorry for the delay.
>
...
>
> Javier, are you ok with your name being added to THANKS?

Sure, but I do not know if my contribution is enough to be in that file ;)

Thanks for working on this.

Regards
-- 
Javier Jardón Cabezas

Information forwarded to owner <at> debbugs.gnu.org, bug-automake <at> gnu.org:
bug#8168; Package automake. (Fri, 01 Apr 2011 07:46:02 GMT) Full text and rfc822 format available.

Message #18 received at 8168 <at> debbugs.gnu.org (full text, mbox):

From: Ralf Wildenhues <Ralf.Wildenhues <at> gmx.de>
To: Stefano Lattarini <stefano.lattarini <at> gmail.com>
Cc: 8168 <at> debbugs.gnu.org, Javier Jardón <jjardon <at> gnome.org>,
	automake-patches <at> gnu.org
Subject: Re: bug#8168: macros directory not created automatically
Date: Fri, 1 Apr 2011 09:45:41 +0200

* Stefano Lattarini wrote on Mon, Mar 14, 2011 at 08:44:03PM CET:
>   <http://lists.gnu.org/archive/html/automake/2011-03/msg00000.html>
>   <http://lists.gnu.org/archive/html/bug-automake/2011-03/msg00004.html>

> Attached is a patch series (2 patches) that should address the issue.
> The series is based off of maint -- as I'm not yet sure whether it
> would be better to merge it to master only, or to maint too.

> Ralf, OK to apply?

I'm debating a couple of questions:

Patch 2:
- Should `--install -I foo/bar/m4' create intermediate directories, or
  would we suspect a typo?
- Should `--install -I $dir' also create an absolute $dir?  Does it with
  your patch?  (I think "no" with both questions, but it would be good
  to be sure.)

Patch 1:
- Should the warning/erroring bits differentiate between relative or
  absolute directory names?  I'm considering to not warn at all about
  absolute names, as we might not have any control over the tree there.
- For a relative directory, a warning seems appropriate; and verb is not
  the right function for that.  The most fitting category would be
  syntax, barring anything better?  (And yes, that means aclocal -Werror
  would then error out, but that could be considered a good thing).
  But it seems 'verb' would be appropriate for absolute directories.

What do you think?

> If yes, where (maint, or master only)?

Hmm, 1/2 can be seen as a bug fix, but I'd regard 2/2 as a new feature.
Maybe base both of them off maint, and merge 1/2 to maint when we're
done with the semantics?

Further, a couple of trivial nits inline.

Thanks,
Ralf

> Subject: [PATCH 1/2] aclocal: `-I' does not bail out on invalid directories.
> 
> Related to automake bug#8168.
> 
> * aclocal.in (scan_m4_dirs): If a user-specified "include
> directory" is unreadable or non-existent, do not issue a
> fatal error anymore, but simply issue a warning, and only
> when running in verbose mode.
> * NEWS: Update.
> * tests/aclocal-bad-dirlist-include-dir.test: New test.
> * tests/aclocal-bad-local-include-dir.test: Likewise.
> * tests/aclocal-bad-system-include-dir.test: Likewise.
> * tests/Makefile.am (TESTS): Update.
> * tests/.gitignore: Update.

> --- a/NEWS
> +++ b/NEWS
> @@ -5,6 +5,11 @@ New in 1.11.0a:
>    - The `lzma' compression scheme and associated automake option `dist-lzma'
>      is obsoleted by `xz' and `dist-xz' due to upstream changes.
>  
> +* Changes to aclocal:
> +
> +  - aclocal does not issue a fatal error anymore if one of the directories
> +    specified with `-I' does not exist, or is not readable.
> +
>  Bugs fixed in 1.11.0a:
>  
>  * Bugs introduced by 1.11:
> diff --git a/aclocal.in b/aclocal.in
> index 2210fe3..3b9beab 100644
> --- a/aclocal.in
> +++ b/aclocal.in
> @@ -312,7 +312,15 @@ sub scan_m4_dirs ($@)
>      {
>        if (! opendir (DIR, $m4dir))
>  	{
> -	  fatal "couldn't open directory `$m4dir': $!";
> +	  if ($type == FT_USER)
> +	    {
> +	      verb "cannot open directory `$m4dir': $!";
> +	      next;
> +	    }
> +	  else
> +	    {
> +	      fatal "couldn't open directory `$m4dir': $!";
> +	    }
>  	}
>  
>        # We reverse the directory contents so that foo2.m4 gets

> --- /dev/null
> +++ b/tests/aclocal-bad-dirlist-include-dir.test

Wouldn't "bad-dirlist-include" be a long-enough and precise-enough name?

> +# Check that `aclocal' errors out when passed a non-readable directory
> +# with the `dirlist' file.
> +
> +. ./defs || Exit 1
> +
> +set -e
> +
> +cat > configure.in <<END
> +AC_INIT([$me], [1.0])
> +END
> +
> +mkdir dirlist-test
> +chmod a-r dirlist-test
> +ls -l disrlist-test && Exit 77

Typo disrlist

> +$ACLOCAL 2>stderr && { cat stderr >&2; Exit 1; }

Please add a comment before this line:
  # See the m4/dirlist file in the source tree.

which I needed to understand why the test was working at all.

> +cat stderr >&2
> +grep " couldn't open directory .*dirlist-test" stderr

> --- /dev/null
> +++ b/tests/aclocal-bad-local-include-dir.test

> +# Check that `aclocal' does not bail out when passed a non-existent
> +# or non-readable directory with the `-I' option.  Also check that
> +# warns appropriately when `--verbose' is used.

The second sentence is missing a subject ('it'?).

> +cat > configure.in <<END
> +AC_INIT([$me], [1.0])
> +MY_MACRO
> +NOT_A_MACRO
> +END
> +
> +mkdir m4
> +cat > m4/my-defs.m4 <<END
> +AC_DEFUN([MY_MACRO], [my--macro--expansion])
> +END
> +
> +mkdir unreadable unopenable unopenable/sub private
> +
> +cat > unreadable/not-defs.m4 <<END
> +AC_DEFUN([NOT_A_MACRO], [invalid--expansion])
> +END
> +cp unreadable/not-defs.m4 unopenable/sub
> +cp unreadable/not-defs.m4 private
> +
> +chmod a-r unreadable
> +chmod a-x unopenable
> +chmod a-rwx private
> +
> +ls -l unreadable && Exit 77
> +ls -l unopenable/sub && Exit 77
> +ls -l private && Exit 77
> +
> +aclocal_call ()
> +{
> +  $ACLOCAL -I m4 \
> +           -I non-existent \
> +           -I "$cwd"/non-existent \
> +           -I unreadable \
> +           -I "$cwd"/unreadable \
> +           -I unopenable/sub \
> +           -I "$cwd"/unopenable/sub \
> +           -I private \
> +           -I "$cwd"/private \
> +           ${1+"$@"} >stdout 2>stderr
> +}
> +
> +cwd=`pwd` || Exit 99
> +
> +aclocal_call && test ! -s stdout && test ! -s stderr \
> +  || { cat stdout; cat stderr >&2; Exit 1; }
> +
> +$EGREP '(MY_MACRO|my-defs\.m4)' aclocal.m4
> +$EGREP '(NOT_A_MACRO|not-defs\.m4)' aclocal.m4 && Exit 1
> +
> +$AUTOCONF
> +
> +$FGREP MY_MACRO configure && Exit 1
> +$FGREP my--macro--expansion configure
> +$FGREP NOT_A_MACRO configure
> +$FGREP invalid--expansion configure && Exit 1
> +
> +aclocal_call --verbose || { cat stdout; cat stderr >&2; Exit 1; }
> +cat stdout
> +cat stderr >&2
> +
> +for d in non-existent unreadable unopenable/sub private; do
> +  grep " cannot open .*$d" stderr
> +  grep " cannot open .*$cwd/$d" stderr
> +done

> --- /dev/null
> +++ b/tests/aclocal-bad-system-include-dir.test

> +# Check that `aclocal' errors out when passed a non-existent or
> +# non-readable directory with the `dirlist' file.

> +cat > configure.in <<END
> +AC_INIT([$me], [1.0])
> +END
> +
> +mkdir fake-acdir
> +chmod a-r fake-acdir
> +ls -l fake-acdir && Exit 77
> +
> +$ACLOCAL --acdir fake-acdir 2>stderr && { cat stderr >&2; Exit 1; }
> +cat stderr >&2
> +grep " couldn't open directory .*fake-acdir" stderr



> Subject: [PATCH 2/2] aclocal: create local directory where to install m4 files

> Before this change, a call like `aclocal -I m4 --install' would
> fail if the `m4' directory wasn't pre-existing.  This could be
> particularly annoying when running in a checked-out version
> from a VCS like git, which doesn't allow empty directories to
> be tracked.
> 
> Closes automake bug#8168.
> 
> * aclocal.in (install_file): Change signature: take as second
> argument the destination directory rather than the destination
> file.  Crate the destination directory if it doesn't already
> exist.  In verbose mode, tell what is being copied where.
> (write_aclocal: Update.
> * NEWS: Update.
> * THANKS: Update.
> * tests/aclocal-install-fail.test: New test.
> * tests/aclocal-install-mkdir.test: Likewise.
> * tests/aclocal-no-install-no-mkdir.test: Likewise.
> * tests/aclocal-verbose-install.test: Likewise.
> * tests/Makefile.am (TESTS): Update.

> --- a/NEWS
> +++ b/NEWS
> @@ -10,6 +10,10 @@ New in 1.11.0a:
>    - aclocal does not issue a fatal error anymore if one of the directories
>      specified with `-I' does not exist, or is not readable.
>  
> +  - If `aclocal --install' is used, and the first directory specified with
> +    `-I' is non-existent, aclocal will now create it before trying to copy
> +    files in it.
> +
>  Bugs fixed in 1.11.0a:
>  
>  * Bugs introduced by 1.11:

> --- a/aclocal.in
> +++ b/aclocal.in
> @@ -207,12 +207,15 @@ sub reset_maps ()
>    undef &search;
>  }
>  
> -# install_file ($SRC, $DEST)
> +# install_file ($SRC, $DESTDIR)
>  sub install_file ($$)
>  {
> -  my ($src, $dest) = @_;
> +  my ($src, $destdir) = @_;
> +  my $dest = $destdir . "/" . basename $src;
>    my $diff_dest;
>  
> +  verb "installing $src to $dest";
> +
>    if ($force_output
>        || !exists $file_contents{$dest}
>        || $file_contents{$src} ne $file_contents{$dest})
> @@ -265,6 +268,8 @@ sub install_file ($$)
>  	}
>        elsif (!$dry_run)
>  	{
> +	  xsystem ('mkdir', $destdir)
> +            unless -d $destdir;

Perl has a mkdir function, there is no need for xsystem.

>  	  xsystem ('cp', $src, $dest);
>  	}
>      }
> @@ -778,9 +783,7 @@ sub write_aclocal ($@)
>  	      my $dest;
>  	      for my $ifile (@{$file_includes{$file}}, $file)
>  		{
> -		  $dest = "$user_includes[0]/" . basename $ifile;
> -		  verb "installing $ifile to $dest";
> -		  install_file ($ifile, $dest);
> +		  install_file ($ifile, $user_includes[0]);
>  		}
>  	      $installed = 1;
>  	    }

> --- /dev/null
> +++ b/tests/aclocal-install-fail.test

> +# Check that `aclocal --install' fails when it should.

This test should use required=ro-dir I think.

> +. ./defs || Exit 1
> +
> +set -e
> +
> +cat > configure.in <<END
> +AC_INIT([$me], [1.0])
> +MY_MACRO
> +END
> +
> +mkdir dirlist-test
> +cat > dirlist-test/my-defs.m4 <<END
> +AC_DEFUN([MY_MACRO], [:])
> +END
> +
> +: > a-regular-file
> +mkdir unwritable-dir
> +chmod a-w unwritable-dir
> +
> +ACLOCAL_TESTSUITE_FLAGS='-I a-regular-file' $ACLOCAL --install 2>stderr \
> +  && { cat stderr >&2; Exit 1; }
> +cat stderr >&2
> +grep 'mkdir:.*a-regular-file' stderr
> +
> +ACLOCAL_TESTSUITE_FLAGS='-I unwritable-dir/sub' $ACLOCAL --install \
> +  2>stderr && { cat stderr >&2; Exit 1; }
> +cat stderr >&2
> +grep 'mkdir:.*unwritable-dir/sub' stderr
> +
> +ACLOCAL_TESTSUITE_FLAGS='-I unwritable-dir' $ACLOCAL --install 2>stderr \
> +  && { cat stderr >&2; Exit 1; }
> +cat stderr >&2
> +grep 'cp:.*unwritable-dir' stderr

> --- /dev/null
> +++ b/tests/aclocal-install-mkdir.test

> +# Check that `aclocal --install' create the local m4 directory if
> +# necessary.
> +
> +. ./defs || Exit 1
> +
> +set -e
> +
> +cat > configure.in <<END
> +AC_INIT([$me], [1.0])
> +MY_MACRO
> +END
> +
> +mkdir dirlist-test
> +cat > dirlist-test/my-defs.m4 <<END
> +AC_DEFUN([MY_MACRO], [:])
> +END
> +
> +ACLOCAL_TESTSUITE_FLAGS='-I foo' $ACLOCAL --install
> +ls -l . foo
> +test -f foo/my-defs.m4
> +
> +cwd=`pwd`
> +case $pwd in
> +  *$sp*|*$tab*)
> +    : cannot run this check
> +    ;;
> +  *)
> +    ACLOCAL_TESTSUITE_FLAGS="-I $cwd/bar" $ACLOCAL --install
> +    ls -l . bar
> +    test -f bar/my-defs.m4
> +    ;;
> +esac
> +
> +mkdir zardoz
> +# What should happen:
> +#  * quux should be created, and required m4 files copied into there.
> +#  * zardoz shouldn't be preferred to quux, if if the former exists while
> +#    the latter does not.
> +#  * blah shouldn't be uselessly created.
> +ACLOCAL_TESTSUITE_FLAGS='-I quux -I zardoz -I blah' $ACLOCAL --install
> +ls -l . quux zardoz
> +grep MY_MACRO quux/my-defs.m4
> +ls zardoz | grep . && Exit 1
> +test -d blah || test -r blah && Exit 1

> --- /dev/null
> +++ b/tests/aclocal-no-install-no-mkdir.test
> @@ -0,0 +1,39 @@

> +# Check that `aclocal' do not create non-existent local m4 directory

s/do/does/
a non-existent

> +# if the `--install' option is not given.
> +
> +. ./defs || Exit 1
> +
> +set -e
> +
> +cat > configure.in <<END
> +AC_INIT([$me], [1.0])
> +MY_MACRO
> +END
> +
> +mkdir dirlist-test
> +cat > dirlist-test/my-defs.m4 <<END
> +AC_DEFUN([MY_MACRO], [:])
> +END
> +
> +# Ignore the exit status of aclocal; that is checked in other tests.

Why?  Can't hurt to also test that it fails, no?

> +ACLOCAL_TESTSUITE_FLAGS='-I foo' $ACLOCAL -I bar || :
> +test ! -d foo && test ! -r foo
> +test ! -d bar && test ! -r bar

> --- /dev/null
> +++ b/tests/aclocal-verbose-install.test

> +# Check verbose messages by `aclocal --install'.
> +
> +. ./defs || Exit 1
> +
> +set -e
> +
> +cat > configure.in <<END
> +AC_INIT([$me], [1.0])
> +MY_MACRO_BAR
> +MY_MACRO_QUUX
> +END
> +
> +mkdir dirlist-test
> +cat > dirlist-test/bar.m4 <<END
> +AC_DEFUN([MY_MACRO_BAR], [:])
> +END
> +cat > dirlist-test/quux.m4 <<END
> +AC_DEFUN([MY_MACRO_QUUX], [:])
> +END
> +
> +mkdir foodir
> +: > foodir/bar.m4
> +
> +ACLOCAL_TESTSUITE_FLAGS='-I foodir' $ACLOCAL --install --verbose 2>stderr \
> + || { cat stderr >&2; Exit 1; }
> +cat stderr >&2
> +grep ' installing .*dirlist-test/bar\.m4.* to .*foodir/bar\.m4' stderr
> +grep ' installing .*dirlist-test/quux\.m4.* to .*foodir/quux\.m4' stderr
> +grep ' overwriting .*foodir/bar\.m4.* with .*dirlist-test/bar\.m4' stderr
> +grep ' installing .*foodir/quux\.m4.* from .*dirlist-test/quux\.m4' stderr
> +
> +# Sanity checks.
> +ls -l foodir
> +grep MY_MACRO_BAR foodir/bar.m4
> +grep MY_MACRO_QUUX foodir/quux.m4

Information forwarded to owner <at> debbugs.gnu.org, bug-automake <at> gnu.org:
bug#8168; Package automake. (Fri, 01 Apr 2011 13:14:02 GMT) Full text and rfc822 format available.

Message #21 received at 8168 <at> debbugs.gnu.org (full text, mbox):

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: Ralf Wildenhues <Ralf.Wildenhues <at> gmx.de>
Cc: 8168 <at> debbugs.gnu.org, Javier Jardón <jjardon <at> gnome.org>,
	automake-patches <at> gnu.org
Subject: Re: bug#8168: macros directory not created automatically
Date: Fri, 1 Apr 2011 15:13:19 +0200

On Friday 01 April 2011, Ralf Wildenhues wrote:
> * Stefano Lattarini wrote on Mon, Mar 14, 2011 at 08:44:03PM CET:
> >   <http://lists.gnu.org/archive/html/automake/2011-03/msg00000.html>
> >   <http://lists.gnu.org/archive/html/bug-automake/2011-03/msg00004.html>
> 
> > Attached is a patch series (2 patches) that should address the issue.
> > The series is based off of maint -- as I'm not yet sure whether it
> > would be better to merge it to master only, or to maint too.
> 
> > Ralf, OK to apply?
> 
> I'm debating a couple of questions:
> 
> Patch 2:
> - Should `--install -I foo/bar/m4' create intermediate directories, or
>   would we suspect a typo?
>
I'd say the latter.  It should be good enough for the all legitimate uses,
and will make the implementation slightly simpler (we can use the builtin
function `mkdir' instead of the `mkpath' function from module File::Path).
Also, in case the users start complaining about this limitation, we can
still easily lift it, without breaking backward-compatibility.

> - Should `--install -I $dir' also create an absolute $dir?
>
I think so.  Why shouldn't it?

> Does it with your patch?
>
Yes, and that is tested in `aclocal-install-mkdir.test'; see this hunk:

  cwd=`pwd`
  case $cwd in
    *$sp*|*$tab*)
      : cannot run this check
      ;;
    *)
      ACLOCAL_TESTSUITE_FLAGS="-I $cwd/bar" $ACLOCAL --install
      ls -l . bar
      test -f bar/my-defs.m4
      ;;
  esac

(Well, in fact there was a typo in this hunk, which I've corrected in
the meantime; see below).

> (I think "no" with both questions, but it would be good to be sure.)
>
The answer to both question is in truth "yes".  Good you asked :-)

> Patch 1:
> - Should the warning/erroring bits differentiate between relative or
>   absolute directory names?  I'm considering to not warn at all about
>   absolute names, as we might not have any control over the tree there.
>
I agree about not having a warning. But a message with `verb' (thus
displayed only when the user requests verbose output) would be useful
also in this case, no?

> - For a relative directory, a warning seems appropriate; and verb is
>   not the right function for that.
>
Well, in truth, I didn't intend for that message to be a warning -- it's
just a verbose mesage that can help in debugging.  I think `verb' is
appropriate for such an usage.  Probably I should fix the ChangeLog
entry to be more consistent with the intended behaviour; i.e., from

    * aclocal.in (scan_m4_dirs): If a user-specified "include
    directory" is unreadable or non-existent, do not issue a
    fatal error anymore, but simply issue a warning, and only
    when running in verbose mode.

to:

    * aclocal.in (scan_m4_dirs): If a user-specified "include
    directory" is unreadable or non-existent, do not issue a
    fatal error anymore, but only give a proper message when
    running in verbose mode.

Would that be better?

>   The most fitting category would be
>   syntax, barring anything better?  (And yes, that means aclocal -Werror
>   would then error out, but that could be considered a good thing).
>   But it seems 'verb' would be appropriate for absolute directories.
> 
> What do you think?
>
I think that we should behave similarly to how gcc, m4 and perl (and
probably many other programs) behave -- i.e., no warning on `-I' used
with non-existent directories, either relative or absolute:
  $ gcc -I /none foo.c
  $ m4 -I /none </dev/null
  $ perl -I /none -e '1;'

> > If yes, where (maint, or master only)?
> 
> Hmm, 1/2 can be seen as a bug fix, but I'd regard 2/2 as a new feature.
> Maybe base both of them off maint, and merge 1/2 to maint when we're
> done with the semantics?
>
Fine with me.

> Further, a couple of trivial nits inline.
> 
> Thanks,
> Ralf

> > Subject: [PATCH 1/2] aclocal: `-I' does not bail out on invalid directories.
> > 
> > Related to automake bug#8168.
> > 
> > * aclocal.in (scan_m4_dirs): If a user-specified "include
> > directory" is unreadable or non-existent, do not issue a
> > fatal error anymore, but simply issue a warning, and only
> > when running in verbose mode.
> > * NEWS: Update.
> > * tests/aclocal-bad-dirlist-include-dir.test: New test.
> > * tests/aclocal-bad-local-include-dir.test: Likewise.
> > * tests/aclocal-bad-system-include-dir.test: Likewise.
> > * tests/Makefile.am (TESTS): Update.
> > * tests/.gitignore: Update.

> > --- /dev/null
> > +++ b/tests/aclocal-bad-dirlist-include-dir.test
> 
> Wouldn't "bad-dirlist-include" be a long-enough and precise-enough name?
>
Hmm... I prefero to keep the `aclocal' in there, to keep the test
names esier to search.  So I went for `aclocal-bad-dirlist-incl.test'.
Similarly for the other two tests.

> > +# Check that `aclocal' errors out when passed a non-readable directory
> > +# with the `dirlist' file.
> > +
> > +. ./defs || Exit 1
> > +
> > +set -e
> > +
> > +cat > configure.in <<END
> > +AC_INIT([$me], [1.0])
> > +END
> > +
> > +mkdir dirlist-test
> > +chmod a-r dirlist-test
> > +ls -l disrlist-test && Exit 77
> 
> Typo disrlist
>
Oops, well spotted.  Fixed.

> > +$ACLOCAL 2>stderr && { cat stderr >&2; Exit 1; }
> 
> Please add a comment before this line:
>   # See the m4/dirlist file in the source tree.
> 
> which I needed to understand why the test was working at all.
>
Done (and I agree the comment is useful).

> > +cat stderr >&2
> > +grep " couldn't open directory .*dirlist-test" stderr


> > --- /dev/null
> > +++ b/tests/aclocal-bad-local-include-dir.test
> 
> > +# Check that `aclocal' does not bail out when passed a non-existent
> > +# or non-readable directory with the `-I' option.  Also check that
> > +# warns appropriately when `--verbose' is used.
> 
> The second sentence is missing a subject ('it'?).
>
Yep.  Fixed.

> > Subject: [PATCH 2/2] aclocal: create local directory where to install m4 files
> 
> > Before this change, a call like `aclocal -I m4 --install' would
> > fail if the `m4' directory wasn't pre-existing.  This could be
> > particularly annoying when running in a checked-out version
> > from a VCS like git, which doesn't allow empty directories to
> > be tracked.
> > 
> > Closes automake bug#8168.
> > 
> > * aclocal.in (install_file): Change signature: take as second
> > argument the destination directory rather than the destination
> > file.  Crate the destination directory if it doesn't already
> > exist.  In verbose mode, tell what is being copied where.
> > (write_aclocal: Update.
> > * NEWS: Update.
> > * THANKS: Update.
> > * tests/aclocal-install-fail.test: New test.
> > * tests/aclocal-install-mkdir.test: Likewise.
> > * tests/aclocal-no-install-no-mkdir.test: Likewise.
> > * tests/aclocal-verbose-install.test: Likewise.
> > * tests/Makefile.am (TESTS): Update.
> 
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -10,6 +10,10 @@ New in 1.11.0a:
> >    - aclocal does not issue a fatal error anymore if one of the directories
> >      specified with `-I' does not exist, or is not readable.
> >  
> > +  - If `aclocal --install' is used, and the first directory specified with
> > +    `-I' is non-existent, aclocal will now create it before trying to copy
> > +    files in it.
> > +
> >  Bugs fixed in 1.11.0a:
> >  
> >  * Bugs introduced by 1.11:
> 
> > --- a/aclocal.in
> > +++ b/aclocal.in
> > @@ -207,12 +207,15 @@ sub reset_maps ()
> >    undef &search;
> >  }
> >  
> > -# install_file ($SRC, $DEST)
> > +# install_file ($SRC, $DESTDIR)
> >  sub install_file ($$)
> >  {
> > -  my ($src, $dest) = @_;
> > +  my ($src, $destdir) = @_;
> > +  my $dest = $destdir . "/" . basename $src;
> >    my $diff_dest;
> >  
> > +  verb "installing $src to $dest";
> > +
> >    if ($force_output
> >        || !exists $file_contents{$dest}
> >        || $file_contents{$src} ne $file_contents{$dest})
> > @@ -265,6 +268,8 @@ sub install_file ($$)
> >  	}
> >        elsif (!$dry_run)
> >  	{
> > +	  xsystem ('mkdir', $destdir)
> > +            unless -d $destdir;
> 
> Perl has a mkdir function, there is no need for xsystem.
>
Agreed (and testcases updated accordingly).

> >  	  xsystem ('cp', $src, $dest);
> >  	}
> >      }
> > @@ -778,9 +783,7 @@ sub write_aclocal ($@)
> >  	      my $dest;
> >  	      for my $ifile (@{$file_includes{$file}}, $file)
> >  		{
> > -		  $dest = "$user_includes[0]/" . basename $ifile;
> > -		  verb "installing $ifile to $dest";
> > -		  install_file ($ifile, $dest);
> > +		  install_file ($ifile, $user_includes[0]);
> >  		}
> >  	      $installed = 1;
> >  	    }
 
> > --- /dev/null
> > +++ b/tests/aclocal-install-fail.test
> 
> > +# Check that `aclocal --install' fails when it should.
> 
> This test should use required=ro-dir I think.
>
Agreed. Fixed.

> > +. ./defs || Exit 1
> > +
> > +set -e
> > +
> > +cat > configure.in <<END
> > +AC_INIT([$me], [1.0])
> > +MY_MACRO
> > +END
> > +
> > +mkdir dirlist-test
> > +cat > dirlist-test/my-defs.m4 <<END
> > +AC_DEFUN([MY_MACRO], [:])
> > +END
> > +
> > +: > a-regular-file
> > +mkdir unwritable-dir
> > +chmod a-w unwritable-dir
> > +
> > +ACLOCAL_TESTSUITE_FLAGS='-I a-regular-file' $ACLOCAL --install 2>stderr \
> > +  && { cat stderr >&2; Exit 1; }
> > +cat stderr >&2
> > +grep 'mkdir:.*a-regular-file' stderr
> > +
> > +ACLOCAL_TESTSUITE_FLAGS='-I unwritable-dir/sub' $ACLOCAL --install \
> > +  2>stderr && { cat stderr >&2; Exit 1; }
> > +cat stderr >&2
> > +grep 'mkdir:.*unwritable-dir/sub' stderr
> > +
> > +ACLOCAL_TESTSUITE_FLAGS='-I unwritable-dir' $ACLOCAL --install 2>stderr \
> > +  && { cat stderr >&2; Exit 1; }
> > +cat stderr >&2
> > +grep 'cp:.*unwritable-dir' stderr


> > --- /dev/null
> > +++ b/tests/aclocal-install-mkdir.test
> 
> > +# Check that `aclocal --install' create the local m4 directory if
> > +# necessary.
> > +
> > +. ./defs || Exit 1
> > +
> > +set -e
> > +
> > +cat > configure.in <<END
> > +AC_INIT([$me], [1.0])
> > +MY_MACRO
> > +END
> > +
> > +mkdir dirlist-test
> > +cat > dirlist-test/my-defs.m4 <<END
> > +AC_DEFUN([MY_MACRO], [:])
> > +END
> > +
> > +ACLOCAL_TESTSUITE_FLAGS='-I foo' $ACLOCAL --install
> > +ls -l . foo
> > +test -f foo/my-defs.m4
> > +
> > +cwd=`pwd`
> > +case $pwd in
>
This should be `$cwd'.  Fixed.

> > +  *$sp*|*$tab*)
> > +    : cannot run this check
> > +    ;;
> > +  *)
> > +    ACLOCAL_TESTSUITE_FLAGS="-I $cwd/bar" $ACLOCAL --install
> > +    ls -l . bar
> > +    test -f bar/my-defs.m4
> > +    ;;
> > +esac
> > +
> > +mkdir zardoz
> > +# What should happen:
> > +#  * quux should be created, and required m4 files copied into there.
> > +#  * zardoz shouldn't be preferred to quux, if if the former exists while
> > +#    the latter does not.
> > +#  * blah shouldn't be uselessly created.
> > +ACLOCAL_TESTSUITE_FLAGS='-I quux -I zardoz -I blah' $ACLOCAL --install
> > +ls -l . quux zardoz
> > +grep MY_MACRO quux/my-defs.m4
> > +ls zardoz | grep . && Exit 1
> > +test -d blah || test -r blah && Exit 1


> > --- /dev/null
> > +++ b/tests/aclocal-no-install-no-mkdir.test
> > @@ -0,0 +1,39 @@
> 
> > +# Check that `aclocal' do not create non-existent local m4 directory
> 
> s/do/does/
> a non-existent
>
Fixed.

> > +# if the `--install' option is not given.
> > +
> > +. ./defs || Exit 1
> > +
> > +set -e
> > +
> > +cat > configure.in <<END
> > +AC_INIT([$me], [1.0])
> > +MY_MACRO
> > +END
> > +
> > +mkdir dirlist-test
> > +cat > dirlist-test/my-defs.m4 <<END
> > +AC_DEFUN([MY_MACRO], [:])
> > +END
> > +
> > +# Ignore the exit status of aclocal; that is checked in other tests.
> 
> Why?
>
Better separation of concerns between different tests (sorta).

> Can't hurt to also test that it fails, no?
>
As you prefer -- I really have no strong feelings here.

> > +ACLOCAL_TESTSUITE_FLAGS='-I foo' $ACLOCAL -I bar || :
> > +test ! -d foo && test ! -r foo
> > +test ! -d bar && test ! -r bar
> 

Thanks,
  Stefano

Information forwarded to owner <at> debbugs.gnu.org, bug-automake <at> gnu.org:
bug#8168; Package automake. (Fri, 01 Apr 2011 13:27:01 GMT) Full text and rfc822 format available.

Message #24 received at 8168 <at> debbugs.gnu.org (full text, mbox):

From: Ralf Wildenhues <Ralf.Wildenhues <at> gmx.de>
To: Stefano Lattarini <stefano.lattarini <at> gmail.com>
Cc: 8168 <at> debbugs.gnu.org, Javier Jardón <jjardon <at> gnome.org>,
	automake-patches <at> gnu.org
Subject: Re: bug#8168: macros directory not created automatically
Date: Fri, 1 Apr 2011 15:26:47 +0200

* Stefano Lattarini wrote on Fri, Apr 01, 2011 at 03:13:19PM CEST:
> On Friday 01 April 2011, Ralf Wildenhues wrote:
> > Patch 2:
> > - Should `--install -I foo/bar/m4' create intermediate directories, or
> >   would we suspect a typo?
> >
> I'd say the latter.  It should be good enough for the all legitimate uses,
> and will make the implementation slightly simpler (we can use the builtin
> function `mkdir' instead of the `mkpath' function from module File::Path).
> Also, in case the users start complaining about this limitation, we can
> still easily lift it, without breaking backward-compatibility.

OK.

> > - Should `--install -I $dir' also create an absolute $dir?
> >
> I think so.  Why shouldn't it?

Well, I don't understand what a legitimate use case would be, that's
why.  You need a relative path anyway for --install to copy files there.
aclocal won't install to an absolute first -I directory, because that
is usually not what was intended (it typically comes from having to work
around the non-existence of ACLOCAL_PATH by specifying ACLOCAL='aclocal
-I /some/system/dir').

Do you see know what it may not be a good idea to try to create such a
directory?

> > (I think "no" with both questions, but it would be good to be sure.)
> >
> The answer to both question is in truth "yes".  Good you asked :-)


> > Patch 1:
> > - Should the warning/erroring bits differentiate between relative or
> >   absolute directory names?  I'm considering to not warn at all about
> >   absolute names, as we might not have any control over the tree there.
> >
> I agree about not having a warning. But a message with `verb' (thus
> displayed only when the user requests verbose output) would be useful
> also in this case, no?

Sure.

> > - For a relative directory, a warning seems appropriate; and verb is
> >   not the right function for that.
> >
> Well, in truth, I didn't intend for that message to be a warning -- it's
> just a verbose mesage that can help in debugging.  I think `verb' is
> appropriate for such an usage.  Probably I should fix the ChangeLog
> entry to be more consistent with the intended behaviour; i.e., from
> 
>     * aclocal.in (scan_m4_dirs): If a user-specified "include
>     directory" is unreadable or non-existent, do not issue a
>     fatal error anymore, but simply issue a warning, and only
>     when running in verbose mode.
> 
> to:
> 
>     * aclocal.in (scan_m4_dirs): If a user-specified "include
>     directory" is unreadable or non-existent, do not issue a
>     fatal error anymore, but only give a proper message when
>     running in verbose mode.
> 
> Would that be better?

But why would a warning be inappropriate?  It can be turned off, and it
can signal a command-line typo.  Very few users use --verbose, and those
that do, do not run it in order to get notified of typos.

We don't have to work identical to a preprocessor here.  Unlike a
preprocessor, aclocal is much more at the whim of bogus (or missing)
macro files, because we scan all files in a directory, not just
explicitly included ones, so some control over the search path is a
good thing in general.

> >   The most fitting category would be
> >   syntax, barring anything better?  (And yes, that means aclocal -Werror
> >   would then error out, but that could be considered a good thing).
> >   But it seems 'verb' would be appropriate for absolute directories.
> > 
> > What do you think?
> >
> I think that we should behave similarly to how gcc, m4 and perl (and
> probably many other programs) behave -- i.e., no warning on `-I' used
> with non-existent directories, either relative or absolute:
>   $ gcc -I /none foo.c
>   $ m4 -I /none </dev/null
>   $ perl -I /none -e '1;'

See above.

> > > +	  xsystem ('mkdir', $destdir)
> > > +            unless -d $destdir;
> > 
> > Perl has a mkdir function, there is no need for xsystem.
> >
> Agreed (and testcases updated accordingly).

Be sure to check for errors.

Thanks,
Ralf

Information forwarded to owner <at> debbugs.gnu.org, bug-automake <at> gnu.org:
bug#8168; Package automake. (Fri, 01 Apr 2011 13:57:02 GMT) Full text and rfc822 format available.

Message #27 received at 8168 <at> debbugs.gnu.org (full text, mbox):

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: Ralf Wildenhues <Ralf.Wildenhues <at> gmx.de>
Cc: 8168 <at> debbugs.gnu.org, Javier Jardón <jjardon <at> gnome.org>,
	automake-patches <at> gnu.org
Subject: Re: bug#8168: macros directory not created automatically
Date: Fri, 1 Apr 2011 15:55:44 +0200

On Friday 01 April 2011, Ralf Wildenhues wrote:
> * Stefano Lattarini wrote on Fri, Apr 01, 2011 at 03:13:19PM CEST:
> > On Friday 01 April 2011, Ralf Wildenhues wrote:
> > > Patch 2:
> > > - Should `--install -I $dir' also create an absolute $dir?
> > >
> > I think so.  Why shouldn't it?
> 
> Well, I don't understand what a legitimate use case would be, that's
> why.  You need a relative path anyway for --install to copy files there.
> aclocal won't install to an absolute first -I directory, because that
> is usually not what was intended
>
Ahhh, now your position makes perfect sense, and I agree with it.
I'll fix the patch and the testuite shortly(ish).

BTW, I dimly remembered some non-obvious interaction between `--install'
and absolute dirnames, and I had looked at the documentation searching
for it before answering your previous, but nothing came up.  Now I see
that the behaviour is indeed documented, but not in a completely
"appropriate" place (it's only in the introduction to aclocal, not in
the documentation of the `--install' option).  Maybe this could be
improved.

> (it typically comes from having to work
> around the non-existence of ACLOCAL_PATH by specifying ACLOCAL='aclocal
> -I /some/system/dir').
> 
> Do you see know what it may not be a good idea to try to create such a
> directory?
>
Yes, and I agree with you now.

> > > Patch 1:
> > > - For a relative directory, a warning seems appropriate; and verb is
> > >   not the right function for that.
> > >
> > Well, in truth, I didn't intend for that message to be a warning -- it's
> > just a verbose mesage that can help in debugging.  I think `verb' is
> > appropriate for such an usage.  Probably I should fix the ChangeLog
> > entry to be more consistent with the intended behaviour; i.e., from
> > 
> >     * aclocal.in (scan_m4_dirs): If a user-specified "include
> >     directory" is unreadable or non-existent, do not issue a
> >     fatal error anymore, but simply issue a warning, and only
> >     when running in verbose mode.
> > 
> > to:
> > 
> >     * aclocal.in (scan_m4_dirs): If a user-specified "include
> >     directory" is unreadable or non-existent, do not issue a
> >     fatal error anymore, but only give a proper message when
> >     running in verbose mode.
> > 
> > Would that be better?
> 
> But why would a warning be inappropriate?  It can be turned off,
>
Only turning off other warnings too ...

> and it can signal a command-line typo.  Very few users use --verbose,
> and those that do, do not run it in order to get notified of typos.
>
Maybe a new warning category could be introduced?

> We don't have to work identical to a preprocessor here.  Unlike a
> preprocessor, aclocal is much more at the whim of bogus (or missing)
> macro files, because we scan all files in a directory, not just
> explicitly included ones, so some control over the search path is a
> good thing in general.
>
OK, you have a point here.

> > >   The most fitting category would be syntax, barring anything
> > >   better?
>
But we would need something better IMHO.

> > >   (And yes, that means aclocal -Werror
> > >   would then error out, but that could be considered a good thing).
>
Yes, but only if the warning categories have a sufficient granularity.

> > >   But it seems 'verb' would be appropriate for absolute directories.

 
> > > > +	  xsystem ('mkdir', $destdir)
> > > > +            unless -d $destdir;
> > > 
> > > Perl has a mkdir function, there is no need for xsystem.
> > >
> > Agreed (and testcases updated accordingly).
> 
> Be sure to check for errors.
>
I did (and the testsuite would have caught a missing check anyway ;-)


Regards,
   Stefano

Severity set to 'wishlist' from 'normal' Request was from Stefano Lattarini <stefano.lattarini <at> gmail.com> to control <at> debbugs.gnu.org. (Thu, 15 Mar 2012 16:31:01 GMT) Full text and rfc822 format available.

Merged 8168 10816. Request was from Stefano Lattarini <stefano.lattarini <at> gmail.com> to control <at> debbugs.gnu.org. (Thu, 15 Mar 2012 16:31:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-automake <at> gnu.org:
bug#8168; Package automake. (Thu, 15 Mar 2012 20:29:01 GMT) Full text and rfc822 format available.

Message #34 received at 8168 <at> debbugs.gnu.org (full text, mbox):

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: automake-patches <at> gnu.org
Cc: 8168 <at> debbugs.gnu.org, dwheeler <at> dwheeler.com, jjardon <at> gnome.org,
	10816 <at> debbugs.gnu.org
Subject: [PATCH] aclocal: create local directory where to install m4 files
Date: Thu, 15 Mar 2012 20:57:22 +0100

Fixes automake bug#8168 and bug#10816.

A call like "aclocal -I m4 --install" used to fail if the 'm4'
directory wasn't pre-existing.  This could be particularly
annoying when running in a checked-out version from a VCS like
git, which doesn't allow empty directories to be tracked.

* aclocal.in (File::Path): New import.
(scan_m4_dirs): Don't die if the first directory of type FT_USER
doesn't exist and the '--install' option was given; that directory
will be created later in ...
(install_file): ... here.  Change signature: now it takes as second
argument the destination directory rather than the destination file.
Crate the destination directory if it doesn't already exist.  In
verbose mode, tell what is being copied where.
(write_aclocal): Update.
* NEWS, THANKS: Update.
* tests/aclocal-install-fail.test: New test.
* tests/aclocal-install-mkdir.test: Likewise.
* tests/aclocal-no-install-no-mkdir.test: Likewise.
* tests/aclocal-verbose-install.test: Likewise.
* tests/list-of-tests.mk: Add them.

Signed-off-by: Stefano Lattarini <stefano.lattarini <at> gmail.com>
---

 I will push this in a couple of days if there is no objection.
 Thanks to David and Javier, and sorry for the awful delay.

 Regards,
   Stefano

 NEWS                                   |    4 ++
 THANKS                                 |    2 +
 aclocal.in                             |   40 +++++++++++++++----
 tests/aclocal-install-fail.test        |   65 ++++++++++++++++++++++++++++++++
 tests/aclocal-install-mkdir.test       |   62 ++++++++++++++++++++++++++++++
 tests/aclocal-no-install-no-mkdir.test |   39 +++++++++++++++++++
 tests/aclocal-verbose-install.test     |   54 ++++++++++++++++++++++++++
 tests/list-of-tests.mk                 |    4 ++
 8 files changed, 261 insertions(+), 9 deletions(-)
 create mode 100755 tests/aclocal-install-fail.test
 create mode 100755 tests/aclocal-install-mkdir.test
 create mode 100755 tests/aclocal-no-install-no-mkdir.test
 create mode 100755 tests/aclocal-verbose-install.test

diff --git a/NEWS b/NEWS
index b998ce4..2b5f68f 100644
--- a/NEWS
+++ b/NEWS
@@ -122,6 +122,10 @@ New in 1.11.0a:
     optional space between the -I, -L and -l options and their respective
     arguments, for better POSIX compliance.
 
+  - If "aclocal --install" is used, and the first directory specified with
+    '-I' is non-existent, aclocal will now create it before trying to copy
+    files in it.
+
 Bugs fixed in 1.11.0a:
 
 * Bugs introduced by 1.11.2:
diff --git a/THANKS b/THANKS
index 8363126..d7b3658 100644
--- a/THANKS
+++ b/THANKS
@@ -73,6 +73,7 @@ Dave Brolley		brolley <at> redhat.com
 Dave Korn		dave.korn.cygwin <at> googlemail.com
 Dave Morrison		dave <at> bnl.gov
 David A. Swierczek	swiercze <at> mr.med.ge.com
+David A. Wheeler	dwheeler <at> dwheeler.com
 David Byron		dbyron <at> dbyron.com
 Davyd Madeley		davyd <at> fugro-fsi.com.au
 David Pashley		david <at> davidpashley.com
@@ -149,6 +150,7 @@ Janos Farkas		chexum <at> shadow.banki.hu
 Jared Davis		abiword <at> aiksaurus.com
 Jason Duell		jcduell <at> lbl.gov
 Jason Molenda		crash <at> cygnus.co.jp
+Javier Jardón		jjardon <at> gnome.org
 Jeff Bailey		Jbailey <at> phn.ca
 Jeff Garzik		jgarzik <at> pobox.com
 Jeff Squyres		jsquyres <at> lam-mpi.org
diff --git a/aclocal.in b/aclocal.in
index ed1d8d9..ca15732 100644
--- a/aclocal.in
+++ b/aclocal.in
@@ -7,9 +7,7 @@ eval 'case $# in 0) exec @PERL@ -S "$0";; *) exec @PERL@ -S "$0" "$@";; esac'
 
 # aclocal - create aclocal.m4 by scanning configure.ac
 
-# Copyright (C) 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004,
-# 2005, 2006, 2007, 2008, 2009, 2010, 2011 Free Software Foundation,
-# Inc.
+# Copyright (C) 1996-2012 Free Software Foundation, Inc.
 
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -45,6 +43,7 @@ use Automake::FileUtils;
 use File::Basename;
 use File::stat;
 use Cwd;
+use File::Path ();
 
 # Some globals.
 
@@ -181,6 +180,17 @@ sub unlink_tmp
 $SIG{'INT'} = $SIG{'TERM'} = $SIG{'QUIT'} = $SIG{'HUP'} = 'unlink_tmp';
 END { unlink_tmp }
 
+sub xmkdir_p ($)
+{
+  my $dir = shift;
+  local $@ = undef;
+  return
+    if -d $dir or eval { File::Path::mkpath $dir };
+  chomp $@;
+  $@ =~ s/\s+at\s.*\bline\s\d+.*$//;
+  fatal "could not create directory '$dir': $@";
+}
+
 # Check macros in acinclude.m4.  If one is not used, warn.
 sub check_acinclude ()
 {
@@ -210,12 +220,15 @@ sub reset_maps ()
   undef &search;
 }
 
-# install_file ($SRC, $DEST)
+# install_file ($SRC, $DESTDIR)
 sub install_file ($$)
 {
-  my ($src, $dest) = @_;
+  my ($src, $destdir) = @_;
+  my $dest = $destdir . "/" . basename ($src);
   my $diff_dest;
 
+  verb "installing $src to $dest";
+
   if ($force_output
       || !exists $file_contents{$dest}
       || $file_contents{$src} ne $file_contents{$dest})
@@ -268,6 +281,7 @@ sub install_file ($$)
 	}
       elsif (!$dry_run)
 	{
+          xmkdir_p ($destdir);
 	  xsystem ('cp', $src, $dest);
 	}
     }
@@ -307,6 +321,7 @@ sub list_compare (\@\@)
 # --------------------------
 # Scan all M4 files installed in @DIRS for new macro definitions.
 # Register each file as of type $TYPE (one of the FT_* constants).
+my $first_user_m4dir = 1;
 sub scan_m4_dirs ($@)
 {
   my ($type, @dirlist) = @_;
@@ -315,7 +330,16 @@ sub scan_m4_dirs ($@)
     {
       if (! opendir (DIR, $m4dir))
 	{
-	  fatal "couldn't open directory `$m4dir': $!";
+	  if ($install && $type == FT_USER && $first_user_m4dir)
+            {
+              # We will try to create this directory later, so don't
+              # complain if it doesn't exist.
+              # TODO: maybe we should avoid complaining only if errno
+              # is ENONENT?
+              $first_user_m4dir = 0;
+              next;
+            }
+	  fatal "couldn't open directory '$m4dir': $!";
 	}
 
       # We reverse the directory contents so that foo2.m4 gets
@@ -773,9 +797,7 @@ sub write_aclocal ($@)
 	      my $dest;
 	      for my $ifile (@{$file_includes{$file}}, $file)
 		{
-		  $dest = "$user_includes[0]/" . basename $ifile;
-		  verb "installing $ifile to $dest";
-		  install_file ($ifile, $dest);
+		  install_file ($ifile, $user_includes[0]);
 		}
 	      $installed = 1;
 	    }
diff --git a/tests/aclocal-install-fail.test b/tests/aclocal-install-fail.test
new file mode 100755
index 0000000..cf493aa
--- /dev/null
+++ b/tests/aclocal-install-fail.test
@@ -0,0 +1,65 @@
+#! /bin/sh
+# Copyright (C) 2012 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that "aclocal --install" fails when it should.
+# FIXME: this is a good candidate for a conversion to TAP.
+
+am_create_testdir=empty
+required=ro-dir
+. ./defs || Exit 1
+
+set -e
+
+cat > configure.in <<END
+AC_INIT([$me], [1.0])
+MY_MACRO
+END
+
+mkdir sys-acdir
+cat > sys-acdir/my-defs.m4 <<END
+AC_DEFUN([MY_MACRO], [:])
+END
+
+ACLOCAL="$ACLOCAL -Wnone --system-acdir=sys-acdir"
+
+: > a-regular-file
+mkdir unwritable-dir
+chmod a-w unwritable-dir
+
+$ACLOCAL -I a-regular-file --install 2>stderr \
+  && { cat stderr >&2; Exit 1; }
+cat stderr >&2
+$EGREP '(mkdir:|directory ).*a-regular-file' stderr
+test ! -f aclocal.m4
+
+$ACLOCAL --install -I unwritable-dir/sub 2>stderr \
+  && { cat stderr >&2; Exit 1; }
+cat stderr >&2
+$EGREP '(mkdir:|directory ).*unwritable-dir/sub' stderr
+test ! -f aclocal.m4
+
+$ACLOCAL -I unwritable-dir --install 2>stderr \
+  && { cat stderr >&2; Exit 1; }
+cat stderr >&2
+$EGREP '(cp:|copy ).*unwritable-dir' stderr
+test ! -f aclocal.m4
+
+# Sanity check.
+mkdir m4
+$ACLOCAL -I m4 --install && test -f aclocal.m4 \
+  || fatal_ "aclocal failed also when expected to succeed"
+
+:
diff --git a/tests/aclocal-install-mkdir.test b/tests/aclocal-install-mkdir.test
new file mode 100755
index 0000000..61a3d19
--- /dev/null
+++ b/tests/aclocal-install-mkdir.test
@@ -0,0 +1,62 @@
+#! /bin/sh
+# Copyright (C) 2012 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that "aclocal --install" creates the local m4 directory if
+# necessary.
+# FIXME: this is a good candidate for a conversion to TAP.
+
+am_create_testdir=empty
+. ./defs || Exit 1
+
+set -e
+
+cat > configure.in <<END
+AC_INIT([$me], [1.0])
+MY_MACRO
+END
+
+mkdir sys-acdir
+cat > sys-acdir/my-defs.m4 <<END
+AC_DEFUN([MY_MACRO], [:])
+END
+
+ACLOCAL="$ACLOCAL --system-acdir=sys-acdir"
+
+$ACLOCAL -I foo --install
+test -f foo/my-defs.m4
+
+$ACLOCAL --install -I "`pwd`/bar"
+test -f bar/my-defs.m4
+
+$ACLOCAL --install -I baz/sub/sub2
+test -f baz/sub/sub2/my-defs.m4
+
+mkdir zardoz
+# What should happen:
+#  * quux should be created, and required m4 files copied into there.
+#  * zardoz shouldn't be preferred to quux, even if the former exists
+#    while the latter does not.
+$ACLOCAL --install -I quux -I zardoz
+test -d quux
+grep MY_MACRO quux/my-defs.m4
+ls zardoz | grep . && Exit 1
+
+# It's better if aclocal doesn't create the first include dir on failure.
+$ACLOCAL --install -I none -I none2 && Exit 1
+test ! -d none
+test ! -d none2
+
+:
diff --git a/tests/aclocal-no-install-no-mkdir.test b/tests/aclocal-no-install-no-mkdir.test
new file mode 100755
index 0000000..73a6116
--- /dev/null
+++ b/tests/aclocal-no-install-no-mkdir.test
@@ -0,0 +1,39 @@
+#! /bin/sh
+# Copyright (C) 2012 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that aclocal does not create a non-existent local m4 directory
+# if the '--install' option is not given.
+
+am_create_testdir=empty
+. ./defs || Exit 1
+
+set -e
+
+cat > configure.in <<END
+AC_INIT([$me], [1.0])
+MY_MACRO
+END
+
+mkdir sys-acdir
+cat > sys-acdir/my-defs.m4 <<END
+AC_DEFUN([MY_MACRO], [:])
+END
+
+$ACLOCAL -I foo --system-acdir=sys-acdir && Exit 1
+test ! -d foo
+test ! -r foo
+
+:
diff --git a/tests/aclocal-verbose-install.test b/tests/aclocal-verbose-install.test
new file mode 100755
index 0000000..589d540
--- /dev/null
+++ b/tests/aclocal-verbose-install.test
@@ -0,0 +1,54 @@
+#! /bin/sh
+# Copyright (C) 2011 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check verbose messages by `aclocal --install'.
+
+am_create_testdir=empty
+. ./defs || Exit 1
+
+set -e
+
+cat > configure.in <<END
+AC_INIT([$me], [1.0])
+MY_MACRO_BAR
+MY_MACRO_QUUX
+END
+
+mkdir sys-acdir
+cat > sys-acdir/bar.m4 <<END
+AC_DEFUN([MY_MACRO_BAR], [:])
+END
+cat > sys-acdir/quux.m4 <<END
+AC_DEFUN([MY_MACRO_QUUX], [:])
+END
+
+mkdir foodir
+: > foodir/bar.m4
+
+$ACLOCAL --system-acdir=sys-acdir --install --verbose -I foodir 2>stderr \
+ || { cat stderr >&2; Exit 1; }
+cat stderr >&2
+grep ' installing .*sys-acdir/bar\.m4.* to .*foodir/bar\.m4' stderr
+grep ' installing .*sys-acdir/quux\.m4.* to .*foodir/quux\.m4' stderr
+grep ' overwriting .*foodir/bar\.m4.* with .*sys-acdir/bar\.m4' stderr
+grep ' installing .*foodir/quux\.m4.* from .*sys-acdir/quux\.m4' stderr
+
+# Sanity checks.
+ls -l foodir
+grep MY_MACRO_BAR foodir/bar.m4
+grep MY_MACRO_QUUX foodir/quux.m4
+
+:
diff --git a/tests/list-of-tests.mk b/tests/list-of-tests.mk
index b5a604c..f9109c0 100644
--- a/tests/list-of-tests.mk
+++ b/tests/list-of-tests.mk
@@ -64,6 +64,10 @@ aclocal-path-install.test \
 aclocal-path-install-serial.test \
 aclocal-path-nonexistent.test \
 aclocal-path-precedence.test \
+aclocal-install-fail.test \
+aclocal-install-mkdir.test \
+aclocal-no-install-no-mkdir.test \
+aclocal-verbose-install.test \
 acoutnoq.test \
 acoutpt.test \
 acoutpt2.test \
-- 
1.7.9

Message #35 received at 8168-done <at> debbugs.gnu.org (full text, mbox):

From: Stefano Lattarini <stefano.lattarini <at> gmail.com>
To: automake-patches <at> gnu.org
Cc: 10816-done <at> debbugs.gnu.org, 8168-done <at> debbugs.gnu.org,
	dwheeler <at> dwheeler.com, jjardon <at> gnome.org
Subject: Re: bug#10816: [PATCH] aclocal: create local directory where to
	install m4 files
Date: Sat, 17 Mar 2012 09:46:30 +0100

On 03/15/2012 08:57 PM, Stefano Lattarini wrote:
> Fixes automake bug#8168 and bug#10816.
> 
> A call like "aclocal -I m4 --install" used to fail if the 'm4'
> directory wasn't pre-existing.  This could be particularly
> annoying when running in a checked-out version from a VCS like
> git, which doesn't allow empty directories to be tracked.
> 
> * aclocal.in (File::Path): New import.
> (scan_m4_dirs): Don't die if the first directory of type FT_USER
> doesn't exist and the '--install' option was given; that directory
> will be created later in ...
> (install_file): ... here.  Change signature: now it takes as second
> argument the destination directory rather than the destination file.
> Crate the destination directory if it doesn't already exist.  In
> verbose mode, tell what is being copied where.
> (write_aclocal): Update.
> * NEWS, THANKS: Update.
> * tests/aclocal-install-fail.test: New test.
> * tests/aclocal-install-mkdir.test: Likewise.
> * tests/aclocal-no-install-no-mkdir.test: Likewise.
> * tests/aclocal-verbose-install.test: Likewise.
> * tests/list-of-tests.mk: Add them.
> 
> Signed-off-by: Stefano Lattarini <stefano.lattarini <at> gmail.com>
> ---
> 
>  I will push this in a couple of days if there is no objection.
>  Thanks to David and Javier, and sorry for the awful delay.
> 
No objection seen so far, so I've pushed this patch (with minor
adjustments, see commit a75a1a52).  I'm also closing the relevant
bug reports.

Regards,
  Stefano

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Sat, 14 Apr 2012 11:24:02 GMT) Full text and rfc822 format available.

This bug report was last modified 13 years and 127 days ago.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.

GNU bug report logs - #8168 macros directory not created automatically

GNU bug report logs - #8168
macros directory not created automatically