Skip to content

Add CMake build and a limited subset of tests for llvm-mingw #1216

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 17 commits into from
Nov 9, 2022

Conversation

alvinhochun
Copy link
Contributor

@alvinhochun alvinhochun commented Oct 21, 2022

This add a CMake build for building cppwinrt and a limited subset of the existing tests using the llvm-mingw toolchain. A GHA CI build using this toolchain is also included.

More tests will be added later.

Depends on #1212 and #1215

CC @mstorsjo

@kennykerr
Copy link
Collaborator

So getting mingw to work with msbuild like we do for Clang is not practical?

Comment on lines +10 to +12
# alternative to midl that can produce winmd files. Also, even if we do manage
# to reuse the MSVC-compiled binaries, mingw-w64 is still missing
# windowsnumerics.impl.h which is needed to provide the types
Copy link
Collaborator

Choose a reason for hiding this comment

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

The Clang builds depend on the Windows SDK, just like MSVC. I take it mingw doesn't?

Copy link
Contributor

Choose a reason for hiding this comment

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

They have their own equivalent headers due to the Windows SDK license not allowing redistribution, and the SDK headers depending on too many MSVC-isms

Copy link

Choose a reason for hiding this comment

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

It's also important to distinguish key aspect here: Clang can target both MSVC ABI (using Windows SDK) and GCC ABI (using mingw-w64).

@sylveon
Copy link
Contributor

sylveon commented Oct 25, 2022

For clang, msbuild supports it through clang-cl. In GCC/mingw's case, there is no such tool, and VS doesn't have native support

@alvinhochun
Copy link
Contributor Author

Yeah, most has been said already. Another point is that mingw-w64 is commonly used as a way to cross-compile applications for Windows from another Unix-like OS, so headers and tools from MSVC or the Windows SDK usually does not work even if we ignore the licensing issues. (I will also have to look into this aspect later -- I'm not sure how we can use the cppwinrt tool and/or its generated headers for cross-compilation.)

@alvinhochun alvinhochun force-pushed the alvin/llvm-mingw-cmake2 branch from 884b2ad to a6876cd Compare October 29, 2022 14:08
@github-actions
Copy link

github-actions bot commented Nov 9, 2022

This pull request is stale because it has been open 10 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@alvinhochun alvinhochun force-pushed the alvin/llvm-mingw-cmake2 branch from a6876cd to 132239c Compare November 9, 2022 11:36
@alvinhochun alvinhochun changed the title [RFC] Add CMake build and a limited subset of tests for llvm-mingw Add CMake build and a limited subset of tests for llvm-mingw Nov 9, 2022
@alvinhochun alvinhochun marked this pull request as ready for review November 9, 2022 11:39
@alvinhochun
Copy link
Contributor Author

I would like this to be merged so we can at least have some CI coverage for llvm-mingw. I can continue to add more tests later.

@@ -0,0 +1,13 @@
;
Copy link
Collaborator

Choose a reason for hiding this comment

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

These are very simple def files. I'm not sure what value it provides to have them generated by a tool and what licensing issues may occur as a result. Can we remove the comments?

Copy link

Choose a reason for hiding this comment

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

Also, if it'd be possible to update to a newer snapshot of mingw-w64 (unfortunately I don't have an actual release of my toolchains that includes it) this wouldn't be needed iirc.

Other than that, removing the comment should surely be fine.

@kennykerr kennykerr merged commit 5ca626a into microsoft:master Nov 9, 2022
@alvinhochun alvinhochun deleted the alvin/llvm-mingw-cmake2 branch December 11, 2022 16:46
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.

5 participants