[Automake-NG] [PATCH 4/5] [ng] uninstall: reimplement various recipes using more GNU make features

[Stefano Lattarini]

This change likely introduces subtle semantic changes in corner cases
and tricky situation.  Given the simplifications and improved uniformity
it offers, we consider that completely acceptable.

With this change, the test 't/instmany-python.sh' passes once again.

* lib/am/header-vars.mk (am.uninst.cmd): New private make function.
(am.uninst.cmd.aux): Likewise, used internally by the above (and
to be considered an implementation detail of it).
(am__uninstall_files_from_dir): Delete, no more needed.
* lib/am/data.am: Rewrite the uninstall recipe(s) to take advantage
of $(am.uninst.cmd).
* lib/am/libs.am: Likewise.
* lib/am/lisp.am: Likewise.
* lib/am/mans.am: Likewise.
* lib/am/python.am: Likewise.
* lib/am/progs.am: Likewise.
* lib/am/scripts.am: Likewise.
* lib/am/texinfos.am: Likewise (and also of other unrelated GNU make
features while we are at it).

Signed-off-by: Stefano Lattarini <address@hidden>
---
 lib/am/data.am        |  8 ++++----
 lib/am/header-vars.mk | 36 +++++++++++++++++++++++-------------
 lib/am/libs.am        |  8 ++++----
 lib/am/lisp.am        | 13 ++++++-------
 lib/am/mans.am        | 16 ++++++++--------
 lib/am/progs.am       | 23 +++++++++++------------
 lib/am/python.am      | 11 +++++------
 lib/am/scripts.am     | 16 +++++++++-------
 lib/am/texinfos.am    | 22 +++++++---------------
 9 files changed, 77 insertions(+), 76 deletions(-)

diff --git a/lib/am/data.am b/lib/am/data.am
index 9b28d20..dd604d4 100644
--- a/lib/am/data.am
+++ b/lib/am/data.am
@@ -73,10 +73,10 @@ if %?INSTALL%
 .PHONY uninstall-am: uninstall-%DIR%%PRIMARY%
 uninstall-%DIR%%PRIMARY%:
        @$(NORMAL_UNINSTALL)
-       @$(if $(and $(%DIR%_%PRIMARY%),$(%NDIR%dir)), \
-?BASE? files='$(notdir $(%DIR%_%PRIMARY%))'; \
-?!BASE?        files='$(patsubst $(srcdir)/%,%,$(%DIR%_%PRIMARY%))'; \
-       dir='$(DESTDIR)$(%NDIR%dir)'; $(am__uninstall_files_from_dir))
+       $(call am.uninst.cmd,$(%NDIR%dir), \
+?BASE?   $(notdir $(%DIR%_%PRIMARY%)) \
+?!BASE?          $(patsubst $(srcdir)/%,%,$(%DIR%_%PRIMARY%)) \
+       )
 endif %?INSTALL%
 
 
diff --git a/lib/am/header-vars.mk b/lib/am/header-vars.mk
index 92b74f6..4ae77e9 100644
--- a/lib/am/header-vars.mk
+++ b/lib/am/header-vars.mk
@@ -381,13 +381,15 @@ am__base_list = \
   sed '$$!N;$$!N;$$!N;$$!N;$$!N;$$!N;$$!N;s/\n/ /g' | \
   sed '$$!N;$$!N;$$!N;$$!N;s/\n/ /g'
 
-# A shell code fragment to uninstall files from a given directory.
-# It expects the $dir and $files shell variables to be defined respectively
-# to the directory where the files to be removed are, and to the list of
-# such files.
-# Some rm implementations complain if 'rm -f' is used without arguments,
-# so the fist "test -z" check (FIXME: this is probably obsolete; see
-# automake bug#10828).
+# $(call am.uninst.cmd,DIR,FILES,[RM-OPTS])
+# -----------------------------------------
+# Uninstall the given files from the given directory, avoiding to hit
+# command line length limits, and honoring $(DESTDIR).  If the given DIR
+# is actually an empty name, or it it refers to a non-existing file, it
+# is assumed nothing is to be removed.  The RM-OPTS (if present) are
+# passed to the rm invocation removing the files (this ca be useful in
+# case such files are actually directories (as happens with the HTML
+# documentation), in which case rm should be passed the '-r' option.
 # At least Solaris /bin/sh still lacks 'test -e', so we use the multiple
 # "test ! -[fdr]" below instead (FIXME: this should become obsolete when
 # we can assume the $SHELL set by Autoconf-generated configure scripts is
