-
-
Notifications
You must be signed in to change notification settings - Fork 619
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
Multiple gmake2 fixes #959
Conversation
* gmake2 elseif pattern for configurations. https://ghosthub.corp.blizzard.net/premake/premake-core/issues/131 * fix error message. * don't output else if there is no if statements.
@@ -15,6 +15,7 @@ | |||
trigger = "gmake2", | |||
shortname = "Alternative GNU Make", | |||
description = "Generate GNU makefiles for POSIX, MinGW, and Cygwin", | |||
toolset = "gcc", |
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.
How is this default limited to linux?
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.
it's not.... but I can't make it optional, and not setting it will result in the toolset beeing 'nil' when using gmake... unless you're project sets it... in which case this change is moot anyway...
In my opinion it's better to have a default, then to have 'nil', but if you want to make this conditional on some other setting instead please feel free to make this even better... as long as nil doesn't show up...
filter { 'toolset:gcc' } currently doesn't work on linux, even when using gmake/gmake2, and even though gcc is indeed the default because there is stupid code everywhere that does toolset = toolset or 'gcc'
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.
Would it be better to put something like
filter { "system:linux" }
toolset "gcc"
somewhere in bootup?
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.
That is indeed the alternative for linux.... for any other platform the action would still be broken.
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.
I agree... this should be merged, but I lines like those above should probably also exist for windows/osx/freebsd, etc.
No description provided.