Skip to content

Handle --server entrypoint locally#2197

Merged
oferchen merged 1 commit into
masterfrom
ensure-server-implementation-matches-upstream-rsync
Nov 21, 2025
Merged

Handle --server entrypoint locally#2197
oferchen merged 1 commit into
masterfrom
ensure-server-implementation-matches-upstream-rsync

Conversation

@oferchen
Copy link
Copy Markdown
Owner

Summary

  • route --server invocations through the local CLI so they render the standard help text and usage diagnostics
  • adjust CLI and embedding tests to expect local server-mode handling and error trailers

Testing

  • cargo test -p cli server_mode_renders_help_and_usage_error -- --nocapture

Codex Task

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +100 to +104
let program_name = super::detect_program_name(args.first().map(OsString::as_os_str));
let help = super::render_help(program_name);

fn terminate_server_process(
child: &mut Child,
stdout_thread: &mut Option<thread::JoinHandle<()>>,
stderr_thread: &mut Option<thread::JoinHandle<()>>,
) {
let _ = child.kill();
let _ = child.wait();
join_server_thread(stdout_thread);
join_server_thread(stderr_thread);
}
// Upstream prints the standard help text when `--server` is invoked directly.
let _ = stdout.write_all(help.as_bytes());
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P0 Badge Remote --server invocations exit before serving

run_server_mode now renders local help text and then returns a syntax/usage error instead of delegating --server invocations to the upstream rsync binary. Any SSH transfer that launches oc-rsync --server ... on the remote host will now hit this early exit and fail before the server protocol starts, eliminating remote copy/receive support that previously worked via the fallback server implementation.

Useful? React with 👍 / 👎.

@oferchen oferchen merged commit 7f2dca7 into master Nov 21, 2025
5 of 6 checks passed
@oferchen oferchen deleted the ensure-server-implementation-matches-upstream-rsync branch November 21, 2025 08:07
oferchen added a commit that referenced this pull request May 16, 2026
…RSE I2) (#4143)

Adds INC_RECURSE diagnostic counter I2 (#2197): two process-global
AtomicU64 counters that track every encode_and_send_segment invocation
on the generator file-list path and the cumulative wall time spent
inside the helper across all generator transfers.

The counters fire from a single record_segment_dispatch helper bumped
from encode_and_send_segment itself, which wraps the existing body in
an Instant::now() pair. The zero-entry early-return path is still
counted so per-transfer dispatch rate stays comparable to segment
scheduler activity, but contributes a near-zero ns delta.

Totals are sampled in GeneratorContext::run via
segment_dispatch_totals() in the same finalize block used by PR #4103
(I1 first-byte latency), PR #4120 (I4 NDX partition_point), PR #4121
(I3 flush rate) and PR #4142 (I5 prepare_pending_acl), and emitted
via:

- debug_log!(Genr, 1, "generator encode_and_send_segment totals: \
  calls=N elapsed_ns=M")
- tracing::debug! event on rsync::generator::segment_dispatch when
  the optional tracing feature is enabled

Two unit tests in crates/transfer/src/generator/tests.rs cover the
new counters:

- segment_dispatch_call_counter_increments asserts the call counter
  and the cumulative elapsed_ns counter grow by at least N and
  sum-of-durations after N synthetic record_segment_dispatch
  invocations. The assertion uses >= because the counter is shared
  across the process and other tests may run concurrently.
- segment_dispatch_totals_observable_without_dispatch asserts the
  totals snapshot is a pure read - constructing a generator does not
  bump the counter on its own, so two adjacent reads with no
  intervening dispatch call must return identical values.

No new CLI flag, no wire-format change, no change to the public
encode_and_send_segment signature.

Closes #2197
oferchen added a commit that referenced this pull request May 18, 2026
…RSE I2) (#4143)

Adds INC_RECURSE diagnostic counter I2 (#2197): two process-global
AtomicU64 counters that track every encode_and_send_segment invocation
on the generator file-list path and the cumulative wall time spent
inside the helper across all generator transfers.

The counters fire from a single record_segment_dispatch helper bumped
from encode_and_send_segment itself, which wraps the existing body in
an Instant::now() pair. The zero-entry early-return path is still
counted so per-transfer dispatch rate stays comparable to segment
scheduler activity, but contributes a near-zero ns delta.

Totals are sampled in GeneratorContext::run via
segment_dispatch_totals() in the same finalize block used by PR #4103
(I1 first-byte latency), PR #4120 (I4 NDX partition_point), PR #4121
(I3 flush rate) and PR #4142 (I5 prepare_pending_acl), and emitted
via:

- debug_log!(Genr, 1, "generator encode_and_send_segment totals: \
  calls=N elapsed_ns=M")
- tracing::debug! event on rsync::generator::segment_dispatch when
  the optional tracing feature is enabled

Two unit tests in crates/transfer/src/generator/tests.rs cover the
new counters:

- segment_dispatch_call_counter_increments asserts the call counter
  and the cumulative elapsed_ns counter grow by at least N and
  sum-of-durations after N synthetic record_segment_dispatch
  invocations. The assertion uses >= because the counter is shared
  across the process and other tests may run concurrently.
- segment_dispatch_totals_observable_without_dispatch asserts the
  totals snapshot is a pure read - constructing a generator does not
  bump the counter on its own, so two adjacent reads with no
  intervening dispatch call must return identical values.

No new CLI flag, no wire-format change, no change to the public
encode_and_send_segment signature.

Closes #2197
oferchen added a commit that referenced this pull request May 18, 2026
…RSE I2) (#4143)

Adds INC_RECURSE diagnostic counter I2 (#2197): two process-global
AtomicU64 counters that track every encode_and_send_segment invocation
on the generator file-list path and the cumulative wall time spent
inside the helper across all generator transfers.

The counters fire from a single record_segment_dispatch helper bumped
from encode_and_send_segment itself, which wraps the existing body in
an Instant::now() pair. The zero-entry early-return path is still
counted so per-transfer dispatch rate stays comparable to segment
scheduler activity, but contributes a near-zero ns delta.

Totals are sampled in GeneratorContext::run via
segment_dispatch_totals() in the same finalize block used by PR #4103
(I1 first-byte latency), PR #4120 (I4 NDX partition_point), PR #4121
(I3 flush rate) and PR #4142 (I5 prepare_pending_acl), and emitted
via:

- debug_log!(Genr, 1, "generator encode_and_send_segment totals: \
  calls=N elapsed_ns=M")
- tracing::debug! event on rsync::generator::segment_dispatch when
  the optional tracing feature is enabled

Two unit tests in crates/transfer/src/generator/tests.rs cover the
new counters:

- segment_dispatch_call_counter_increments asserts the call counter
  and the cumulative elapsed_ns counter grow by at least N and
  sum-of-durations after N synthetic record_segment_dispatch
  invocations. The assertion uses >= because the counter is shared
  across the process and other tests may run concurrently.
- segment_dispatch_totals_observable_without_dispatch asserts the
  totals snapshot is a pure read - constructing a generator does not
  bump the counter on its own, so two adjacent reads with no
  intervening dispatch call must return identical values.

No new CLI flag, no wire-format change, no change to the public
encode_and_send_segment signature.

Closes #2197
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