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.

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

Fix wrong clang used on Windows #157

Merged
merged 1 commit into from
Dec 30, 2024
Merged

Conversation

Fernthedev
Copy link
Contributor

In relation to the issue I created here #156, I started looking deeper into Rust Bindgen issues and found some things that indicated bindgen was using the wrong clang with clang-sys. The environment provided allows clang-sys to use Android NDK clang and setup properly. The extension is required on Windows.

It seems to be working properly, at least finding the headers.

Relation:
rust-lang/rust-bindgen#2962
https://github.com/KyleMayes/clang-sys?tab=readme-ov-file#environment-variables

Fixing this resolves #156 in my use case at least.

@bbqsrc
Copy link
Owner

bbqsrc commented Dec 30, 2024

If you only have the CLANG_PATH variable change, does that solve the problem? Adding explicit .exe handling doesn’t strike me as something that will have any effect due to how windows resolves executables, and the space after the -I will probably cause some surprising nightmares down the line.

@Fernthedev
Copy link
Contributor Author

Well surprisingly, yes it does! I get a warning that the path is invalid without the extension, as it seems to assume it's a directory.

As for the space, that was an accident leftover from by debugging. I'll go resolve that now, sorry!

@Fernthedev
Copy link
Contributor Author

For further context, only the CLANG_PATH environment variable cares about the extension path. The rest of the changes were just for consistency, but I can revert those changes and only give the extension to the CLANG_PATH variable.

@bbqsrc
Copy link
Owner

bbqsrc commented Dec 30, 2024

That would be much preferable. 😊

@Fernthedev Fernthedev force-pushed the fix-clang-path branch 2 times, most recently from 6132ff6 to 25f6c89 Compare December 30, 2024 20:45
@Fernthedev
Copy link
Contributor Author

Done, and I verified it still works. Thanks

@bbqsrc bbqsrc merged commit 8261932 into bbqsrc:main Dec 30, 2024
8 checks passed
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.

Bindgen inconsistency between Windows and Linux
2 participants