mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joseph Wu <jos...@mesosphere.io>
Subject Re: Review Request 69140: Allowed for unbundled libarchive on cmake builds.
Date Wed, 24 Oct 2018 18:21:30 GMT

-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message