----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69140/#review209976 -----------------------------------------------------------
We could probably do away with some of the double-negatives (i.e. `not UNBUNDLED` -> `BUNDLED`) in the new variable names and logic. 3rdparty/cmake/FindLIBARCHIVE.cmake Lines 21-24 (patched) <https://reviews.apache.org/r/69140/#comment294618> Prefix this with: ``` # NOTE: If this fails, stderr is ignored, and the output variable is empty. # This has no deleterious effect on our path search. ``` To note that non-OSX builds are unaffected by this command not existing. 3rdparty/cmake/FindLIBARCHIVE.cmake Lines 26-29 (patched) <https://reviews.apache.org/r/69140/#comment294619> You might get some unexpected results on OSX if you don't reset the `POSSIBLE_*` variables outside the conditional. ``` set(POSSIBLE_LIBARCHIVE_INCLUDE_DIRS "") set(POSSIBLE_LIBARCHIVE_LIB_DIRS "") if (NOT "${LIBARCHIVE_PREFIX}" STREQUAL "") list(APPEND POSSIBLE_LIBARCHIVE_INCLUDE_DIRS ${LIBARCHIVE_PREFIX}/include) list(APPEND POSSIBLE_LIBARCHIVE_LIB_DIRS ${LIBARCHIVE_PREFIX}/lib) endif() ``` cmake/CompilationConfigure.cmake Lines 82-84 (patched) <https://reviews.apache.org/r/69140/#comment294620> The `LIBARCHIVE_ROOT_DIR` variable doesn't need to be inside this conditional. The variable would have no effect when we are using a bundled library, and would still be set/read on using an installed library. - Joseph Wu On Oct. 24, 2018, 5:25 a.m., Till Toenshoff wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69140/ > ----------------------------------------------------------- > > (Updated Oct. 24, 2018, 5:25 a.m.) > > > Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, James > Peach, and Joseph Wu. > > > Repository: mesos > > > Description > ------- > > Allowed for unbundled libarchive on cmake builds. > > > Diffs > ----- > > 3rdparty/CMakeLists.txt 9584f537cc2a862ce17037224fd0628ffda54b2b > 3rdparty/cmake/FindLIBARCHIVE.cmake PRE-CREATION > cmake/CompilationConfigure.cmake 5d2be0adc55ac5302c2e0e62131feb65657466be > > > Diff: https://reviews.apache.org/r/69140/diff/1/ > > > Testing > ------- > > cmake .. -DUNBUNDLED_LIBARCHIVE=ON > cmake .. -DUNBUNDLED_LIBARCHIVE=OFF > cmake .. -DUNBUNDLED_LIBARCHIVE=ON > -DLIBARCHIVE_ROOT_DIR=/usr/local/opt/libarchive > cmake .. > > > Thanks, > > Till Toenshoff > >
