-
-
Notifications
You must be signed in to change notification settings - Fork 642
Remove mingw toolset #2396
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
Remove mingw toolset #2396
Conversation
|
|
||
| filter "toolset:mingw" | ||
| links { "crypt32", "bcrypt" } | ||
| filter { "system:windows", "toolset:not msc" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about LLVM with Visual Studio? How should this be handled?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So big build matrix! It seems I would have to add extra CI config.
I honestly don't know
- if clang under visual should have the link
- if clang under mingw should have the link.
It seems to beusagefrom curl.
I can use filter { "system:windows", "toolset:gcc" } to not change those configs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels like a bigger change like we discussed, does this solve an immediate problem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussion was adding extra API which would be a big change IMO.
Here I (propose to) remove the strange toolset mingw (which is no even web documented)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added #2576 for LLVM with visual.
Once merged, I would rebase this PR which should answer the original question.
1c6a8cc to
6d4c187
Compare
6d4c187 to
76c1b1a
Compare
76c1b1a to
ec8eaa7
Compare
ec8eaa7 to
a0a05ab
Compare
| { "clang", "Clang (clang)" }, | ||
| { "gcc", "GNU GCC (gcc/g++)" }, | ||
| { "mingw", "MinGW GCC (gcc/g++)" }, | ||
| { "mingw", "MinGW GCC (gcc/g++)" }, -- deprecated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Requesting a follow-up PR to allow for deprecation of "allowed" values in "newoption" (via an API)
What does this PR do?
It removes toolset mingw
which is mostly an alias of "gcc on windows" (for filtering).
How does this PR change Premake's behavior?
filter { "toolset:mingw" }is broken, and alternative should be used, asfilter { "system:windows", "toolset:not msc" }Anything else we should know?
So some works should be done if we want to handle mingw correctly.
Did you check all the boxes?
closes #XXXXin comment to auto-close issue when PR is merged)