You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Chun-Hung Hsiao <ch...@mesosphere.io> on 2017/08/04 18:55:51 UTC

Review Request 61433: Checked openssl and zlib as required libraries.

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

Review request for mesos, Benjamin Bannier and Jie Yu.


Bugs: MESOS-7809
    https://issues.apache.org/jira/browse/MESOS-7809


Repository: mesos


Description
-------

This patch is for grpc support in Mesos. Since grpc depends on protobuf,
ssl and zlib, we make ssl and zlib required packages for Mesos to build.
Despite of the requirement changes, The the `--enable-ssl` and
`--enable-zlib` flags are still kept, since these flags control whether
we want to compress and encrypt the communications.

We also pass the ssl lib path as an `-rpath` option to the linker so the
`AC_RUN_IFELSE` tests won't fail.


Diffs
-----

  configure.ac b2eeedab65d70718451cb8bbe556794b6d8e8db6 


Diff: https://reviews.apache.org/r/61433/diff/1/


Testing
-------

make


Thanks,

Chun-Hung Hsiao

Re: Review Request 61433: Refactored OpenSSL library checks in Mesos.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61433/#review182517
-----------------------------------------------------------


Fix it, then Ship it!





configure.ac
Lines 1967-1968 (patched)
<https://reviews.apache.org/r/61433/#comment258427>

    We are not working around linker bugs, but our autotools setup where we globally mess with `LIBS` not taking into account e.g., setting up library lookup paths.
    
    How about just
    
        # NOTE: We clear `LIBS` here to prevent linking in libraries unrelated to
        # the test. These libraries might not be in the linker lookup paths.


- Benjamin Bannier


