Skip to content

Commit c65d55f

Browse files
committed
Fix sync to always respect walkpath order.
Specifically, we could run into issues where a parent doesn't exist yet when copying files. ``` julia> index = Dict(Tuple(setdiff(p.segments, src.segments)) => p for p in walkpath(src)) Dict{Tuple{String,Vararg{String,N} where N},PosixPath} with 5 entries: ("folder1", "folder2") => /var/folders/h_/vkjv56410m7f75g9ffp46mf80000gp/T/tmpcJbzPa/folder1/folder2 ("folder1",) => /var/folders/h_/vkjv56410m7f75g9ffp46mf80000gp/T/tmpcJbzPa/folder1 ("folder1", "file2") => /var/folders/h_/vkjv56410m7f75g9ffp46mf80000gp/T/tmpcJbzPa/folder1/file2 ("folder1", "folder2", "file3") => /var/folders/h_/vkjv56410m7f75g9ffp46mf80000gp/T/tmpcJbzPa/folder1/folder2/file3 ("file1",) => /var/folders/h_/vkjv56410m7f75g9ffp46mf80000gp/T/tmpcJbzPa/file1 ``` This would try and copy folder1/folder2 before creating folder1 because of how the segments were hashed. An OrderedDict would also resolve this, but we don't want to depend on external dependencies in FilePathsBase.
1 parent fb115af commit c65d55f

File tree

2 files changed

+36
-5
lines changed

2 files changed

+36
-5
lines changed

src/path.jl

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -522,15 +522,19 @@ exist at the source will be removed.
522522
"""
523523
function sync(src::AbstractPath, dst::AbstractPath; delete=false)
524524
# Create an index of all of the source files
525-
index = Dict(Tuple(setdiff(p.segments, src.segments)) => p for p in walkpath(src))
525+
src_paths = collect(walkpath(src))
526+
index = Dict(
527+
Tuple(setdiff(p.segments, src.segments)) => i for (i, p) in enumerate(src_paths)
528+
)
526529

527530
if exists(dst)
528531
for p in walkpath(dst)
529532
k = Tuple(setdiff(p.segments, dst.segments))
530533

531534
if haskey(index, k)
532-
if modified(index[k]) > modified(p)
533-
cp(index[k], p; force=true)
535+
src_path = src_paths[index[k]]
536+
if modified(src_path) > modified(p)
537+
cp(src_path, p; force=true)
534538
end
535539

536540
delete!(index, k)
@@ -540,8 +544,12 @@ function sync(src::AbstractPath, dst::AbstractPath; delete=false)
540544
end
541545

542546
# Finally, copy over files that don't exist at the destination
543-
for (seg, p) in index
544-
cp(p, Path(dst, tuple(dst.segments..., seg...)); force=true)
547+
# But we need to iterate through it in a way that respects the original
548+
# walkpath order otherwise we may end up trying to copy a file before its parents.
549+
index_pairs = collect(pairs(index))
550+
index_pairs = index_pairs[sortperm(last.(index_pairs))]
551+
for (seg, i) in index_pairs
552+
cp(src_paths[i], Path(dst, tuple(dst.segments..., seg...)); force=true)
545553
end
546554
else
547555
cp(src, dst)

src/test.jl

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -640,6 +640,29 @@ module TestPaths
640640
sync(ps.foo, ps.qux / "foo"; delete=true)
641641
@test !exists(ps.qux / "foo" / "test.txt")
642642
rm(ps.qux / "foo"; recursive=true)
643+
644+
# Test a condtion where the index could reorder the walkpath order.
645+
tmp_src = ps.root / "tmp-src"
646+
mkdir(tmp_src)
647+
src_file = tmp_src / "file1"
648+
write(src_file, "Hello World!")
649+
650+
src_folder = tmp_src / "folder1"
651+
mkdir(src_folder)
652+
src_folder_file = src_folder / "file2"
653+
write(src_folder_file, "") # empty file
654+
655+
src_folder2 = src_folder / "folder2" # nested folders
656+
mkdir(src_folder2)
657+
src_folder2_file = src_folder2 / "file3"
658+
write(src_folder2_file, "Test")
659+
660+
tmp_dst = ps.root / "tmp_dst"
661+
mkdir(tmp_dst)
662+
sync(tmp_src, tmp_dst)
663+
@test exists(tmp_dst / "folder1" / "folder2" / "file3")
664+
rm(tmp_src; recursive=true)
665+
rm(tmp_dst; recursive=true)
643666
end
644667
end
645668

0 commit comments

Comments
 (0)