You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@mesos.apache.org by Andrew Schwartzmeyer <an...@schwartzmeyer.com> on 2017/09/22 18:34:42 UTC

Review Request 62511: Fixed OpenSSL support in libprocess for Windows.

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

Review request for mesos, John Kordich, Joseph Wu, and Till Toenshoff.

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

Repository: mesos

Description
-------

This patch adds support for building with OpenSSL on Windows. This
requires an installation of OpenSSL, such as 1.0.2L from Shining Light
Productions. Mesos must be configured like:

-DENABLE_LIBEVENT=ON -DENABLE_SSL=ON
-DOPENSSL_ROOT_DIR="C:\OpenSSL-Win64"

The use of `_POSIX_HOST_NAME_MAX` was replaced with `MAXHOSTNAMELEN`,
consistent with the rest of libprocess.

Windows does not define `in_addr_t`, but the type as declared by our
cURL dependency for Windows is `unsigned long`.

OpenSSL on Windows requires the adapater module `openssl/applink.c` to
be compiled as part of the consuming project to deal with Windows
runtime library differences. Not doing so manifests itself as the "no
OPENSSL_Applink" runtime error. The OpenSSL FAQ recommends simply
`#include`-ing it in one of the project's source files, e.g.
libprocess's `ssl/utilities.cpp`.

Diffs
-----

3rdparty/libprocess/include/process/ssl/utilities.hpp 797353ce36e805d72959b3f394d3e1d11e2cd89b
3rdparty/libprocess/src/openssl.hpp e74db5f52b8a7d944317de15eadde277aee85e06
3rdparty/libprocess/src/openssl.cpp d16cc1d092518a5adbde840683168f3c79e5f07b
3rdparty/libprocess/src/ssl/utilities.cpp d752acb8af734f8f918dba6ba4fccc2802c81ec5

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

Testing
-------

Thanks,

Andrew Schwartzmeyer

Re: Review Request 62511: Fixed OpenSSL support in libprocess for Windows.

Posted by Joseph Wu <jo...@mesosphere.io>.

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


Ship it!




LGTM.


3rdparty/libprocess/include/process/ssl/utilities.hpp
Lines 19-20 (patched)
<https://reviews.apache.org/r/62511/#comment264756>

    I'm going to append a `NOTE:` to these comments.


- Joseph Wu


On Oct. 2, 2017, 4:48 p.m., Andrew Schwartzmeyer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62511/
> -----------------------------------------------------------
> 
> (Updated Oct. 2, 2017, 4:48 p.m.)
> 
> 
> Review request for mesos, John Kordich, Joseph Wu, and Till Toenshoff.
> 
> 
> Bugs: MESOS-7992
>     https://issues.apache.org/jira/browse/MESOS-7992
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch adds support for building with OpenSSL on Windows. This
> requires an installation of OpenSSL, such as 1.0.2L from Shining Light
> Productions. Mesos must be configured like:
> 
>     -DENABLE_LIBEVENT=ON -DENABLE_SSL=ON
>     -DOPENSSL_ROOT_DIR="C:\OpenSSL-Win64"
> 
> The use of `_POSIX_HOST_NAME_MAX` was replaced with `MAXHOSTNAMELEN`,
> consistent with the rest of libprocess.
> 
> Windows does not define `in_addr_t`, but the type as declared by our
> cURL dependency for Windows is `unsigned long`.
> 
> OpenSSL on Windows requires the adapater module `openssl/applink.c` to
> be compiled as part of the consuming project to deal with Windows
> runtime library differences. Not doing so manifests itself as the "no
> OPENSSL_Applink" runtime error. The OpenSSL FAQ recommends simply
> `#include`-ing it in one of the project's source files, e.g.
> libprocess's `ssl/utilities.cpp`.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/ssl/utilities.hpp 797353ce36e805d72959b3f394d3e1d11e2cd89b 
>   3rdparty/libprocess/src/openssl.hpp e74db5f52b8a7d944317de15eadde277aee85e06 
>   3rdparty/libprocess/src/openssl.cpp d16cc1d092518a5adbde840683168f3c79e5f07b 
>   3rdparty/libprocess/src/ssl/utilities.cpp d752acb8af734f8f918dba6ba4fccc2802c81ec5 
> 
> 
> Diff: https://reviews.apache.org/r/62511/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 62511: Fixed OpenSSL support in libprocess for Windows.

Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.

(Updated Oct. 2, 2017, 4:48 p.m.)

Review request for mesos, John Kordich, Joseph Wu, and Till Toenshoff.

Changes
-------

Used localized typedef instead of pre-processor definition.

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

Repository: mesos

Description
-------

This patch adds support for building with OpenSSL on Windows. This
requires an installation of OpenSSL, such as 1.0.2L from Shining Light
Productions. Mesos must be configured like:

-DENABLE_LIBEVENT=ON -DENABLE_SSL=ON
-DOPENSSL_ROOT_DIR="C:\OpenSSL-Win64"

The use of `_POSIX_HOST_NAME_MAX` was replaced with `MAXHOSTNAMELEN`,
consistent with the rest of libprocess.

Windows does not define `in_addr_t`, but the type as declared by our
cURL dependency for Windows is `unsigned long`.

