Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: calling programs with spaces on Windows #4515

Merged
merged 12 commits into from
Jul 26, 2024
Merged

fix: calling programs with spaces on Windows #4515

merged 12 commits into from
Jul 26, 2024

Conversation

Kha
Copy link
Member

@Kha Kha commented Jun 20, 2024

This entire API has been carefully optimized for maximum pain output

@Kha Kha added the release-ci Enable all CI checks for a PR, like is done for releases label Jun 20, 2024
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc June 20, 2024 13:40 Inactive
tydeu
tydeu reviewed Jun 20, 2024
View reviewed changes
src/runtime/process.cpp Outdated Show resolved Hide resolved
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc June 20, 2024 14:46 Inactive
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc June 20, 2024 14:56 Inactive
@Kha Kha force-pushed the fix-createprocess branch from 945d56a to 140c3a7 Compare June 20, 2024 15:05
@Kha Kha force-pushed the fix-createprocess branch from 140c3a7 to 3a7b7a5 Compare June 20, 2024 15:10
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc June 21, 2024 07:56 Inactive
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc June 21, 2024 09:41 Inactive
@Kha
Copy link
Member Author

Kha commented Jun 21, 2024

Well, something broke, but it seems more likely to be a Unicode problem with Lake (I think we're still not setting the code page to UTF-8) or something on the way to it.

tydeu
tydeu reviewed Jun 21, 2024
View reviewed changes
tests/pkg/spaces/test.sh Outdated Show resolved Hide resolved
@Kha @tydeu
Update tests/pkg/spaces/test.sh
632030a 
Co-authored-by: Mac Malone <tydeu@hatpress.net>
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc July 1, 2024 09:21 Inactive
@Kha
Copy link
Member Author

Kha commented Jul 1, 2024

Well, now the test isn't broken even without escaping. It might be some weird edge case like "CreateProcess with A\B C.exe "an arg" works, but only if A\B does not exist by itself" 🤷 .

@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc July 24, 2024 14:15 Inactive
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc July 24, 2024 16:34 Inactive
@Kha
Copy link
Member Author

Kha commented Jul 25, 2024

Hah, successfully failed the test. Now to add back the fix.

error: unspecified system_category error (error code: 193)

@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc July 25, 2024 09:43 Inactive
@Kha Kha marked this pull request as ready for review July 25, 2024 10:54
@Kha Kha enabled auto-merge July 25, 2024 10:54
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc July 25, 2024 10:58 Inactive
@Kha Kha changed the title fix: calling programs with spaces on Windows? fix: calling programs with spaces on Windows Jul 26, 2024
@Kha Kha removed the release-ci Enable all CI checks for a PR, like is done for releases label Jul 26, 2024
@Kha Kha added this pull request to the merge queue Jul 26, 2024
@github-actions github-actions bot added the toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN label Jul 26, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

Mathlib CI status (docs):

  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 1f732bb3b7fa8e93ca01c1b257668aee14d72d2d --onto 39e0b41fe1ab4d16f15efb0dc9bd7607a8941713. (2024-07-26 17:37:33)

Merged via the queue into master with commit 9b342ef Jul 26, 2024
23 of 24 checks passed
@Kha Kha deleted the fix-createprocess branch August 10, 2024 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tydeu tydeu tydeu left review comments

Assignees
No one assigned
Labels
toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Kha @leanprover-community-mathlib4-bot @tydeu