Paths

Table of Contentst

Differential D1380

Build: precompiled logic update
ClosedPublic
Actions

Authored by Silier on Mar 11 2018, 10:53 PM.

Details

Reviewers

Itms

Commits

rP23314: Precompiled logic update [Windows build]

Summary

Nuke HAVE_PCH and use only one macro for PCH logic.
This macro is enabled (=1) or disabled (=0) based on --without-pch and no_pch parameters in premake5
Visual studio no more overrides this logic

Test Plan

check that game builds with without-pch flag

Windows builds

Diff Detail

Repository

rP 0 A.D. Public Repository

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Silier created this revision.Mar 11 2018, 10:53 PM

Owners added a subscriber: Restricted Owners Package.Mar 11 2018, 10:53 PM

Silier retitled this revision from Precompiled logic disable update to Precompiled logic disable PCH.Mar 11 2018, 11:17 PM

Stan added a subscriber: Stan.Mar 11 2018, 11:21 PM

Stan added inline comments.

build/premake/premake5.lua
474 ↗	(On Diff #6149)	Add back the newline :)

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/233/display/redirect

Harbormaster completed remote builds in B5501: Diff 6149.Mar 12 2018, 6:44 AM

Silier retitled this revision from Precompiled logic disable PCH to Build: precompiled logic update.Mar 12 2018, 7:12 AM

Oof this one is old. Many sorries for the awful queue ?

At first sight this adds a confusing option. I would have expected the patch to define CONFIG_ENABLE_PCH to 0 when --without-pch is passed to premake, no?

Silier updated this revision to Diff 7733.Apr 12 2019, 8:26 PM

Silier edited the summary of this revision. (Show Details)

Silier edited the test plan for this revision. (Show Details)

Silier edited the summary of this revision. (Show Details)Apr 12 2019, 8:32 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/1202/display/redirect

Harbormaster completed remote builds in B7168: Diff 7733.Apr 12 2019, 8:51 PM

I think this is a good first step, but CONFIG_ENABLE_PCH is kind of a dumb variable. Ideally, either precompiled headers are enabled and the new include the whole content of them, or they aren't and we include nothing.

Right now our precompiled headers also act as global include files, when they probably shouldn't.

Thanks for the patch and I owe you a beer or something for the delay ? The changes look perfect. Let's get them in before the copyright year becomes wrong...

Can you rebase and reupload before committing? That way it will be tested under Windows and macOS ? And I can build without pch on Linux too.

This revision is now accepted and ready to land.Dec 30 2019, 5:56 PM

In D1380#79218, @wraitii wrote:

Right now our precompiled headers also act as global include files, when they probably shouldn't.

I think our use of the precompiled headers is clever in that sense, we use them for two purposes. I agree it might be confusing and I don't know if this is a good practice, but I like it.

rebase

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/11/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/916/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1434/display/redirect

Harbormaster completed remote builds in B10579: Diff 10831.Dec 31 2019, 1:07 PM

Also builds and runs --without-pch on Linux (gcc 7). ?

Closed by commit rP23314: Precompiled logic update [Windows build] (authored by Silier). · Explain WhyDec 31 2019, 2:59 PM

This revision was automatically updated to reflect the committed changes.

Silier added a commit: rP23314: Precompiled logic update [Windows build].