Page MenuHomeWildfire Games

Build without PCH - windows fix
ClosedPublic

Authored by Angen on Mar 7 2018, 8:30 PM.

Details

Reviewers
vladislavbelov
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP21964: Fix build without PCH for some targets.
Summary

Currently build with --without-pch and/or with HAVE_PCH 0 does not build on windows because of missing includes

Test Plan

test that it builds with --without-pch falg and on windows also set HAVE_PCH 0 in \source\lib\sysdep\compiler.h

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

Angen created this revision.Mar 7 2018, 8:30 PM
Owners added subscribers: Restricted Owners Package, Restricted Owners Package.Mar 7 2018, 8:30 PM
elexis awarded a token.Mar 7 2018, 8:31 PM
Stan awarded a token.Mar 7 2018, 8:36 PM
Angen added a parent revision: D1333: Precompiled update.
Vulcan added a subscriber: Vulcan.Mar 7 2018, 11:40 PM

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

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

elexis added a subscriber: elexis.Mar 8 2018, 1:40 PM
elexis added inline comments.
source/graphics/ShaderDefines.h
25 ↗(On Diff #6057)

last change to this file was rP15481 2014-07-03, I thought build without PCH was working at least last year sometime? Itms, leper and Imarok had compiled with that too afaik. Maybe it's only on windows.
But the patch is probably correct and complete for this windows use case if it compiles for you.

Angen added inline comments.Mar 8 2018, 2:13 PM
source/graphics/ShaderDefines.h
25 ↗(On Diff #6057)

to this file maybe, but it is possible that precompiled politic changed, because when HAVE PCH is 0, the file with <map> and other includes is whole ignored and not included

Stan added a subscriber: Stan.Mar 8 2018, 2:29 PM
Stan added inline comments.
source/graphics/ShaderDefines.h
25 ↗(On Diff #6057)

Yeah I guess it did compile with a peculiar bug, or maybe they didn't have it by then so it compiled fine :)

Angen added inline comments.Mar 8 2018, 2:46 PM
source/graphics/ShaderDefines.h
25 ↗(On Diff #6057)

next thing about date when it broke is that h files are not compiled directly but included to the cpp and they are compiled so since they had missing includes or used pch it worked. But this h file requiers map to work correctly so I gave it here instead before every include of this h in cpp

Angen updated this revision to Diff 6091.Mar 9 2018, 5:51 PM

years and one unimportant include delete from prev version

Vulcan added a comment.Mar 9 2018, 6:20 PM

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

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

Stan added a reviewer: Restricted Owners Package.Mar 10 2018, 10:31 AM
Angen updated this revision to Diff 6568.May 18 2018, 5:06 PM
Angen retitled this revision from Build without PCH to Build without PCH - windows fix.
Angen edited the summary of this revision. (Show Details)
Angen edited the test plan for this revision. (Show Details)

update wnuma.cpp because year in file have changed

and could we have this pushed pls? Another patch is waiting for this.

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

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

Stan added a comment.EditedDec 28 2018, 3:23 PM

@Angen does the test project build for you without pch ?

When setting (→ is for replacing)

In lib/config.h

# define CONFIG_ENABLE_PCH 1 → 0 // improve build performance

and in lib/sysdep/compiler.h

// are PreCompiled Headers supported?
#if MSC_VERSION
# define HAVE_PCH 1 → 0
#elif defined(USING_PCH)
# define HAVE_PCH 1 → 0
#else
# define HAVE_PCH 0
#endif

I get four C2317 errors about missing try catch in the test project

vladislavbelov accepted this revision.Dec 28 2018, 3:25 PM

The patch looks good for me. I'll fix notes in the commit.

source/scriptinterface/ScriptInterface.h
1 ↗(On Diff #6568)

Bump year.

22 ↗(On Diff #6568)

If the patch rearranges something, then do the same thing for other touched files.

This revision is now accepted and ready to land.Dec 28 2018, 3:25 PM
This revision was automatically updated to reflect the committed changes.

@Angen Thanks for the patch!