From 4245f1153e997d71d733b9d88cf6c8f947405ead Mon Sep 17 00:00:00 2001 From: Paolo Holinski Date: Thu, 11 Jan 2024 14:50:50 +0100 Subject: [PATCH 01/11] rename attrs -> ex postVisitDirectory gets passed the exception that caused processing of the subtree to terminate early as the second arg --- src/babashka/fs.cljc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/babashka/fs.cljc b/src/babashka/fs.cljc index 5329255..2ddf119 100644 --- a/src/babashka/fs.cljc +++ b/src/babashka/fs.cljc @@ -209,8 +209,8 @@ (preVisitDirectory [_ dir attrs] (-> (pre-visit-dir dir attrs) file-visit-result)) - (postVisitDirectory [_ dir attrs] - (-> (post-visit-dir dir attrs) + (postVisitDirectory [_ dir ex] + (-> (post-visit-dir dir ex) file-visit-result)) (visitFile [_ path attrs] (-> (visit-file path attrs) From 7009a71c80efe10c9996f284e27344bdd22c024c Mon Sep 17 00:00:00 2001 From: Paolo Holinski Date: Thu, 11 Jan 2024 14:52:07 +0100 Subject: [PATCH 02/11] Fix copy-tree for non-writable directories --- src/babashka/fs.cljc | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/babashka/fs.cljc b/src/babashka/fs.cljc index 2ddf119..f92964a 100644 --- a/src/babashka/fs.cljc +++ b/src/babashka/fs.cljc @@ -424,6 +424,9 @@ ([path {:keys [:posix-file-permissions]}] (Files/createDirectories (as-path path) (posix->attrs posix-file-permissions)))) +(declare posix-file-permissions) +(declare u+wx) + (defn copy-tree "Copies entire file tree from src to dest. Creates dest if needed using `create-dirs`, passing it the `:posix-file-permissions` @@ -453,7 +456,8 @@ (when-not (Files/exists to-dir link-options) (Files/copy ^Path dir to-dir ^"[Ljava.nio.file.CopyOption;" - copy-options))) + copy-options) + (u+wx to-dir))) :continue) :visit-file (fn [from-path _attrs] (let [rel (relativize from from-path) @@ -462,7 +466,13 @@ ^"[Ljava.nio.file.CopyOption;" copy-options) :continue) - :continue)})))) + :continue) + :post-visit-dir (fn [dir _ex] + (let [rel (relativize from dir) + to-dir (path to rel)] + (let [perms (posix-file-permissions (file dir))] + (Files/setPosixFilePermissions to-dir perms)) + :continue))})))) (defn temp-dir "Returns `java.io.tmpdir` property as path." From f0a366f58ec6ac9113ab10574523335e914ca67d Mon Sep 17 00:00:00 2001 From: Paolo Holinski Date: Thu, 11 Jan 2024 15:03:15 +0100 Subject: [PATCH 03/11] Add test --- test/babashka/fs_test.clj | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/babashka/fs_test.clj b/test/babashka/fs_test.clj index eabbd30..556a30c 100644 --- a/test/babashka/fs_test.clj +++ b/test/babashka/fs_test.clj @@ -218,6 +218,14 @@ (is (fs/exists? (fs/file tmp"foo2" "foo" "1")) "The nested destination directory is not created when it doesn't exist"))) +(deftest copy-tree-nested-ro-dir-test + (fs/with-temp-dir [tmp {}] + ;; https://github.com/babashka/fs/issues/122 + (fs/create-dirs (fs/path tmp "src" "foo" "bar")) + (fs/set-posix-file-permissions (fs/path tmp "src" "foo") "r-xr-xr-x") + (fs/copy-tree (fs/path tmp "src") (fs/path tmp "dst")) + (is (fs/exists? (fs/path tmp "dst" "foo" "bar"))))) + (deftest components-test (let [paths (map normalize (fs/components (fs/path (temp-dir) "foo")))] (is (= "foo" (last paths))) From c371e492f3cde35e13c8370f46b1f9efc3d4b4c6 Mon Sep 17 00:00:00 2001 From: Paolo Holinski Date: Thu, 11 Jan 2024 15:05:11 +0100 Subject: [PATCH 04/11] Changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 73721a1..27af8e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ For a list of breaking changes, check [here](#breaking-changes). Babashka [fs](https://github.com/babashka/fs): file system utility library for Clojure +## Unreleased + +- [#122](https://github.com/babashka/fs/issues/122): `fs/copy-tree`: fix copying read-only directories with children ([@sohalt](https://github.com/sohalt)) + ## v0.5.20 (2023-12-21) - [#119](https://github.com/babashka/fs/issues/119): `fs/delete-tree`: add `:force` flag to delete read-only directories/files. Set the flag to true in `fs/with-temp-dir` ([@jlesquembre](https://github.com/jlesquembre)) From cf2e953f35e903900f9d34a07883cc50751e4a15 Mon Sep 17 00:00:00 2001 From: Paolo Holinski Date: Thu, 11 Jan 2024 15:30:43 +0100 Subject: [PATCH 05/11] Fix copy-tree on win --- src/babashka/fs.cljc | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/babashka/fs.cljc b/src/babashka/fs.cljc index f92964a..9ed6c7e 100644 --- a/src/babashka/fs.cljc +++ b/src/babashka/fs.cljc @@ -470,8 +470,11 @@ :post-visit-dir (fn [dir _ex] (let [rel (relativize from dir) to-dir (path to rel)] - (let [perms (posix-file-permissions (file dir))] - (Files/setPosixFilePermissions to-dir perms)) + (if win? + (when-not (.canWrite (file from)) + (.setWritable (file dir) false)) + (let [perms (posix-file-permissions (file dir))] + (Files/setPosixFilePermissions to-dir perms))) :continue))})))) (defn temp-dir From ca92e51b491fc878684062cf8086f18755a00724 Mon Sep 17 00:00:00 2001 From: Paolo Holinski Date: Thu, 11 Jan 2024 15:31:36 +0100 Subject: [PATCH 06/11] expand test --- test/babashka/fs_test.clj | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/babashka/fs_test.clj b/test/babashka/fs_test.clj index 556a30c..afc0252 100644 --- a/test/babashka/fs_test.clj +++ b/test/babashka/fs_test.clj @@ -224,7 +224,8 @@ (fs/create-dirs (fs/path tmp "src" "foo" "bar")) (fs/set-posix-file-permissions (fs/path tmp "src" "foo") "r-xr-xr-x") (fs/copy-tree (fs/path tmp "src") (fs/path tmp "dst")) - (is (fs/exists? (fs/path tmp "dst" "foo" "bar"))))) + (is (fs/exists? (fs/path tmp "dst" "foo" "bar"))) + (is (not (fs/writable? (fs/path tmp "dst" "foo")))))) (deftest components-test (let [paths (map normalize (fs/components (fs/path (temp-dir) "foo")))] From d1c887099b79e87208f3b9fbc7d7fdbee751ae47 Mon Sep 17 00:00:00 2001 From: Paolo Holinski Date: Thu, 11 Jan 2024 18:55:20 +0100 Subject: [PATCH 07/11] fix test on windows --- test/babashka/fs_test.clj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/babashka/fs_test.clj b/test/babashka/fs_test.clj index afc0252..5c16790 100644 --- a/test/babashka/fs_test.clj +++ b/test/babashka/fs_test.clj @@ -222,7 +222,7 @@ (fs/with-temp-dir [tmp {}] ;; https://github.com/babashka/fs/issues/122 (fs/create-dirs (fs/path tmp "src" "foo" "bar")) - (fs/set-posix-file-permissions (fs/path tmp "src" "foo") "r-xr-xr-x") + (.setWritable (fs/file tmp "src" "foo") false) (fs/copy-tree (fs/path tmp "src") (fs/path tmp "dst")) (is (fs/exists? (fs/path tmp "dst" "foo" "bar"))) (is (not (fs/writable? (fs/path tmp "dst" "foo")))))) From e84795d7f65b5da216019fdc0c8eac8c256062f9 Mon Sep 17 00:00:00 2001 From: Paolo Holinski Date: Thu, 11 Jan 2024 19:00:36 +0100 Subject: [PATCH 08/11] maybe like this? --- test/babashka/fs_test.clj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/babashka/fs_test.clj b/test/babashka/fs_test.clj index 5c16790..e4cf557 100644 --- a/test/babashka/fs_test.clj +++ b/test/babashka/fs_test.clj @@ -222,7 +222,7 @@ (fs/with-temp-dir [tmp {}] ;; https://github.com/babashka/fs/issues/122 (fs/create-dirs (fs/path tmp "src" "foo" "bar")) - (.setWritable (fs/file tmp "src" "foo") false) + (.setReadOnly (fs/file tmp "src" "foo")) (fs/copy-tree (fs/path tmp "src") (fs/path tmp "dst")) (is (fs/exists? (fs/path tmp "dst" "foo" "bar"))) (is (not (fs/writable? (fs/path tmp "dst" "foo")))))) From d78addd034f881d0cef81cb8f5d152bda8314496 Mon Sep 17 00:00:00 2001 From: Paolo Holinski Date: Thu, 11 Jan 2024 19:05:46 +0100 Subject: [PATCH 09/11] only check not writable on non-windows sytems --- test/babashka/fs_test.clj | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/babashka/fs_test.clj b/test/babashka/fs_test.clj index e4cf557..5f85da7 100644 --- a/test/babashka/fs_test.clj +++ b/test/babashka/fs_test.clj @@ -225,7 +225,10 @@ (.setReadOnly (fs/file tmp "src" "foo")) (fs/copy-tree (fs/path tmp "src") (fs/path tmp "dst")) (is (fs/exists? (fs/path tmp "dst" "foo" "bar"))) - (is (not (fs/writable? (fs/path tmp "dst" "foo")))))) + (when (not windows?) + ;; you can always write to directories on Windows, even if they are read-only + ;; https://answers.microsoft.com/en-us/windows/forum/all/all-folders-are-now-read-only-windows-10/0ca1880f-e997-46af-bd85-042a53fc078e + (is (not (fs/writable? (fs/path tmp "dst" "foo"))))))) (deftest components-test (let [paths (map normalize (fs/components (fs/path (temp-dir) "foo")))] From 982969c6a68a41572ef2824a2608057f4d6810b8 Mon Sep 17 00:00:00 2001 From: Paolo Holinski Date: Fri, 12 Jan 2024 16:04:11 +0100 Subject: [PATCH 10/11] move functions up to get rid of declare --- src/babashka/fs.cljc | 60 ++++++++++++++++++++++++-------------------- 1 file changed, 33 insertions(+), 27 deletions(-) diff --git a/src/babashka/fs.cljc b/src/babashka/fs.cljc index 9ed6c7e..144f7c5 100644 --- a/src/babashka/fs.cljc +++ b/src/babashka/fs.cljc @@ -424,8 +424,26 @@ ([path {:keys [:posix-file-permissions]}] (Files/createDirectories (as-path path) (posix->attrs posix-file-permissions)))) -(declare posix-file-permissions) -(declare u+wx) +(defn set-posix-file-permissions + "Sets posix file permissions on f. Accepts a string like `\"rwx------\"` or a set of PosixFilePermission." + [f posix-file-permissions] + (Files/setPosixFilePermissions (as-path f) (->posix-file-permissions posix-file-permissions))) + +(defn posix-file-permissions + "Gets f's posix file permissions. Use posix->str to view as a string." + ([f] (posix-file-permissions f nil)) + ([f {:keys [:nofollow-links]}] + (Files/getPosixFilePermissions (as-path f) (->link-opts nofollow-links)))) + +(defn- u+wx + [f] + (if win? + (.setWritable (file f) true) + (let [^HashSet perms (posix-file-permissions f) + p1 (.add perms PosixFilePermission/OWNER_WRITE) + p2 (.add perms PosixFilePermission/OWNER_EXECUTE)] + (when (or p1 p2) + (set-posix-file-permissions f perms))))) (defn copy-tree "Copies entire file tree from src to dest. Creates dest if needed @@ -474,8 +492,20 @@ (when-not (.canWrite (file from)) (.setWritable (file dir) false)) (let [perms (posix-file-permissions (file dir))] - (Files/setPosixFilePermissions to-dir perms))) + (Files/setPosixFilePermissions to-dir perms))) :continue))})))) +(declare posix-file-permissions) +(declare set-posix-file-permissions) + +(defn- u+wx + [f] + (if win? + (.setWritable (file f) true) + (let [^HashSet perms (posix-file-permissions f) + p1 (.add perms PosixFilePermission/OWNER_WRITE) + p2 (.add perms PosixFilePermission/OWNER_EXECUTE)] + (when (or p1 p2) + (set-posix-file-permissions f perms))))) (defn temp-dir "Returns `java.io.tmpdir` property as path." @@ -572,19 +602,6 @@ [f] (Files/isSymbolicLink (as-path f))) -(declare posix-file-permissions) -(declare set-posix-file-permissions) - -(defn- u+wx - [f] - (if win? - (.setWritable (file f) true) - (let [^HashSet perms (posix-file-permissions f) - p1 (.add perms PosixFilePermission/OWNER_WRITE) - p2 (.add perms PosixFilePermission/OWNER_EXECUTE)] - (when (or p1 p2) - (set-posix-file-permissions f perms))))) - (defn delete-tree "Deletes a file tree using `walk-file-tree`. Similar to `rm -rf`. Does not follow symlinks. `force` ensures read-only directories/files are deleted. Similar to `chmod -R +wx` + `rm -rf`" @@ -646,17 +663,6 @@ (.deleteOnExit (as-file f)) f) -(defn set-posix-file-permissions - "Sets posix file permissions on f. Accepts a string like `\"rwx------\"` or a set of PosixFilePermission." - [f posix-file-permissions] - (Files/setPosixFilePermissions (as-path f) (->posix-file-permissions posix-file-permissions))) - -(defn posix-file-permissions - "Gets f's posix file permissions. Use posix->str to view as a string." - ([f] (posix-file-permissions f nil)) - ([f {:keys [:nofollow-links]}] - (Files/getPosixFilePermissions (as-path f) (->link-opts nofollow-links)))) - (defn same-file? "Returns true if this is the same file as other." [this other] From d5c6b5e6d9026686d8de5c95900be1f3d5a40a6d Mon Sep 17 00:00:00 2001 From: Paolo Holinski Date: Fri, 12 Jan 2024 16:05:36 +0100 Subject: [PATCH 11/11] No need to make directories writable on windows --- src/babashka/fs.cljc | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/babashka/fs.cljc b/src/babashka/fs.cljc index 144f7c5..a7e5648 100644 --- a/src/babashka/fs.cljc +++ b/src/babashka/fs.cljc @@ -475,7 +475,8 @@ (Files/copy ^Path dir to-dir ^"[Ljava.nio.file.CopyOption;" copy-options) - (u+wx to-dir))) + (when-not win? + (u+wx to-dir)))) :continue) :visit-file (fn [from-path _attrs] (let [rel (relativize from from-path) @@ -488,9 +489,7 @@ :post-visit-dir (fn [dir _ex] (let [rel (relativize from dir) to-dir (path to rel)] - (if win? - (when-not (.canWrite (file from)) - (.setWritable (file dir) false)) + (when-not win? (let [perms (posix-file-permissions (file dir))] (Files/setPosixFilePermissions to-dir perms))) :continue))}))))