Page MenuHomeWildfire Games

Fix non-PCH build after recent include changes
ClosedPublic

Authored by Itms on Jan 29 2018, 3:05 PM.

Details

Summary

I am not sure when it happened (did not bisect), but the CStr.h inclusion disappeared in JSInterface_Mod.h. This didn't break PCH builds because this header is precompiled in the engine project.

Test Plan

Build with update-workspaces.sh --without-pch.

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

Itms created this revision.Jan 29 2018, 3:05 PM
Vulcan added a subscriber: Vulcan.Jan 29 2018, 4:33 PM

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

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (309 tests).....................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (309 tests).....................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...
Executing section Default...
Executing section Source...
Executing section JS...
Imarok accepted this revision.Jan 29 2018, 6:05 PM
Imarok added a subscriber: Imarok.

Could reproduce the issue and confirm the fix. Looks reasonable.

According to the files' changelog, this seems to be the case since ever.

This revision is now accepted and ready to land.Jan 29 2018, 6:05 PM
elexis added a subscriber: elexis.Jan 29 2018, 6:36 PM

the CStr.h inclusion disappeared in JSInterface_Mod.h

Are you sure the CStr include was present in the headers at any time? Since there is a CStr but no CStr include in the header of this file when it was introduced in rP15677.

Itms added a comment.Jan 29 2018, 9:35 PM
In D1273#51507, @Imarok wrote:

Could reproduce the issue and confirm the fix. Looks reasonable.

Thanks for testing! I will commit tomorrow.

In D1273#51507, @Imarok wrote:

According to the files' changelog, this seems to be the case since ever.

In D1273#51511, @elexis wrote:

Are you sure the CStr include was present in the headers at any time? Since there is a CStr but no CStr include in the header of this file when it was introduced in rP15677.

Yes sorry I was unclear: I'm not saying the include used to be in that file, but it was included somewhere in the chain of includes starting from scriptinterface/ScriptInterface.h until recently. Indeed, I successfully built without PCH when I fixed the premake5 scripts a big couple of weeks ago.

This revision was automatically updated to reflect the committed changes.