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

MinGW build fix #362

Merged
merged 3 commits into from
Dec 16, 2022
Merged

MinGW build fix #362

merged 3 commits into from
Dec 16, 2022

Conversation

brainrom
Copy link
Contributor

@brainrom brainrom commented Dec 7, 2022

.def and .pdb are MSVC-specific resources. MinGW can correctly build the OpenXR loader, but only with these changes.

@CLAassistant
Copy link

CLAassistant commented Dec 7, 2022

CLA assistant check
All committers have signed the CLA.

@rpavlik
Copy link
Contributor

rpavlik commented Dec 13, 2022

The one thing I'm not sure about, is that I am pretty sure that LLVM/Clang on Windows can make PDB's. I was hoping CMake would have a variable or property that says "this toolchain makes pdb" but I haven't found one. This is an improvement but I wonder if we can flip the condition, only exclude for mingw?

@brainrom
Copy link
Contributor Author

brainrom commented Dec 13, 2022

Hmm. Okay, then if(WIN32 AND NOT MINGW AND DYNAMIC_LOADER)?

@brainrom
Copy link
Contributor Author

Changed the condition.

@rpavlik rpavlik enabled auto-merge (rebase) December 16, 2022 17:01
@rpavlik
Copy link
Contributor

rpavlik commented Dec 16, 2022

Thanks very much!

@rpavlik rpavlik merged commit 70e01d5 into KhronosGroup:main Dec 16, 2022
@brainrom brainrom deleted the patch-1 branch December 16, 2022 17:19
@rpavlik
Copy link
Contributor

rpavlik commented Mar 30, 2023

We received #387 to correct the exported symbols, which effectively undoes this. Can you work with that submitter to figure out the correct answer here that will work for both of you?

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