Skip to content

ci: Fix cancel parallel jobs on windows #20084

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.

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

Merged
merged 1 commit into from
Jun 25, 2025

Conversation

ShoyuVanilla
Copy link
Member

@ShoyuVanilla ShoyuVanilla commented Jun 24, 2025

Fixes #20079

@ShoyuVanilla ShoyuVanilla force-pushed the fix-windows-cancel-par branch 3 times, most recently from 4345a52 to 6ab76a8 Compare June 24, 2025 13:39
@lnicola
Copy link
Member

lnicola commented Jun 24, 2025

I think we can run curl.exe (not that there's anything wrong with Invoke-RestMethod, so it doesn't matter). I wonder if it wouldn't be nicer to write that command on a single line and avoid having two versions.

@ShoyuVanilla ShoyuVanilla force-pushed the fix-windows-cancel-par branch 3 times, most recently from 0f63b6d to ef4ddda Compare June 24, 2025 13:59
Copy link
Member

@Veykril Veykril left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ShoyuVanilla ShoyuVanilla force-pushed the fix-windows-cancel-par branch from ef4ddda to be7f564 Compare June 24, 2025 14:00
@ShoyuVanilla
Copy link
Member Author

I think we can run curl.exe (not that there's anything wrong with Invoke-RestMethod, so it doesn't matter). I wonder if it wouldn't be nicer to write that command on a single line and avoid having two versions.

Can't we just change the shell for windows to bash? https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#defaultsrunshell

I was going to do this in a breeze but pwsh was more harder than I thought and you saw my follies 😅

Okay, I'll try with bash and curl.exe

@ShoyuVanilla ShoyuVanilla force-pushed the fix-windows-cancel-par branch from be7f564 to 4ed9bfc Compare June 24, 2025 14:08
@ShoyuVanilla
Copy link
Member Author

ShoyuVanilla commented Jun 24, 2025

Oh, this worked well. I should have tried bash first 😂
https://github.com/rust-lang/rust-analyzer/actions/runs/15852789504/job/44690400616
image

@ShoyuVanilla ShoyuVanilla force-pushed the fix-windows-cancel-par branch from 4ed9bfc to dfd2c0e Compare June 24, 2025 14:11
@ShoyuVanilla ShoyuVanilla marked this pull request as ready for review June 24, 2025 14:11
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 24, 2025
@ShoyuVanilla
Copy link
Member Author

So the following was my chain of thoughts:

  • There might be no msys/mingw on Windows runner by default
  • Even so, there might be no curl on it
  • Even so, it might be not on $PATH by default

Thus, I just try to do things with Invoke-RestMethod, which is aliased to curl in pwsh and I thought it would work well, but it somehow fails to deliver auth header correctly and I don't know why.

What I lacked of were faith to belive Windows runner and determination to try brand new alternative on my first fail 😅

@lnicola
Copy link
Member

lnicola commented Jun 24, 2025

Welp, I knew about bash on Windows but lately I'm a little too biased against changing the defaults.

But I just realized we couldn't have kept a single step if we ran curl.exe 🤦.

@ShoyuVanilla
Copy link
Member Author

ShoyuVanilla commented Jun 24, 2025

Yes, might be hard to correctly locate curl.exe in a single script.
BTW, I looked into the above screenshot with bash carefully and just found out that the initial Invoke-RestMethod thing actually worked with the same result and the only difference was its exit code was not 0

https://github.com/rust-lang/rust-analyzer/actions/runs/15852059976/job/44687827150
image
(Responses are same with the former screenshot)

Besides those exit code behaviors (might be) related to HTTP status code, I have no idea why GitHub returns 403 but handles the requests as intended 🙃

@Veykril Veykril added this pull request to the merge queue Jun 25, 2025
Merged via the queue into rust-lang:master with commit 3eb4925 Jun 25, 2025
26 of 28 checks passed
@ShoyuVanilla ShoyuVanilla deleted the fix-windows-cancel-par branch June 25, 2025 08:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cancelling workflows from Windows jobs fails
4 participants