Page MenuHomePhabricator

Update xcode project with new header search paths
AbandonedPublic

Authored by jon on Jul 5 2022, 12:16 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Nov 5, 2:41 PM
Unknown Object (File)
Tue, Nov 5, 2:41 PM
Unknown Object (File)
Tue, Nov 5, 2:41 PM
Unknown Object (File)
Tue, Nov 5, 2:41 PM
Unknown Object (File)
Tue, Nov 5, 2:41 PM
Unknown Object (File)
Tue, Nov 5, 2:41 PM
Unknown Object (File)
Tue, Nov 5, 2:41 PM
Unknown Object (File)
Tue, Nov 5, 2:41 PM

Details

Reviewers
atul
marcin
ashoat
Summary

Should fix iOS build, as headers
will be able to be found

Depends on D4448

Test Plan

Build xcode project

Diff Detail

Repository
rCOMM Comm
Branch
joringer/andriod (branched from master)
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Jul 5 2022, 12:26 PM
Harbormaster failed remote builds in B10306: Diff 14206!
Harbormaster returned this revision to the author for changes because remote builds failed.Jul 5 2022, 1:28 PM
Harbormaster failed remote builds in B10309: Diff 14213!

Looks like the issue for the iOS build was:

fatal error: 'CryptoTools/CryptoModule.h' file not found
#include <CryptoTools/CryptoModule.h>

Yea, a bit odd, since "$(SRCROOT)/../cpp/CommonCpp", should resolve to native/cpp/CommonCpp, which should be the correct path

Harbormaster returned this revision to the author for changes because remote builds failed.Jul 6 2022, 10:56 AM
Harbormaster failed remote builds in B10324: Diff 14231!

Attempt to use PROJECT_DIR instead

Harbormaster returned this revision to the author for changes because remote builds failed.Jul 6 2022, 11:13 AM
Harbormaster failed remote builds in B10325: Diff 14232!

Figured out why this is failing, The Compiled Sources don't inherit the HEADER_SEARCH_PATH, that only gets used by files that are directly included into the project. I have to manually had the -I../cpp/CommonCpp path to each of them

Harbormaster returned this revision to the author for changes because remote builds failed.Jul 7 2022, 12:51 PM
Harbormaster failed remote builds in B10381: Diff 14291!

Fix Notification Services target as well

Harbormaster returned this revision to the author for changes because remote builds failed.Jul 7 2022, 2:58 PM
Harbormaster failed remote builds in B10384: Diff 14294!
Harbormaster returned this revision to the author for changes because remote builds failed.Jul 7 2022, 3:17 PM
Harbormaster failed remote builds in B10386: Diff 14296!

Inherit more android fixes :)

For the xcode project, I probably over-did it on header search paths, but didn't want to painfully determine which ones actually needed it, since I don't have a local environment yet

ashoat added a reviewer: atul.
ashoat added a reviewer: marcin.

Resigning since I'm usually not a good person to set as a first-pass reviewer. Exceptions here

Looks good since everything builds. IIRC in the past opening up an Xcode project/workspace in the Xcode GUI could sometimes "throw away" changes made "manually" to .pbxproj files.

I'll try patching this in locally, building in Xcode GUI, and seeing what happens

I made all the changes through the GUI, just to make sure I didn't invalidate something.

In D4452#127734, @jonringer-comm wrote:

I made all the changes through the GUI, just to make sure I didn't invalidate something.

Ah gotcha, thanks for clarifying. Wasn't sure because of

Unfortunately until I get setup with a Apple developer account, I can't really make use of XCode directly yet

in another diff.

atul requested changes to this revision.EditedJul 8 2022, 12:10 PM

ah dang, while the Comm target builds successfully... the NotificationService target fails

going to quickly add NotificationService target to the CI, sorry about that


Getting the following:

/Users/atul/comm/native/cpp/CommonCpp/NativeModules/InternalModules/NetworkModule.cpp error build: Build input file cannot be found: '/Users/atul/comm/native/cpp/CommonCpp/NativeModules/InternalModules/NetworkModule.cpp'
This revision now requires changes to proceed.Jul 8 2022, 12:10 PM

Update imported cpp file paths

NotificationServices should build now, in the process of getting my apple account in order

I'm running into the following:

acf9b1.png (522×2 px, 217 KB)

atul requested changes to this revision.Jul 14 2022, 9:52 AM
This revision now requires changes to proceed.Jul 14 2022, 9:52 AM
marcin requested changes to this revision.Jul 15 2022, 1:19 AM

I patched this differential, run yarn cleaninstall, attempted to build app for Physical device and I experienced the same error as @atul . However I quickly found a solution to this problem. I needed to change two lines:

  1. <olm/session.hh> to "olm/session.hh" in CryptoModule.cpp
  2. <sqlite_orm.h> to "sqlite_orm.h" in SQLiteQueryExecutor.cpp.

We either need to introduce those changes or (more probably) find a reason why using <> instead of "" for includes fails in new settings. I am not sure if it is helpful but sqlite_orm.h is a 3-rd party library but it is included in our repo as a regular file.

Need to update this, as it now has conflicts with master. And solving the header structure has been de-prioritized