[PATCH] Prefer lstat over stat, don't chmod symlinks

[Elliot Murphy]

On 06/11/2007 07:40 PM, Martin Pool wrote:
> To make sure that this works, we should add a test in test_osutils
> that make_readonly works correctly when the target is a symlink.  (In
> fact it looks like at the moment there are no direct tests for those
> functions at all?)

Sure thing! Here is an updated patch that includes tests for both
make_readonly and make_writable, and verifies that they don't blow up
when handed a dangling symlink.

=== modified file 'bzrlib/osutils.py'
--- bzrlib/osutils.py	2007-05-15 22:40:39 +0000
+++ bzrlib/osutils.py	2007-06-11 22:00:01 +0000
@@ -69,18 +69,23 @@
 # OR with 0 on those platforms
 O_BINARY = getattr(os, 'O_BINARY', 0)

+# On posix, use lstat instead of stat so that we can
+# operate on broken symlinks. On Windows revert to stat.
+lstat = getattr(os, 'lstat', os.stat)

 def make_readonly(filename):
     """Make a filename read-only."""
-    mod = os.stat(filename).st_mode
-    mod = mod & 0777555
-    os.chmod(filename, mod)
+    mod = lstat(filename).st_mode
+    if not stat.S_ISLNK(mod):
+        mod = mod & 0777555
+        os.chmod(filename, mod)


 def make_writable(filename):
-    mod = os.stat(filename).st_mode
-    mod = mod | 0200
-    os.chmod(filename, mod)
+    mod = lstat(filename).st_mode
+    if not stat.S_ISLNK(mod):
+        mod = mod | 0200
+        os.chmod(filename, mod)


 _QUOTE_RE = None

=== modified file 'bzrlib/tests/test_osutils.py'
--- bzrlib/tests/test_osutils.py	2007-04-16 21:12:29 +0000
+++ bzrlib/tests/test_osutils.py	2007-06-12 03:27:35 +0000
@@ -247,6 +247,28 @@
         self.assertEqual(baz_path, osutils.dereference_path(foo_baz_path))


+    def test_changing_access(self):
+        f = file('file', 'w')
+        f.write('monkey')
+        f.close()
+
+        # Make a file readonly
+        osutils.make_readonly('file')
+        mode = osutils.lstat('file').st_mode
+        self.assertEqual(mode, mode & 0777555)
+
+        # Make a file writable
+        osutils.make_writable('file')
+        mode = osutils.lstat('file').st_mode
+        self.assertEqual(mode, mode | 0200)
+
+        if osutils.has_symlinks():
+            # should not error when handed a symlink
+            os.symlink('nonexistent', 'dangling')
+            osutils.make_readonly('dangling')
+            osutils.make_writable('dangling')
+
+
     def test_kind_marker(self):
         self.assertEqual("", osutils.kind_marker("file"))
         self.assertEqual("/", osutils.kind_marker(osutils._directory_kind))

-- 
Elliot Murphy | https://launchpad.net/~emurphy/