@@ -396,9 +398,17 @@ am__base_list = \
 # We expect $dir to be either non-existent or a directory, so the
 # failure we'll experience if it is a regular file is indeed desired
 # and welcome (better to fail loudly than silently).
-am__uninstall_files_from_dir = { \
-  test -z "$$files" \
-    || { test ! -d "$$dir" && test ! -f "$$dir" && test ! -r "$$dir"; } \
-    || { echo " ( cd '$$dir' && rm -f" $$files ")"; \
-         cd "$$dir" && rm -f $$files; }; \
-  }
+# Similarly to the 'am.clean-cmd.f' above, this function is only meant to
+# be used in a "sub-recipe" by its own.
+am.uninst.cmd.aux = \
+  $(if $(and $2,$1), \
+    { test ! -d '$(DESTDIR)'$2 \
+      && test ! -f '$(DESTDIR)'$2 \
+      && test ! -r '$(DESTDIR)'$2; } \
+    || { \
+      echo " cd '$(DESTDIR)$2' && rm -f $1" \
+        && cd '$(DESTDIR)$2' \
+       && rm -f$(if $3, $3) $1; }$(am.chars.newline))
+am.uninst.cmd = \
+  @$(call am.xargs-map,$0.aux,$(strip $2),$(strip $1),$(strip $3))
+
diff --git a/lib/am/libs.am b/lib/am/libs.am
index d5b0b63..6ca7dad 100644
--- a/lib/am/libs.am
+++ b/lib/am/libs.am
@@ -86,10 +86,10 @@ if %?INSTALL%
 .PHONY uninstall-am: uninstall-%DIR%LIBRARIES
 uninstall-%DIR%LIBRARIES:
        @$(NORMAL_UNINSTALL)
-       @$(if $(and $(%DIR%_LIBRARIES),$(%NDIR%dir)), \
-?BASE? files='$(notdir $(%DIR%_LIBRARIES))'; \
-?!BASE?        files='$(patsubst $(srcdir)/%,%,$(%DIR%_LIBRARIES))'; \
-       dir='$(DESTDIR)$(%NDIR%dir)'; $(am__uninstall_files_from_dir))
+       $(call am.uninst.cmd,$(%NDIR%dir), \
+?BASE?   $(notdir $(%DIR%_LIBRARIES)) \
+?!BASE?          $(patsubst $(srcdir)/%,%,$(%DIR%_LIBRARIES)) \
+       )
 endif %?INSTALL%
 
 
diff --git a/lib/am/lisp.am b/lib/am/lisp.am
index ab7f0c3..0c40354 100644
--- a/lib/am/lisp.am
+++ b/lib/am/lisp.am
@@ -88,14 +88,13 @@ if %?INSTALL%
 uninstall-%DIR%LISP:
        @$(NORMAL_UNINSTALL)
 ## Do not uninstall anything if EMACS was not found.
-       @test "$(EMACS)" != no || exit 0; \
-       $(if $(and $(%DIR%_LISP),$(%NDIR%dir)), \
-       files='$(foreach i,\
-?BASE?          $(notdir $(%DIR%_LISP)), \
-?!BASE?                 $(patsubst $(srcdir)/%,%,$(%DIR%_LISP)), \
+## FIXME: we should actually check more strictly for $(EMACS) = "no".
+       $(call am.uninst.cmd,$(if $(filter no,$(EMACS)),,$(%NDIR%dir)), \
+          $(foreach i,\
+?BASE?      $(notdir $(%DIR%_LISP)), \
+?!BASE?             $(patsubst $(srcdir)/%,%,$(%DIR%_LISP)), \
 ## Also remove the '.elc' byte-compiled versions (if any).
-                $(i) $(i)c)'; \
-       dir='$(DESTDIR)$(%NDIR%dir)'; $(am__uninstall_files_from_dir))
+            $(i) $(i)c))
 endif %?INSTALL%
 
 
