remove: try deleting before trying to chmod

dvc/utils/fs.py

@@ -1,3 +1,4 @@
+import sys
 import errno
 import logging
 import os
@@ -129,6 +130,13 @@ def _chmod(func, p, excinfo):
     func(p)
 
 
+def _unlink(path, onerror):
+    try:
+        os.unlink(path)
+    except OSError:
+        onerror(os.unlink, path, sys.exc_info())
+
+
 def remove(path):
     logger.debug("Removing '%s'", path)
 
@@ -137,7 +145,7 @@ def remove(path):
         if os.path.isdir(path):
             shutil.rmtree(path, onerror=_chmod)
         else:
-            _chmod(os.unlink, path, None)
+            _unlink(path, _chmod)
     except OSError as exc:
         if exc.errno != errno.ENOENT:
             raise

tests/func/test_unprotect.py

@@ -24,11 +24,14 @@ def test(self):
 
         self.assertTrue(os.access(self.FOO, os.W_OK))
 
-        # NOTE: cache is now unprotected, bceause we try to set write perms
-        # on files that we try to delete, as some filesystems require that
-        # (e.g. NTFS). But it should be restored after the next cache check,
-        # hence why we call `dvc status` here.
-        self.assertTrue(os.access(cache, os.W_OK))
-        ret = main(["status"])
-        self.assertEqual(ret, 0)
+        if os.name == "nt":
+            # NOTE: cache is now unprotected, because NTFS doesn't allow
+            # deleting read-only files, so we have to try to set write perms
+            # on files that we try to delete, which propagates to the cache
+            # file. But it should be restored after the next cache check, hence
+            # why we call `dvc status` here.
+            self.assertTrue(os.access(cache, os.W_OK))
+            ret = main(["status"])
+            self.assertEqual(ret, 0)
+
         self.assertFalse(os.access(cache, os.W_OK))

tests/unit/remote/test_local.py

@@ -54,11 +54,12 @@ def test_is_protected(tmp_dir, dvc, link_name):
     remote.unprotect(link)
 
     assert not remote.is_protected(link)
-    if link_name == "symlink" and os.name == "nt":
-        # NOTE: Windows symlink perms don't propagate to the target
-        assert remote.is_protected(foo)
-    else:
+    if os.name == "nt" and link_name == "hardlink":
+        # NOTE: NTFS doesn't allow deleting read-only files, which forces us to
+        # set write perms on the link, which propagates to the source.
         assert not remote.is_protected(foo)
+    else:
+        assert remote.is_protected(foo)
 
 
 @pytest.mark.parametrize("err", [errno.EPERM, errno.EACCES])