Review fixes for Phase 7 Wave D.
#1 (HIGH): STATUS_DIR_TIMES entries no longer create directories. A new
receiver-only File.dir_time_only flag marks dir-time entries; file_save_to_disk_full
short-circuits them as FILE_SAVE_SKIPPED before any device/dir branch, so the sink
still accumulates metadata into the deferred DirTimeList but creates nothing. Empty
source dirs stay untransferred (-a), -m/--prune-empty-dirs semantics are preserved,
and a pre-existing regular file/symlink at an empty-dir mirror path no longer aborts
the transfer. dir_time_list_apply fstatat()s the leaf (AT_SYMLINK_NOFOLLOW) and skips
absent/non-directory paths QUIETLY; only a real existing directory is stamped.
Also initialize File.dir_time_only in file_create() (uninitialised garbage otherwise).
#2 (MED): send_dir_times() chunks entries into repeated STATUS_DIR_TIMES frames of at
most MAX_MANIFEST_ENTRIES, matching the receiver's per-frame bound; the tautological
> INT_MAX check is gone.
#3 (LOW): dir_time_list_add() assigns each grown array right after its realloc (no
dangling) and advances capacity only after both succeed.
#4 (LOW): RSYNC_COMPAT.md -- STATUS_MKDIR carries metadata, dir times are transmitted
via STATUS_DIR_TIMES and applied at the end, empty dirs are still never created; -m
rationale, -O row and Wave D notes updated. Summary counts untouched.
#5 (LOW): integration tests for the three #1 scenarios (empty-dir non-creation under
-a and -a -m, collision non-abort), scanner test now covers empty-dir capture, and
test_file_restore_symlink_metadata asserts the positive apply path when supported.
PROTOCOL_VERSION stays 2.17.0; config-frame layout unchanged.
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.
- fake_super_restore_fd now sanitizes mode like metadata_mode (never grants
S_IWGRP|S_IWOTH; 0666 -> 0644), fixing a privilege regression
- --sparse takes precedence over --preallocate (skip posix_fallocate when
sparse) so holes are not re-allocated; docs corrected
- --partial retention disabled under --no_replace (ignore/existing) and only
marks write_attempted after the write begins (no empty-temp retention)
- accept --block-size=SIZE / --delta-block=SIZE inline forms; neutral messages
- fake-super EPERM/EACCES skipped silently (docs aligned); EINVAL still logged
- sparse unit test now memcmp's the full buffer; TestBlockSize integration keeps
the destination basis so delta is genuinely exercised
- unit 37/37, cppcheck 0, clang-format 0
- write_all_sparse: skips all-zero runs >= 4096 bytes via lseek(SEEK_CUR) and
ftruncates the final size, wired into the atomic temp+rename and --inplace
paths with no wire change (full image already in memory).
- --partial retention: on a save failure after the temp held data, rename the
already-written temp to the destination path (best-effort; falls through to
unlink; never retains when --partial is off) so --append/--append-verify can
resume; tested by forcing futimens EINVAL with an out-of-range nsec.
- --block-size aliases --delta-block; verified config->delta_block_size is
honored by the delta engine end-to-end (unit + integration tests).
- fake_super_restore_fd: parses and re-applies user.fastsync.stat fd-relative
(fchown best-effort/non-root skipped, fchmod, futimens); a save under
--fake-super now both records and re-applies.
- -N/--crtimes and --stderr=client promoted to a new 'Impossible/Divergence'
status bucket (Summary: 136/2/4/3/2 = 147).
- cppcheck/clang-format clean; unit 37/37; integration 407 passed.
- 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
-c -> --checksum, -m -> --prune-empty-dirs, -M -> --remote-option,
-f -> --filter, -s -> --secluded-args, -p -> --perms, -T -> --temp-dir;
-a/--archive is now real rsync -rlptgoD (links+metadata+devices+specials).
FastSync's own flags moved to long-form-only or new shorts:
-j/--threads (multithreading), --preserve (metadata), --sendfile,
--chunk-serialization, --timeout, --ssh-port. Client-side only; the
wire config fields are unchanged (no PROTOCOL_VERSION bump). The server
keeps -p as its port. Docs (README, RSYNC_COMPAT summary 129->132) and
unit/integration tests updated. 37/37 unit, 400-pass integration.
Implements the client-only residual-batch feature end-to-end:
- src/shared/batch.{c,h}: self-contained single-file batch codec using the
existing chunk_serialize/chunk_deserialize codec (byte-identical by
construction). Magic+format-version header (metadata mode is persisted into
the header so a batch is self-describing across machines), length-prefixed
chunk records, bounded reads that reject malformed/truncated/oversized
records cleanly.
- src/client/client_send.c: write_batch_from_source (deterministic separate
scan pass, loads every chunk's file images, emits header+records) and
apply_batch_to_dest (local apply to a destination root via
file_save_to_disk_full). No wire change, no server involved.
- src/client/client_validation.c: --write-batch XOR --only-write-batch;
--read-batch exclusive with both; --read-batch needs only a DEST,
--only-write-batch only a SOURCE.
- src/client/client_cli.c: main() drives the three batch modes without
connecting/transferring for read/only-write; --write-batch runs the live
transfer (single-threaded so the config survives) then emits the batch.
- tests/test_batch.{c,h} (unit: byte-identical roundtrip with and without
metadata; bad-magic/truncated/oversized rejection) + tests/integration/
test_batch.py (only-write no-server, read-batch no-source roundtrip,
--write-batch with a live transfer, conflict rejections).
- clang-format: realign PART-1 config.h comment block.
No PROTOCOL_VERSION bump, no config-frame field, no server flag.
- test_credentials.c: NUL-terminate the overlong-line stack buffer before
make_tmp_file's strlen() (was a stack-buffer-overflow READ under ASan);
still exercises the overlong-rejection path.
- Add redacted protocol string variants (protocol_send_str_redacted /
receive + fd send_str_redacted/receive_str_redacted) and use them for the
daemon auth username/digest so --verbose / LOG_DEBUG_ALL never logs a
replayable credential while other protocol strings keep their debug trace.
- credentials_verify/gate: replace byte-wise-short-circuiting strcmp with a
fixed-length constant-time username compare (closes user-enumeration oracle);
update doc comment to match.
- read_secret_file: preserve password exact bytes (only strip trailing CR/LF)
and burn the stack line buffer; document the whitespace behavior.
- test_server_cli.c: note the parser zero-inits opts on failure.
- Add debug-level daemon test asserting the digest never appears under --verbose.
PROTOCOL_VERSION stays 2.15.0.
file_send_special now takes const File*; test_chunk declares the deserialized
Chunk* const. cppcheck (--error-exitcode=1) now exits clean; clang-format
clean; unit 29/29.
Capture+transmit source atime (pre-read stat; O_NOATIME sender guard) and birth
time (statx STATX_BTIME); receiver restores atime with mtime (crtime not settable
portably -> transmitted, explicitly not applied). --open-noatime is client-only.
-O/-J documented as accepted no-ops (FastSync never preserves dir/symlink times).
Wire: metadata frame gains atime/crtime val+sec+nsec; PROTOCOL_VERSION
2.11.0->2.12.0. Review fixes: gate atime capture to Linux (no epoch clobber on
non-Linux), close fd on fdopen failure, honest -O/-J status (Compat no-op).
Source files sharing (st_dev,st_ino) are recreated as hard links on the
destination; only the first member's data crosses the wire (siblings ride a
payload-less STATUS_HARDLINK frame). Ordering requires the single-FIFO-writer
receiver + forced sequential scan (documented). link()-failure falls back to a
byte-identical local copy. Rejects -s/--append. PROTOCOL_VERSION 2.11.0->2.12.0.
Review fixes: delete the dead HardLinkRegistry (ordering holds by FIFO writer),
and --existing no longer aborts when the first member is absent but the sibling
exists (leaves the sibling in place).
identity_parse_map freed the str_dup'd list before logging the offending rule
( points into that buffer), causing an invalid read caught by CI valgrind
(MSAN/MSAN-style; the ONLY definite valgrind error in the suite). Log before
freeing. valgrind now reports 0 errors / 0 definite leaks in both the parent
and the forked wire-roundtrip child.
identity_active_enabled() only gates identity_apply_ownership, which runs only
when metadata is present, so --numeric-ids must stay in the set: combined with
-M it activates raw-id application, while a standalone --numeric-ids (no
ownership-affecting flag) carries no metadata and correctly stays inert. My
earlier review fix removed it and broke 'owner not applied' for -M --numeric-ids
(uid 0 instead of the source ids). Revert that removal.
Receiver allocates the destination file's full size before streaming data
(posix_fallocate preferred, ftruncate fallback on EOPNOTSUPP/ENOSYS) so an
out-of-space transfer fails fast instead of partway. Additive config bool
crossing the wire; PROTOCOL_VERSION 2.10.0 -> 2.11.0. Threaded through all
store paths (atomic, inplace, partial/delay-updates staging, link-dest copy
fallback). Review hardening: explicit lseek(0) before the data write so
correctness does not depend on posix_fallocate leaving the fd offset unchanged.
Receiver-side ownership application, opt-in and privilege-gated:
- OFF for every existing transfer (plain -M/--preserve still never applies
ownership); only triggers on an explicit identity flag + receiver permission.
- EPERM/EACCES warn-and-continue (never aborts); other fchown errors escalate.
- fd-relative fchown after the file is written (symlink-safe, confined).
- New src/shared/identity.{c,h}; config fields numeric_ids / chown uid/gid /
usermap + groupmap id-pair tables cross the wire; PROTOCOL_VERSION 2.10.0
-> 2.11.0. CLI in client_cli.c; per-connection snapshot in server.c.
- Review fixes: EPERM/EACCES-only warn-and-continue, prominent root-receiver
notice, identity_clear_active on early server error paths, --numeric-ids
kept inert standalone (removed from activation trigger set).
- Replace --dist=loadgroup (a no-op: it only groups by xdist_group marks) with
--dist=load, and make module teardowns remove only their own SOURCE_DIR/DEST_DIR
(never the shared worker-keyed TEST_DATA_DIR) so interleaved modules can't wipe
each other's fixtures. Add a session-scoped cleanup of the per-worker dir.
(loadfile grouped the whole test_features.py module onto one worker and slowed
the full suite to 761s vs 224s with load.)
- Mark the two setpriv privilege tests with a 'setpriv' marker and run the full
merge suite as -m "not setpriv", so the dev/main gate can't go red on the
env-dependent /root-traversal tests regardless of the runner uid.
- Add a TLS basic test to the ci subset, add workflow_dispatch for on-demand full
runs, and fix trailing newlines.
- Measured: smoke -m ci = 28 passed/39s; full -n4 = 280 passed/11 skipped/1 xpassed
in 224s (was 860s serial).
- Add pytest-xdist to the CI Dockerfile (image -> v10).
- Worker-isolate TEST_DATA_DIR (PYTEST_XDIST_WORKER) so concurrent xdist
workers never collide on shared-filesystem fixtures.
- Register a 'ci' marker and tag a fast representative subset of integration
tests (basic TCP, incremental, compression, delete, basis, append).
- ci.yaml: lint + build + unit + the marked subset (-n4) on every PR; the
full integration suite plus sanitizer/fuzz/coverage/valgrind run only on
push to dev/main.
- Integration step runtime drops from ~860s (serial) to ~230s (-n4); the PR
gate lands well under ~3 minutes.
Unit: manifest_delete_missing_args treats a deeper missing entry whose
destination parent directory does not exist as a no-op (run continues, nothing
created). Integration (TestMissingArgs): a deeper missing entry with an absent
parent -R bare and full-mirror layouts, single-threaded and -m, no longer
aborts --delete (the extras walk still removes an unrelated extra); --dirs +
--files-from with --ignore-missing-args skips a listed-but-missing entry and
transfers the rest.
BLOCKER: an absent mirror whose PARENT directory does not exist on the
destination (a deeper --files-from missing entry, -R or full-mirror layout) was
treated as a hard failure because file_open_secure_parent returned -1 when the
parent was missing. That aborted the whole run, skipped the --delete extras
walk, and tore down an early --delete-before/during connection, contradicting
'missing mirror = no-op / all-missing succeeds'. A parent-open failure is now a
no-op when errno is ENOENT/ENOTDIR (matching file_remove_tree_secure); only a
genuine I/O error fails the run.
Also: an already-absent mirror reached via unlinkat-ENOENT no longer prints
'Deleted: <path>' (a no-op dressed as a deletion); the per-path 'Deleted:' line
is printed only when an entry was actually removed.
Precise notes for both rows: supported algorithms (xxh64/xxhash, md5 via EVP),
rejection of unsupported names, seed semantics (full 64-bit for whole-file,
low 32 bits for the delta block hash, ignored by md5), the --cc alias, the
length-prefixed bounded handshake digest, the 2.9.0 -> 2.10.0 protocol bump,
default byte-for-byte preservation, and the divergence from rsync's randomized
seed (FastSync defaults to seed 0). Summary count line is intentionally untouched.
Adds real algorithm selection (xxh64 default, plus md5 via OpenSSL EVP) and a
64-bit seed for the per-file whole-file digest used by the --incremental/
--checksum handshake and basis-dir content verification. The seed also feeds
the delta path's per-block xxHash32 strong checksum (low 32 bits) so an
explicit seed deterministically changes those digests too. Sender and receiver
hash identically: the algorithm id and seed cross the config wire frame and the
STATUS_CHECK handshake now carries a length-prefixed, bounded digest instead of
a fixed 64-bit value. Unsupported algorithm names are rejected at parse time
(never a silent no-op). PROTOCOL_VERSION bumped 2.9.0 -> 2.10.0; defaults
(xxh64, seed 0) preserve prior byte-for-byte behavior.
Flip both rows to Implemented with precise Notes covering the tail-resume
model, the prefix-verify semantics (and the plain-append rsync-parity safety
statement), the new wire frames, and the PROTOCOL_VERSION 2.9.0 -> 2.10.0 bump.
Add a Phase-3 append-wave implementation-notes block. The Summary count line
is deliberately left untouched.
- CLI: --append/--append-verify acceptance (parse + imply --incremental,
validate) and incompatibility rejection with -s and --whole-file; both
removed from the unimplemented reject list.
- Config: on-the-wire append/append_verify round-trip.
- Unit: append_resume_eligible / append_tail_length pure resume math.
- Integration (test_append.py): matching-prefix resume is byte-identical and
tail-only (wire bytes << source size); --append with a wrong prefix keeps
prefix+tail (rsync parity) while --append-verify detects the mismatch and
falls back to a byte-exact full transfer; --append with --inplace and -m.
Implement rsync's append modes: when an existing destination file is SHORTER
than the source, the receiver negotiates a resume offset and only the tail is
transferred; the full file (retained prefix + tail) is rebuilt and installed
through the normal atomic store path, so the result is byte-identical to the
source whenever the prefix matches.
- --append: sends the tail without content-verifying the retained prefix
(rsync parity; the documented prefix-trust risk).
- --append-verify: verifies the retained prefix against the source's prefix
xxHash64 before appending and, on a mismatch, falls back to a clean full
transfer (never a corrupt prefix+tail blend).
New wire frames STATUS_APPEND / STATUS_APPEND_SIG / STATUS_APPEND_OK /
STATUS_APPEND_DATA; PROTOCOL_VERSION bumped 2.9.0 -> 2.10.0 (peers must match).
Both flags imply --incremental and are incompatible with -s (chunk
serialization) and --whole-file (rejected up front). Respects --inplace,
--partial/--partial-dir and --delay-updates via the shared store engine.
Precise Notes on the arg model (files-from entries), implies-relationships
(delete-missing implies ignore, independent of --delete), delete/timing/force/
exclude-protection interplay, the third manifest section and the 2.9.0 -> 2.10.0
wire bump, and the divergences (empty-list hard error, no negation, max-delete
not applied to explicit deletions). The files-from row now references the flags.
The Summary count line is intentionally left for a recount commit.
TapTap