On Aug. 9, 2017, 5 a.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61433/
> -----------------------------------------------------------
> 
> (Updated Aug. 9, 2017, 5 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Till Toenshoff.
> 
> 
> Bugs: MESOS-7870
>     https://issues.apache.org/jira/browse/MESOS-7870
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Since gRPC requires OpenSSL, we checks if libssl and libcrypto exist no
> matter if the `--enable-ssl` flag is on. This enables us to have a
> non-SSL-enabled Mesos build with gRPC support. Also fixed errors that
> a bad linker might link the configure tests with libssl unnecessarily,
> which would cause runtime failures if libssl is not in the runtime
> library search path.
> 
> 
> Diffs
> -----
> 
>   configure.ac 5a6e42c61e6752c3742532502dbc753071b31323 
> 
> 
> Diff: https://reviews.apache.org/r/61433/diff/3/
> 
> 
> Testing
> -------
> 
> Ran `make check` on the following two configurations:
> 1. ../configure
> 2. ../configure --enable-ssl --enable-libevent
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 61433: Refactored OpenSSL library checks in Mesos.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61433/#review182638
-----------------------------------------------------------


Ship it!




Ship It!

- Jie Yu


On Aug. 10, 2017, 9:58 p.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61433/
> -----------------------------------------------------------
> 
> (Updated Aug. 10, 2017, 9:58 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Till Toenshoff.
> 
> 
> Bugs: MESOS-7870
>     https://issues.apache.org/jira/browse/MESOS-7870
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Since gRPC requires OpenSSL, we checks if libssl and libcrypto exist no
> matter if the `--enable-ssl` flag is on. This enables us to have a
> non-SSL-enabled Mesos build with gRPC support. Also fixed errors that
> a bad linker might link the configure tests with libssl unnecessarily,
> which would cause runtime failures if libssl is not in the runtime
> library search path.
> 
> 
> Diffs
> -----
> 
>   configure.ac 307f0aea7f19932befba37c5467851718d317c92 
> 
> 
> Diff: https://reviews.apache.org/r/61433/diff/6/
> 
> 
> Testing
> -------
> 
> Ran `make check` on the following two configurations:
> 1. ../configure
> 2. ../configure --enable-ssl --enable-libevent
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 61433: Refactored OpenSSL library checks in Mesos.

Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61433/
-----------------------------------------------------------

(Updated Aug. 10, 2017, 9:58 p.m.)


Review request for mesos, Benjamin Bannier, Jie Yu, and Till Toenshoff.


Changes
-------

Rebased.


Bugs: MESOS-7870
    https://issues.apache.org/jira/browse/MESOS-7870


Repository: mesos


Description
-------

Since gRPC requires OpenSSL, we checks if libssl and libcrypto exist no
matter if the `--enable-ssl` flag is on. This enables us to have a
non-SSL-enabled Mesos build with gRPC support. Also fixed errors that
a bad linker might link the configure tests with libssl unnecessarily,
which would cause runtime failures if libssl is not in the runtime
library search path.


Diffs (updated)
-----

  configure.ac 307f0aea7f19932befba37c5467851718d317c92 


Diff: https://reviews.apache.org/r/61433/diff/6/

Changes: https://reviews.apache.org/r/61433/diff/5-6/


Testing
-------

Ran `make check` on the following two configurations:
1. ../configure
2. ../configure --enable-ssl --enable-libevent


Thanks,

Chun-Hung Hsiao

Re: Review Request 61433: Refactored OpenSSL library checks in Mesos.

Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61433/
-----------------------------------------------------------

(Updated Aug. 9, 2017, 11:47 p.m.)


Review request for mesos, Benjamin Bannier, Jie Yu, and Till Toenshoff.


Changes
-------

Addressed @tillt's comments.


Bugs: MESOS-7870
    https://issues.apache.org/jira/browse/MESOS-7870


Repository: mesos


Description
-------

Since gRPC requires OpenSSL, we checks if libssl and libcrypto exist no
matter if the `--enable-ssl` flag is on. This enables us to have a
non-SSL-enabled Mesos build with gRPC support. Also fixed errors that
a bad linker might link the configure tests with libssl unnecessarily,
which would cause runtime failures if libssl is not in the runtime
library search path.


Diffs (updated)
-----

  configure.ac 307f0aea7f19932befba37c5467851718d317c92 


Diff: https://reviews.apache.org/r/61433/diff/5/

Changes: https://reviews.apache.org/r/61433/diff/4-5/


Testing
-------

Ran `make check` on the following two configurations:
1. ../configure
2. ../configure --enable-ssl --enable-libevent


Thanks,

Chun-Hung Hsiao

Re: Review Request 61433: Refactored OpenSSL library checks in Mesos.

Posted by Till Toenshoff <to...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61433/#review182537
-----------------------------------------------------------


Ship it!





configure.ac
Lines 1727-1729 (patched)
<https://reviews.apache.org/r/61433/#comment258442>

    Your formatting is well readable but not entirely consistent with the other checks.
    
    ```
    AC_CHECK_HEADERS([openssl/ssl.h],
                     [AC_CHECK_LIB([ssl],
                                   [SSL_CTX_new],
                                   [found_ssl=yes],
                                   [],
                                   [-lcrypto])])
    ```
    
    I am a fan of consistent formatting but leaving it up to you to decide :) - hence not an "issue".


- Till Toenshoff


On Aug. 9, 2017, 10:46 p.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61433/
> -----------------------------------------------------------
> 
> (Updated Aug. 9, 2017, 10:46 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Till Toenshoff.
> 
> 
> Bugs: MESOS-7870
>     https://issues.apache.org/jira/browse/MESOS-7870
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Since gRPC requires OpenSSL, we checks if libssl and libcrypto exist no
> matter if the `--enable-ssl` flag is on. This enables us to have a
> non-SSL-enabled Mesos build with gRPC support. Also fixed errors that
> a bad linker might link the configure tests with libssl unnecessarily,
> which would cause runtime failures if libssl is not in the runtime
> library search path.
> 
> 
> Diffs
> -----
> 
>   configure.ac 5a6e42c61e6752c3742532502dbc753071b31323 
> 
> 
> Diff: https://reviews.apache.org/r/61433/diff/4/
> 
> 
> Testing
> -------
> 
> Ran `make check` on the following two configurations:
> 1. ../configure
> 2. ../configure --enable-ssl --enable-libevent
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 61433: Refactored OpenSSL library checks in Mesos.

Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61433/
-----------------------------------------------------------

(Updated Aug. 9, 2017, 10:46 p.m.)


Review request for mesos, Benjamin Bannier, Jie Yu, and Till Toenshoff.


Changes
-------

Addressed @bbannier's comments.


Bugs: MESOS-7870
    https://issues.apache.org/jira/browse/MESOS-7870


Repository: mesos


Description
-------

Since gRPC requires OpenSSL, we checks if libssl and libcrypto exist no
matter if the `--enable-ssl` flag is on. This enables us to have a
non-SSL-enabled Mesos build with gRPC support. Also fixed errors that
a bad linker might link the configure tests with libssl unnecessarily,
which would cause runtime failures if libssl is not in the runtime
library search path.


Diffs (updated)
-----

  configure.ac 5a6e42c61e6752c3742532502dbc753071b31323 


Diff: https://reviews.apache.org/r/61433/diff/4/

Changes: https://reviews.apache.org/r/61433/diff/3-4/


Testing
-------

Ran `make check` on the following two configurations:
1. ../configure
2. ../configure --enable-ssl --enable-libevent


Thanks,

Chun-Hung Hsiao

Re: Review Request 61433: Refactored OpenSSL library checks in Mesos.

Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61433/
-----------------------------------------------------------

(Updated Aug. 9, 2017, 3 a.m.)


Review request for mesos, Benjamin Bannier, Jie Yu, and Till Toenshoff.


Changes
-------

Made OpenSSL optional.


Summary (updated)
-----------------

Refactored OpenSSL library checks in Mesos.


Bugs: MESOS-7870
    https://issues.apache.org/jira/browse/MESOS-7870


Repository: mesos


Description (updated)
-------

Since gRPC requires OpenSSL, we checks if libssl and libcrypto exist no
matter if the `--enable-ssl` flag is on. This enables us to have a
non-SSL-enabled Mesos build with gRPC support. Also fixed errors that
a bad linker might link the configure tests with libssl unnecessarily,
which would cause runtime failures if libssl is not in the runtime
library search path.


Diffs (updated)
-----

  configure.ac 5a6e42c61e6752c3742532502dbc753071b31323 


Diff: https://reviews.apache.org/r/61433/diff/3/

Changes: https://reviews.apache.org/r/61433/diff/2-3/


Testing (updated)
-------

Ran `make check` on the following two configurations:
1. ../configure
2. ../configure --enable-ssl --enable-libevent


Thanks,

Chun-Hung Hsiao

Re: Review Request 61433: Checked openssl and zlib as required libraries for Mesos.

Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61433/
-----------------------------------------------------------

(Updated Aug. 8, 2017, 12:44 a.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Summary (updated)
-----------------

Checked openssl and zlib as required libraries for Mesos.


Bugs: MESOS-7809
    https://issues.apache.org/jira/browse/MESOS-7809


Repository: mesos


Description (updated)
-------

This patch is for grpc support in Mesos. Since grpc depends on protobuf,
ssl and zlib, we make ssl and zlib required packages for Mesos to build.
Despite of the requirement changes, the `--enable-ssl` and
`--enable-zlib` flags are still kept, since these flags control whether
we want to compress and encrypt the communications.


Diffs
-----

  configure.ac b2eeedab65d70718451cb8bbe556794b6d8e8db6 


Diff: https://reviews.apache.org/r/61433/diff/2/


Testing
-------

make


Thanks,

Chun-Hung Hsiao

Re: Review Request 61433: Checked openssl and zlib as required libraries.

Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61433/
-----------------------------------------------------------

(Updated Aug. 8, 2017, 12:37 a.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
-------

Avoided setting `-rpath`.


Bugs: MESOS-7809
    https://issues.apache.org/jira/browse/MESOS-7809


Repository: mesos


Description (updated)
-------

This patch is for grpc support in Mesos. Since grpc depends on protobuf,
ssl and zlib, we make ssl and zlib required packages for Mesos to build.
Despite of the requirement changes, The the `--enable-ssl` and
`--enable-zlib` flags are still kept, since these flags control whether
we want to compress and encrypt the communications.


Diffs (updated)
-----

  configure.ac b2eeedab65d70718451cb8bbe556794b6d8e8db6 


Diff: https://reviews.apache.org/r/61433/diff/2/

Changes: https://reviews.apache.org/r/61433/diff/1-2/


Testing
-------

make


Thanks,

Chun-Hung Hsiao