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.

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

Fix gmake dependencies #1005

Merged
merged 3 commits into from
Mar 22, 2018
Merged

Fix gmake dependencies #1005

merged 3 commits into from
Mar 22, 2018

Conversation

TurkeyMan
Copy link
Contributor

@TurkeyMan TurkeyMan commented Feb 16, 2018

Attempt to fix #981

Note, this patch doesn't implement any logic to prevent execution of the prebuild step if the project is not dirty... it will ALWAYS run prebuild, just like gmake output always has, but the deps should now be structured that parallel builds work.

@tvandijck
Copy link
Contributor

Make sure you discuss this change with @bwhittle (at World of Warcraft).
He did most of the PCH related fixes earlier last year.

Other then the test failures, the change looks fine to me.

@TurkeyMan
Copy link
Contributor Author

This has been tested now on a large project.

@TurkeyMan
Copy link
Contributor Author

If someone wants to bless, this makes parallel builds work with make.

Copy link
Member

@starkos starkos left a comment

Choose a reason for hiding this comment

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

@TurkeyMan, do you not have the ability to approve? Thought you did…I'll approve now.

Copy link
Member

@starkos starkos left a comment

Choose a reason for hiding this comment

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

Actually, hold on…the branch is out of date. I'm going to merge now, and if the tests still pass (and you don't see any issues) we can approve.

Copy link
Member

@starkos starkos left a comment

Choose a reason for hiding this comment

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

Approving…thanks!

@starkos starkos merged commit 3e20452 into master Mar 22, 2018
@TurkeyMan
Copy link
Contributor Author

Sure, I can approve til the cows come home (I created this github account!) :P
But I totally wanted someone else to give this one a good once-over, since it feels pretty dangerous to me.

@TurkeyMan TurkeyMan deleted the fix_deps branch April 26, 2018 04:32
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.

Makefile target dependencies aren't correct
3 participants