fix(stack): force C locale on git subprocesses to fix push under non-English locales#1336
Conversation
Merge ProtectionsYour pull request matches the following merge protections and will not be merged until they are valid. 🟢 🤖 Continuous IntegrationWonderful, this rule succeeded.
🟢 👀 Review RequirementsWonderful, this rule succeeded.
🟢 Enforce conventional commitWonderful, this rule succeeded.Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
🟢 🔎 ReviewsWonderful, this rule succeeded.
🟢 📕 PR descriptionWonderful, this rule succeeded.
|
There was a problem hiding this comment.
Pull request overview
This PR aims to make stack push resilient to non-English Git locales by standardizing subprocess locale where stack-related Git output is parsed. It touches the shared async command runner, the git patch-id helper used during stack push, and adds a regression test around environment propagation.
Changes:
- Added a shared helper to build a subprocess environment with
LC_ALL/LANG/LANGUAGE=C. - Updated the async command runner and
git patch-idsubprocess path to pass that environment through. - Added a regression test that mocks
create_subprocess_execand verifies the forced locale variables are propagated.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
mergify_cli/utils.py |
Adds the locale-forcing subprocess environment helper and wires it into the shared async subprocess runner. |
mergify_cli/tests/test_utils.py |
Adds a regression test that checks run_command() forwards the forced C-locale environment. |
mergify_cli/stack/push.py |
Passes the same forced locale environment to the git patch-id subprocess used during stack push change classification. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…English locales `mergify stack push` crashed on first push when the user's locale is not English. `fetch_notes_ref` and `read_reasons` match git error output by substring (e.g. `b"couldn't find remote ref"`, `b"no note found"`), but git emits translated messages — under fr_FR git says `fatal : impossible de trouver la référence distante`, the substring check fails, and the exception is re-raised. Force `LC_ALL=C LANG=C LANGUAGE=C` on git subprocesses spawned by `run_command` and `_git_patch_id` so the output we parse is stable regardless of the user's locale. The other git call sites (ci/queue, ci/scopes, stack/reorder) parse by exit code or null-delimited output and are deliberately left alone — `git rebase -i` in particular shows its messages to the user, and forcing English would degrade UX. Add a regression test that mocks `create_subprocess_exec` and asserts `run_command` propagates the C-locale env even when the surrounding shell is fr_FR. Change-Id: Ia3b14565355b3031809ce3f2b13f0d145bceeb30
Revision history
|
800dcbb to
ebe7ceb
Compare
Pull request has been modified.
Merge Queue Status
This pull request spent 12 minutes 4 seconds in the queue, including 11 minutes 44 seconds running CI. Required conditions to merge
|
mergify stack pushcrashed on first push when the user's locale is notEnglish.
fetch_notes_refandread_reasonsmatch git error output bysubstring (e.g.
b"couldn't find remote ref",b"no note found"), butgit emits translated messages — under fr_FR git says
fatal : impossible de trouver la référence distante, the substringcheck fails, and the exception is re-raised.
Force
LC_ALL=C LANG=C LANGUAGE=Con git subprocesses spawned byrun_commandand_git_patch_idso the output we parse is stableregardless of the user's locale. The other git call sites (ci/queue,
ci/scopes, stack/reorder) parse by exit code or null-delimited output
and are deliberately left alone —
git rebase -iin particular showsits messages to the user, and forcing English would degrade UX.
Add a regression test that mocks
create_subprocess_execand assertsrun_commandpropagates the C-locale env even when the surroundingshell is fr_FR.