mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew Schwartzmeyer <and...@schwartzmeyer.com>
Subject Re: Review Request 62105: Enabled building of the Cyrus SASL CRAM MD5 lib on Windows.
Date Wed, 06 Sep 2017 20:48:37 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62105/#review184728
-----------------------------------------------------------




3rdparty/CMakeLists.txt
Lines 194 (patched)
<https://reviews.apache.org/r/62105/#comment260920>

    Unless `cyrus_sasl` can build as both shared and static on Windows, we should code this
specifically to `STATIC` instead of `${LIBRARY_LINKAGE}`.



3rdparty/CMakeLists.txt
Lines 197-198 (patched)
<https://reviews.apache.org/r/62105/#comment260921>

    I think Joe ended up committing slightly different formatting conventions here, with another
line break, e.g.:
    
    ```
    set_target_properties(
        cyrus_sasl PROPERTIES
        ...
        )
    ```



3rdparty/CMakeLists.txt
Lines 208-209 (patched)
<https://reviews.apache.org/r/62105/#comment260922>

    These two lines should be removed as `ExternalProject_Add` will default to "doing the
right thing" and building with CMake (and we shouldn't ever hard code the generator here).
    
    Also I don't mean "set to `${CMAKE_NOOP}` but instead not set at all (lines deleted).



3rdparty/cmake/Versions.cmake
Lines 7-8 (patched)
<https://reviews.apache.org/r/62105/#comment260924>

    This is a question for Joe: should we place these two lines under the `else ()` below,
since this is only bundled on Windows? Otherwise it may confusingly indicate that all builds
(including Linux) use this version, but on Linux we actually use the system library.



3rdparty/cyrus_sasl-2.1.27rc3.patch
Lines 1 (patched)
<https://reviews.apache.org/r/62105/#comment260923>

    Since it's a patch, I won't review it too much. However, we should ensure we submit this
change upstream to libsasl2, and at that point clean it up a bit.


- Andrew Schwartzmeyer


On Sept. 6, 2017, 1:35 p.m., John Kordich wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62105/
> -----------------------------------------------------------
> 
> (Updated Sept. 6, 2017, 1:35 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Jeff Coffler, and Joseph Wu.
> 
> 
> Bugs: MESOS-3384
>     https://issues.apache.org/jira/browse/MESOS-3384
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Enabled building of the Cyrus SASL CRAM MD5 lib on Windows.
> 
> 
> Diffs
> -----
> 
>   3rdparty/CMakeLists.txt 2a183a4a46bf8bc97455fe02648407ba561b38e7 
>   3rdparty/cmake/Versions.cmake 0b62ae96817ade616f8dfc52cf8df995f8fd8927 
>   3rdparty/cyrus_sasl-2.1.27rc3.patch PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/62105/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> John Kordich
> 
>


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