You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Alex Clemmer <cl...@gmail.com> on 2016/12/07 02:27:06 UTC

Review Request 54462: Windows: Added APR include path to libprocess configuration.

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

Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.


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


Repository: mesos


Description
-------

Windows: Added APR include path to libprocess configuration.

Partially addresses MESOS-3447, as APR is a dependency of the SVN
facilities of Stout.

On Unix builds, APR is expected to have been installed on the system
prior to building Mesos (usually by a package manager). Since Windows
does not have a package manager or a reasonble way of automatically
discovering where a package is installed (aside from the registry), our
CMake build system takes it upon itself to manage these system
dependencies.  manage this dependency itself. This means that on
Windows, we need to configure the build to look for the APR headers in
our custom-downloaded APR repository.  Currently, though, we are not
doing this, so when we'll hit a compile-time error if we try to build
(e.g.) `svn.hpp`.

This commit will introduce the APR include paths as part of the build
against Stout. Since Stout is a header-only library, it is (right now)
incumbent on whoever is bundling Stout up to manage the third-party
dependencies of Stout. In our current implementation, libprocess manages
the APR dependency for Stout, so (just as we do for other dependencies
like cURL) we will have it manage this logic as well.


Diffs
-----

  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 786e47e63dc03ab4851c93ec2030f85c049cebe9 

Diff: https://reviews.apache.org/r/54462/diff/


Testing
-------


Thanks,

Alex Clemmer


Re: Review Request 54462: Windows: Added APR include path to libprocess configuration.

Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54462/#review158375
-----------------------------------------------------------



There's a stray clause in the commit message:

>   manage this dependency itself.

- Andrew Schwartzmeyer


On Dec. 7, 2016, 2:27 a.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54462/
> -----------------------------------------------------------
> 
> (Updated Dec. 7, 2016, 2:27 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.
> 
> 
> Bugs: MESOS-3447
>     https://issues.apache.org/jira/browse/MESOS-3447
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Windows: Added APR include path to libprocess configuration.
> 
> Partially addresses MESOS-3447, as APR is a dependency of the SVN
> facilities of Stout.
> 
> On Unix builds, APR is expected to have been installed on the system
> prior to building Mesos (usually by a package manager). Since Windows
> does not have a package manager or a reasonble way of automatically
> discovering where a package is installed (aside from the registry), our
> CMake build system takes it upon itself to manage these system
> dependencies.  manage this dependency itself. This means that on
> Windows, we need to configure the build to look for the APR headers in
> our custom-downloaded APR repository.  Currently, though, we are not
> doing this, so when we'll hit a compile-time error if we try to build
> (e.g.) `svn.hpp`.
> 
> This commit will introduce the APR include paths as part of the build
> against Stout. Since Stout is a header-only library, it is (right now)
> incumbent on whoever is bundling Stout up to manage the third-party
> dependencies of Stout. In our current implementation, libprocess manages
> the APR dependency for Stout, so (just as we do for other dependencies
> like cURL) we will have it manage this logic as well.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 786e47e63dc03ab4851c93ec2030f85c049cebe9 
> 
> Diff: https://reviews.apache.org/r/54462/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>


Re: Review Request 54462: Windows: Added APR include path to libprocess configuration.

Posted by Alex Clemmer <cl...@gmail.com>.

> On Dec. 8, 2016, 1:17 a.m., Joseph Wu wrote:
> > I suppose we still need to bundle a libsvn before the `svn_tests.cpp` will work.

We're getting there, but that's correct. :)


- Alex


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


