From e8829c698bbbba7309a2f6218a172d59ccb58f5c Mon Sep 17 00:00:00 2001
From: "Marcel R." <github.riga@icloud.com>
Date: Thu, 16 Jan 2025 16:32:07 +0100
Subject: [PATCH] Fix local target permission handling.

---
 law/target/file.py  | 29 ++++++++++++++++++++++-------
 law/target/local.py |  6 +++---
 2 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/law/target/file.py b/law/target/file.py
index 6c802315..32b7d31b 100644
--- a/law/target/file.py
+++ b/law/target/file.py
@@ -324,10 +324,15 @@ def chmod(self, perm, *, silent: bool = False, **kwargs) -> bool:
         return self.fs.chmod(self.path, perm, silent=silent, **kwargs)
 
     def makedirs(self, *args, **kwargs) -> None:
+        # overwrites luigi's makedirs method
         parent = self.parent
         if parent:
             parent.touch(*args, **kwargs)
 
+    def _prepare_dir(self, **kwargs) -> None:
+        dir_target = self if isinstance(self, self.directory_class) else self.parent
+        dir_target.touch(**kwargs)  # type: ignore[union-attr]
+
     @abstractproperty
     def fs(self) -> FileSystem:
         ...
@@ -478,8 +483,11 @@ def copy_to(
         dir_perm: int | None = None,
         **kwargs,
     ) -> str:
+        if isinstance(dst, FileSystemTarget):
+            dst._prepare_dir(perm=dir_perm, **kwargs)
+
         # TODO: complain when dst not local? forward to copy_from request depending on protocol?
-        return self.fs.copy(self.path, get_path(dst), perm=perm, dir_perm=dir_perm, **kwargs)
+        return self.fs.copy(self.path, get_path(dst), perm=perm, **kwargs)
 
     def copy_from(
         self,
@@ -489,12 +497,14 @@ def copy_from(
         dir_perm: int | None = None,
         **kwargs,
     ) -> str:
+        self._prepare_dir(perm=dir_perm, **kwargs)
+
         if isinstance(src, FileSystemFileTarget):
-            return src.copy_to(self.abspath, perm=perm, dir_perm=dir_perm, **kwargs)
+            return src.copy_to(self.abspath, perm=perm or self.fs.default_file_perm, **kwargs)
 
-        # when src is a plain string, let the fs handle it
         # TODO: complain when src not local? forward to copy_to request depending on protocol?
-        return self.fs.copy(get_path(src), self.path, perm=perm, dir_perm=dir_perm, **kwargs)
+        # when src is a plain string, let the fs handle it
+        return self.fs.copy(get_path(src), self.path, perm=perm, **kwargs)
 
     def move_to(
         self,
@@ -504,8 +514,11 @@ def move_to(
         dir_perm: int | None = None,
         **kwargs,
     ) -> str:
+        if isinstance(dst, FileSystemTarget):
+            dst._prepare_dir(perm=dir_perm, **kwargs)
+
         # TODO: complain when dst not local? forward to copy_from request depending on protocol?
-        return self.fs.move(self.path, get_path(dst), perm=perm, dir_perm=dir_perm, **kwargs)
+        return self.fs.move(self.path, get_path(dst), perm=perm, **kwargs)
 
     def move_from(
         self,
@@ -515,12 +528,14 @@ def move_from(
         dir_perm: int | None = None,
         **kwargs,
     ) -> str:
+        self._prepare_dir(perm=dir_perm, **kwargs)
+
         if isinstance(src, FileSystemFileTarget):
-            return src.move_to(self.abspath, perm=perm, dir_perm=dir_perm, **kwargs)
+            return src.move_to(self.abspath, perm=perm or self.fs.default_file_perm, **kwargs)
 
         # when src is a plain string, let the fs handle it
         # TODO: complain when src not local? forward to copy_to request depending on protocol?
-        return self.fs.move(get_path(src), self.path, perm=perm, dir_perm=dir_perm, **kwargs)
+        return self.fs.move(get_path(src), self.path, perm=perm, **kwargs)
 
 
 class FileSystemDirectoryTarget(FileSystemTarget):
diff --git a/law/target/local.py b/law/target/local.py
index 1dc94e2e..ce321bd2 100644
--- a/law/target/local.py
+++ b/law/target/local.py
@@ -200,7 +200,7 @@ def mkdir(  # type: ignore[override]
 
         # the mode passed to os.mkdir or os.makedirs is ignored on some systems, so the strategy
         # here is to disable the process' current umask, create the directories and use chmod again
-        orig = os.umask(0) if perm is not None else None
+        orig = os.umask(0o0770 - perm) if perm is not None else None
         func = os.makedirs if recursive else os.mkdir
         try:
             try:
@@ -578,7 +578,7 @@ def localize(
 
                     # move back again
                     if tmp.exists():
-                        tmp.copy_to_local(self, perm=perm, dir_perm=dir_perm)
+                        self.copy_from_local(tmp, perm=perm, dir_perm=dir_perm)
                     else:
                         logger.warning(
                             "cannot move non-existing localized target to actual representation "
@@ -663,7 +663,7 @@ def localize(
                     # TODO: keep track of changed contents in "a" mode and copy only those?
                     if tmp.exists():
                         self.remove()
-                        tmp.copy_to_local(self, perm=perm, dir_perm=dir_perm)
+                        self.copy_from_local(tmp, perm=perm, dir_perm=dir_perm)
                     else:
                         logger.warning(
                             "cannot move non-existing localized target to actual representation "