Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/17118 )
Change subject: IMPALA-10488: Add jwt-cpp 0.5.0 to the toolchain ...................................................................... Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/17118/3/source/jwt-cpp/build.sh File source/jwt-cpp/build.sh: http://gerrit.cloudera.org:8080/#/c/17118/3/source/jwt-cpp/build.sh@33 PS3, Line 33: # jwt-cpp is currently header-only, so it really is only copying files around > Should we add header only libraries to native toolchain? I think it is fairly harmless for native toolchain to have header only libraries. There are some nice properties about doing this through the native toolchain: - native toolchain builds on all the Linux versions. If the header-only projects have validations through CMake, then those validations confirm that there isn't anything incompatible with all the Linux versions we support. - The version of the library is clear. Any patches we do on top of it are kept separately. I think I prefer this to the be/src/thirdparty approach. I think there are options that may get the best of both. There are some CMake extensions that can go fetch a project as of a particular revision and build it (see https://cmake.org/cmake/help/latest/module/ExternalProject.html ). If we had something like that working in the Impala build system, I would use it. I'm going to split this off from the stack with the compression changes since I don't need it yet. -- To view, visit http://gerrit.cloudera.org:8080/17118 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I77aa3b36b45e8ef3c2d7873327948197c2c65d11 Gerrit-Change-Number: 17118 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Comment-Date: Thu, 08 Apr 2021 03:10:22 +0000 Gerrit-HasComments: Yes