You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by John Kordich via Review Board <no...@reviews.apache.org> on 2018/06/01 21:58:43 UTC
Re: Review Request 67064: Added libarchive, bzip2,
and xz patches and associated build changes.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67064/
-----------------------------------------------------------
(Updated June 1, 2018, 9:58 p.m.)
Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Chun-Hung Hsiao, Eric Mumau, Jie Yu, Joseph Wu, Li Li, and Radhika Jandhyala.
Changes
-------
Addressed Joseph Wu's build issues:
Added ENABLE_LZ4 option to the CMake build of libarchive, so that we can disable it.
Disabled all flags to libarchive that are unnecessary as part of the autotools build.
Bugs: MESOS-8064
https://issues.apache.org/jira/browse/MESOS-8064
Repository: mesos
Description
-------
These libraries being available will allow stout to be changed such that
stout can invoke libarchive instead of shelling out to tar to extract
archives. On Windows, tar usually doesn't exist, and even if it does, it
rarely supports most compression formats. On Windows, we will build
libarchive to support each of those compression formats.
Diffs (updated)
-----
3rdparty/CMakeLists.txt ecb6946401d9b81c6610cf9f33dcf2caa9ff0f04
3rdparty/Makefile.am 062df18643cadb4f8f5bcbd56c1dad63a7b8f894
3rdparty/bzip2-1.0.6.patch PRE-CREATION
3rdparty/cmake/Versions.cmake 2828acd2aa00c8778c6c462bc594716b2b6289ba
3rdparty/libarchive-3.3.2.patch PRE-CREATION
3rdparty/versions.am ada998d62d012a45b2cf17256c1ae16b92d611f2
3rdparty/xz-5.2.3.patch PRE-CREATION
configure.ac 03d333d65bcab2e46cff0b1329e5ad7bb26da697
src/Makefile.am b7184ceccef5f2e985d905c155156f95c7a7c7b4
Diff: https://reviews.apache.org/r/67064/diff/7/
Changes: https://reviews.apache.org/r/67064/diff/6-7/
Testing
-------
I've built on Windows with CMake and have run all tests, which all pass.
I've also built on Linux with both the autotools build and cmake build and have run all tests, which all pass.
Thanks,
John Kordich
Re: Review Request 67064: Added libarchive, bzip2,
and xz patches and associated build changes.
Posted by Joseph Wu <jo...@mesosphere.io>.
> On June 4, 2018, 1:50 p.m., Joseph Wu wrote:
> > src/Makefile.am
> > Lines 218 (patched)
> > <https://reviews.apache.org/r/67064/diff/7/?file=2034659#file2034659line218>
> >
> > Won't need this line when linking against the bundled libarchive since we'll be adding the library to libmesos further down the makefile.
Ah, nevermind. This becomes necessary as the build rules for libmesos and mesos-fetcher are slightly separate in the automake build.
- Joseph
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67064/#review204069
-----------------------------------------------------------
On June 4, 2018, 12:11 p.m., John Kordich wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67064/
> -----------------------------------------------------------
>
> (Updated June 4, 2018, 12:11 p.m.)
>
>
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Chun-Hung Hsiao, Eric Mumau, Jie Yu, Joseph Wu, Li Li, and Radhika Jandhyala.
>
>
> Bugs: MESOS-8064
> https://issues.apache.org/jira/browse/MESOS-8064
>
>
> Repository: mesos
>
>
> Description
> -------
>
> These libraries being available will allow stout to be changed such that
> stout can invoke libarchive instead of shelling out to tar to extract
> archives. On Windows, tar usually doesn't exist, and even if it does, it
> rarely supports most compression formats. On Windows, we will build
> libarchive to support each of those compression formats.
>
>
> Diffs
> -----
>
> 3rdparty/CMakeLists.txt ecb6946401d9b81c6610cf9f33dcf2caa9ff0f04
> 3rdparty/Makefile.am 062df18643cadb4f8f5bcbd56c1dad63a7b8f894
> 3rdparty/bzip2-1.0.6.patch PRE-CREATION
> 3rdparty/cmake/Versions.cmake 2828acd2aa00c8778c6c462bc594716b2b6289ba
> 3rdparty/libarchive-3.3.2.patch PRE-CREATION
> 3rdparty/versions.am ada998d62d012a45b2cf17256c1ae16b92d611f2
> 3rdparty/xz-5.2.3.patch PRE-CREATION
> configure.ac 03d333d65bcab2e46cff0b1329e5ad7bb26da697
> src/Makefile.am b7184ceccef5f2e985d905c155156f95c7a7c7b4
>
>
> Diff: https://reviews.apache.org/r/67064/diff/8/
>
>
> Testing
> -------
>
> I've built on Windows with CMake and have run all tests, which all pass.
> I've also built on Linux with both the autotools build and cmake build and have run all tests, which all pass.
>
>
> Thanks,
>
> John Kordich
>
>
Re: Review Request 67064: Added libarchive, bzip2,
and xz patches and associated build changes.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67064/#review204069
-----------------------------------------------------------
Fix it, then Ship it!
A couple of small things I'll fix before committing:
3rdparty/CMakeLists.txt
Lines 937-948 (patched)
<https://reviews.apache.org/r/67064/#comment286677>
We should add `-DENABLE_LibGCC=OFF` just to be safe.
The option could have an effect on Windows if the condition `NOT WITHOUT_PCRE_STATIC AND NOT PCRE_STATIC AND PCRE_FOUND` is satsified. AFAICT, none of these variables are usually defined.
3rdparty/CMakeLists.txt
Lines 954 (patched)
<https://reviews.apache.org/r/67064/#comment286675>
Hum... tests appear to be disabled on all platforms...
I'll change this to be a note about the linkage against other compression libraries (bzip2, xz, zlib).
3rdparty/Makefile.am
Line 234 (original), 237 (patched)
<https://reviews.apache.org/r/67064/#comment286674>
Some extraneous spacing changes (mostly tabs to spaces) seem to have snuck into this patch. I'll remove them before committing.
3rdparty/Makefile.am
Lines 281-290 (patched)
<https://reviews.apache.org/r/67064/#comment286683>
We'll need to disable building bzip2 and xz via:
```
--without-bz2lib
--without-lzma
```
As these are not explicit dependencies. The libarchive build attempts to find these on the system, and if found, it will require symbols found in those libraries. We'd then get a link-time failure as Mesos does not link to `-lbz2` and `-lxz`.
3rdparty/libarchive-3.3.2.patch
Lines 32-39 (patched)
<https://reviews.apache.org/r/67064/#comment286684>
The whitespace changes in the patch are extraneous and can be removed. (We don't need to prettify 3rdparty sources :)
configure.ac
Lines 1469 (patched)
<https://reviews.apache.org/r/67064/#comment286671>
Missing a part of the conditional here:
`|| test "x$enable_bundled" != "xyes"`
I can add this before committing.
src/Makefile.am
Lines 218 (patched)
<https://reviews.apache.org/r/67064/#comment286672>
Won't need this line when linking against the bundled libarchive since we'll be adding the library to libmesos further down the makefile.
3rdparty/CMakeLists.txt
Lines 987 (patched)
<https://reviews.apache.org/r/67064/#comment286685>
I almost didn't notice the odd capitalization on these:
`SET_target_properties`
vs
`set_target_properties`
I'll fix up the three instances of this before committing.
- Joseph Wu
On June 4, 2018, 12:11 p.m., John Kordich wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67064/
> -----------------------------------------------------------
>
> (Updated June 4, 2018, 12:11 p.m.)
>
>
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Chun-Hung Hsiao, Eric Mumau, Jie Yu, Joseph Wu, Li Li, and Radhika Jandhyala.
>
>
> Bugs: MESOS-8064
> https://issues.apache.org/jira/browse/MESOS-8064
>
>
> Repository: mesos
>
>
> Description
> -------
>
> These libraries being available will allow stout to be changed such that
> stout can invoke libarchive instead of shelling out to tar to extract
> archives. On Windows, tar usually doesn't exist, and even if it does, it
> rarely supports most compression formats. On Windows, we will build
> libarchive to support each of those compression formats.
>
>
> Diffs
> -----
>
> 3rdparty/CMakeLists.txt ecb6946401d9b81c6610cf9f33dcf2caa9ff0f04
> 3rdparty/Makefile.am 062df18643cadb4f8f5bcbd56c1dad63a7b8f894
> 3rdparty/bzip2-1.0.6.patch PRE-CREATION
> 3rdparty/cmake/Versions.cmake 2828acd2aa00c8778c6c462bc594716b2b6289ba
> 3rdparty/libarchive-3.3.2.patch PRE-CREATION
> 3rdparty/versions.am ada998d62d012a45b2cf17256c1ae16b92d611f2
> 3rdparty/xz-5.2.3.patch PRE-CREATION
> configure.ac 03d333d65bcab2e46cff0b1329e5ad7bb26da697
> src/Makefile.am b7184ceccef5f2e985d905c155156f95c7a7c7b4
>
>
> Diff: https://reviews.apache.org/r/67064/diff/8/
>
>
> Testing
> -------
>
> I've built on Windows with CMake and have run all tests, which all pass.
> I've also built on Linux with both the autotools build and cmake build and have run all tests, which all pass.
>
>
> Thanks,
>
> John Kordich
>
>
Re: Review Request 67064: Added libarchive, bzip2,
and xz patches and associated build changes.
Posted by John Kordich via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67064/
-----------------------------------------------------------
(Updated June 4, 2018, 7:11 p.m.)
Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Chun-Hung Hsiao, Eric Mumau, Jie Yu, Joseph Wu, Li Li, and Radhika Jandhyala.
Changes
-------
Addressed Andy's comments
Bugs: MESOS-8064
https://issues.apache.org/jira/browse/MESOS-8064
Repository: mesos
Description
-------
These libraries being available will allow stout to be changed such that
stout can invoke libarchive instead of shelling out to tar to extract
archives. On Windows, tar usually doesn't exist, and even if it does, it
rarely supports most compression formats. On Windows, we will build
libarchive to support each of those compression formats.
Diffs (updated)
-----
3rdparty/CMakeLists.txt ecb6946401d9b81c6610cf9f33dcf2caa9ff0f04
3rdparty/Makefile.am 062df18643cadb4f8f5bcbd56c1dad63a7b8f894
3rdparty/bzip2-1.0.6.patch PRE-CREATION
3rdparty/cmake/Versions.cmake 2828acd2aa00c8778c6c462bc594716b2b6289ba
3rdparty/libarchive-3.3.2.patch PRE-CREATION
3rdparty/versions.am ada998d62d012a45b2cf17256c1ae16b92d611f2
3rdparty/xz-5.2.3.patch PRE-CREATION
configure.ac 03d333d65bcab2e46cff0b1329e5ad7bb26da697
src/Makefile.am b7184ceccef5f2e985d905c155156f95c7a7c7b4
Diff: https://reviews.apache.org/r/67064/diff/8/
Changes: https://reviews.apache.org/r/67064/diff/7-8/
Testing
-------
I've built on Windows with CMake and have run all tests, which all pass.
I've also built on Linux with both the autotools build and cmake build and have run all tests, which all pass.
Thanks,
John Kordich