Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,19 @@
# 0.1.23

**Critical Bug Fix:**
- Fix cleanup_folder to respect 'only' filter and prevent unintended deletions
- When force=true was used with an 'only' filter, cleanup_folder was deleting all files in the destination that weren't explicitly ignored, including unrelated folders not being managed by the mapping
- This caused data loss for folders like cabal/ and ghc/ that weren't in the only list
- The fix ensures only paths matching the inclusion filter are cleaned up, leaving unmanaged content intact

**Test Coverage:**
- Add comprehensive test coverage for the cleanup_folder bug fix
- Test preserving unrelated folders when using force + only
- Test only cleaning managed paths specified in only list
- Test edge case with empty source and unmanaged dest content
- Update existing test to reflect correct behavior
- Total: 326 examples, 0 failures, 2 pending with 96.41% line coverage

# 0.1.22

**Testing & Quality:**
Expand Down
2 changes: 1 addition & 1 deletion Gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
PATH
remote: .
specs:
dotsync (0.1.22)
dotsync (0.1.23)
fileutils (~> 1.7.3)
find (~> 0.2.0)
listen (~> 3.9.0)
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Dotsync

[![Gem Version](https://badge.fury.io/rb/dotsync.svg)](https://rubygems.org/gems/dotsync)
[![Ruby Gem Test Status](https://github.com/dsaenztagarro/dotsync/actions/workflows/gem-push.yml/badge.svg)](https://github.com/dsaenztagarro/dotsync/actions)
[![CI](https://github.com/dsaenztagarro/dotsync/actions/workflows/gem-push.yml/badge.svg)](https://github.com/dsaenztagarro/dotsync/actions)
[![License: MIT](https://img.shields.io/badge/License-MIT-yellow.svg)](https://opensource.org/licenses/MIT)
[![Ruby Version](https://img.shields.io/badge/ruby-%3E%3D%203.2-blue)](https://www.ruby-lang.org/)

Expand Down
10 changes: 10 additions & 0 deletions lib/dotsync/utils/file_transfer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -144,10 +144,20 @@ def cleanup_folder(target_dir)
Find.find(target_dir) do |path|
next if path == target_dir
abs_path = File.expand_path(path)

# Skip if ignored
if @mapping.ignore?(abs_path)
Find.prune if File.directory?(path)
next
end

# When inclusions (only) are specified, only clean up paths that match the inclusion filter
# This ensures we don't delete unrelated files/folders that aren't being managed
unless @mapping.include?(abs_path)
Find.prune if File.directory?(path)
next
end

if File.file?(path)
FileUtils.rm(path)
elsif File.directory?(path) && Dir.empty?(path)
Expand Down
2 changes: 1 addition & 1 deletion lib/dotsync/version.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# frozen_string_literal: true

module Dotsync
VERSION = "0.1.22"
VERSION = "0.1.23"
end
85 changes: 82 additions & 3 deletions spec/dotsync/utils/file_transfer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -241,15 +241,94 @@
expect(File.read(File.join(dest, "folder3", "subfolder2", "file4.txt"))).to eq("dest content")
expect(File.read(File.join(dest, "file7.txt"))).to eq("dest content")

# Paths not included in "only" option
# expect(File.read(File.join(dest, "folder3", "subfolder3", "sub2folder2", "file6.txt"))).to eq("dest content")
expect(File.exist?(File.join(dest, "folder3", "subfolder3", "sub2folder2", "file6.txt"))).to be false
# Paths not included in "only" option should be preserved (not deleted)
expect(File.read(File.join(dest, "folder3", "subfolder3", "sub2folder2", "file6.txt"))).to eq("dest content")

# Paths included in "only" option
expect(File.read(File.join(dest, "folder1", "file1.txt"))).to eq("src content")
expect(File.read(File.join(dest, "folder3", "subfolder1", "file3.txt"))).to eq("src content")
expect(File.read(File.join(dest, "file8.txt"))).to eq("src content")
end

context "with unrelated folders in destination" do
let(:only) { ["folder1", "file8.txt"] }
let(:ignore) { [] }

before do
# Create source files that match the 'only' filter
FileUtils.rm_rf(src)
FileUtils.mkdir_p(File.join(src, "folder1"))
File.write(File.join(src, "folder1", "file1.txt"), "new src content")
File.write(File.join(src, "file8.txt"), "new src content")

# Create unrelated folders in destination that should NOT be deleted
FileUtils.mkdir_p(File.join(dest, "cabal"))
File.write(File.join(dest, "cabal", "config"), "cabal config content")

FileUtils.mkdir_p(File.join(dest, "ghc"))
File.write(File.join(dest, "ghc", "ghci.conf"), "ghc config content")

# Create additional files in managed folders that should be cleaned up
FileUtils.mkdir_p(File.join(dest, "folder1"))
File.write(File.join(dest, "folder1", "old_file.txt"), "old content to remove")
end

it "does not delete unrelated folders that are not in the only list" do
subject.transfer

# Unrelated folders should be preserved
expect(File.exist?(File.join(dest, "cabal", "config"))).to be true
expect(File.read(File.join(dest, "cabal", "config"))).to eq("cabal config content")
expect(File.exist?(File.join(dest, "ghc", "ghci.conf"))).to be true
expect(File.read(File.join(dest, "ghc", "ghci.conf"))).to eq("ghc config content")

# Managed folders should be cleaned and synced
expect(File.exist?(File.join(dest, "folder1", "file1.txt"))).to be true
expect(File.read(File.join(dest, "folder1", "file1.txt"))).to eq("new src content")
expect(File.exist?(File.join(dest, "folder1", "old_file.txt"))).to be false

# Only included files should be transferred
expect(File.exist?(File.join(dest, "file8.txt"))).to be true
expect(File.read(File.join(dest, "file8.txt"))).to eq("new src content")
end

it "only cleans up files within the managed paths specified in only list" do
# Add more dest files not in the only list
FileUtils.mkdir_p(File.join(dest, "other_folder"))
File.write(File.join(dest, "other_folder", "other_file.txt"), "other content")
File.write(File.join(dest, "unmanaged.txt"), "unmanaged content")

subject.transfer

# Files not in the only list should be untouched
expect(File.exist?(File.join(dest, "other_folder", "other_file.txt"))).to be true
expect(File.read(File.join(dest, "other_folder", "other_file.txt"))).to eq("other content")
expect(File.exist?(File.join(dest, "unmanaged.txt"))).to be true
expect(File.read(File.join(dest, "unmanaged.txt"))).to eq("unmanaged content")
end
end

context "when source is empty and only filter is specified" do
let(:only) { ["folder1"] }
let(:ignore) { [] }

before do
FileUtils.rm_rf(src)
FileUtils.mkdir_p(src)

# Create dest with files that shouldn't be touched since nothing is in src
FileUtils.mkdir_p(File.join(dest, "unmanaged_folder"))
File.write(File.join(dest, "unmanaged_folder", "important_file.txt"), "important content")
end

it "does not delete unmanaged folders when source is empty" do
subject.transfer

# Unmanaged folders should be preserved
expect(File.exist?(File.join(dest, "unmanaged_folder", "important_file.txt"))).to be true
expect(File.read(File.join(dest, "unmanaged_folder", "important_file.txt"))).to eq("important content")
end
end
end
end
end
Expand Down