On Dec. 7, 2016, 11:06 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54462/
> -----------------------------------------------------------
> 
> (Updated Dec. 7, 2016, 11:06 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.
> 
> 
> Bugs: MESOS-3447
>     https://issues.apache.org/jira/browse/MESOS-3447
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Windows: Added APR include path to libprocess configuration.
> 
> Partially addresses MESOS-3447, as APR is a dependency of the SVN
> facilities of Stout.
> 
> On Unix builds, APR is expected to have been installed on the system
> prior to building Mesos (usually by a package manager). Since Windows
> does not have a package manager or a reasonble way of automatically
> discovering where a package is installed (aside from the registry), our
> CMake build system takes it upon itself to manage these system
> dependencies.  manage this dependency itself. This means that on
> Windows, we need to configure the build to look for the APR headers in
> our custom-downloaded APR repository.  Currently, though, we are not
> doing this, so when we'll hit a compile-time error if we try to build
> (e.g.) `svn.hpp`.
> 
> This commit will introduce the APR include paths as part of the build
> against Stout. Since Stout is a header-only library, it is (right now)
> incumbent on whoever is bundling Stout up to manage the third-party
> dependencies of Stout. In our current implementation, libprocess manages
> the APR dependency for Stout, hence, we put this logic in libprocess.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 786e47e63dc03ab4851c93ec2030f85c049cebe9 
> 
> Diff: https://reviews.apache.org/r/54462/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>


Re: Review Request 54462: Windows: Added APR include path to libprocess configuration.

Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54462/#review158469
-----------------------------------------------------------


Ship it!




I suppose we still need to bundle a libsvn before the `svn_tests.cpp` will work.

- Joseph Wu


On Dec. 7, 2016, 3:06 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54462/
> -----------------------------------------------------------
> 
> (Updated Dec. 7, 2016, 3:06 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.
> 
> 
> Bugs: MESOS-3447
>     https://issues.apache.org/jira/browse/MESOS-3447
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Windows: Added APR include path to libprocess configuration.
> 
> Partially addresses MESOS-3447, as APR is a dependency of the SVN
> facilities of Stout.
> 
> On Unix builds, APR is expected to have been installed on the system
> prior to building Mesos (usually by a package manager). Since Windows
> does not have a package manager or a reasonble way of automatically
> discovering where a package is installed (aside from the registry), our
> CMake build system takes it upon itself to manage these system
> dependencies.  manage this dependency itself. This means that on
> Windows, we need to configure the build to look for the APR headers in
> our custom-downloaded APR repository.  Currently, though, we are not
> doing this, so when we'll hit a compile-time error if we try to build
> (e.g.) `svn.hpp`.
> 
> This commit will introduce the APR include paths as part of the build
> against Stout. Since Stout is a header-only library, it is (right now)
> incumbent on whoever is bundling Stout up to manage the third-party
> dependencies of Stout. In our current implementation, libprocess manages
> the APR dependency for Stout, hence, we put this logic in libprocess.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 786e47e63dc03ab4851c93ec2030f85c049cebe9 
> 
> Diff: https://reviews.apache.org/r/54462/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>


Re: Review Request 54462: Windows: Added APR include path to libprocess configuration.

Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54462/
-----------------------------------------------------------

(Updated Dec. 7, 2016, 11:06 p.m.)


Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.


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


Repository: mesos


Description (updated)
-------

Windows: Added APR include path to libprocess configuration.

Partially addresses MESOS-3447, as APR is a dependency of the SVN
facilities of Stout.

On Unix builds, APR is expected to have been installed on the system
prior to building Mesos (usually by a package manager). Since Windows
does not have a package manager or a reasonble way of automatically
discovering where a package is installed (aside from the registry), our
CMake build system takes it upon itself to manage these system
dependencies.  manage this dependency itself. This means that on
Windows, we need to configure the build to look for the APR headers in
our custom-downloaded APR repository.  Currently, though, we are not
doing this, so when we'll hit a compile-time error if we try to build
(e.g.) `svn.hpp`.

This commit will introduce the APR include paths as part of the build
against Stout. Since Stout is a header-only library, it is (right now)
incumbent on whoever is bundling Stout up to manage the third-party
dependencies of Stout. In our current implementation, libprocess manages
the APR dependency for Stout, hence, we put this logic in libprocess.


Diffs
-----

  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 786e47e63dc03ab4851c93ec2030f85c049cebe9 

Diff: https://reviews.apache.org/r/54462/diff/


Testing
-------


Thanks,

Alex Clemmer