Skip to content

Stop pinning clang in windows ucrt - #47

Closed
jbourassa wants to merge 9 commits into
oxidize-rb:mainfrom
jbourassa:main
Closed

Stop pinning clang in windows ucrt#47
jbourassa wants to merge 9 commits into
oxidize-rb:mainfrom
jbourassa:main

Conversation

@jbourassa

@jbourassa jbourassa commented Jan 2, 2025

Copy link
Copy Markdown
Collaborator

Ruby 3.4 + Windows is failing CI on wasmtime-rb, see this error.

Ruby 3.4 detects and uses <stdckdint.h> if it's present (source). For some reason, on Windows with that clang version the compiler thinks <stdckdint.h> is available, but then errors when includeing it.

I tried using the built-in clang version and looks like it worked, see this CI run, hence this PR.

clang 15 fails to compile Ruby 3.4, but looks like rb-sys' bindgen
properly works with more recent clang now.
@jbourassa
jbourassa marked this pull request as ready for review January 2, 2025 14:49
@gjtorikian

Copy link
Copy Markdown
Member

This should also close #46.

@gjtorikian

Copy link
Copy Markdown
Member

I realized this morning I have merge permissions for this repo, so I created a test build for Commonmarker using this action: gjtorikian/commonmarker#335

Since it passed (thank you!), I was ready to merge this in, before realizing I don't have access to one thing: context. 😆 I'm not sure why this was pinned in the first place, so I don't feel comfortable merging it in without @ianks' 👀 .

@jbourassa

Copy link
Copy Markdown
Collaborator Author

IIRC, it was pinned to workaround incompatibilities between the latest bindgen version supported by rb-sys' MRSV and recent clang. But we're now on a more recent bindgen version, so that reason no longer applies.

I'm going off what I remember and could be wrong, I too would feel better if Ian had a look before merging.

@ianks

ianks commented Jan 8, 2025

Copy link
Copy Markdown
Contributor

annoyingly, i think bindgen breaks without clang-15 still :/ https://github.com/jbourassa/oxidize-rb-actions/actions/runs/12583936454/job/35072741493#step:8:4505

@jbourassa

Copy link
Copy Markdown
Collaborator Author

Ah, that's unfortunate, I don't know what's causing it nor how to fix it.

I tried updating bindgen in hope that it'd fix the issue (oxidize-rb/rb-sys#482), but that won't work given the recent bindgen require rust 1.70 (and the current MSRV is 1.65).

Not sure where to go from here.

@gjtorikian

gjtorikian commented Jan 10, 2025

Copy link
Copy Markdown
Member

FWIW I insert https://github.com/lhotari/action-upterm as a step in situations like these. That way one can just SSH into the runner and debug from there.

@jbourassa

Copy link
Copy Markdown
Collaborator Author

Oh, I didn't realize I spamming y'all! Unfortunately the upterm action does not work on Windows.

Going to close this PR and open a new one with a different approach.

@gjtorikian

Copy link
Copy Markdown
Member

No spam at all! I'm grateful you're looking into it. I just thought I'd offer another way to debug.

@gjtorikian

Copy link
Copy Markdown
Member

Is there a reason MSRV is set to what it is? Can it be raised?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants