Skip to content

Fix Process changing GAP's directory on Cygwin - #6776

Merged
fingolfin merged 1 commit into
masterfrom
mh-claude/cygwin-process-cwd
Oct 23, 2026
Merged

fingolfin merged 1 commit into
masterfrom
mh-claude/cygwin-process-cwd

Conversation

@fingolfin

Copy link
Copy Markdown
Member

spawnve cannot set the child's working directory, so the Cygwin variant of SyExecuteProcess changes GAP's own, and never back: every Process or RunProcess call with a directory leaves GAP there.

Change the directory only around spawnve and restore it afterwards, keeping errno from chdir or spawnve for the caller. Doing both after the descriptor setup leaves no early return between them.

Assisted-by: Generative AI

@fingolfin fingolfin added kind: bug Issues describing general bugs, and PRs fixing them os: windows Issues and PRs that are (at least partially) specific to Windows topic: kernel release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes labels Oct 19, 2026
@codecov

codecov Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 70.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.70%. Comparing base (f6ee13b) to head (3a740d7).

Files with missing lines Patch % Lines
src/iostream.c 70.00% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6776      +/-   ##
==========================================
+ Coverage   77.11%   79.70%   +2.59%     
==========================================
  Files         683      684       +1     
  Lines      295304   295580     +276     
  Branches     8667     8690      +23     
==========================================
+ Hits       227709   235599    +7890     
+ Misses      65712    58164    -7548     
+ Partials     1883     1817      -66     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@fingolfin
fingolfin force-pushed the mh-claude/cygwin-process-cwd branch from ec0f580 to 29d7f04 Compare October 19, 2026 11:08
@fingolfin fingolfin closed this Oct 22, 2026
@fingolfin fingolfin reopened this Oct 22, 2026
@fingolfin
fingolfin enabled auto-merge (squash) October 22, 2026 22:00
spawnve cannot set the child's working directory, so the Cygwin
variant of SyExecuteProcess changes GAP's own, and never back: every
`Process` or `RunProcess` call with a directory leaves GAP there.

Change the directory only around spawnve and restore it afterwards,
keeping errno from chdir or spawnve for the caller. Doing both after
the descriptor setup leaves no early return between them.

Assisted-by: Generative AI
@fingolfin
fingolfin force-pushed the mh-claude/cygwin-process-cwd branch from 29d7f04 to 3a740d7 Compare October 22, 2026 22:22
@fingolfin
fingolfin merged commit 4186b95 into master Oct 23, 2026
34 checks passed
@fingolfin
fingolfin deleted the mh-claude/cygwin-process-cwd branch October 23, 2026 00:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind: bug Issues describing general bugs, and PRs fixing them os: windows Issues and PRs that are (at least partially) specific to Windows release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes topic: kernel

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants