severity 10697 minor
tags 10697 + patch
thanks
Reference: <http://debbugs.gnu.org/cgi/bugreport.cgi?bug=10697>
Hi Jim, sorry for the awful delay.
On 02/02/2012 05:06 PM, Jim Meyering wrote:
> In cppi (http://git.savannah.gnu.org/cgit/cppi.git), I am now using non-
> recursive make via subdir-objects, modeled after the way bison does it.
> I see that "make clean" is inefficient: one "rm -f" per .o file:
>
> rm -fr *.o
> rm -f *.o
> rm -f lib/calloc.o
> rm -f lib/close-stream.o
> ...
> rm -f lib/xstrtol.o
> rm -f lib/xstrtoul.o
> rm -f src/cppi.o
>
> cppi has so few .o files that it's not a problem, but with hundreds
> (coreutils has over 600), it could be noticeable.
>
> Contrast that with its removal of *.o above and of all tests/*.log files
> using just one rm invocation each, I think it must simply be an oversight.
> Seems like it'd be worth fixing some day.
>
I agree, and today could be the day :-)
See the attached patch; I will push it by tomorrow if there is no objection
(and if anyone would like to give it a try on a real project, that would be
much appreciated).
Regards,
Stefano
>From 06dfdbe38e78c5eedb03f688f0264ec0097a4e21 Mon Sep 17 00:00:00 2001
Message-Id: <06dfdbe38e78c5eedb03f688f0264ec0097a4e21.1339330238.git.stefano.lattar...@gmail.com>
From: Stefano Lattarini <stefano.lattar...@gmail.com>
Date: Sun, 10 Jun 2012 13:38:58 +0200
Subject: [PATCH] subdir-objects: improve "make mostlyclean" efficiency and
flexibility
Fixes automake bug#10697.
Before this change, the generated Makefile issued one 'rm' invocation
for each subdir object file. Not only was this very inefficient when
there were several such files, but it also caused stale object files
to be left behind when a source file was renamed or removed.
* automake.in (handle_single_transform): When a subdir object is seen,
update '%compile_clean_files' to clean all the compiled objects in its
same subdirectory, and all the libtool compiled objects ('.lo') there
as well is that subdir object is a libtool one.
* t/subobj-clean-pr10697.sh: New test.
* t/subobj-clean-lt-pr10697.sh: Likewise.
* t/list-of-tests.mk: Add them.
* NEWS: Update.
Signed-off-by: Stefano Lattarini <stefano.lattar...@gmail.com>
---
NEWS | 8 ++
automake.in | 28 ++++---
t/list-of-tests.mk | 2 +
t/subobj-clean-lt-pr10697.sh | 169 ++++++++++++++++++++++++++++++++++++++++++
t/subobj-clean-pr10697.sh | 164 ++++++++++++++++++++++++++++++++++++++++
5 files changed, 359 insertions(+), 12 deletions(-)
create mode 100755 t/subobj-clean-lt-pr10697.sh
create mode 100755 t/subobj-clean-pr10697.sh
diff --git a/NEWS b/NEWS
index cf45836..588e8ed 100644
--- a/NEWS
+++ b/NEWS
@@ -60,6 +60,14 @@ New in 1.12.2:
input file. Such a warning will also be present in the next
Autoconf version (2.70).
+* Cleaning rules:
+
+ - Cleaning rules for compiled objects (both "plain" and libtool) work
+ better when subdir objects are involved, not triggering a distinct
+ 'rm' invocation for each such object. They do so by removing *any*
+ compiled object file that is in the same directory of a subdir
+ object. See automake bug#10697.
+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
New in 1.12.1:
diff --git a/automake.in b/automake.in
index 5cf5a2c..6f8ac0c 100644
--- a/automake.in
+++ b/automake.in
@@ -1963,18 +1963,22 @@ sub handle_single_transform ($$$$$%)
err_am "'$full' should not contain a '..' component";
}
- # Make sure object is removed by 'make mostlyclean'.
- $compile_clean_files{$object} = MOSTLY_CLEAN;
- # If we have a libtool object then we also must remove
- # the ordinary .o.
- if ($object =~ /\.lo$/)
- {
- (my $xobj = $object) =~ s,lo$,\$(OBJEXT),;
- $compile_clean_files{$xobj} = MOSTLY_CLEAN;
-
- # Remove any libtool object in this directory.
- $libtool_clean_directories{$directory} = 1;
- }
+ # Make sure *all* objects files in the subdirectory are
+ # removed by "make mostlyclean". Not only this is more
+ # efficient than listing the object files to be removed
+ # individually (which would cause an 'rm' invocation for
+ # each of them -- very inefficient, see bug#10697), it
+ # would also leave stale object files in the subdirectory
+ # whenever a source file there is removed or renamed.
+ $compile_clean_files{"$directory/*.\$(OBJEXT)"} = MOSTLY_CLEAN;
+ if ($object =~ /\.lo$/)
+ {
+ # If we have a libtool object, then we also must remove
+ # any '.lo' objects in its same subdirectory.
+ $compile_clean_files{"$directory/*.lo"} = MOSTLY_CLEAN;
+ # Remember to cleanup .libs/ in this directory.
+ $libtool_clean_directories{$directory} = 1;
+ }
push (@dep_list, require_build_directory ($directory));
diff --git a/t/list-of-tests.mk b/t/list-of-tests.mk
index 446157c..76e94b3 100644
--- a/t/list-of-tests.mk
+++ b/t/list-of-tests.mk
@@ -1043,6 +1043,8 @@ t/subobj11a.sh \
t/subobj11b.sh \
t/subobj11c.sh \
t/subobjname.sh \
+t/subobj-clean-pr10697.sh \
+t/subobj-clean-lt-pr10697.sh \
t/subpkg.sh \
t/subpkg2.sh \
t/subpkg3.sh \
diff --git a/t/subobj-clean-lt-pr10697.sh b/t/subobj-clean-lt-pr10697.sh
new file mode 100755
index 0000000..95a732c
--- /dev/null
+++ b/t/subobj-clean-lt-pr10697.sh
@@ -0,0 +1,169 @@
+#! /bin/sh
+# Copyright (C) 1998-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/>.
+
+# Removing subdir objects does not cause too much 'rm' invocations.
+# Also, if we rename a source file in a subdirectory, the stale
+# compiled object corresponding to the old name still gets removed
+# by "make mostlyclean". See automake bug#10697.
+# This is the libtool case. Keep this test in sync with sister test
+# 'subobj-clean-pr10697.sh', which deals with the non-libtool case.
+
+required='cc libtoolize'
+. ./defs || Exit 1
+
+cat >> configure.ac << 'END'
+AM_PROG_AR
+AC_PROG_LIBTOOL
+AC_PROG_CC
+AM_PROG_CC_C_O
+AC_OUTPUT
+END
+
+oPATH=$PATH
+ocwd=`pwd` || fatal_ "getting current working directory"
+
+# An rm(1) wrapper that fails when invoked too many times.
+mkdir rm-wrap
+max_rm_invocations=6
+count_file=$ocwd/rm-wrap/count
+cat > rm-wrap/rm <<END
+#!/bin/sh
+set -e
+count=\`cat '$count_file'\`
+count=\`expr \$count + 1\`
+if test \$count -le $max_rm_invocations; then :; else
+ echo "rm invoked more than $max_rm_invocations times" >&2
+ exit 1
+fi
+echo "\$count" > '$count_file'
+PATH='$oPATH'; export PATH
+exec rm "\$@"
+END
+chmod a+x rm-wrap/rm
+echo "0" > rm-wrap/count
+
+cat > Makefile.am <<'END'
+.PHONY: sanity-check-rm
+sanity-check-rm:
+ rm -f 1
+ rm -f 2
+ rm -f 3
+ rm -f 4
+ rm -f 5
+ rm -f 6
+ rm -f x && exit 1; :
+ echo "0" > rm-wrap/count
+
+AUTOMAKE_OPTIONS = subdir-objects
+lib_LTLIBRARIES = libfoo.la
+libfoo_la_SOURCES = \
+ sub1/a.c \
+ sub1/b.c \
+ sub1/c.c \
+ sub1/d.c \
+ sub1/e.c \
+ sub1/f.c \
+ sub2/a.c \
+ sub2/b.c \
+ sub2/c.c \
+ sub2/d.c \
+ sub2/e.c \
+ sub2/f.c \
+ main.c
+END
+
+mkdir sub1 sub2
+echo 'int libmain (void)' > main.c
+echo '{' >> main.c
+for i in 1 2; do
+ for j in a b c d e f; do
+ echo "void $j$i (void) { }" > sub$i/$j.c
+ echo " $j$i ();" >> main.c
+ done
+done
+echo ' return 0;' >> main.c
+echo '}' >> main.c
+cat main.c # For debugging.
+
+libtoolize
+$ACLOCAL
+$AUTOCONF
+$AUTOMAKE -a
+
+./configure
+
+# The use of this variable is only meant to keep us better in sync
+# with the sister test 'subobj-clean-pr10697.sh'.
+OBJEXT=lo
+
+$MAKE
+
+# This must go after configure, since that will invoke rm many times.
+PATH=$ocwd/rm-wrap:$PATH; export PATH
+$MAKE sanity-check-rm || fatal_ "rm wrapper doesn't work as expected"
+
+$MAKE mostlyclean
+ls -l . sub1 sub2
+for i in 1 2; do
+ for j in a b c d e f; do
+ test ! -f sub$i/$j.o
+ test ! -f sub$i/$j.obj
+ test ! -f sub$i/$j.lo
+ test -f sub$i/$j.c || Exit 99 # Sanity check
+ done
+done
+
+PATH=$oPATH; export PATH
+rm -rf rm-wrap
+
+$MAKE clean
+$MAKE
+test -f sub1/a.$OBJEXT
+test -f sub2/d.$OBJEXT
+
+mv -f sub2/d.c sub2/x.c
+rm -f sub1/a.c
+
+sed -e '/ a1 ()/d' main.c > t
+mv -f t main.c
+
+sed -e '/sub1\/a\.c/d' -e 's|sub2/d\.c|sub2/x.c|' Makefile.am > t
+mv -f t Makefile.am
+
+using_gmake || $MAKE Makefile
+$MAKE
+test -f sub2/x.$OBJEXT
+
+# The stale objects are still there after a mere "make all" ...
+test -f sub1/a.$OBJEXT
+test -f sub2/a.$OBJEXT
+
+# ... but they get removed by "make mostlyclean" ...
+$MAKE mostlyclean
+test ! -f sub1/a.$OBJEXT
+test ! -f sub2/d.$OBJEXT
+
+# ... and do not get rebuilt ...
+$MAKE clean
+$MAKE all
+test ! -f sub1/a.$OBJEXT
+test ! -f sub2/d.$OBJEXT
+
+# ... while the non-stale files do.
+test -f sub1/b.$OBJEXT
+test -f sub2/x.$OBJEXT
+
+:
diff --git a/t/subobj-clean-pr10697.sh b/t/subobj-clean-pr10697.sh
new file mode 100755
index 0000000..3b51cf1
--- /dev/null
+++ b/t/subobj-clean-pr10697.sh
@@ -0,0 +1,164 @@
+#! /bin/sh
+# Copyright (C) 1998-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/>.
+
+# Removing subdir objects does not cause too much 'rm' invocations.
+# Also, if we rename a source file in a subdirectory, the stale
+# compiled object corresponding to the old name still gets removed by
+# "make mostlyclean". See automake bug#10697.
+# This is the non-libtool case. Keep this test in sync with sister test
+# 'subobj-clean-lt-pr10697.sh', which deals with the libtool case.
+
+required=cc
+. ./defs || Exit 1
+
+cat >> configure.ac << 'END'
+AC_PROG_CC
+AM_PROG_CC_C_O
+AC_CONFIG_FILES([get-objext.sh:get-objext.in])
+AC_OUTPUT
+END
+
+echo "OBJEXT='@OBJEXT@'" > get-objext.in
+
+oPATH=$PATH
+ocwd=`pwd` || fatal_ "getting current working directory"
+
+# An rm(1) wrapper that fails when invoked too many times.
+mkdir rm-wrap
+max_rm_invocations=3
+count_file=$ocwd/rm-wrap/count
+cat > rm-wrap/rm <<END
+#!/bin/sh
+set -e
+count=\`cat '$count_file'\`
+count=\`expr \$count + 1\`
+if test \$count -le $max_rm_invocations; then :; else
+ echo "rm invoked more than $max_rm_invocations times" >&2
+ exit 1
+fi
+echo "\$count" > '$count_file'
+PATH='$oPATH'; export PATH
+exec rm "\$@"
+END
+chmod a+x rm-wrap/rm
+echo "0" > rm-wrap/count
+
+cat > Makefile.am <<'END'
+.PHONY: sanity-check-rm
+sanity-check-rm:
+ rm -f 1
+ rm -f 2
+ rm -f 3
+ rm -f x && exit 1; :
+ echo "0" > rm-wrap/count
+
+AUTOMAKE_OPTIONS = subdir-objects
+bin_PROGRAMS = foo
+foo_SOURCES = \
+ sub1/a.c \
+ sub1/b.c \
+ sub1/c.c \
+ sub1/d.c \
+ sub1/e.c \
+ sub1/f.c \
+ sub2/a.c \
+ sub2/b.c \
+ sub2/c.c \
+ sub2/d.c \
+ sub2/e.c \
+ sub2/f.c \
+ main.c
+END
+
+mkdir sub1 sub2
+echo 'int main (void)' > main.c
+echo '{' >> main.c
+for i in 1 2; do
+ for j in a b c d e f; do
+ echo "void $j$i (void) { }" > sub$i/$j.c
+ echo " $j$i ();" >> main.c
+ done
+done
+echo ' return 0;' >> main.c
+echo '}' >> main.c
+cat main.c # For debugging.
+
+$ACLOCAL
+$AUTOCONF
+$AUTOMAKE -a
+
+./configure
+
+test -f get-objext.sh
+. ./get-objext.sh
+
+$MAKE
+
+# This must go after configure, since that will invoke rm many times.
+PATH=$ocwd/rm-wrap:$PATH; export PATH
+$MAKE sanity-check-rm || fatal_ "rm wrapper doesn't work as expected"
+
+$MAKE mostlyclean
+ls -l . sub1 sub2
+for i in 1 2; do
+ for j in a b c d e f; do
+ test ! -f sub$i/$j.o
+ test ! -f sub$i/$j.obj
+ test -f sub$i/$j.c || Exit 99 # Sanity check
+ done
+done
+
+PATH=$oPATH; export PATH
+rm -rf rm-wrap
+
+$MAKE clean
+$MAKE
+test -f sub1/a.$OBJEXT
+test -f sub2/d.$OBJEXT
+
+mv -f sub2/d.c sub2/x.c
+rm -f sub1/a.c
+
+sed -e '/ a1 ()/d' main.c > t
+mv -f t main.c
+
+sed -e '/sub1\/a\.c/d' -e 's|sub2/d\.c|sub2/x.c|' Makefile.am > t
+mv -f t Makefile.am
+
+using_gmake || $MAKE Makefile
+$MAKE
+test -f sub2/x.$OBJEXT
+
+# The stale objects are still there after a mere "make all" ...
+test -f sub1/a.$OBJEXT
+test -f sub2/a.$OBJEXT
+
+# ... but they get removed by "make mostlyclean" ...
+$MAKE mostlyclean
+test ! -f sub1/a.$OBJEXT
+test ! -f sub2/d.$OBJEXT
+
+# ... and do not get rebuilt ...
+$MAKE clean
+$MAKE all
+test ! -f sub1/a.$OBJEXT
+test ! -f sub2/d.$OBJEXT
+
+# ... while the non-stale files do.
+test -f sub1/b.$OBJEXT
+test -f sub2/x.$OBJEXT
+
+:
--
1.7.9.5