diff --git a/lib/am/mans.am b/lib/am/mans.am
index dc08f27..32ae50f 100644
--- a/lib/am/mans.am
+++ b/lib/am/mans.am
@@ -125,21 +125,22 @@ uninstall-man%SECTION%:
        @$(NORMAL_UNINSTALL)
 if %?NOTRANS_MANS%
 ## Handle MANS with notrans_ prefix
-       @list='%NOTRANS_SECT_LIST%'; test -n "$(man%SECTION%dir)" || exit 0; \
-       files=`{ for i in $$list; do echo "$$i"; done; \
+       $(call am.uninst.cmd,$(man%SECTION%dir),$(shell \
+       { list='%NOTRANS_SECT_LIST%'; \
+       for i in $$list; do echo "$$i"; done; \
 ## Extract all items from notrans_man_MANS that should go in this section.
 ## This must be done dynamically to support conditionals.
 ?HAVE_NOTRANS? l2='%NOTRANS_LIST%'; for i in $$l2; do echo "$$i"; done | \
 ## Accept for 'man1' files like 'foo.1c' but not 'sub.1/foo.2' or 'foo-2.1.4'.
 ?HAVE_NOTRANS?   sed -n '/\.%SECTION%[a-z]*$$/p'; \
 ## Extract basename of manpage, change the extension if needed.
-       } | sed 's,.*/,,;s,\.[^%SECTION%][0-9a-z]*$$,.%SECTION%,'`; \
-       dir='$(DESTDIR)$(man%SECTION%dir)'; $(am__uninstall_files_from_dir)
+       } | sed -e 's,.*/,,;s,\.[^%SECTION%][0-9a-z]*$$,.%SECTION%,'))
 endif %?NOTRANS_MANS%
 if %?TRANS_MANS%
 ## Handle MANS without notrans_ prefix
-       @list='%TRANS_SECT_LIST%'; test -n "$(man%SECTION%dir)" || exit 0; \
-       files=`{ for i in $$list; do echo "$$i"; done; \
+       $(call am.uninst.cmd,$(man%SECTION%dir),$(shell \
+       { list='%TRANS_SECT_LIST%'; \
+       for i in $$list; do echo "$$i"; done; \
 ## Extract all items from man_MANS that should go in this section.
 ## This must be done dynamically to support conditionals.
 ?HAVE_TRANS?   l2='%TRANS_LIST%'; for i in $$l2; do echo "$$i"; done | \
@@ -148,6 +149,5 @@ if %?TRANS_MANS%
 ## Extract basename of manpage, run it through the program rename
 ## transform, and change the extension if needed.
        } | sed -e 's,.*/,,;h;s,.*\.,,;s,^[^%SECTION%][0-9a-z]*$$,%SECTION%,;x' 
\
-             -e 's,\.[0-9a-z]*$$,,;$(transform);G;s,\n,.,'`; \
-       dir='$(DESTDIR)$(man%SECTION%dir)'; $(am__uninstall_files_from_dir)
+               -e 's,\.[0-9a-z]*$$,,;$(transform);G;s,\n,.,'))
 endif %?TRANS_MANS%
diff --git a/lib/am/progs.am b/lib/am/progs.am
index 6dac5a8..a221a66 100644
--- a/lib/am/progs.am
+++ b/lib/am/progs.am
@@ -84,18 +84,17 @@ if %?INSTALL%
 .PHONY uninstall-am: uninstall-%DIR%PROGRAMS
 uninstall-%DIR%PROGRAMS:
        @$(NORMAL_UNINSTALL)
