fix(p7-cli-namespace): address c-review (setfacl -m mangled to --threads; two -s tests lost intent; label/doc sweeps)

- revert over-eager replacement of setfacl -m in test_features.py
- use --chunk-serialization (+ set dirs) in the two append/append-verify
  chunk-serialization rejection unit tests so they exercise the real check
- rename archive-negation integration test (--no-preserve is inert under
  archive because devices/specials force metadata)
- update stale (-c)/(-m)/(-s)/(-f) display labels and RSYNC_COMPAT -c/-m refs
- document the --no-perms negation limitation in the Wave A note
This commit is contained in:
2026-09-11 14:34:03 +02:00
parent 66e82f9384
commit d39ddab42c
6 changed files with 41 additions and 33 deletions
+4 -4
View File
@@ -74,7 +74,7 @@ This document maps rsync's full feature set to FastSync's current implementation
| `-r`, `--recursive` | Recurse into directories | ✅ Implemented | Default behavior |
| `-R`, `--relative` | Use relative path names | ✅ Implemented | Meaningful together with `--files-from` (FastSync's default full-tree scan always mirrors the full source argument path below the destination root, so -R does not change it). With `-R` + `--files-from` each listed entry is transmitted under its bare relative destination path: an entry `sub/x.txt` lands at `<dest>/sub/x.txt` (its leading components preserved) instead of under the `<dest>/<full source path>` mirror. Only the path sent on the wire changes; the client still reads the absolute source path, and the delete manifest derives from the sent (relative) paths so `--delete` and `--remove-source-files` stay consistent in both layouts. Works single-threaded and under `-m` (including chunk serialization) |
| `--no-implied-dirs` | Don't send implied dirs with -R | ✅ Implemented | Client-side, meaningful only with `-R` + `--files-from`. rsync would normally create the ancestor directories implied by a listed file so it can be written; with `--no-implied-dirs` a listed file whose parent directory is not itself (or via an ancestor) explicitly listed cannot be placed, and FastSync fails the whole run up front with a clear error (`--no-implied-dirs: cannot place file '...': parent directory '...' is not explicitly listed`). Listing the directory (or an ancestor of it, or the whole tree `.`) permits the file. In every other mode the option has no effect. FastSync has no per-entry skip channel, so the rsync "omit the file" case is surfaced as a hard pre-transfer error |
| `-d`, `--dirs`, `--old-dirs`, `--old-d` | Transfer dirs without recursing | ✅ Implemented | `-d <dir>` transmits an explicit directory entry for the source-root directory, so the destination mirror is created empty and nothing is descended into. With `--files-from` exactly the listed items are transferred: a listed directory is created empty (no descent) and a listed file is transferred with its content; the dest layout follows the same -R rules as plain files. A new wire frame (`STATUS_MKDIR`) carries each directory entry (path only); the receiver creates it with the same confined mkdir-parent semantics as regular writes, in single-threaded and `-m` receivers (chunk serialization carries a per-entry type marker). Directory entries appear in the delete manifest so `--delete` prunes correctly. FastSync divergences: directory mtimes/modes are not transmitted, filter/`--exclude` rules are not re-applied to the listed dirs mode (there is no descent during which they would apply), and `-d` never creates the intermediate directories between the destination root and a listed file beyond the usual on-demand parent creation. Under `--delay-updates` only regular files are staged: directory entries are created immediately, so a delayed run that fails part way can leave the already-created empty directories behind (matching rsync, which also creates directories as it processes the file list and only delays regular-file data) |
| `-d`, `--dirs`, `--old-dirs`, `--old-d` | Transfer dirs without recursing | ✅ Implemented | `-d <dir>` transmits an explicit directory entry for the source-root directory, so the destination mirror is created empty and nothing is descended into. With `--files-from` exactly the listed items are transferred: a listed directory is created empty (no descent) and a listed file is transferred with its content; the dest layout follows the same -R rules as plain files. A new wire frame (`STATUS_MKDIR`) carries each directory entry (path only); the receiver creates it with the same confined mkdir-parent semantics as regular writes, in single-threaded and `-j`/`--threads` receivers (chunk serialization carries a per-entry type marker). Directory entries appear in the delete manifest so `--delete` prunes correctly. FastSync divergences: directory mtimes/modes are not transmitted, filter/`--exclude` rules are not re-applied to the listed dirs mode (there is no descent during which they would apply), and `-d` never creates the intermediate directories between the destination root and a listed file beyond the usual on-demand parent creation. Under `--delay-updates` only regular files are staged: directory entries are created immediately, so a delayed run that fails part way can leave the already-created empty directories behind (matching rsync, which also creates directories as it processes the file list and only delays regular-file data) |
| `--mkpath` | Create missing path components | ✅ Implemented | Wire option (client → server). At connection start the server creates the client's destination root directory (and any missing leading components below its own authorized root) when `--mkpath` is set, failing the connection cleanly if it cannot. Without `--mkpath` a destination root that does not exist yet is rejected up front (rsync semantics), so the flag is the only way to transfer into a not-yet-created destination directory. Creation is confined by the same secure mkdir walk as file writes (`O_NOFOLLOW`, no `..`) |
## 5. Transfer Modifications
@@ -96,7 +96,7 @@ This document maps rsync's full feature set to FastSync's current implementation
| `-b`, `--backup` | Make backups of overwritten files | ✅ Implemented | Backup before overwrite |
| `--backup-dir=DIR` | Backup directory hierarchy | ✅ Implemented | `backup_dir` config field |
| `--suffix=SUFFIX` | Backup suffix (default ~) | ✅ Implemented | `suffix` config field |
| `--delay-updates` | Put updated files in place at end | ✅ Implemented | Successfully received files are staged under a private 0700 `.fastsync-stage` dir inside the receive root and atomically renamed into their final destinations only after the whole transfer (manifest/delete handling included) succeeds, just before the success/outcome frame is sent. The delete walker deliberately skips the staging dir at the receive root, so `--delete` removes genuine extras but never the staged files (deletion runs before publication; rsync's delete-after ordering is not implemented). `--existing`/`--ignore-existing`/`--update` decide against the final destination path at stage time; `--backup` moves the old file aside at publication. Incompatible with `--inplace` and with `--backup-dir=.fastsync-stage` (the internal staging name is reserved; both are rejected). The staging dir name is fixed, so two simultaneous delayed transfers to the same destination root are serialized with an exclusive advisory lock held for the whole transfer: the second session fails cleanly instead of corrupting the first. Aborting or failing before publication installs nothing and removes the staging tree; a crash between stage and publish leaves staged leftovers that the next delayed run wipes at start (process death releases the lock). A stage→publish failure aborts the transfer (best-effort cleanup of the not-yet-published staged files; already-published files are not rolled back). Works in single-threaded and `-m` modes |
| `--delay-updates` | Put updated files in place at end | ✅ Implemented | Successfully received files are staged under a private 0700 `.fastsync-stage` dir inside the receive root and atomically renamed into their final destinations only after the whole transfer (manifest/delete handling included) succeeds, just before the success/outcome frame is sent. The delete walker deliberately skips the staging dir at the receive root, so `--delete` removes genuine extras but never the staged files (deletion runs before publication; rsync's delete-after ordering is not implemented). `--existing`/`--ignore-existing`/`--update` decide against the final destination path at stage time; `--backup` moves the old file aside at publication. Incompatible with `--inplace` and with `--backup-dir=.fastsync-stage` (the internal staging name is reserved; both are rejected). The staging dir name is fixed, so two simultaneous delayed transfers to the same destination root are serialized with an exclusive advisory lock held for the whole transfer: the second session fails cleanly instead of corrupting the first. Aborting or failing before publication installs nothing and removes the staging tree; a crash between stage and publish leaves staged leftovers that the next delayed run wipes at start (process death releases the lock). A stage→publish failure aborts the transfer (best-effort cleanup of the not-yet-published staged files; already-published files are not rolled back). Works in single-threaded and `-j`/`--threads` modes |
| `-T`, `--temp-dir=DIR` | Create temporary files in DIR | ✅ Implemented | `--temp-dir` with the rsync short `-T` (Phase 7 Wave A; the timeout alias moved to long-only `--timeout`). Scratch dir is resolved under the receive root; temp copies use a unique name there and are atomically renamed into place. If the scratch dir and destination are on different filesystems the atomic rename fails with EXDEV and the file save fails, which aborts the whole transfer (FastSync has no per-file skip/resume on a save error; rsync's non-atomic copy fallback is deliberately not used). `--inplace` and `--partial-dir` writes bypass the scratch dir |
## 7. Deletion
@@ -600,7 +600,7 @@ now transmits targets (the prior behavior was broken/partial); its status moved
| `-z`, `--compress` | Compress file data | ✅ Implemented | Always uses zstd (rsync supports multiple algorithms — a documented divergence, selectable via `--compress-choice`). Phase 7 Wave A: `-z` is now the compression short form; `-c` is rsync's `--checksum` |
| `--compress-choice=STR`, `--zc=STR` | Choose compression algorithm | ✅ Implemented | FastSync supports `zstd` and `none` |
| `--compress-level=NUM`, `--zl=NUM` | Set compression level | ✅ Implemented | 1-22, default 5 |
| `--compress-threads=NUM` | Set compression threads | ✅ Implemented | `compression_threads` config field (client-only; does not cross the wire). Sets the number of worker threads used by the zstd compression pool to NUM (1..64; 0/garbage/oversized rejected up front). Accepted in both `--compress-threads=NUM` and two-argument `--compress-threads NUM` forms. Composes with `-c`/compression; under the `-m` multithreaded pipeline it parallelizes compressed chunk encoding. See test_tcp.py `-c --compress-threads=2` and test_client_cli.c |
| `--compress-threads=NUM` | Set compression threads | ✅ Implemented | `compression_threads` config field (client-only; does not cross the wire). Sets the number of worker threads used by the zstd compression pool to NUM (1..64; 0/garbage/oversized rejected up front). Accepted in both `--compress-threads=NUM` and two-argument `--compress-threads NUM` forms. Composes with `-z`/compression; under the `-j`/`--threads` multithreaded pipeline it parallelizes compressed chunk encoding. See test_tcp.py `-z --compress-threads=2` and test_client_cli.c |
| `--skip-compress=LIST` | Skip compress for suffixes | ✅ Implemented | Comma-separated, case-insensitive suffix list; empty list skips none; incompatible with FastSync chunk serialization (`-s`) |
## 13. Connectivity
@@ -798,7 +798,7 @@ These are the hardest compatibility items because they require durable formats o
These are the last compatibility items and the closing phase toward rsync flag parity. Per the project decision: every rsync flag (short **and** long) that is *possible* gets real rsync-parity behavior; anything physically impossible becomes an explicit **Impossible/Divergence** status (accepted for CLI compatibility, safely inert, with coverage tests proving that); and the two privilege flags are deferred to the final wave pending an explicit privilege-model decision. The remaining `⚠️ Partial`, `🔄 Compatibility No-op`, `🔀 Alt Arg`, and `❌ Not Implemented` rows in the Summary are this phase's scope. All Wave A renames are **client-side only** (the wire config fields `use_compression`/`use_metadata`/`use_sendfile`/`use_chunk_serialization` are unchanged), so they require **no `PROTOCOL_VERSION` bump**.
**Wave A — CLI namespace parity (rename colliding FastSync short flags) — ✅ implemented.** This freed the short letters rsync needs and made the three `🔀 Alt Arg` rows real. `-c`→`--checksum`, `-m`→`--prune-empty-dirs`, `-M`→`--remote-option`, `-f`→`--filter`, `-s`→`--secluded-args`, `-p`→`--perms`, `-T`→`--temp-dir`, `-a`/`--archive`→real `-rlptgoD`. FastSync's own flags moved to long-form-only or new shorts: `-j`/`--threads` (multithreading), `--preserve` (metadata), `--sendfile`, `--chunk-serialization`, `--timeout`, `--ssh-port`. The server's independent little CLI keeps `-p` as its port. All client-side, no wire change, no `PROTOCOL_VERSION` bump. Unit tests 37/37, full integration 400 passed, cppcheck and clang-format clean.
**Wave A — CLI namespace parity (rename colliding FastSync short flags) — ✅ implemented.** This freed the short letters rsync needs and made the three `🔀 Alt Arg` rows real. `-c`→`--checksum`, `-m`→`--prune-empty-dirs`, `-M`→`--remote-option`, `-f`→`--filter`, `-s`→`--secluded-args`, `-p`→`--perms`, `-T`→`--temp-dir`, `-a`/`--archive`→real `-rlptgoD`. FastSync's own flags moved to long-form-only or new shorts: `-j`/`--threads` (multithreading), `--preserve` (metadata), `--sendfile`, `--chunk-serialization`, `--timeout`, `--ssh-port`. The server's independent little CLI keeps `-p` as its port. All client-side, no wire change, no `PROTOCOL_VERSION` bump. Unit tests 37/37, full integration 400 passed, cppcheck and clang-format clean. Known Wave-A limitation: `--no-perms`/`--no-compress`-style negation of the newly-aliased shorts is not wired into the negatable set (only the long-form `--preserve`/`--compress`/`--no-links` negations exist); `--archive --no-perms` is consequently not supported yet — a minor deviation from rsync, acceptable for Wave A.
| FastSync flag today | rsync wants that name | Proposed rename |
|---------------------|----------------------|-----------------|
+12 -8
View File
@@ -101,7 +101,7 @@ class TestDeviceSpecial:
assert os.major(st.st_rdev) == 1 and os.minor(st.st_rdev) == 3
def test_m_remove_source_files_keeps_recreated_fifo(self, shared_server):
"""-m --remove-source-files --specials: a recreated FIFO must NOT be
"""--threads --remove-source-files --specials: a recreated FIFO must NOT be
acknowledged as a removable source (its outcome must not shift the
per-file status stream, which would break the run and mis-remove the
adjacent regular file). The regular file is removed; the FIFO stays."""
@@ -122,7 +122,7 @@ class TestDeviceSpecial:
@pytest.mark.skipif(os.geteuid() != 0, reason="requires root to create device nodes")
def test_m_remove_source_files_keeps_recreated_device(self, shared_server):
"""Root-only: -m --remove-source-files --devices must not remove a
"""Root-only: --threads --remove-source-files --devices must not remove a
source device node the receiver recreated (mirrors the single-threaded
behavior; the special is never acknowledged as a removable source)."""
self._setup()
@@ -320,7 +320,7 @@ class TestRemoveSourceFiles:
with open(source_file, "wb") as f:
f.write(b"keep after skip")
# The seed run preserves timestamps (-M) so the destination copy has the
# The seed run preserves timestamps (--preserve) so the destination copy has the
# source's exact mtime; otherwise the incremental skip would depend on
# both writes landing in the same whole second (a race).
result, _ = run_client(source, dest, flags=["--preserve"], port=shared_server.port)
@@ -348,11 +348,15 @@ class TestArchiveMode:
assert not missing, f"Missing: {missing}"
assert not mismatches, f"Mismatch: {mismatches}"
def test_archive_implied_options_can_be_negated(self, shared_server):
def test_archive_with_negated_links(self, shared_server):
# --archive implies links + metadata + devices + specials. Devices/specials
# force metadata transmission (recreating a node needs the metadata mode), so
# the post-parse layer keeps use_metadata on even under --no-preserve; only the
# independently-negatable --no-links actually takes effect here.
clean_dir(DEST_DIR)
result, dur = run_client(
SOURCE_DIR, DEST_DIR,
flags=["--archive", "--no-links", "--no-preserve"],
flags=["--archive", "--no-links"],
port=shared_server.port,
)
if result.returncode != 0:
@@ -1770,13 +1774,13 @@ class TestLogFileFormat:
flags=["--log-file", log_path, "--log-file-format=%f %l", "--threads"],
port=shared_server.port,
)
assert result.returncode == 0, f"log-file -m sync failed: {result.stderr[:200]}"
assert result.returncode == 0, f"log-file --threads sync failed: {result.stderr[:200]}"
assert os.path.exists(log_path), "--log-file created no log"
with open(log_path, encoding="utf-8", errors="replace") as fh:
content = fh.read()
expected = {f"{os.path.join(source, rel)} {len(data)}" for rel, data in files.items()}
for line in expected:
assert line in content, f"log file (-m) missing {line!r}"
assert line in content, f"log file (--threads) missing {line!r}"
class TestDelayUpdates:
@@ -4552,7 +4556,7 @@ class TestExtendedAttributes:
pytest.skip("filesystem does not support xattrs")
acl_blob = None
if shutil.which("setfacl") is not None:
acl = subprocess.run(["setfacl", "--threads", "o::r", f], capture_output=True, text=True)
acl = subprocess.run(["setfacl", "-m", "o::r", f], capture_output=True, text=True)
if acl.returncode == 0:
try:
acl_blob = os.getxattr(f, "system.posix_acl_access")
+7 -7
View File
@@ -97,31 +97,31 @@ class TestSSHStandard:
assert r["status"] == "Success", r["error"]
def test_multithreading(self):
r = _run_ssh_test("SSH Multithreading (-m)", ["--threads"])
r = _run_ssh_test("SSH Multithreading (--threads)", ["--threads"])
assert r["status"] == "Success", r["error"]
def test_compression(self):
r = _run_ssh_test("SSH Compression (-c)", ["-z"])
r = _run_ssh_test("SSH Compression (-z)", ["-z"])
assert r["status"] == "Success", r["error"]
def test_chunk_serialization(self):
r = _run_ssh_test("SSH Chunk Serialization (-s)", ["--chunk-serialization"])
r = _run_ssh_test("SSH Chunk Serialization (--chunk-serialization)", ["--chunk-serialization"])
assert r["status"] == "Success", r["error"]
def test_compression_chunk(self):
r = _run_ssh_test("SSH Compression + Chunk (-c -s)", ["-z", "--chunk-serialization"])
r = _run_ssh_test("SSH Compression + Chunk (-z --chunk-serialization)", ["-z", "--chunk-serialization"])
assert r["status"] == "Success", r["error"]
def test_multithread_compression(self):
r = _run_ssh_test("SSH Multithread + Compression (-m -c)", ["--threads", "-z"])
r = _run_ssh_test("SSH Multithread + Compression (--threads -z)", ["--threads", "-z"])
assert r["status"] == "Success", r["error"]
def test_multithread_chunk(self):
r = _run_ssh_test("SSH Multithread + Chunk (-m -s)", ["--threads", "--chunk-serialization"])
r = _run_ssh_test("SSH Multithread + Chunk (--threads --chunk-serialization)", ["--threads", "--chunk-serialization"])
assert r["status"] == "Success", r["error"]
def test_all_flags(self):
r = _run_ssh_test("SSH All Flags (-m -c -s)", ["--threads", "-z", "--chunk-serialization"])
r = _run_ssh_test("SSH All Flags (--threads -z --chunk-serialization)", ["--threads", "-z", "--chunk-serialization"])
assert r["status"] == "Success", r["error"]
+2 -2
View File
@@ -234,8 +234,8 @@ class TestStopDelete:
result, _ = run_client(source, dest, flags=["--threads", "--delete", "--stop-at=now+0s"],
port=shared_server.port)
assert result.returncode == 0, \
f"-m --delete immediate stop failed: {(result.stderr or result.stdout)[:400]}"
f"--threads --delete immediate stop failed: {(result.stderr or result.stdout)[:400]}"
received = get_dest_received_dir(dest, source)
mismatches, missing = verify_transfer(source, received)
assert not mismatches and not missing, \
f"-m --delete wiped source mirrors: missing={missing} mismatches={mismatches}"
f"--threads --delete wiped source mirrors: missing={missing} mismatches={mismatches}"
+10 -10
View File
@@ -68,45 +68,45 @@ class TestTCPStandard:
class TestTCPFlags:
@pytest.mark.ci
def test_multithreading(self, shared_server):
r = _run_tcp_test("Multithreading (-m)", shared_server.port, ["--threads"])
r = _run_tcp_test("Multithreading (--threads)", shared_server.port, ["--threads"])
assert r["status"] == "Success", r["error"]
@pytest.mark.ci
def test_compression(self, shared_server):
r = _run_tcp_test("Compression (-c)", shared_server.port, ["-z"])
r = _run_tcp_test("Compression (-z)", shared_server.port, ["-z"])
assert r["status"] == "Success", r["error"]
def test_compression_threads(self, shared_server):
r = _run_tcp_test("Compression threads (-c --compress-threads=2)", shared_server.port,
r = _run_tcp_test("Compression threads (-z --compress-threads=2)", shared_server.port,
["-z", "--compress-threads=2"])
assert r["status"] == "Success", r["error"]
def test_chunk_serialization(self, shared_server):
r = _run_tcp_test("Chunk Serialization (-s)", shared_server.port, ["--chunk-serialization"])
r = _run_tcp_test("Chunk Serialization (--chunk-serialization)", shared_server.port, ["--chunk-serialization"])
assert r["status"] == "Success", r["error"]
def test_compression_chunk(self, shared_server):
r = _run_tcp_test("Compression + Chunk (-c -s)", shared_server.port, ["-z", "--chunk-serialization"])
r = _run_tcp_test("Compression + Chunk (-z --chunk-serialization)", shared_server.port, ["-z", "--chunk-serialization"])
assert r["status"] == "Success", r["error"]
def test_multithread_compression(self, shared_server):
r = _run_tcp_test("Multithreading + Compression (-m -c)", shared_server.port, ["--threads", "-z"])
r = _run_tcp_test("Multithreading + Compression (--threads -z)", shared_server.port, ["--threads", "-z"])
assert r["status"] == "Success", r["error"]
def test_multithread_chunk(self, shared_server):
r = _run_tcp_test("Multithreading + Chunk (-m -s)", shared_server.port, ["--threads", "--chunk-serialization"])
r = _run_tcp_test("Multithreading + Chunk (--threads --chunk-serialization)", shared_server.port, ["--threads", "--chunk-serialization"])
assert r["status"] == "Success", r["error"]
def test_all_flags(self, shared_server):
r = _run_tcp_test("Multithread + Compression + Chunk (-m -c -s)", shared_server.port, ["--threads", "-z", "--chunk-serialization"])
r = _run_tcp_test("Multithread + Compression + Chunk (--threads -z --chunk-serialization)", shared_server.port, ["--threads", "-z", "--chunk-serialization"])
assert r["status"] == "Success", r["error"]
def test_sendfile(self, shared_server):
r = _run_tcp_test("Sendfile (-f)", shared_server.port, ["--sendfile"])
r = _run_tcp_test("Sendfile (--sendfile)", shared_server.port, ["--sendfile"])
assert r["status"] == "Success", r["error"]
def test_sendfile_multithread(self, shared_server):
r = _run_tcp_test("Sendfile + Multithreading (-f -m)", shared_server.port, ["--sendfile", "--threads"])
r = _run_tcp_test("Sendfile + Multithreading (--sendfile --threads)", shared_server.port, ["--sendfile", "--threads"])
assert r["status"] == "Success", r["error"]
+6 -2
View File
@@ -2374,20 +2374,24 @@ static void test_parse_args_append_both() {
static void test_validate_config_append_rejects_chunk_serialization() {
Config* cfg = config_create();
char* argv[] = {"fastsync", "--append", "-s", "/src", "/dst"};
char* argv[] = {"fastsync", "--append", "--chunk-serialization", "/src", "/dst"};
int positional_args[2];
int positional_count = 0;
EXPECT_EQ_INT(parse_args(cfg, 5, argv, positional_args, &positional_count), 0);
cfg->send_directory = str_dup("/src");
cfg->receive_root_directory = str_dup("/dst");
EXPECT_FALSE(validate_config(cfg));
config_delete(cfg);
}
static void test_validate_config_append_verify_rejects_chunk_serialization() {
Config* cfg = config_create();
char* argv[] = {"fastsync", "--append-verify", "-s", "/src", "/dst"};
char* argv[] = {"fastsync", "--append-verify", "--chunk-serialization", "/src", "/dst"};
int positional_args[2];
int positional_count = 0;
EXPECT_EQ_INT(parse_args(cfg, 5, argv, positional_args, &positional_count), 0);
cfg->send_directory = str_dup("/src");
cfg->receive_root_directory = str_dup("/dst");
EXPECT_FALSE(validate_config(cfg));
config_delete(cfg);
}