feat: reject --append with --whole-file at config build time#4049
Merged
Conversation
Upstream rsync (options.c:2382) rejects the combination `--append --whole-file` with the message `--append cannot be used with --whole-file` and exits with RERR_SYNTAX (1). oc-rsync was silently accepting both flags together. Add the conflict check to both validation layers that gate config construction: - `ClientConfigBuilder::validate` (core) - the primary path used by the CLI; surfaces a `ConfigConflict` whose `Display` matches upstream wording. - `LocalCopyOptionsBuilder::validate` (engine) - keeps the engine-level builder in sync so programmatic embedders also reject the combination. Only an explicit `--whole-file` (`Some(true)`) conflicts; the default (`None`) and `--no-whole-file` (`Some(false)`) remain accepted so existing call sites that set the tri-state from auto-detection are unaffected. Tests cover the engine builder, the core config builder (including the exact upstream error string), and an end-to-end CLI integration test that drives `cli::run` and asserts the RERR_SYNTAX exit code plus the diagnostic mentions both flags. Refs: #2145, #2146, #2147
3 tasks
oferchen
added a commit
that referenced
this pull request
May 16, 2026
Convert the compatibility-flags mutual-exclusion audit from a current-state inventory into a historical record. Each gap row (G1-G4) now cites the PR that landed the remediation: - G1: --checksum-choice=none promotes whole_file=1 (PR #4066) - G2: --inplace --partial-dir gated on CF_INPLACE_PARTIAL_DIR (PR #4064) - G3: --append --whole-file rejected at config build time (PR #4049) - G4: --delay-updates partial_dir promotion centralised (PR #4050) Section 5 is replaced with a resolution-summary table. Footnotes [g] and [j] are updated to describe the current behaviour and reference the resolving PRs. The divergence descriptions are preserved so the audit remains a record of the upstream-vs-oc-rsync conflict matrix prior to remediation. Refs: #2151, #2106
oferchen
added a commit
that referenced
this pull request
May 16, 2026
) Convert the compatibility-flags mutual-exclusion audit from a current-state inventory into a historical record. Each gap row (G1-G4) now cites the PR that landed the remediation: - G1: --checksum-choice=none promotes whole_file=1 (PR #4066) - G2: --inplace --partial-dir gated on CF_INPLACE_PARTIAL_DIR (PR #4064) - G3: --append --whole-file rejected at config build time (PR #4049) - G4: --delay-updates partial_dir promotion centralised (PR #4050) Section 5 is replaced with a resolution-summary table. Footnotes [g] and [j] are updated to describe the current behaviour and reference the resolving PRs. The divergence descriptions are preserved so the audit remains a record of the upstream-vs-oc-rsync conflict matrix prior to remediation. Refs: #2151, #2106
oferchen
added a commit
that referenced
this pull request
May 18, 2026
Upstream rsync (options.c:2382) rejects the combination `--append --whole-file` with the message `--append cannot be used with --whole-file` and exits with RERR_SYNTAX (1). oc-rsync was silently accepting both flags together. Add the conflict check to both validation layers that gate config construction: - `ClientConfigBuilder::validate` (core) - the primary path used by the CLI; surfaces a `ConfigConflict` whose `Display` matches upstream wording. - `LocalCopyOptionsBuilder::validate` (engine) - keeps the engine-level builder in sync so programmatic embedders also reject the combination. Only an explicit `--whole-file` (`Some(true)`) conflicts; the default (`None`) and `--no-whole-file` (`Some(false)`) remain accepted so existing call sites that set the tri-state from auto-detection are unaffected. Tests cover the engine builder, the core config builder (including the exact upstream error string), and an end-to-end CLI integration test that drives `cli::run` and asserts the RERR_SYNTAX exit code plus the diagnostic mentions both flags. Refs: #2145, #2146, #2147
oferchen
added a commit
that referenced
this pull request
May 18, 2026
) Convert the compatibility-flags mutual-exclusion audit from a current-state inventory into a historical record. Each gap row (G1-G4) now cites the PR that landed the remediation: - G1: --checksum-choice=none promotes whole_file=1 (PR #4066) - G2: --inplace --partial-dir gated on CF_INPLACE_PARTIAL_DIR (PR #4064) - G3: --append --whole-file rejected at config build time (PR #4049) - G4: --delay-updates partial_dir promotion centralised (PR #4050) Section 5 is replaced with a resolution-summary table. Footnotes [g] and [j] are updated to describe the current behaviour and reference the resolving PRs. The divergence descriptions are preserved so the audit remains a record of the upstream-vs-oc-rsync conflict matrix prior to remediation. Refs: #2151, #2106
oferchen
added a commit
that referenced
this pull request
May 18, 2026
Upstream rsync (options.c:2382) rejects the combination `--append --whole-file` with the message `--append cannot be used with --whole-file` and exits with RERR_SYNTAX (1). oc-rsync was silently accepting both flags together. Add the conflict check to both validation layers that gate config construction: - `ClientConfigBuilder::validate` (core) - the primary path used by the CLI; surfaces a `ConfigConflict` whose `Display` matches upstream wording. - `LocalCopyOptionsBuilder::validate` (engine) - keeps the engine-level builder in sync so programmatic embedders also reject the combination. Only an explicit `--whole-file` (`Some(true)`) conflicts; the default (`None`) and `--no-whole-file` (`Some(false)`) remain accepted so existing call sites that set the tri-state from auto-detection are unaffected. Tests cover the engine builder, the core config builder (including the exact upstream error string), and an end-to-end CLI integration test that drives `cli::run` and asserts the RERR_SYNTAX exit code plus the diagnostic mentions both flags. Refs: #2145, #2146, #2147
oferchen
added a commit
that referenced
this pull request
May 18, 2026
) Convert the compatibility-flags mutual-exclusion audit from a current-state inventory into a historical record. Each gap row (G1-G4) now cites the PR that landed the remediation: - G1: --checksum-choice=none promotes whole_file=1 (PR #4066) - G2: --inplace --partial-dir gated on CF_INPLACE_PARTIAL_DIR (PR #4064) - G3: --append --whole-file rejected at config build time (PR #4049) - G4: --delay-updates partial_dir promotion centralised (PR #4050) Section 5 is replaced with a resolution-summary table. Footnotes [g] and [j] are updated to describe the current behaviour and reference the resolving PRs. The divergence descriptions are preserved so the audit remains a record of the upstream-vs-oc-rsync conflict matrix prior to remediation. Refs: #2151, #2106
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
1 participant
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
options.c:2382) rejects--append --whole-fileat parse time withRERR_SYNTAX(exit code 1) and the message--append cannot be used with --whole-file. oc-rsync was silently accepting the combination, then producing inconsistent behavior at transfer time depending on which layer was reached first.ClientConfigBuilder::validate(core) - the primary path the CLI uses; surfaces aConfigConflictwhoseDisplaymatches upstream wording exactly.LocalCopyOptionsBuilder::validate(engine) - keeps the engine-level builder in sync so programmatic embedders also reject the combination.--whole-file(Some(true)) conflicts; the default (None) and--no-whole-file(Some(false)) remain accepted so existing call sites that set the tri-state from auto-detection are unaffected.Motivation
Audit follow-up tasks #2145, #2146, #2147 flagged that the
--append --whole-filecombination must be rejected at config build time to mirror upstream rsync's syntax check (options.c:2382, functionparse_arguments). Silent acceptance produced upstream-incompatible behaviour and made transfer-time diagnostics confusing.Test plan
cargo fmt --all -- --checkcrates/engine/.../builder/tests.rs::append_and_whole_file_conflictand the two accept-path tests (append_with_no_whole_file_ok,append_with_default_whole_file_ok).crates/core/.../builder/tests.rs::validate_append_with_whole_file_conflictsplus the accept-path companions; the error-string assertion locks the exact upstream wording.crates/cli/tests/mutually_exclusive_options.rs::test_append_and_whole_file_rejecteddrivescli::run, asserts exit code 1, and confirms the diagnostic names both flags using upstream phrasing.