-       @list='$(%DIR%_PROGRAMS)'; test -n "$(%NDIR%dir)" || list=; \
-       files=`for p in $$list; do echo "$$p"; done | \
-## Remove any leading directory before applying $(transform),
-## but keep the directory part in the hold buffer, in order to
-## reapply it again afterwards in the nobase case.  Append $(EXEEXT).
-         sed -e 'h;s,^.*/,,;s/$(EXEEXT)$$//;$(transform)' \
-             -e 's/$$/$(EXEEXT)/' \
-?!BASE?              -e 'x;s,[^/]*$$,,;G;s,\n,,' \
-       `; \
-       test -n "$$list" || exit 0; \
-       echo " ( cd '$(DESTDIR)$(%NDIR%dir)' && rm -f" $$files ")"; \
-       cd "$(DESTDIR)$(%NDIR%dir)" && rm -f $$files
+## The need to apply $(transform) is quite tricky, and forces us to
+## go though a $(shell) invocation.
+       $(call am.uninst.cmd,$(%NDIR%dir),$(shell \
+         list='$(%DIR%_PROGRAMS)'; for p in $$list; do echo "$$p"; done | \
+## Remove any leading directory before applying $(transform), but keep
+## the directory part in the hold buffer, in order to reapply it again
+## afterwards in the nobase case.  Append $(EXEEXT).
+           sed -e 'h;s,^.*/,,;s/$(EXEEXT)$$//;$(transform)' \
+               -e 's/$$/$(EXEEXT)/' \
+?!BASE?                -e 'x;s,[^/]*$$,,;G;s,\n,,' \
+        ))
 endif %?INSTALL%
 
 
diff --git a/lib/am/python.am b/lib/am/python.am
index 026b2d9..fb5ae50 100644
--- a/lib/am/python.am
+++ b/lib/am/python.am
@@ -78,13 +78,12 @@ if %?INSTALL%
 .PHONY uninstall-am: uninstall-%DIR%PYTHON
 uninstall-%DIR%PYTHON:
        @$(NORMAL_UNINSTALL)
-       @$(if $(and $(%DIR%_PYTHON),$(%NDIR%dir)), \
-       files='$(foreach i,\
-?BASE?          $(notdir $(%DIR%_PYTHON)), \
-?!BASE?                 $(patsubst $(srcdir)/%,%,$(%DIR%_PYTHON)), \
+       $(call am.uninst.cmd,$(%NDIR%dir), \
+         $(foreach i,\
+?BASE?     $(notdir $(%DIR%_PYTHON)), \
+?!BASE?            $(patsubst $(srcdir)/%,%,$(%DIR%_PYTHON)), \
 ## Also remove the '.pyc' and '.py'o byte-compiled versions.
-                $(i) $(i)c $(i)o)'; \
-       dir='$(DESTDIR)$(%NDIR%dir)'; $(am__uninstall_files_from_dir))
+             $(i) $(i)c $(i)o))
 endif %?INSTALL%
 
 
diff --git a/lib/am/scripts.am b/lib/am/scripts.am
index 1d453ab..4054ba5 100644
--- a/lib/am/scripts.am
+++ b/lib/am/scripts.am
@@ -54,13 +54,15 @@ if %?INSTALL%
 .PHONY uninstall-am: uninstall-%DIR%SCRIPTS
 uninstall-%DIR%SCRIPTS:
        @$(NORMAL_UNINSTALL)