Diffs (updated)
-----

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

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

Testing
-------

Thanks,

Andrew Schwartzmeyer

Re: Review Request 62511: Fixed OpenSSL support in libprocess for Windows.

Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.


> On Oct. 2, 2017, 1:50 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/src/ssl/utilities.cpp
> > Lines 16-18 (patched)
> > <https://reviews.apache.org/r/62511/diff/1/?file=1832921#file1832921line16>
> >
> >     If possible, a `typedef` would be preferable.
> >     
> >     Is this type required by any of the openssl headers?  Or just the single occurrence in this file (line 256)?

This is the single occurence for Mesos. It does not occur in the OpenSSL headers. Only cURL also has a mention of it.


- Andrew


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


On Sept. 22, 2017, 11:34 a.m., Andrew Schwartzmeyer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62511/
> -----------------------------------------------------------
> 
> (Updated Sept. 22, 2017, 11:34 a.m.)
> 
> 
> Review request for mesos, John Kordich, Joseph Wu, and Till Toenshoff.
> 
> 
> Bugs: MESOS-7992
>     https://issues.apache.org/jira/browse/MESOS-7992
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch adds support for building with OpenSSL on Windows. This
> requires an installation of OpenSSL, such as 1.0.2L from Shining Light
> Productions. Mesos must be configured like:
> 
>     -DENABLE_LIBEVENT=ON -DENABLE_SSL=ON
>     -DOPENSSL_ROOT_DIR="C:\OpenSSL-Win64"
> 
> The use of `_POSIX_HOST_NAME_MAX` was replaced with `MAXHOSTNAMELEN`,
> consistent with the rest of libprocess.
> 
> Windows does not define `in_addr_t`, but the type as declared by our
> cURL dependency for Windows is `unsigned long`.
> 
> OpenSSL on Windows requires the adapater module `openssl/applink.c` to
> be compiled as part of the consuming project to deal with Windows
> runtime library differences. Not doing so manifests itself as the "no
> OPENSSL_Applink" runtime error. The OpenSSL FAQ recommends simply
> `#include`-ing it in one of the project's source files, e.g.
> libprocess's `ssl/utilities.cpp`.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/ssl/utilities.hpp 797353ce36e805d72959b3f394d3e1d11e2cd89b 
>   3rdparty/libprocess/src/openssl.hpp e74db5f52b8a7d944317de15eadde277aee85e06 
>   3rdparty/libprocess/src/openssl.cpp d16cc1d092518a5adbde840683168f3c79e5f07b 
>   3rdparty/libprocess/src/ssl/utilities.cpp d752acb8af734f8f918dba6ba4fccc2802c81ec5 
> 
> 
> Diff: https://reviews.apache.org/r/62511/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 62511: Fixed OpenSSL support in libprocess for Windows.

Posted by Joseph Wu <jo...@mesosphere.io>.

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


Ship it!




LGTM.


3rdparty/libprocess/src/ssl/utilities.cpp
Lines 16-18 (patched)
<https://reviews.apache.org/r/62511/#comment263764>

    If possible, a `typedef` would be preferable.
    
    Is this type required by any of the openssl headers?  Or just the single occurrence in this file (line 256)?


- Joseph Wu


On Sept. 22, 2017, 11:34 a.m., Andrew Schwartzmeyer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62511/
> -----------------------------------------------------------
> 
> (Updated Sept. 22, 2017, 11:34 a.m.)
> 
> 
> Review request for mesos, John Kordich, Joseph Wu, and Till Toenshoff.
> 
> 
> Bugs: MESOS-7992
>     https://issues.apache.org/jira/browse/MESOS-7992
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch adds support for building with OpenSSL on Windows. This
> requires an installation of OpenSSL, such as 1.0.2L from Shining Light
> Productions. Mesos must be configured like:
> 
>     -DENABLE_LIBEVENT=ON -DENABLE_SSL=ON
>     -DOPENSSL_ROOT_DIR="C:\OpenSSL-Win64"
> 
> The use of `_POSIX_HOST_NAME_MAX` was replaced with `MAXHOSTNAMELEN`,
> consistent with the rest of libprocess.
> 
> Windows does not define `in_addr_t`, but the type as declared by our
> cURL dependency for Windows is `unsigned long`.
> 
> OpenSSL on Windows requires the adapater module `openssl/applink.c` to
> be compiled as part of the consuming project to deal with Windows
> runtime library differences. Not doing so manifests itself as the "no
> OPENSSL_Applink" runtime error. The OpenSSL FAQ recommends simply
> `#include`-ing it in one of the project's source files, e.g.
> libprocess's `ssl/utilities.cpp`.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/ssl/utilities.hpp 797353ce36e805d72959b3f394d3e1d11e2cd89b 
>   3rdparty/libprocess/src/openssl.hpp e74db5f52b8a7d944317de15eadde277aee85e06 
>   3rdparty/libprocess/src/openssl.cpp d16cc1d092518a5adbde840683168f3c79e5f07b 
>   3rdparty/libprocess/src/ssl/utilities.cpp d752acb8af734f8f918dba6ba4fccc2802c81ec5 
> 
> 
> Diff: https://reviews.apache.org/r/62511/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>