Count the complete Windows locale encoding suffix by youknowdot · Pull Request #9065 · RustPython/RustPython

youknowdot · 2026-10-10T12:35:47Z

Summary

Extract the Python 3.14-compatible Windows locale suffix guard from #8954.

Count every byte after the first dot toward the existing 15-byte encoding-name limit, including an @modifier. The existing LC_ALL helper still validates semicolon-separated components individually. This matches CPython 3.14.7 and 3.14.8, which reject before calling the CRT.

The original guard from #6416 incorrectly stopped counting at @. Existing canonical long-encoding tests cover other suffix lengths; the small Python snippet adds only the missing modifier cases for LC_CTYPE and LC_ALL. Both test strings exceed the guard and cannot reach the CRT with this fix.

Validation and platform limits

Independent source review confirms both inputs return the existing locale.Error before CString conversion or the host call.
Normal repository hooks and the Linux native build passed.
Both Linux snippet harness entries pass, but the Windows assertions are platform-skipped. This is not Windows runtime validation.
No defective Windows baseline was executed, and no local Windows toolchain is installed.
Linux stdlib and separately configured CAPI Clippy pass with -D warnings. Hosted Windows build and snippet results are required before treating platform validation as complete.

No locale setting is changed by the new regression cases. No canonical assertions, README wording, or implementation-name guards are changed.

Summary by CodeRabbit

Bug Fixes
- On Windows, setting LC_CTYPE or LC_ALL to a locale name with an overlong encoding portion now raises the expected “unsupported locale setting” error. The length check includes the full portion after the period, including any modifier suffix, so these invalid settings are rejected consistently.

Include modifiers in the 15-byte bound before calling the CRT, matching Python 3.14. Assisted-by: OpenAI Codex:model identifier unavailable

coderabbitai · 2026-10-10T12:35:56Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: RustPython/RustPython/.coderabbit.yml
Review profile: CHILL
Plan: Advanced
Run ID: a5836347-4c08-44fb-951c-080c906b1760

📥 Commits

Reviewing files that changed from the base of the PR and between b63c3b0 and 6e7e7e5.

📒 Files selected for processing (2)

crates/stdlib/src/locale.rs
extra_tests/snippets/stdlib_locale.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.

📝 Walkthrough

Walkthrough

Windows locale-name validation now checks the full substring after the period against MAX_CP_LEN. A Windows-only test checks that overlong names for LC_CTYPE and LC_ALL raise the expected error.

Changes

Windows locale validation

Layer / File(s)	Summary
Validate locale name length `crates/stdlib/src/locale.rs`, `extra_tests/snippets/stdlib_locale.py`	The Windows validation checks the full substring after the period against `MAX_CP_LEN`. Tests check that overlong names for `LC_CTYPE` and `LC_ALL` raise `locale.Error("unsupported locale setting")`.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: youknowone

Merge Risk: ⚪ Minimal · up to 6e7e7

Overlong Windows locale suffixes, including modifier bytes, are rejected for LC_CTYPE and LC_ALL. No concrete code blocker is evident; complete the stated hosted Windows validation before considering platform validation complete.

Pre-merge checks | 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: counting the complete Windows locale encoding suffix, including modifiers, within the length limit.
Docstring Coverage		Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create a new PR

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

_{Comment @coderabbitai help to get the list of available commands.}

chatgpt-codex-connector · 2026-10-10T12:57:26Z

Codex Review Summary

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

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-10T13:01:19.412407Z	`6e7e7e5`	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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6e7e7e5cfd

ℹ️ 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".

chatgpt-codex-connector · 2026-10-10T13:01:16Z

+        (locale.LC_CTYPE, "English.1252@" + "x" * 16),
+        (locale.LC_ALL, "LC_CTYPE=English.1252@" + "x" * 16),


Remove the prohibited new locale test

This new snippet adds test logic and data, but the root AGENTS.md restricts test-file changes to adding or removing expected-failure decorators and associated TODO comments; these new LC_CTYPE/LC_ALL cases therefore violate the repository's critical test-modification policy and should be removed from this change.

AGENTS.md reference: AGENTS.md:L273-L279

Useful? React with 👍 / 👎.

codspeed · 2026-10-10T16:10:25Z

Merging this PR will not alter performance

✅ 62 untouched benchmarks
⏩ 4 skipped benchmarks¹

_{Comparing youknowdot:fix-windows-locale-suffix-length (6e7e7e5) with main (b63c3b0)}

4 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

Count the complete Windows locale encoding suffix

6e7e7e5

Include modifiers in the 15-byte bound before calling the CRT, matching Python 3.14. Assisted-by: OpenAI Codex:model identifier unavailable

youknowone approved these changes Oct 10, 2026

View reviewed changes

youknowone marked this pull request as ready for review October 10, 2026 12:57

youknowone enabled auto-merge (squash) October 10, 2026 12:57

chatgpt-codex-connector Bot reviewed Oct 10, 2026

View reviewed changes

youknowone merged commit 9220438 into RustPython:main Oct 10, 2026
31 checks passed

Navigation Menu

Repository navigation

Count the complete Windows locale encoding suffix - #9065

Count the complete Windows locale encoding suffix#9065
youknowone merged 1 commit into
RustPython:mainfrom
youknowdot:fix-windows-locale-suffix-length

youknowdot commented Oct 10, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 10, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

chatgpt-codex-connector Bot commented Oct 10, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Oct 10, 2026

Uh oh!

codspeed Bot commented Oct 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		(locale.LC_CTYPE, "English.1252@" + "x" * 16),
		(locale.LC_ALL, "LC_CTYPE=English.1252@" + "x" * 16),

Conversation

youknowdot commented Oct 10, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Validation and platform limits

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

chatgpt-codex-connector Bot commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Oct 10, 2026

Choose a reason for hiding this comment

Uh oh!

codspeed Bot commented Oct 10, 2026

Merging this PR will not alter performance

Footnotes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

youknowdot commented Oct 10, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 10, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Oct 10, 2026 •

edited

Loading