Fix bug removing symbolic links. (Steve Leblanc)

4 files changed, 42 insertions, 3 deletions

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index f74cb92..621b865 100644
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -51,6 +51,10 @@ RELEASE 0.10 - XXX
   - Documentation fixes: better initial explanation of SConscript files;
     fix a misformatted "table" in the StaticObject explanation.
 
+  From Steven Knight and Steve Leblanc:
+
+  - Fix the -c option so it will remove symlinks.
+
   From Steve Leblanc:
 
   - Add a Clean() method to support removing user-specified targets
diff --git a/src/engine/SCons/Node/FS.py b/src/engine/SCons/Node/FS.py
index eb23cd8..ece238a 100644
--- a/src/engine/SCons/Node/FS.py
+++ b/src/engine/SCons/Node/FS.py
@@ -60,6 +60,12 @@ import SCons.Warnings
 # there should be *no* changes to the external file system(s)...
 #
 
+if hasattr(os, 'symlink'):
+    def _existsp(p):
+        return os.path.exists(p) or os.path.islink(p)
+else:
+    _existsp = os.path.exists
+
 def LinkFunc(target, source, env):
     src = source[0].path
     dest = target[0].path
@@ -940,7 +946,7 @@ class File(Entry):
 
     def remove(self):
         """Remove this file."""
-        if os.path.exists(self.path):
+        if _existsp(self.path):
             os.unlink(self.path)
             return 1
         return None
diff --git a/src/engine/SCons/Node/FSTests.py b/src/engine/SCons/Node/FSTests.py
index b3fac3f..6ca8182 100644
--- a/src/engine/SCons/Node/FSTests.py
+++ b/src/engine/SCons/Node/FSTests.py
@@ -923,6 +923,18 @@ class FSTestCase(unittest.TestCase):
         f = fs.File('exists')
         r = f.remove()
         assert r, r
+        assert not os.path.exists(test.workpath('exists')), "exists was not removed"
+
+        symlink = test.workpath('symlink')
+        try:
+            os.symlink(test.workpath('does_not_exist'), symlink)
+            assert os.path.islink(symlink)
+            f = fs.File('symlink')
+            r = f.remove()
+            assert r, r
+            assert not os.path.islink(symlink), "symlink was not removed"
+        except AttributeError:
+            pass
 
         test.write('can_not_remove', "can_not_remove\n")
         test.writable(test.workpath('.'), 0)
diff --git a/test/option-c.py b/test/option-c.py
index 5d1a714..22523bd 100644
--- a/test/option-c.py
+++ b/test/option-c.py
@@ -47,6 +47,16 @@ env.B(target = 'foo1.out', source = 'foo1.in')
 env.B(target = 'foo2.out', source = 'foo2.xxx')
 env.B(target = 'foo2.xxx', source = 'foo2.in')
 env.B(target = 'foo3.out', source = 'foo3.in')
+import os
+if hasattr(os, 'symlink'):
+    def symlink1(env, target, source):
+        # symlink to a file that exists
+        os.symlink(str(source[0]), str(target[0]))
+    env.Command(target = 'symlink1', source = 'foo1.in', action = symlink1)
+    def symlink2(env, target, source):
+        # force symlink to a file that doesn't exist
+        os.symlink('does_not_exist', str(target[0]))
+    env.Command(target = 'symlink2', source = 'foo1.in', action = symlink2)
 """ % python)
 
 test.write('foo1.in', "foo1.in\n")
@@ -93,6 +103,10 @@ test.fail_test(test.read(test.workpath('foo2.xxx')) != "foo2.in\n")
 test.fail_test(test.read(test.workpath('foo2.out')) != "foo2.in\n")
 test.fail_test(test.read(test.workpath('foo3.out')) != "foo3.in\n")
 
+if hasattr(os, 'symlink'):
+    test.fail_test(not os.path.islink(test.workpath('symlink1')))
+    test.fail_test(not os.path.islink(test.workpath('symlink2')))
+
 test.run(arguments = '-c foo2.xxx',
          stdout = test.wrap_stdout("Removed foo2.xxx\n"))
 
@@ -101,13 +115,16 @@ test.fail_test(os.path.exists(test.workpath('foo2.xxx')))
 test.fail_test(test.read(test.workpath('foo2.out')) != "foo2.in\n")
 test.fail_test(test.read(test.workpath('foo3.out')) != "foo3.in\n")
 
-test.run(arguments = '-c .',
-         stdout = test.wrap_stdout("Removed foo1.out\nRemoved foo2.out\nRemoved foo3.out\n"))
+test.run(arguments = '-c .')
 
 test.fail_test(os.path.exists(test.workpath('foo1.out')))
 test.fail_test(os.path.exists(test.workpath('foo2.out')))
 test.fail_test(os.path.exists(test.workpath('foo3.out')))
 
+if hasattr(os, 'symlink'):
+    test.fail_test(os.path.islink(test.workpath('symlink1')))
+    test.fail_test(os.path.islink(test.workpath('symlink2')))
+
 test.run(arguments = 'foo1.out foo2.out foo3.out')
 
 expect = test.wrap_stdout("""Removed foo1.out