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.

Sign up for GitHub

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

Jump to bottom

Fix MSC LTO, runtime, subsystem #1552

Merged
merged 1 commit into from
Nov 10, 2020
Merged

Fix MSC LTO, runtime, subsystem #1552

merged 1 commit into from
Nov 10, 2020

Conversation

JoelLinn
Copy link
Contributor

@JoelLinn JoelLinn commented Nov 3, 2020 •
edited
Loading

What does this PR do?

  • Link-time optimizations now sets correct cl and ld flags.
  • Run-Time selection now adheres to runtime setting.
  • Set subsystem flag for kind=="WindowedApp"

How does this PR change Premake's behavior?

  • The different Warning flags should not cause any troubles in any consumers.
  • All other changes do in fact change the behavior. They however fix something that was broken or didn't work before.

starkos
starkos requested changes Nov 4, 2020
View reviewed changes
src/tools/msc.lua Outdated Show resolved Hide resolved
src/tools/msc.lua Show resolved Hide resolved
src/tools/msc.lua Outdated Show resolved Hide resolved
tests/tools/test_msc.lua Show resolved Hide resolved
tests/tools/test_msc.lua Outdated Show resolved Hide resolved
@starkos
Copy link
Member

starkos commented Nov 4, 2020

FYI, if you had split these fixes into separate PRs (LTO, WindowedApp, runtime flags, warning changes) we could have approved the simpler changes. Because you submitted them all as one PR, they are all now going to be held up awaiting changes. The first checkbox on our PR template says "Focus on a single fix or feature" for a reason.

@starkos starkos changed the title msc Fix several flags. Fix MSC LTO, runtime, subsystem, warning flags Nov 4, 2020
@JoelLinn JoelLinn changed the title Fix MSC LTO, runtime, subsystem, warning flags Fix MSC LTO, runtime, subsystem Nov 8, 2020
@starkos
Copy link
Member

starkos commented Nov 8, 2020

@samsinsane - does this one look reasonable to you, now that some of the cruft has been trimmed out?

@JoelLinn - you'll need to rebase these changes against the latest master branch (okay if you want to wait for review)

@samsinsane
Copy link
Member

@starkos Yeah, overall it looks good.

@starkos
Copy link
Member

starkos commented Nov 9, 2020

Cool. @JoelLinn If you can rebase and squash these changes I'll get it merged. Thanks for the contribution!

@JoelLinn
msc Fix several flags.
0163b86 
- Link-time optimizations now sets cl and ld flags.
- Run-Time selection now adheres to `runtime` setting.
- Set Subsystem for WindowedApp
@JoelLinn
Copy link
Contributor Author

JoelLinn commented Nov 9, 2020

Done. I don't know what to squash though as there is only one commit...

@starkos starkos merged commit 4138c3e into premake:master Nov 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@samsinsane samsinsane samsinsane approved these changes

@starkos starkos starkos approved these changes

Assignees

@starkos starkos

@samsinsane samsinsane

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@JoelLinn @starkos @samsinsane