- fixed NoClean for multi-target builders

3 files changed, 122 insertions, 25 deletions

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index 9c6f8ae4..5f62c1d4 100644
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -6,6 +6,9 @@
 
 RELEASE 2.3.2.alpha.yyyymmdd - NEW DATE WILL BE INSERTED HERE
 
+  From Amir Szekely:
+    - Fixed NoClean() for multi-target builders (#2353).
+
   From Russel Winder:
     - Revamp of the D language support. Tools for DMD, GDC and LDC provided
       and integrated with the C and C++ linking.   NB  This is only tested with
diff --git a/src/engine/SCons/Script/Main.py b/src/engine/SCons/Script/Main.py
index 93380fb1..7abea3fe 100644
--- a/src/engine/SCons/Script/Main.py
+++ b/src/engine/SCons/Script/Main.py
@@ -339,37 +339,41 @@ class CleanTask(SCons.Taskmaster.AlwaysTask):
         except (IOError, OSError), e:
             print "scons: Could not remove '%s':" % pathstr, e.strerror
 
-    def show(self):
+    def _get_files_to_clean(self):
+        result = []
         target = self.targets[0]
-        if (target.has_builder() or target.side_effect) and not target.noclean:
-            for t in self.targets:
-                if not t.isdir():
-                    display("Removed " + str(t))
-        if target in SCons.Environment.CleanTargets:
-            files = SCons.Environment.CleanTargets[target]
-            for f in files:
-                self.fs_delete(f.abspath, str(f), 0)
+        if target.has_builder() or target.side_effect:
+            result = [t for t in self.targets if not t.noclean]
+        return result
 
-    def remove(self):
+    def _clean_targets(self, remove):
         target = self.targets[0]
-        if (target.has_builder() or target.side_effect) and not target.noclean:
-            for t in self.targets:
-                try:
-                    removed = t.remove()
-                except OSError, e:
-                    # An OSError may indicate something like a permissions
-                    # issue, an IOError would indicate something like
-                    # the file not existing.  In either case, print a
-                    # message and keep going to try to remove as many
-                    # targets aa possible.
-                    print "scons: Could not remove '%s':" % str(t), e.strerror
-                else:
-                    if removed:
-                        display("Removed " + str(t))
         if target in SCons.Environment.CleanTargets:
             files = SCons.Environment.CleanTargets[target]
             for f in files:
-                self.fs_delete(f.abspath, str(f))
+                self.fs_delete(f.abspath, str(f), remove)
+
+    def show(self):
+        for t in self._get_files_to_clean():
+            if not t.isdir():
+                display("Removed " + str(t))
+        self._clean_targets(0)
+
+    def remove(self):
+        for t in self._get_files_to_clean():
+            try:
+                removed = t.remove()
+            except OSError, e:
+                # An OSError may indicate something like a permissions
+                # issue, an IOError would indicate something like
+                # the file not existing.  In either case, print a
+                # message and keep going to try to remove as many
+                # targets aa possible.
+                print "scons: Could not remove '%s':" % str(t), e.strerror
+            else:
+                if removed:
+                    display("Removed " + str(t))
+        self._clean_targets(1)
 
     execute = remove
 
diff --git a/test/NoClean.py b/test/NoClean.py
new file mode 100644
index 00000000..01fe209d
--- /dev/null
+++ b/test/NoClean.py
@@ -0,0 +1,90 @@
+#!/usr/bin/env python

+#

+# __COPYRIGHT__

+#

+# Permission is hereby granted, free of charge, to any person obtaining

+# a copy of this software and associated documentation files (the

+# "Software"), to deal in the Software without restriction, including

+# without limitation the rights to use, copy, modify, merge, publish,

+# distribute, sublicense, and/or sell copies of the Software, and to

+# permit persons to whom the Software is furnished to do so, subject to

+# the following conditions:

+#

+# The above copyright notice and this permission notice shall be included

+# in all copies or substantial portions of the Software.

+#

+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY

+# KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE

+# WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND

+# NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE

+# LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION

+# OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION

+# WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.

+#

+

+__revision__ = "__FILE__ __REVISION__ __DATE__ __DEVELOPER__"

+

+#

+# This test ensures that NoClean works correctly, even when it's applied to

+# a single target in the return list of an multi-target Builder.

+#

+import TestSCons

+

+test = TestSCons.TestSCons()

+

+test.write('SConstruct', """

+def action(target, source, env):

+	for t in target: open(t.path, 'w')

+Command('1.out', 'SConstruct', action)

+NoClean('1.out')

+""")

+

+test.write('SConstruct.force', """

+def action(target, source, env):

+	for t in target: open(t.path, 'w')

+	open('4.out', 'w')

+res = Command('3.out', 'SConstruct.force', action)

+Clean('4.out', res)

+NoClean('4.out')

+""")

+

+test.write('SConstruct.multi', """

+def action(target, source, env):

+	for t in target: open(t.path, 'w')

+Command(['5.out', '6.out'], 'SConstruct.multi', action)

+NoClean('6.out')

+""")

+

+#

+# Basic check: NoClean keeps files

+#

+test.run()

+test.run(arguments='-c')

+

+test.must_exist('1.out')

+

+#

+# Check: NoClean overrides Clean

+#

+test.run(arguments=['-f', 'SConstruct.force'])

+test.run(arguments=['-f', 'SConstruct.force', '-c'])

+

+test.must_not_exist('3.out')

+test.must_exist('4.out')

+

+#

+# Check: NoClean works for multi-target Builders

+#

+test.run(arguments=['-f', 'SConstruct.multi'])

+test.run(arguments=['-f', 'SConstruct.multi', '-c'])

+

+test.must_not_exist('5.out')

+test.must_exist('6.out')

+

+test.pass_test()

+

+# Local Variables:

+# tab-width:4

+# indent-tabs-mode:nil

+# End:

+# vim: set expandtab tabstop=4 shiftwidth=4: