Skip to content

Preserve native trailing command-line argument bytes - #9494

Merged
youknowone merged 1 commit into
RustPython:mainfrom
youknowdot:python315-lossless-argv-20261031
Oct 31, 2026
Merged

youknowone merged 1 commit into
RustPython:mainfrom
youknowdot:python315-lossless-argv-20261031

Conversation

@youknowdot

Copy link
Copy Markdown
Contributor

Summary

  • Preserve native trailing command-line arguments as OsString instead of rejecting them during CLI UTF-8 conversion.
  • Decode sys.argv and sys.orig_argv through the existing filesystem-decoding path; use args_os() for original arguments and program-name initialization.
  • Adapt startup error reporting and the syslog default identifier to the retained native strings.
  • Restore only CmdLineTest.test_invalid_utf8_arg through scripts/update_lib migrate, retaining every other failure marker and the explicit RUSTPYTHONPATH adapter. No test assertion or payload changes.

Based on main 8051c05ae4e100bf35eeb32044d3456d2546dc75, after #9492 landed. No dependency or Cargo.lock changes.

Public Rust API change

Settings.argv changes from Vec<String> to Vec<OsString> and Paths.program_name from String to OsString. Embedders assigning these public fields must convert their values (for example with .into() or OsString::from). This is not a source-compatible field-type change.

Scope and limits

This fixes native-byte transport for trailing arguments and original-argument construction under RustPython's existing filesystem-decoding policy. It does not implement complete locale-dependent decoding, non-Unicode script/module selectors, invalid -c diagnostics, or invalid argv[0] compatibility. Those behaviors are not claimed by this PR. The undecodable-code expected-failure marker remains.

Validation

  • cargo fmt --all -- --check and git diff --check passed.
  • cargo check and strict cargo clippy -- -D warnings passed for --bin rustpython --no-default-features --features threading,stdlib,stdio,importlib,host_env,doc,ucd_3_2_0 with the unchanged lockfile.
  • Fresh CLI build passed with the same features; the actual VM compiler command used opt-level 3.
  • Fresh bounded observations compared the exact two canonical test_invalid_utf8_arg payloads under C.UTF-8/default, C locale, -X utf8, and -X utf8=0: eight candidate and eight pinned CPython 3.15.0 processes. Every process exited successfully without stderr; os.fsencode(sys.argv[1]) and os.fsencode(sys.orig_argv[-1]) matched the original bytes and the reference. Input manifests remained unchanged and all owned process groups exited.
  • The canonical test method's AST, including its remaining platform decorator, matches CPython commit d220409a13b77de7a6c30abe16ea90db01528edc.

The observations directly ran the bounded payload observer with -B -S; they did not execute the original subprocess unittest harness, the entire test_cmd_line module, the full workspace/CAPI suites, or Windows/macOS runtime tests. The reference was built with test extension modules disabled. CI remains the next cross-platform check, not proof of untested compatibility.

AI-assisted implementation and validation; commit disclosure included.

Carry arguments and program names as OsString until Python filesystem
decoding, and restore the canonical invalid UTF-8 argument test through
update_lib. Keep remaining markers and startup adapters unchanged.

Assisted-by: OpenAI Codex: model version unavailable
@coderabbitai

coderabbitai Bot commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Caution

Review failed

The pull request is closed.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@github-actions

Copy link
Copy Markdown
Contributor

📦 Library Dependencies

The following Lib/ modules were modified. Here are their dependencies:

[ ] test: cpython/Lib/test/test_cmd_line.py (TODO: 12)

dependencies:

dependent tests: (no tests depend on cmd_line)

Legend:

  • [+] path exists in CPython
  • [x] up-to-date, [ ] outdated

@youknowone
youknowone marked this pull request as ready for review October 31, 2026 10:36
@youknowone
youknowone merged commit c2bb450 into RustPython:main Oct 31, 2026
29 checks passed
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-10-31T10:36:07.622534Z
We couldn’t complete this review. Comment @codex review to try again.
51c3a30 Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants