Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12054 )
Change subject: [build] Fix bulding codegen on MacOS Mojave ...................................................................... Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/12054/3/src/kudu/codegen/CMakeLists.txt File src/kudu/codegen/CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/12054/3/src/kudu/codegen/CMakeLists.txt@73 PS3, Line 73: Once this is working we'll need a big block comment here explaining what's going on. http://gerrit.cloudera.org:8080/#/c/12054/3/src/kudu/codegen/CMakeLists.txt@74 PS3, Line 74: if (NOT EXISTS /usr/include) Isn't the below correct to do regardless of whether /usr/include exists? http://gerrit.cloudera.org:8080/#/c/12054/3/src/kudu/codegen/CMakeLists.txt@82 PS3, Line 82: --sysroot="${SDKPATH}") I guess this is fine if it works, but isn't -I sufficient? I'm reading through https://clang.llvm.org/docs/CrossCompilation.html. http://gerrit.cloudera.org:8080/#/c/12054/3/src/kudu/codegen/CMakeLists.txt@95 PS3, Line 95: set(IR_FLAGS If you go down the --sysroot path, I think I'd prefer adding it directly to IR_FLAGS rather than to PREFIXED_IR_INCLUDES, because strictly speaking it's not an "include"; it's an additional configuration for clang. -- To view, visit http://gerrit.cloudera.org:8080/12054 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I33f7510c7bf0cc2a7708191653b23c04e6df8a29 Gerrit-Change-Number: 12054 Gerrit-PatchSet: 3 Gerrit-Owner: Attila Bukor <abu...@apache.org> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Grant Henke <granthe...@apache.org> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com> Gerrit-Comment-Date: Fri, 07 Dec 2018 18:18:46 +0000 Gerrit-HasComments: Yes
