Skip to content

Ensure nextest failures fail the Linux test workflow#1914

Merged
oferchen merged 1 commit into
masterfrom
implement-missing-components-for-oc-rsync-5hrvfv
Nov 3, 2025
Merged

Ensure nextest failures fail the Linux test workflow#1914
oferchen merged 1 commit into
masterfrom
implement-missing-components-for-oc-rsync-5hrvfv

Conversation

@oferchen
Copy link
Copy Markdown
Owner

@oferchen oferchen commented Nov 3, 2025

Summary

  • ensure the Linux test workflow respects cargo nextest failures by removing the unconditional success guard

Testing

  • cargo test --locked

https://chatgpt.com/codex/tasks/task_e_69091f3ec794832392f6819450f8f956

@oferchen oferchen merged commit 0c9cb77 into master Nov 3, 2025
1 check passed
@oferchen oferchen deleted the implement-missing-components-for-oc-rsync-5hrvfv branch November 3, 2025 21:51
oferchen added a commit that referenced this pull request May 1, 2026
…1911)

Closes #1911 by completing the local-copy bridge between the parsed
`IconvSetting` on `ClientConfig` and the engine's `LocalCopyOptions`,
mirroring the SSH/daemon bridge that PR #3458 wired through
`apply_common_server_flags`.

The audit at `docs/audits/iconv-inert.md` (PR #3526) labels #1911 as
"Partial" because the local-copy path bypasses
`apply_common_server_flags` and therefore had no route by which a
resolved `FilenameConverter` could reach the engine.

Changes:

- `engine::local_copy::LocalCopyOptions` and
  `LocalCopyOptionsBuilder` gain an `iconv: Option<FilenameConverter>`
  field with a `with_iconv` setter and `iconv()` accessor on the
  options struct, plus an `iconv()` setter on the builder.
- `core::client::run::LocalCopyOptionsBuilder::build` (the internal
  wrapper around the engine builder) calls
  `config.iconv().resolve_converter()` and propagates the result via
  the new setter.
- Unit tests in `crates/engine/src/local_copy/options/filters.rs`
  cover default-none, attach, and clear.
- Unit tests in `crates/core/src/client/run/mod.rs` cover the
  `IconvSetting -> LocalCopyOptions.iconv` mapping for unspecified,
  disabled, locale-default, explicit, and unsupported-charset variants.

This PR plumbs the converter only; it does not yet apply it on
sender file-list emit (#1912), receiver file-list ingest (#1913),
or filter-rule path matching (#1914). Those are tracked separately.
oferchen added a commit that referenced this pull request May 2, 2026
#1914) (#3550)

Audit the filter-evaluation entry points to confirm they operate on
local-charset names, matching upstream rsync 3.4.1 exactly.

Upstream rsync's exclude.c contains zero iconv references. Filter rules
match raw bytes in the local charset on both sender and receiver. The
hand-off happens at the flist boundary: sender filters before
iconvbufs(ic_send,...), receiver filters after iconvbufs(ic_recv,...).
Pattern strings are stored verbatim from CLI/merge-files in the local
charset.

oc-rsync follows the same model:
- Sender FilterChain::allows() in generator/file_list/walk.rs sees raw
  Path bytes from the local filesystem before encoding conversion.
- Receiver FilterChain::allows_deletion() in receiver/directory/deletion.rs
  sees post-ic_recv local-charset names from the file list and entry
  names from read_dir().
- Pattern strings flow through CLI -> filters crate without iconv hops.

No code change required. Wiring a FilenameConverter into the filter
chain would double-convert names and break pattern matches for non-ASCII
patterns.
oferchen added a commit that referenced this pull request May 5, 2026
…1911)

Closes #1911 by completing the local-copy bridge between the parsed
`IconvSetting` on `ClientConfig` and the engine's `LocalCopyOptions`,
mirroring the SSH/daemon bridge that PR #3458 wired through
`apply_common_server_flags`.

The audit at `docs/audits/iconv-inert.md` (PR #3526) labels #1911 as
"Partial" because the local-copy path bypasses
`apply_common_server_flags` and therefore had no route by which a
resolved `FilenameConverter` could reach the engine.

Changes:

- `engine::local_copy::LocalCopyOptions` and
  `LocalCopyOptionsBuilder` gain an `iconv: Option<FilenameConverter>`
  field with a `with_iconv` setter and `iconv()` accessor on the
  options struct, plus an `iconv()` setter on the builder.
- `core::client::run::LocalCopyOptionsBuilder::build` (the internal
  wrapper around the engine builder) calls
  `config.iconv().resolve_converter()` and propagates the result via
  the new setter.
- Unit tests in `crates/engine/src/local_copy/options/filters.rs`
  cover default-none, attach, and clear.
- Unit tests in `crates/core/src/client/run/mod.rs` cover the
  `IconvSetting -> LocalCopyOptions.iconv` mapping for unspecified,
  disabled, locale-default, explicit, and unsupported-charset variants.

This PR plumbs the converter only; it does not yet apply it on
sender file-list emit (#1912), receiver file-list ingest (#1913),
or filter-rule path matching (#1914). Those are tracked separately.
oferchen added a commit that referenced this pull request May 5, 2026
#1914) (#3550)

Audit the filter-evaluation entry points to confirm they operate on
local-charset names, matching upstream rsync 3.4.1 exactly.

Upstream rsync's exclude.c contains zero iconv references. Filter rules
match raw bytes in the local charset on both sender and receiver. The
hand-off happens at the flist boundary: sender filters before
iconvbufs(ic_send,...), receiver filters after iconvbufs(ic_recv,...).
Pattern strings are stored verbatim from CLI/merge-files in the local
charset.

oc-rsync follows the same model:
- Sender FilterChain::allows() in generator/file_list/walk.rs sees raw
  Path bytes from the local filesystem before encoding conversion.
- Receiver FilterChain::allows_deletion() in receiver/directory/deletion.rs
  sees post-ic_recv local-charset names from the file list and entry
  names from read_dir().
- Pattern strings flow through CLI -> filters crate without iconv hops.

No code change required. Wiring a FilenameConverter into the filter
chain would double-convert names and break pattern matches for non-ASCII
patterns.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant