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/16 23:16:57 UTC
Review Request 54827: Made CMake build default to use brew APR on OS
X.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54827/
-----------------------------------------------------------
Review request for mesos and Michael Park.
Repository: mesos
Description
-------
This review is a follow-up to the chain starting at #54278, which
transitions the Autotools Mesos build to use homebrew's APR on OS X
builds.
The goal of this commit is to fix the things that could confuse the
CMake build into selecting the wrong APR* headers and libraries. The big
ones are:
* Header and library paths are missing for brew defaults. Since APR* is
currently keg-only, we don't expect it to show up in systems paths,
and we should add the brew key paths ourselves.
* Header and library paths are ordered incorrectly. So, it is possible
to select headers from `/usr/include`, but libraries from
`/usr/local/opt`. Additionally, we want to check the user-specific
paths before system paths.
* The call to `find_library` passes the suggested paths into the `PATHS`
argument instead of the `HINTS` argument. In the case of the former,
system paths are checked first, and then our suggested paths are
checked. But, we'd like the opposite.
* Linking flags are incorrect. We should be passing `lib/` paths into
the `-L` flags, but we're passing in paths to the actual library files
instead. The library names should go as `-l` flags, or we `-L` to the
`lib/` directory paths.
Diffs
-----
3rdparty/stout/cmake/FindApr.cmake 9d55ba0f63d0215619472e86ade8c486364fae7e
3rdparty/stout/cmake/StoutConfigure.cmake d8da0f0702eb5bde1e1105accdffb82abe0cd24b
Diff: https://reviews.apache.org/r/54827/diff/
Testing
-------
`make check` on OS X and Windows.
Thanks,
Alex Clemmer
Re: Review Request 54827: Made CMake build default to use brew APR on
OS X.
Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54827/#review159524
-----------------------------------------------------------
Ship it!
Ship It!
- Michael Park
On Dec. 16, 2016, 3:46 p.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54827/
> -----------------------------------------------------------
>
> (Updated Dec. 16, 2016, 3:46 p.m.)
>
>
> Review request for mesos and Michael Park.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This review is a follow-up to the chain starting at #54278, which
> transitions the Autotools Mesos build to use homebrew's APR on OS X
> builds.
>
> The goal of this commit is to fix the things that could confuse the
> CMake build into selecting the wrong APR* headers and libraries. The big
> ones are:
>
> * Header and library paths are missing for brew defaults. Since APR* is
> currently keg-only, we don't expect it to show up in systems paths,
> and we should add the brew key paths ourselves.
> * Header and library paths are ordered incorrectly. So, it is possible
> to select headers from `/usr/include`, but libraries from
> `/usr/local/opt`. Additionally, we want to check the user-specific
> paths before system paths.
> * The call to `find_library` passes the suggested paths into the `PATHS`
> argument instead of the `HINTS` argument. In the case of the former,
> system paths are checked first, and then our suggested paths are
> checked. But, we'd like the opposite.
> * Linking flags are incorrect. We should be passing `lib/` paths into
> the `-L` flags, but we're passing in paths to the actual library files
> instead. The library names should go as `-l` flags, or we `-L` to the
> `lib/` directory paths.
> * There's a typo causing us to never search `POSSIBLE_APRUTIL_LIB_DIRS`.
>
>
> Diffs
> -----
>
> 3rdparty/stout/cmake/FindApr.cmake 9d55ba0f63d0215619472e86ade8c486364fae7e
> 3rdparty/stout/cmake/StoutConfigure.cmake d8da0f0702eb5bde1e1105accdffb82abe0cd24b
>
> Diff: https://reviews.apache.org/r/54827/diff/
>
>
> Testing
> -------
>
> `make check` on OS X and Windows.
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 54827: Made CMake build default to use brew APR on
OS X.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54827/
-----------------------------------------------------------
(Updated Dec. 16, 2016, 11:46 p.m.)
Review request for mesos and Michael Park.
Changes
-------
Use `brew --prefix` instead of a hard-coded brew path.
Repository: mesos
Description
-------
This review is a follow-up to the chain starting at #54278, which
transitions the Autotools Mesos build to use homebrew's APR on OS X
builds.
The goal of this commit is to fix the things that could confuse the
CMake build into selecting the wrong APR* headers and libraries. The big
ones are:
* Header and library paths are missing for brew defaults. Since APR* is
currently keg-only, we don't expect it to show up in systems paths,
and we should add the brew key paths ourselves.
* Header and library paths are ordered incorrectly. So, it is possible
to select headers from `/usr/include`, but libraries from
`/usr/local/opt`. Additionally, we want to check the user-specific
paths before system paths.
* The call to `find_library` passes the suggested paths into the `PATHS`
argument instead of the `HINTS` argument. In the case of the former,
system paths are checked first, and then our suggested paths are
checked. But, we'd like the opposite.
* Linking flags are incorrect. We should be passing `lib/` paths into
the `-L` flags, but we're passing in paths to the actual library files
instead. The library names should go as `-l` flags, or we `-L` to the
`lib/` directory paths.
* There's a typo causing us to never search `POSSIBLE_APRUTIL_LIB_DIRS`.
Diffs (updated)
-----
3rdparty/stout/cmake/FindApr.cmake 9d55ba0f63d0215619472e86ade8c486364fae7e
3rdparty/stout/cmake/StoutConfigure.cmake d8da0f0702eb5bde1e1105accdffb82abe0cd24b
Diff: https://reviews.apache.org/r/54827/diff/
Testing
-------
`make check` on OS X and Windows.
Thanks,
Alex Clemmer
Re: Review Request 54827: Made CMake build default to use brew APR on
OS X.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54827/
-----------------------------------------------------------
(Updated Dec. 16, 2016, 11:18 p.m.)
Review request for mesos and Michael Park.
Repository: mesos
Description (updated)
-------
This review is a follow-up to the chain starting at #54278, which
transitions the Autotools Mesos build to use homebrew's APR on OS X
builds.
The goal of this commit is to fix the things that could confuse the
CMake build into selecting the wrong APR* headers and libraries. The big
ones are:
* Header and library paths are missing for brew defaults. Since APR* is
currently keg-only, we don't expect it to show up in systems paths,
and we should add the brew key paths ourselves.
* Header and library paths are ordered incorrectly. So, it is possible
to select headers from `/usr/include`, but libraries from
`/usr/local/opt`. Additionally, we want to check the user-specific
paths before system paths.
* The call to `find_library` passes the suggested paths into the `PATHS`
argument instead of the `HINTS` argument. In the case of the former,
system paths are checked first, and then our suggested paths are
checked. But, we'd like the opposite.
* Linking flags are incorrect. We should be passing `lib/` paths into
the `-L` flags, but we're passing in paths to the actual library files
instead. The library names should go as `-l` flags, or we `-L` to the
`lib/` directory paths.
* There's a typo causing us to never search `POSSIBLE_APRUTIL_LIB_DIRS`.
Diffs
-----
3rdparty/stout/cmake/FindApr.cmake 9d55ba0f63d0215619472e86ade8c486364fae7e
3rdparty/stout/cmake/StoutConfigure.cmake d8da0f0702eb5bde1e1105accdffb82abe0cd24b
Diff: https://reviews.apache.org/r/54827/diff/
Testing
-------
`make check` on OS X and Windows.
Thanks,
Alex Clemmer