From bd91ddce57ffc1d4dedd4e84d1ed85510c76e5b9 Mon Sep 17 00:00:00 2001 From: Sylvain Joyeux Date: Fri, 13 Mar 2026 17:35:56 -0300 Subject: [PATCH] chore: use the lock file to know whether a folder is a Roby log dir We were using info.yml so far, but info.yml is archived itself. This breaks archiving in the presence of remote process servers, when the remote transfers files after the info.yml got archived. --- lib/syskit/cli/log_runtime_archive.rb | 2 +- test/cli/test_log_runtime_archive.rb | 49 ++++++++++++++--------- test/cli/test_log_runtime_archive_main.rb | 2 +- 3 files changed, 31 insertions(+), 22 deletions(-) diff --git a/lib/syskit/cli/log_runtime_archive.rb b/lib/syskit/cli/log_runtime_archive.rb index 7d6f1cf76..b903c71ca 100644 --- a/lib/syskit/cli/log_runtime_archive.rb +++ b/lib/syskit/cli/log_runtime_archive.rb @@ -361,7 +361,7 @@ def self.find_all_dataset_folders(root_dir) child = (root_dir / child) next unless child.directory? - child if (child / "info.yml").file? + child if (child / Roby::Application::LOCK_FILE_EXT).file? end candidates.compact.sort_by { |a| a.basename.to_s } diff --git a/test/cli/test_log_runtime_archive.rb b/test/cli/test_log_runtime_archive.rb index eff6033f1..8485d061b 100644 --- a/test/cli/test_log_runtime_archive.rb +++ b/test/cli/test_log_runtime_archive.rb @@ -11,6 +11,11 @@ module CLI @root = make_tmppath @archive_path = (make_tmppath / "archive.tar") @in_files = [] + @lock_files = {} + end + + after do + @lock_files.each_value(&:close) end describe ".find_all_dataset_folders" do @@ -148,11 +153,10 @@ module CLI describe ".archive_dataset" do it "in full mode, archives only the rotated logs if there are some" do - dataset = make_valid_folder("20220434-2023") + dataset = make_valid_folder("20220434-2023", locked: false) make_in_file "test.0.log", "test0", root: dataset make_in_file "test.1.log", "test1", root: dataset make_in_file "something.txt", "something", root: dataset - make_in_file ".lock", "something", root: dataset ret = @archive_path.open("w") do |archive_io| flexmock(LogRuntimeArchive) @@ -174,9 +178,8 @@ module CLI it "in full mode, archives the non-rotated logs " \ "if there are no rotated logs" do - dataset = make_valid_folder("20220434-2023") + dataset = make_valid_folder("20220434-2023", locked: false) make_in_file "something.txt", "something", root: dataset - make_in_file ".lock", "something", root: dataset ret = @archive_path.open("w") do |archive_io| flexmock(LogRuntimeArchive) @@ -401,13 +404,9 @@ module CLI end it "archives all folders, the locked ones partially" do - dataset0 = make_valid_folder("20220434-2023") - dataset1 = make_valid_folder("20220434-2024") - dataset2 = make_valid_folder("20220434-2025") - - (dataset0 / ".lock").write("") # unlocked dir - # No lock file means locked for dataset1 - (dataset2 / ".lock").write("") # unlocked dir + dataset0 = make_valid_folder("20220434-2023", locked: false) + dataset1 = make_valid_folder("20220434-2024", locked: true) + dataset2 = make_valid_folder("20220434-2025", locked: false) should_archive_dataset(dataset0, "20220434-2023.0.tar", full: true) should_archive_dataset(dataset1, "20220434-2024.0.tar", full: false) @@ -523,15 +522,13 @@ module CLI end it "gathers all non-rotated logs in the very last archive" do - dataset = make_valid_folder("20220434-2023") + dataset = make_valid_folder("20220434-2023", locked: false) make_random_file "test.0.log", root: dataset make_random_file "test.1.log", root: dataset make_random_file "test.2.log", root: dataset make_random_file "test.txt", root: dataset make_random_file "test-PID.txt", root: dataset - (dataset / ".lock").write("") - @process.process_root_folder entries = read_archive(path: @archive_dir / "20220434-2023.0.tar") @@ -592,13 +589,12 @@ def create_server(params) describe ".process_root_folder_transfer" do it "transfers all finished dataset files from root folder " \ "through FTP" do - dataset_a = make_valid_folder("20220434-2023") + dataset_a = make_valid_folder("20220434-2023", locked: false) dataset_b = make_valid_folder("20220434-2024") make_random_file "test.0.log", root: dataset_a make_random_file "test.1.log", root: dataset_a make_random_file "test.0.log", root: dataset_b make_random_file "test.1.log", root: dataset_b - (dataset_a / ".lock").write("") @process.process_root_folder_transfer(@params) @@ -612,14 +608,14 @@ def create_server(params) end describe ".process_dataset_transfer" do - it "ignores the .lock file" do + it "ignores the .roby-logdir file" do dataset = make_valid_folder("PATH") - make_random_file ".lock", root: dataset + make_random_file ".roby-logdir", root: dataset @process.process_dataset_transfer( dataset, @params, @root, full: true ) - refute(File.exist?(@target_dir / "PATH" / ".lock")) + refute(File.exist?(@target_dir / "PATH" / ".roby-logdir")) end it "transfers all files from a folder through FTP" do @@ -872,13 +868,26 @@ def assert_deleted_files(deleted_files, directory: @archive_dir) end end - def make_valid_folder(name) + def make_valid_folder(name, locked: true) path = (@root / name) path.mkpath + lockfile_path = path / ".roby-logdir" + FileUtils.touch(lockfile_path) + + if locked + io = lockfile_path.open(File::RDWR | File::CREAT, 0o644) + @lock_files[lockfile_path] = io + io.flock(File::LOCK_EX | File::LOCK_NB) + end + FileUtils.touch(path / "info.yml") path end + def unlock_folder(path) + @lock_files.delete(path).close + end + def make_random_file(name, root: @root, size: 1024) content = Base64.encode64(Random.bytes(size)) make_in_file name, content, root: root diff --git a/test/cli/test_log_runtime_archive_main.rb b/test/cli/test_log_runtime_archive_main.rb index 6bacec733..fdb804366 100644 --- a/test/cli/test_log_runtime_archive_main.rb +++ b/test/cli/test_log_runtime_archive_main.rb @@ -291,7 +291,7 @@ def call_archive(root_path, archive_path, low_limit, freed_limit) @server = call_create_server(root_tmp_path, @server_params) dataset_a = make_dataset(dataset_tmp_path, "19981222-1301") - (dataset_a / ".lock").write("") # unlock the dir + (dataset_a / ".roby-logdir").write("") # unlock the dir call_transfer(dataset_tmp_path) assert(File.exist?(root_tmp_path / "19981222-1301" / "test.0.log"))