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

msbuild: fix overbuilding of externals and lessen rebuild likelyhood #10933

Merged
merged 2 commits into from Aug 22, 2022

Conversation

shuffle2
Copy link
Contributor

This partially reverts #9092 (which created Externals\ExternalsReferenceAll.props)
Instead, Externals now "publish" exports.props files that referencing projects can import in order to 1) reference the project (for dependency graph) and 2) include any ClCompile settings (+etc) exported by the external (used for include directories and definitions).

This also splits some of Base.props out into a seperate file, such that Base.props is now only for truly global settings, whereas the new file is for settings shared between dolphin-authored code (but not Externals).

While working on PRs recently, I've noticed that msbuild would sometimes overbuild after making small edits. This PR should fix that (please let me know if you ever see such a thing). It also makes rebuilds of Externals less likely, as Base.props does not need to be edited as frequently. The new behavior is also more correct in the sense that include paths and definitions for all External libraries are not broadcast into everything (fun fact: this was extremely vestigial dolphin behavior, based on how SCons was setup, when dolphin used that).

For full rebuilds, there is a slight perf regression for machines with a lot of cpu cores (like, > 32). However, with some tips from https://devblogs.microsoft.com/cppblog/cpp-build-throughput-investigation-and-tune-up speed can be regained if needed. I also realized most people building dolphin don't have such machines, and would rather use less resources by default (oops..)

@Rumi-Larry
Copy link

Is there a reason some GUIDs are lowercase and others lowercase?

* moves dolphin-specific settings out of Base.props
* creates exports.props for externals, allowing to easily import
  individual Externals
* corrects some cruft that accumulated and probably contributed
  to msbuild overbuilding
@shuffle2
Copy link
Contributor Author

I think this PR is ready to merge

@shuffle2
Copy link
Contributor Author

Can someone merge this or indicate why it’s not being merged

@AdmiralCurtiss
Copy link
Contributor

I'll just believe you that this is correct.

@AdmiralCurtiss AdmiralCurtiss merged commit 8b7e6f1 into dolphin-emu:master Aug 22, 2022
11 checks passed
@shuffle2 shuffle2 deleted the vs branch October 21, 2022 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants