feat(p7-times): real directory/symlink time preservation; -O/-J meaningful
Wave D of Phase 7. Make -O/--omit-dir-times and -J/--omit-link-times real by preserving directory and symlink times, and mark --secluded-args as an explicit Impossible/Divergence no-op. Wire: PROTOCOL_VERSION 2.16.0 -> 2.17.0. Adds a terminal STATUS_DIR_TIMES frame (int count + (wire path, metadata) pairs) sent after all file data and the optional delete manifest. STATUS_MKDIR also carries metadata for --dirs entries. Config-frame layout is unchanged. Sender: the recursive scanner captures every traversed source directory (both DirectoryScanner and the parallel scanner root + workers, appends mutex-guarded) into a shared list; the single-threaded and -m paths transmit it last. Receiver: a DirTimeList accumulates received directory metadata and applies it with fd-relative no-follow utimensat only at the very end -- after all children, after the commit-style --delete, and after --delay-updates publication -- in the single-threaded success frame and in server.c after the -m threads join. -O skips the application. Symlink metadata is applied at link creation with utimensat/fchownat/fchmodat AT_SYMLINK_NOFOLLOW; -J suppresses only link times. identity_apply_ownership_link shares the identity resolver with the fd path. Docs: -O/-J rows -> Implemented; --secluded-args -> Impossible/Divergence; --protocol accepted/rejected values and Phase-6/7 notes updated. Tests: unit (scanner dir capture, DirTimeList apply, symlink metadata, protocol version values) and integration (dir mtime round-trip + -O, symlink mtime round-trip + -J, independent suppression), parameterized over single/multithread.
This commit is contained in:
@@ -4672,3 +4672,128 @@ class TestConnectivityClientOptions:
|
||||
f"--blocking-io -c failed: {(result.stderr or result.stdout)[:300]}"
|
||||
mismatches, missing = verify_transfer(source, get_dest_received_dir(dest, source))
|
||||
assert not mismatches and not missing
|
||||
|
||||
|
||||
DISTINCT_MTIME = 1_000_000_000 # 2001-09-09T01:46:40Z; a whole second
|
||||
|
||||
|
||||
class TestDirectoryAndSymlinkTimes:
|
||||
"""P7 Wave D: -O/--omit-dir-times and -J/--omit-link-times are real.
|
||||
|
||||
FastSync now captures and applies directory mtimes (deferred to the end of
|
||||
the transfer, after children) and symlink mtimes (immediate, via no-follow
|
||||
primitives). -O/-J suppress exactly their own class of times.
|
||||
"""
|
||||
|
||||
def _tree(self, name):
|
||||
source = os.path.join(TEST_DATA_DIR, name + "_src")
|
||||
dest = os.path.join(TEST_DATA_DIR, name + "_dst")
|
||||
clean_dir(source)
|
||||
clean_dir(dest)
|
||||
os.makedirs(os.path.join(source, "sub", "deep"), exist_ok=True)
|
||||
with open(os.path.join(source, "sub", "file.txt"), "wb") as fh:
|
||||
fh.write(b"content\n")
|
||||
with open(os.path.join(source, "sub", "deep", "deep.txt"), "wb") as fh:
|
||||
fh.write(b"deeper\n")
|
||||
dirs = (source, os.path.join(source, "sub"), os.path.join(source, "sub", "deep"))
|
||||
for d in dirs:
|
||||
os.utime(d, (DISTINCT_MTIME, DISTINCT_MTIME))
|
||||
if abs(os.stat(source).st_mtime - DISTINCT_MTIME) > 2:
|
||||
pytest.skip("filesystem does not preserve directory mtimes")
|
||||
return source, dest, ("", "sub", os.path.join("sub", "deep"))
|
||||
|
||||
def _link_tree(self, name):
|
||||
source = os.path.join(TEST_DATA_DIR, name + "_src")
|
||||
dest = os.path.join(TEST_DATA_DIR, name + "_dst")
|
||||
clean_dir(source)
|
||||
clean_dir(dest)
|
||||
os.makedirs(os.path.join(source, "sub"), exist_ok=True)
|
||||
with open(os.path.join(source, "sub", "file.txt"), "wb") as fh:
|
||||
fh.write(b"target\n")
|
||||
link = os.path.join(source, "sub", "link")
|
||||
# A same-directory relative target (no ".."): FastSync refuses an
|
||||
# escaping/ambiguous symlink target, and ".." is a deliberate divergence.
|
||||
os.symlink("file.txt", link)
|
||||
os.utime(link, (DISTINCT_MTIME, DISTINCT_MTIME), follow_symlinks=False)
|
||||
if abs(os.lstat(link).st_mtime - DISTINCT_MTIME) > 2:
|
||||
pytest.skip("filesystem does not preserve symlink mtimes")
|
||||
return source, dest, os.path.join("sub", "link")
|
||||
|
||||
def _run(self, source, dest, flags, shared_server):
|
||||
result, _ = run_client(source, dest, flags=flags, port=shared_server.port)
|
||||
assert result.returncode == 0, \
|
||||
f"{flags} failed: {(result.stderr or result.stdout)[:400]}"
|
||||
return get_dest_received_dir(dest, source)
|
||||
|
||||
@pytest.mark.ci
|
||||
@pytest.mark.parametrize("mt", [False, True])
|
||||
def test_directory_mtime_round_trip(self, shared_server, mt):
|
||||
source, dest, rels = self._tree("dirtime")
|
||||
flags = ["-a"] + (["--threads"] if mt else [])
|
||||
received = self._run(source, dest, flags, shared_server)
|
||||
for rel in rels:
|
||||
src_m = os.stat(os.path.join(source, rel)).st_mtime
|
||||
dst_m = os.stat(os.path.join(received, rel)).st_mtime
|
||||
assert abs(dst_m - src_m) < 2, \
|
||||
f"dir '{rel}': source={src_m} dest={dst_m} (flags={flags})"
|
||||
|
||||
@pytest.mark.ci
|
||||
@pytest.mark.parametrize("mt", [False, True])
|
||||
def test_omit_dir_times_suppresses_only_dirs(self, shared_server, mt):
|
||||
source, dest, rels = self._tree("omitdir")
|
||||
flags = ["-a", "-O"] + (["--threads"] if mt else [])
|
||||
received = self._run(source, dest, flags, shared_server)
|
||||
for rel in rels:
|
||||
dst_m = os.stat(os.path.join(received, rel)).st_mtime
|
||||
assert abs(dst_m - DISTINCT_MTIME) > 5, \
|
||||
f"-O must not apply directory times ('{rel}' got {dst_m})"
|
||||
|
||||
@pytest.mark.ci
|
||||
@pytest.mark.parametrize("mt", [False, True])
|
||||
def test_symlink_mtime_round_trip(self, shared_server, mt):
|
||||
source, dest, rel = self._link_tree("linktime")
|
||||
flags = ["-a"] + (["--threads"] if mt else [])
|
||||
received = self._run(source, dest, flags, shared_server)
|
||||
src_link = os.path.join(source, rel)
|
||||
dst_link = os.path.join(received, rel)
|
||||
assert os.path.islink(dst_link), f"{dst_link} is not a symlink"
|
||||
src_m = os.lstat(src_link).st_mtime
|
||||
dst_m = os.lstat(dst_link).st_mtime
|
||||
assert abs(dst_m - src_m) < 2, f"symlink times: source={src_m} dest={dst_m}"
|
||||
|
||||
@pytest.mark.ci
|
||||
@pytest.mark.parametrize("mt", [False, True])
|
||||
def test_omit_link_times_suppresses_only_links(self, shared_server, mt):
|
||||
source, dest, rel = self._link_tree("omitlink")
|
||||
flags = ["-a", "-J"] + (["--threads"] if mt else [])
|
||||
received = self._run(source, dest, flags, shared_server)
|
||||
dst_link = os.path.join(received, rel)
|
||||
assert os.path.islink(dst_link), f"{dst_link} is not a symlink"
|
||||
dst_m = os.lstat(dst_link).st_mtime
|
||||
assert abs(dst_m - DISTINCT_MTIME) > 5, \
|
||||
f"-J must not apply symlink times (got {dst_m})"
|
||||
|
||||
@pytest.mark.ci
|
||||
def test_omit_flags_are_independent(self, shared_server):
|
||||
"""-O suppresses only directory times and -J only symlink times: with
|
||||
-O the symlink time is still preserved, and with -J the dir times are."""
|
||||
source, dest, rel = self._link_tree("omitindep")
|
||||
# Add a subdirectory mtime to check alongside the symlink.
|
||||
sub = os.path.join(source, "sub")
|
||||
os.utime(sub, (DISTINCT_MTIME, DISTINCT_MTIME))
|
||||
|
||||
# -O => dir times omitted, symlink time preserved.
|
||||
clean_dir(dest + "_o")
|
||||
recv_o = self._run(source, dest + "_o", ["-a", "-O"], shared_server)
|
||||
assert abs(os.lstat(os.path.join(recv_o, rel)).st_mtime - DISTINCT_MTIME) < 2, \
|
||||
"-O must not suppress symlink times"
|
||||
assert abs(os.stat(os.path.join(recv_o, "sub")).st_mtime - DISTINCT_MTIME) > 5, \
|
||||
"-O must suppress directory times"
|
||||
|
||||
# -J => symlink times omitted, dir times preserved.
|
||||
clean_dir(dest + "_j")
|
||||
recv_j = self._run(source, dest + "_j", ["-a", "-J"], shared_server)
|
||||
assert abs(os.lstat(os.path.join(recv_j, rel)).st_mtime - DISTINCT_MTIME) > 5, \
|
||||
"-J must suppress symlink times"
|
||||
assert abs(os.stat(os.path.join(recv_j, "sub")).st_mtime - DISTINCT_MTIME) < 2, \
|
||||
"-J must not suppress directory times"
|
||||
|
||||
@@ -94,14 +94,14 @@ def _seed_protocol_source(source):
|
||||
class TestProtocol:
|
||||
@pytest.mark.ci
|
||||
def test_protocol_current_version_accepted(self, shared_server):
|
||||
"""--protocol=2.16.0 (the current PROTOCOL_VERSION) is accepted and the
|
||||
"""--protocol=2.17.0 (the current PROTOCOL_VERSION) is accepted and the
|
||||
transfer completes normally."""
|
||||
source = os.path.join(TEST_DATA_DIR, "proto_ok_src")
|
||||
dest = os.path.join(TEST_DATA_DIR, "proto_ok_dst")
|
||||
shutil.rmtree(dest, ignore_errors=True)
|
||||
os.makedirs(dest)
|
||||
_seed_protocol_source(source)
|
||||
result, _ = run_client(source, dest, flags=["--protocol=2.16.0"],
|
||||
result, _ = run_client(source, dest, flags=["--protocol=2.17.0"],
|
||||
port=shared_server.port)
|
||||
assert result.returncode == 0, \
|
||||
f"--protocol current run failed: {(result.stderr or result.stdout)[:400]}"
|
||||
@@ -118,7 +118,7 @@ class TestProtocol:
|
||||
shutil.rmtree(dest, ignore_errors=True)
|
||||
os.makedirs(dest)
|
||||
_seed_protocol_source(source)
|
||||
for bad in ("2.15.0", "2.17.0", "216", "31"):
|
||||
for bad in ("2.15.0", "2.16.0", "216", "31"):
|
||||
result, _ = run_client(source, dest, flags=[f"--protocol={bad}"],
|
||||
port=shared_server.port)
|
||||
assert result.returncode != 0, f"--protocol={bad} should be rejected"
|
||||
|
||||
@@ -222,7 +222,7 @@ static void test_parse_args_protocol_accept_current() {
|
||||
Config* cfg = valid_client_config();
|
||||
EXPECT_NOT_NULL(cfg);
|
||||
char* argv_equals[] = {"fastsync", "--source-dir", "/src",
|
||||
"--dest-dir", "/dst", "--protocol=2.16.0"};
|
||||
"--dest-dir", "/dst", "--protocol=2.17.0"};
|
||||
int positional_args[2];
|
||||
int positional_count = 0;
|
||||
EXPECT_EQ_INT(parse_args(cfg, 6, argv_equals, positional_args, &positional_count), 0);
|
||||
@@ -232,7 +232,7 @@ static void test_parse_args_protocol_accept_current() {
|
||||
cfg = valid_client_config();
|
||||
EXPECT_NOT_NULL(cfg);
|
||||
char* argv_space[] = {"fastsync", "--source-dir", "/src", "--dest-dir",
|
||||
"/dst", "--protocol", "2.16.0"};
|
||||
"/dst", "--protocol", "2.17.0"};
|
||||
positional_count = 0;
|
||||
EXPECT_EQ_INT(parse_args(cfg, 7, argv_space, positional_args, &positional_count), 0);
|
||||
EXPECT_EQ_STR(cfg->version, PROTOCOL_VERSION);
|
||||
@@ -242,7 +242,7 @@ static void test_parse_args_protocol_accept_current() {
|
||||
/* Any --protocol value other than the current PROTOCOL_VERSION must end in
|
||||
* failure (parse_args simply stores it; validate_config rejects it up front). */
|
||||
static void test_parse_args_protocol_rejects_other_versions() {
|
||||
static const char* const bad_versions[] = {"2.16", "2.15.0", "2.17.0", "216", "31", "abc", ""};
|
||||
static const char* const bad_versions[] = {"2.16", "2.15.0", "2.16.0", "216", "31", "abc", ""};
|
||||
for (size_t i = 0; i < sizeof(bad_versions) / sizeof(bad_versions[0]); i++) {
|
||||
Config* cfg = valid_client_config();
|
||||
EXPECT_NOT_NULL(cfg);
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
#include "test_file.h"
|
||||
#include "file.h"
|
||||
#include "file_receive.h"
|
||||
#include "data.h"
|
||||
#include "config.h"
|
||||
#include "utils.h"
|
||||
@@ -1216,6 +1217,39 @@ void test_trust_sender() {
|
||||
file_set_authorized_root(-1, NULL);
|
||||
}
|
||||
|
||||
/* P7 Wave D: the deferred directory-time list deep-copies entries and applies
|
||||
* them (fd-relative, no-follow) to an existing directory, then frees cleanly. */
|
||||
static void test_dir_time_list() {
|
||||
const char* root = "test_dir_time_root";
|
||||
const char* sub = "test_dir_time_root/sub";
|
||||
file_set_authorized_root(-1, NULL);
|
||||
rmdir(sub);
|
||||
rmdir(root);
|
||||
EXPECT_EQ_INT(mkdir(root, 0755), 0);
|
||||
EXPECT_EQ_INT(mkdir(sub, 0755), 0);
|
||||
|
||||
DirTimeList list;
|
||||
dir_time_list_init(&list);
|
||||
EXPECT_EQ_INT((int)list.count, 0);
|
||||
FileMetadata metadata = {.mtime_sec = 1000000000, .mtime_nsec = 0};
|
||||
EXPECT_TRUE(dir_time_list_add(&list, "sub", &metadata));
|
||||
EXPECT_TRUE(dir_time_list_add(&list, "sub", &metadata));
|
||||
EXPECT_EQ_INT((int)list.count, 2);
|
||||
|
||||
dir_time_list_apply(&list, root);
|
||||
struct stat st;
|
||||
EXPECT_EQ_INT(stat(sub, &st), 0);
|
||||
EXPECT_EQ_INT((int)st.st_mtime, 1000000000);
|
||||
|
||||
dir_time_list_free(&list);
|
||||
EXPECT_EQ_INT((int)list.count, 0);
|
||||
EXPECT_NULL(list.paths);
|
||||
EXPECT_NULL(list.entries);
|
||||
|
||||
rmdir(sub);
|
||||
rmdir(root);
|
||||
}
|
||||
|
||||
void test_file() {
|
||||
test_file_create();
|
||||
test_file_special_rdev_valid();
|
||||
@@ -1254,6 +1288,7 @@ void test_file() {
|
||||
test_file_send_single_calls_metadata_and_path();
|
||||
}
|
||||
test_file_metadata_create();
|
||||
test_dir_time_list();
|
||||
test_inplace_overwrite_clears_special_mode_bits();
|
||||
test_inplace_overwrite_metadata_strips_special_bits();
|
||||
test_inplace_overwrite_truncates_shorter_payload();
|
||||
|
||||
@@ -302,6 +302,39 @@ static void test_chmod_changes() {
|
||||
EXPECT_FALSE(chmod_apply(0777, "a+r,", &result));
|
||||
}
|
||||
|
||||
/* P7 Wave D: symlink metadata is applied with no-follow primitives, and -J
|
||||
* (omit_link_times) suppresses the timestamp. The test is robust to
|
||||
* filesystems that silently ignore symlink timestamps: it mainly proves the
|
||||
* omit path never touches the stored time. */
|
||||
static void test_file_restore_symlink_metadata() {
|
||||
const char* dir = "temp_symlink_md_test";
|
||||
const char* target = "temp_symlink_md_test/target";
|
||||
const char* link = "temp_symlink_md_test/link";
|
||||
EXPECT_EQ_INT(mkdir(dir, 0755), 0);
|
||||
FILE* f = fopen(target, "w");
|
||||
EXPECT_NOT_NULL(f);
|
||||
fputs("t", f);
|
||||
fclose(f);
|
||||
EXPECT_EQ_INT(symlink("target", link), 0);
|
||||
|
||||
FileMetadata applied = {.mtime_sec = 1000000000, .mtime_nsec = 0};
|
||||
file_restore_symlink_metadata(link, &applied, false);
|
||||
struct stat st;
|
||||
EXPECT_EQ_INT(lstat(link, &st), 0);
|
||||
EXPECT_TRUE(S_ISLNK(st.st_mode));
|
||||
time_t t1 = st.st_mtime;
|
||||
|
||||
/* -J: a different time must be left untouched. */
|
||||
FileMetadata newer = {.mtime_sec = 1234567890, .mtime_nsec = 0};
|
||||
file_restore_symlink_metadata(link, &newer, true);
|
||||
EXPECT_EQ_INT(lstat(link, &st), 0);
|
||||
EXPECT_EQ_INT((int)st.st_mtime, (int)t1);
|
||||
|
||||
unlink(link);
|
||||
unlink(target);
|
||||
rmdir(dir);
|
||||
}
|
||||
|
||||
void test_metadata() {
|
||||
test_metadata_to_from_buf_roundtrip();
|
||||
test_metadata_to_buf_null();
|
||||
@@ -315,5 +348,6 @@ void test_metadata() {
|
||||
test_file_restore_metadata_applies_atime();
|
||||
test_file_restore_executability_only();
|
||||
test_directory_restore_executability_only();
|
||||
test_file_restore_symlink_metadata();
|
||||
test_chmod_changes();
|
||||
}
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
#include "test_utils.h"
|
||||
#include "scanner.h"
|
||||
#include "array_list.h"
|
||||
#include "file.h"
|
||||
#include "file_list.h"
|
||||
#include "filter.h"
|
||||
@@ -1262,6 +1263,51 @@ static void test_files_from_relative_send_path() {
|
||||
rmdir(root);
|
||||
}
|
||||
|
||||
/* P7 Wave D: the recursive scan captures every traversed source directory as an
|
||||
* is_dir File (metadata, no payload) in the shared dir_entries list, including
|
||||
* the transfer root, so the sender can transmit directory times at the end. */
|
||||
static void test_scanner_captures_directory_times() {
|
||||
const char* root = "test_scan_dirtime";
|
||||
const char* sub = "test_scan_dirtime/sub";
|
||||
const char* file1 = "test_scan_dirtime/sub/a.txt";
|
||||
EXPECT_EQ_INT(mkdir(root, 0755), 0);
|
||||
EXPECT_EQ_INT(mkdir(sub, 0755), 0);
|
||||
create_test_file(file1, "x");
|
||||
|
||||
ArrayList* dirs = array_list_create(file_destroy);
|
||||
EXPECT_NOT_NULL(dirs);
|
||||
ScannerOptions options = {0};
|
||||
options.use_metadata = true;
|
||||
options.capture_dir_times = true;
|
||||
options.dir_entries = dirs;
|
||||
DirectoryScanner* scanner = directory_scanner_create_with_options(root, &options);
|
||||
EXPECT_NOT_NULL(scanner);
|
||||
Chunk* chunk;
|
||||
while ((chunk = directory_scanner_next(scanner)) != NULL)
|
||||
chunk_destroy(chunk);
|
||||
EXPECT_FALSE(directory_scanner_failed(scanner));
|
||||
|
||||
int found_root = 0;
|
||||
int found_sub = 0;
|
||||
for (int i = 0; i < dirs->size; i++) {
|
||||
const File* file = (const File*)dirs->items[i];
|
||||
EXPECT_TRUE(file->is_dir);
|
||||
EXPECT_NOT_NULL(file->metadata);
|
||||
if (strcmp(file->path, root) == 0)
|
||||
found_root = 1;
|
||||
if (strcmp(file->path, sub) == 0)
|
||||
found_sub = 1;
|
||||
}
|
||||
EXPECT_TRUE(found_root);
|
||||
EXPECT_TRUE(found_sub);
|
||||
|
||||
directory_scanner_destroy(scanner);
|
||||
array_list_delete(dirs);
|
||||
unlink(file1);
|
||||
rmdir(sub);
|
||||
rmdir(root);
|
||||
}
|
||||
|
||||
void test_scanner() {
|
||||
test_scanner_single_file();
|
||||
test_scanner_multiple_files();
|
||||
@@ -1297,4 +1343,5 @@ void test_scanner() {
|
||||
test_dirs_no_descent();
|
||||
test_dirs_files_from();
|
||||
test_files_from_relative_send_path();
|
||||
test_scanner_captures_directory_times();
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user