-       @test -n '$(and $(%DIR%_SCRIPTS),$(%NDIR%dir))' || exit 0; \
-?BASE? files='$(notdir $(%DIR%_SCRIPTS))'; \
-?!BASE?        files='$(patsubst $(srcdir)/%,%,$(%DIR%_SCRIPTS))'; \
-       files=`for f in $$files; do echo "$$f"; done | sed -e \
-?BASE?           '$(transform)'`; \
-?!BASE?                  'h;s,.*/,,;$(transform);x;s|[^/]*$$||;G;s,\n,,'`; \
-       dir='$(DESTDIR)$(%NDIR%dir)'; $(am__uninstall_files_from_dir)
+## The need to apply $(transform) is quite tricky, and forces us to
+## go though a $(shell) invocation.
+       $(call am.uninst.cmd,$(%NDIR%dir),$(shell \
+?BASE?   files='$(notdir $(%DIR%_SCRIPTS))' && \
+?!BASE?          files='$(patsubst $(srcdir)/%,%,$(%DIR%_SCRIPTS))' && \
+         for f in $$files; do echo "$$f"; done | sed -e \
+?BASE?     '$(transform)' \
+?!BASE?            'h;s,.*/,,;$(transform);x;s|[^/]*$$||;G;s,\n,,' \
+        ))
 endif %?INSTALL%
 
 
diff --git a/lib/am/texinfos.am b/lib/am/texinfos.am
index 116053c..9ad7d39 100644
--- a/lib/am/texinfos.am
+++ b/lib/am/texinfos.am
@@ -233,29 +233,27 @@ if %?LOCAL-TEXIS%
 
 uninstall-dvi-am:
        @$(NORMAL_UNINSTALL)
-       $(if $(and $(DVIS),$(dvidir)),rm -f $(addprefix 
'$(DESTDIR)$(dvidir)'/,$(notdir $(DVIS))))
+       $(call am.uninst.cmd,$(dvidir),$(notdir $(DVIS)))
 
 uninstall-pdf-am:
        @$(NORMAL_UNINSTALL)
-       $(if $(and $(PDFS),$(pdfdir)),rm -f $(addprefix 
'$(DESTDIR)$(pdfdir)'/,$(notdir $(PDFS))))
+       $(call am.uninst.cmd,$(pdfdir),$(notdir $(PDFS)))
 
 uninstall-ps-am:
        @$(NORMAL_UNINSTALL)
-       $(if $(and $(PSS),$(psdir)),rm -f $(addprefix 
'$(DESTDIR)$(psdir)'/,$(notdir $(PSS))))
+       $(call am.uninst.cmd,$(psdir),$(notdir $(PSS)))
 
 uninstall-html-am:
        @$(NORMAL_UNINSTALL)
 ## The HTML 'files' can be directories actually, hence the '-r'.
-       $(if $(and $(HTMLS),$(htmldir)),rm -rf $(addprefix 
'$(DESTDIR)$(htmldir)'/,$(notdir $(HTMLS))))
+       $(call am.uninst.cmd,$(htmldir),$(notdir $(HTMLS)),-r)
 
 uninstall-info-am:
        @$(PRE_UNINSTALL)
 ## Run two loops here so that we can handle PRE_UNINSTALL and
 ## NORMAL_UNINSTALL correctly.
        @if test -d '$(DESTDIR)$(infodir)' && $(am__can_run_installinfo); then \
-         list='$(INFO_DEPS)'; \
-         for file in $$list; do \
-           relfile=`echo "$$file" | sed 's|^.*/||'`; \
+         list='$(notdir $(INFO_DEPS))'; for relfile in $$list; do \
 ## install-info needs the actual info file.  We use the installed one,
 ## rather than relying on one still being in srcdir or builddir.
 ## However, "make uninstall && make uninstall" should not fail,
@@ -266,14 +264,8 @@ uninstall-info-am:
          done; \
        else :; fi
        @$(NORMAL_UNINSTALL)
-       @list='$(INFO_DEPS)'; \
-       for file in $$list; do \
-         relfile=`echo "$$file" | sed 's|^.*/||'`; \
-         (if test -d "$(DESTDIR)$(infodir)" && cd "$(DESTDIR)$(infodir)"; then 
\
-            echo " cd '$(DESTDIR)$(infodir)' && rm -f $$relfile 
$$relfile-[0-9] $$relfile-[0-9][0-9]"; \
-            rm -f $$relfile $$relfile-[0-9] $$relfile-[0-9][0-9]; \
-          else :; fi); \
-       done
+       $(call am.uninst.cmd,$(infodir),\
+         $(foreach i,$(notdir $(INFO_DEPS)),$i $i-[0-9] $i-[0-9][0-9]))
 
 endif %?LOCAL-TEXIS%
 
-- 
1.7.12.rc0