Skip to content

Conversation

@Jarod42
Copy link
Contributor

@Jarod42 Jarod42 commented Dec 26, 2024

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, as filter { "system:windows", "toolset:not msc" }

Anything else we should know?

  • Detecting correct mingw-w64/llvm-mingw/msys2 is currently not done anyway
  • Some mingw uses clang and not gcc

So some works should be done if we want to handle mingw correctly.

Did you check all the boxes?

  • Focus on a single fix or feature; remove any unrelated formatting or code changes
  • Add unit tests showing fix or feature works; all tests pass
  • Mention any related issues (put closes #XXXX in comment to auto-close issue when PR is merged)
  • Follow our coding conventions
  • Minimize the number of commits
  • Align documentation to your changes


filter "toolset:mingw"
links { "crypt32", "bcrypt" }
filter { "system:windows", "toolset:not msc" }
Copy link
Member

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?

Copy link
Contributor Author

@Jarod42 Jarod42 Dec 27, 2024

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 be usage from curl.

I can use filter { "system:windows", "toolset:gcc" } to not change those configs

Copy link
Collaborator

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?

Copy link
Contributor Author

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)

Copy link
Contributor Author

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.

@Jarod42 Jarod42 marked this pull request as draft December 29, 2024 09:37
@Jarod42 Jarod42 force-pushed the mingw_toolset_removal branch from 1c6a8cc to 6d4c187 Compare March 23, 2025 13:19
@Jarod42 Jarod42 force-pushed the mingw_toolset_removal branch from 6d4c187 to 76c1b1a Compare December 22, 2025 14:31
@Jarod42 Jarod42 marked this pull request as ready for review December 22, 2025 14:32
@Jarod42 Jarod42 marked this pull request as draft December 22, 2025 14:56
@Jarod42 Jarod42 force-pushed the mingw_toolset_removal branch from 76c1b1a to ec8eaa7 Compare December 22, 2025 15:14
@Jarod42 Jarod42 force-pushed the mingw_toolset_removal branch from ec8eaa7 to a0a05ab Compare December 22, 2025 16:21
@Jarod42 Jarod42 marked this pull request as ready for review December 22, 2025 16:40
{ "clang", "Clang (clang)" },
{ "gcc", "GNU GCC (gcc/g++)" },
{ "mingw", "MinGW GCC (gcc/g++)" },
{ "mingw", "MinGW GCC (gcc/g++)" }, -- deprecated
Copy link
Member

@nickclark2016 nickclark2016 Dec 22, 2025

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)

@nickclark2016 nickclark2016 merged commit 34208df into premake:master Dec 23, 2025
53 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.

3 participants