fix(transfer): interleave itemize rows in flist order (run_pipelined, step 1)#6560
Merged
Conversation
… step 1) The two-phase receiver emitted every directory (.d) itemize row in the directory-creation pass before any file (.f) row from the candidate pass, so `a/ a/f1 b/ b/f2` printed .d a/, .d b/, >f a/f1, >f b/f2. Upstream itemizes in a single flist-index-order walk (generator.c:2329-2344 -> recv_generator -> itemize, log.c:788 rwrite), where a directory row immediately precedes its children: .d a/, >f a/f1, .d b/, >f b/f2. Display-only: transferred data, wire bytes, and metadata application are unchanged; directories are still created before their files. Only the itemize emission moves to a deferred, flist-index-keyed flush. Adds a per-transfer buffer (itemize_rows: RefCell<BTreeMap<usize, Vec<String>>>) plus record_itemize / flush_itemize_rows / a emit_or_record_itemize dispatcher, gated by an opt-in defer_itemize toggle. The dir/file/no-change emit sites in the shared create_directories, build_files_to_transfer, and pipeline loop now route through the dispatcher, which buffers when deferral is on and emits immediately otherwise. record_itemize preserves emit_itemize's client-mode visibility gate exactly (a server receiver's row still travels as wire iflags). Only run_pipelined opts in (sets defer_itemize and flushes before finalize); the sync, incremental, and async receive paths keep emitting immediately and are converted in follow-ups. Requires an lxhost -ii seal before merge.
df149f8 to
57a3757
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Display-only fix for the itemize (
-i/-ii) row ordering on a pull. Transferred data, wire bytes, and metadata application are byte-identical; only the order of itemize rows changes. Directories are still created before their files - only the itemize emission moves to a deferred flush.The divergence
oc's two-phase receiver emits every directory (
.d) itemize row in the directory-creation pass, then every file (.f) row from the candidate pass. Upstream interleaves them in flist index (tree) order, so a directory row immediately precedes its children.For the flist
a/ a/f1 b/ b/f2:.d a/,>f a/f1,.d b/,>f b/f2.d a/,.d b/,>f a/f1,>f b/f2Upstream does a single flist-index-order walk, itemizing each entry as it is reached:
generator.c:2329-2344generate_files- one loop overcur_flist->sorted[i]in index order,recv_generatorper entry.generator.c:1480-1483recv_generator- directoryitemize()at its own flist position.log.c:788rwriteprints the row. The flist is sorted byf_name_cmp(flist.c:1732-1736) = tree order.Design: deferred single-pass ordered flush
oc already reassembles parallel output into upstream serial order, so a post-transfer flush drained in flist-index order is byte-identical to upstream's serial walk.
itemize_rows: RefCell<BTreeMap<usize, Vec<String>>>onReceiverContext(key = flist index -> automatic tree order;Vectolerates a phase-2 redo re-itemizing an entry;RefCellsuffices because every emit site runs on the main driver thread, never a rayon worker).record_itemize(flist_idx, iflags, entry)renders via the existingrender_itemize_lineand buffers the row. It preservesemit_itemize's client-mode visibility gate exactly - a server receiver's row still travels as wire iflags and is printed by the client's sender (log.c:822gates theFCLIENTwrite on!am_server), so nothing is recorded there and no row is doubled.flush_itemize_rows(writer)drains theBTreeMapin ascending key order through the existing client sink, called once immediately beforefinalize_transfer.emit_or_record_itemizedispatcher routes each emit site: it buffers when the per-transferdefer_itemizetoggle is set, and emits immediately otherwise.Scope: only
run_pipelinedis convertedThe emit sites live in helpers (
create_directories,build_files_to_transfer, and the shared pipeline loop) that are also used by the sync, incremental, and async receive paths. To convert onlyrun_pipelinedwithout a sprawling diff, the deferral is an opt-in per-transfer toggle:defer_itemizedefaults tofalse- every other caller emits immediately, exactly as before (behaviour-identical).run_pipelinedsetsdefer_itemize = trueand callsflush_itemize_rowsbefore finalization.Converted emit sites (all now route through the dispatcher): the directory row in
create_directories, the transfer row and the up-to-date/no-change metadata row inbuild_files_to_transfer, and the server-mode transfer row in the pipeline loop (a no-op for display in both the old and new code).Follow-ups (left unchanged here): the
sync,pipelined_incremental, and all async receive twins; the-vname lines; and the--delete-pass rows.Test
deferred_itemize_rows_interleave_in_flist_index_orderdrives the real record sites (create_directories+build_files_to_transfer) with deferral on over ana/ a/f1 b/ b/f2flist and asserts the buffered rows land interleaved in flist-index order (.d a/,>f a/f1,.d b/,>f b/f2), not batched (.d a/,.d b/, ...). It fails if the two-phase batch emission returns: an immediate emit leaves the buffer empty, and recording without the per-index key would order both directory rows ahead of both file rows.Validation
cargo fmt --all- clean.cargo clippy -p transfer --all-targets --all-features --no-deps -- -D warnings- no issues.-iiend-to-end seal against a real upstream binary on the Linux validation host is required before merge.