You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Bannier <be...@mesosphere.io> on 2019/03/25 15:09:22 UTC
Review Request 70295: Enabled launcher sealing for RPM packages.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70295/
-----------------------------------------------------------
Review request for mesos, Gilbert Song, Joseph Wu, and Till Toenshoff.
Repository: mesos
Description
-------
We enable this flag since with it disabled certain public functions
are not available making it hard to e.g., write modules against this
version of Mesos.
While launcher sealing depends on a recent kernel, the platform we
build RPMs for already satisfies the requirements.
Diffs
-----
support/packaging/centos/mesos.spec de905fda3366be5904a0aec8249abcc53b0af9a0
Diff: https://reviews.apache.org/r/70295/diff/1/
Testing
-------
`make check`
Thanks,
Benjamin Bannier
Re: Review Request 70295: Enabled launcher sealing for RPM packages.
Posted by Gilbert Song <so...@gmail.com>.
> On March 27, 2019, 5:57 a.m., James DeFelice wrote:
> > support/packaging/centos/mesos.spec
> > Lines 94 (patched)
> > <https://reviews.apache.org/r/70295/diff/1/?file=2133927#file2133927line94>
> >
> > we probably want to conditionally enable this depending on the centos version. for example I don't think el6 ships w/ the necessary kernel support for launcher-sealing, but I think that el7 does.
> >
> > see https://fedoraproject.org/wiki/Packaging:DistTag
+1
- Gilbert
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70295/#review214102
-----------------------------------------------------------
On March 25, 2019, 8:09 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70295/
> -----------------------------------------------------------
>
> (Updated March 25, 2019, 8:09 a.m.)
>
>
> Review request for mesos, Gilbert Song, Joseph Wu, and Till Toenshoff.
>
>
> Bugs: MESOS-9677
> https://issues.apache.org/jira/browse/MESOS-9677
>
>
> Repository: mesos
>
>
> Description
> -------
>
> We enable this flag since with it disabled certain public functions
> are not available making it hard to e.g., write modules against this
> version of Mesos.
>
> While launcher sealing depends on a recent kernel, the platform we
> build RPMs for already satisfies the requirements.
>
>
> Diffs
> -----
>
> support/packaging/centos/mesos.spec de905fda3366be5904a0aec8249abcc53b0af9a0
>
>
> Diff: https://reviews.apache.org/r/70295/diff/1/
>
>
> Testing
> -------
>
> `make check`
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 70295: Enabled launcher sealing for RPM packages.
Posted by James DeFelice <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70295/#review214102
-----------------------------------------------------------
support/packaging/centos/mesos.spec
Lines 94 (patched)
<https://reviews.apache.org/r/70295/#comment300274>
we probably want to conditionally enable this depending on the centos version. for example I don't think el6 ships w/ the necessary kernel support for launcher-sealing, but I think that el7 does.
see https://fedoraproject.org/wiki/Packaging:DistTag
- James DeFelice
On March 25, 2019, 3:09 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70295/
> -----------------------------------------------------------
>
> (Updated March 25, 2019, 3:09 p.m.)
>
>
> Review request for mesos, Gilbert Song, Joseph Wu, and Till Toenshoff.
>
>
> Bugs: MESOS-9677
> https://issues.apache.org/jira/browse/MESOS-9677
>
>
> Repository: mesos
>
>
> Description
> -------
>
> We enable this flag since with it disabled certain public functions
> are not available making it hard to e.g., write modules against this
> version of Mesos.
>
> While launcher sealing depends on a recent kernel, the platform we
> build RPMs for already satisfies the requirements.
>
>
> Diffs
> -----
>
> support/packaging/centos/mesos.spec de905fda3366be5904a0aec8249abcc53b0af9a0
>
>
> Diff: https://reviews.apache.org/r/70295/diff/1/
>
>
> Testing
> -------
>
> `make check`
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 70295: Enabled launcher sealing for RPM packages.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70295/#review213997
-----------------------------------------------------------
Patch looks great!
Reviews applied: [70295]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers --disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On March 25, 2019, 3:09 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70295/
> -----------------------------------------------------------
>
> (Updated March 25, 2019, 3:09 p.m.)
>
>
> Review request for mesos, Gilbert Song, Joseph Wu, and Till Toenshoff.
>
>
> Bugs: MESOS-9677
> https://issues.apache.org/jira/browse/MESOS-9677
>
>
> Repository: mesos
>
>
> Description
> -------
>
> We enable this flag since with it disabled certain public functions
> are not available making it hard to e.g., write modules against this
> version of Mesos.
>
> While launcher sealing depends on a recent kernel, the platform we
> build RPMs for already satisfies the requirements.
>
>
> Diffs
> -----
>
> support/packaging/centos/mesos.spec de905fda3366be5904a0aec8249abcc53b0af9a0
>
>
> Diff: https://reviews.apache.org/r/70295/diff/1/
>
>
> Testing
> -------
>
> `make check`
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 70295: Enabled launcher sealing for RPM packages.
Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70295/#review215247
-----------------------------------------------------------
Ship it!
Since Benjamin doesn't have time to finish this review, I went ahead and committed it including the changes proposed by @jdef.
- Benno Evers
On March 25, 2019, 3:09 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70295/
> -----------------------------------------------------------
>
> (Updated March 25, 2019, 3:09 p.m.)
>
>
> Review request for mesos, Gilbert Song, Joseph Wu, and Till Toenshoff.
>
>
> Bugs: MESOS-9677
> https://issues.apache.org/jira/browse/MESOS-9677
>
>
> Repository: mesos
>
>
> Description
> -------
>
> We enable this flag since with it disabled certain public functions
> are not available making it hard to e.g., write modules against this
> version of Mesos.
>
> While launcher sealing depends on a recent kernel, the platform we
> build RPMs for already satisfies the requirements.
>
>
> Diffs
> -----
>
> support/packaging/centos/mesos.spec de905fda3366be5904a0aec8249abcc53b0af9a0
>
>
> Diff: https://reviews.apache.org/r/70295/diff/1/
>
>
> Testing
> -------
>
> `make check`
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 70295: Enabled launcher sealing for RPM packages.
Posted by Gilbert Song <so...@gmail.com>.
> On March 25, 2019, 6:21 p.m., Joseph Wu wrote:
> > support/packaging/centos/mesos.spec
> > Lines 94 (patched)
> > <https://reviews.apache.org/r/70295/diff/1/?file=2133927#file2133927line94>
> >
> > I'd expect the `--disable-libtool-wrappers` flag too, but do rpm builds omit libtool wrappers anyway?
> >
> > It's not clear to me when exactly the wrappers are generated and when they aren't
>
> Benjamin Bannier wrote:
> The libtool wrappers (which set up the env to consume dynamic libraries built as part of the build) are generated so executables can be run from the build tree, without installing. They are never installed. OTOH, executables built in autotools projects without libtool wrappers cannot be installed as they are not correctly linked (they'd depend on files from the build tree, not from the install prefix).
>
> Dropping.
@Joseph, in this case, we don't need to disable libtool wrappers since --enable-optimize would make sure there is no .sh init helper generated.
- Gilbert
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70295/#review214019
-----------------------------------------------------------
On March 25, 2019, 8:09 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70295/
> -----------------------------------------------------------
>
> (Updated March 25, 2019, 8:09 a.m.)
>
>
> Review request for mesos, Gilbert Song, Joseph Wu, and Till Toenshoff.
>
>
> Bugs: MESOS-9677
> https://issues.apache.org/jira/browse/MESOS-9677
>
>
> Repository: mesos
>
>
> Description
> -------
>
> We enable this flag since with it disabled certain public functions
> are not available making it hard to e.g., write modules against this
> version of Mesos.
>
> While launcher sealing depends on a recent kernel, the platform we
> build RPMs for already satisfies the requirements.
>
>
> Diffs
> -----
>
> support/packaging/centos/mesos.spec de905fda3366be5904a0aec8249abcc53b0af9a0
>
>
> Diff: https://reviews.apache.org/r/70295/diff/1/
>
>
> Testing
> -------
>
> `make check`
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 70295: Enabled launcher sealing for RPM packages.
Posted by Benjamin Bannier <be...@mesosphere.io>.
> On March 26, 2019, 2:21 a.m., Joseph Wu wrote:
> > support/packaging/centos/mesos.spec
> > Lines 94 (patched)
> > <https://reviews.apache.org/r/70295/diff/1/?file=2133927#file2133927line94>
> >
> > I'd expect the `--disable-libtool-wrappers` flag too, but do rpm builds omit libtool wrappers anyway?
> >
> > It's not clear to me when exactly the wrappers are generated and when they aren't
The libtool wrappers (which set up the env to consume dynamic libraries built as part of the build) are generated so executables can be run from the build tree, without installing. They are never installed. OTOH, executables built in autotools projects without libtool wrappers cannot be installed as they are not correctly linked (they'd depend on files from the build tree, not from the install prefix).
Dropping.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70295/#review214019
-----------------------------------------------------------
On March 25, 2019, 4:09 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70295/
> -----------------------------------------------------------
>
> (Updated March 25, 2019, 4:09 p.m.)
>
>
> Review request for mesos, Gilbert Song, Joseph Wu, and Till Toenshoff.
>
>
> Bugs: MESOS-9677
> https://issues.apache.org/jira/browse/MESOS-9677
>
>
> Repository: mesos
>
>
> Description
> -------
>
> We enable this flag since with it disabled certain public functions
> are not available making it hard to e.g., write modules against this
> version of Mesos.
>
> While launcher sealing depends on a recent kernel, the platform we
> build RPMs for already satisfies the requirements.
>
>
> Diffs
> -----
>
> support/packaging/centos/mesos.spec de905fda3366be5904a0aec8249abcc53b0af9a0
>
>
> Diff: https://reviews.apache.org/r/70295/diff/1/
>
>
> Testing
> -------
>
> `make check`
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 70295: Enabled launcher sealing for RPM packages.
Posted by Benjamin Bannier <be...@mesosphere.io>.
> On March 26, 2019, 2:21 a.m., Joseph Wu wrote:
> > support/packaging/centos/mesos.spec
> > Lines 94 (patched)
> > <https://reviews.apache.org/r/70295/diff/1/?file=2133927#file2133927line94>
> >
> > I'd expect the `--disable-libtool-wrappers` flag too, but do rpm builds omit libtool wrappers anyway?
> >
> > It's not clear to me when exactly the wrappers are generated and when they aren't
>
> Benjamin Bannier wrote:
> The libtool wrappers (which set up the env to consume dynamic libraries built as part of the build) are generated so executables can be run from the build tree, without installing. They are never installed. OTOH, executables built in autotools projects without libtool wrappers cannot be installed as they are not correctly linked (they'd depend on files from the build tree, not from the install prefix).
>
> Dropping.
>
> Gilbert Song wrote:
> @Joseph, in this case, we don't need to disable libtool wrappers since --enable-optimize would make sure there is no .sh init helper generated.
@gilbert, that is incorrect, whether libtool wrappers are used or not is completely orthogonal to the used optimization level.
One must never disable libtool wrappers when planning to install the created binary artifacts.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70295/#review214019
-----------------------------------------------------------
On March 25, 2019, 4:09 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70295/
> -----------------------------------------------------------
>
> (Updated March 25, 2019, 4:09 p.m.)
>
>
> Review request for mesos, Gilbert Song, Joseph Wu, and Till Toenshoff.
>
>
> Bugs: MESOS-9677
> https://issues.apache.org/jira/browse/MESOS-9677
>
>
> Repository: mesos
>
>
> Description
> -------
>
> We enable this flag since with it disabled certain public functions
> are not available making it hard to e.g., write modules against this
> version of Mesos.
>
> While launcher sealing depends on a recent kernel, the platform we
> build RPMs for already satisfies the requirements.
>
>
> Diffs
> -----
>
> support/packaging/centos/mesos.spec de905fda3366be5904a0aec8249abcc53b0af9a0
>
>
> Diff: https://reviews.apache.org/r/70295/diff/1/
>
>
> Testing
> -------
>
> `make check`
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 70295: Enabled launcher sealing for RPM packages.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70295/#review214019
-----------------------------------------------------------
support/packaging/centos/mesos.spec
Lines 94 (patched)
<https://reviews.apache.org/r/70295/#comment300185>
I'd expect the `--disable-libtool-wrappers` flag too, but do rpm builds omit libtool wrappers anyway?
It's not clear to me when exactly the wrappers are generated and when they aren't
- Joseph Wu
On March 25, 2019, 8:09 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70295/
> -----------------------------------------------------------
>
> (Updated March 25, 2019, 8:09 a.m.)
>
>
> Review request for mesos, Gilbert Song, Joseph Wu, and Till Toenshoff.
>
>
> Bugs: MESOS-9677
> https://issues.apache.org/jira/browse/MESOS-9677
>
>
> Repository: mesos
>
>
> Description
> -------
>
> We enable this flag since with it disabled certain public functions
> are not available making it hard to e.g., write modules against this
> version of Mesos.
>
> While launcher sealing depends on a recent kernel, the platform we
> build RPMs for already satisfies the requirements.
>
>
> Diffs
> -----
>
> support/packaging/centos/mesos.spec de905fda3366be5904a0aec8249abcc53b0af9a0
>
>
> Diff: https://reviews.apache.org/r/70295/diff/1/
>
>
> Testing
> -------
>
> `make check`
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 70295: Enabled launcher sealing for RPM packages.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70295/#review213996
-----------------------------------------------------------
PASS: Mesos patch 70295 was successfully built and tested.
Reviews applied: `['70295']`
All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/3010/mesos-review-70295
- Mesos Reviewbot Windows
On March 25, 2019, 3:09 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70295/
> -----------------------------------------------------------
>
> (Updated March 25, 2019, 3:09 p.m.)
>
>
> Review request for mesos, Gilbert Song, Joseph Wu, and Till Toenshoff.
>
>
> Bugs: MESOS-9677
> https://issues.apache.org/jira/browse/MESOS-9677
>
>
> Repository: mesos
>
>
> Description
> -------
>
> We enable this flag since with it disabled certain public functions
> are not available making it hard to e.g., write modules against this
> version of Mesos.
>
> While launcher sealing depends on a recent kernel, the platform we
> build RPMs for already satisfies the requirements.
>
>
> Diffs
> -----
>
> support/packaging/centos/mesos.spec de905fda3366be5904a0aec8249abcc53b0af9a0
>
>
> Diff: https://reviews.apache.org/r/70295/diff/1/
>
>
> Testing
> -------
>
> `make check`
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 70295: Enabled launcher sealing for RPM packages.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70295/#review214010
-----------------------------------------------------------
Is our script also building centos6 rpm? Centos 6 does not have the memfd support I believe
- Gilbert Song
On March 25, 2019, 8:09 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70295/
> -----------------------------------------------------------
>
> (Updated March 25, 2019, 8:09 a.m.)
>
>
> Review request for mesos, Gilbert Song, Joseph Wu, and Till Toenshoff.
>
>
> Bugs: MESOS-9677
> https://issues.apache.org/jira/browse/MESOS-9677
>
>
> Repository: mesos
>
>
> Description
> -------
>
> We enable this flag since with it disabled certain public functions
> are not available making it hard to e.g., write modules against this
> version of Mesos.
>
> While launcher sealing depends on a recent kernel, the platform we
> build RPMs for already satisfies the requirements.
>
>
> Diffs
> -----
>
> support/packaging/centos/mesos.spec de905fda3366be5904a0aec8249abcc53b0af9a0
>
>
> Diff: https://reviews.apache.org/r/70295/diff/1/
>
>
> Testing
> -------
>
> `make check`
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 70295: Enabled launcher sealing for RPM packages.
Posted by James DeFelice <ja...@gmail.com>.
> On April 2, 2019, 5:46 p.m., James DeFelice wrote:
> > support/packaging/centos/mesos.spec
> > Lines 94 (patched)
> > <https://reviews.apache.org/r/70295/diff/1/?file=2133927#file2133927line94>
> >
> > ```
> > %{!?el6:%define launcher_sealing --enable-launcher-sealing}
> >
> > % configure %{launcher_sealing} \
> > ...
> > ```
(minus the space between `%` and `configure`)
- James
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70295/#review214282
-----------------------------------------------------------
On March 25, 2019, 3:09 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70295/
> -----------------------------------------------------------
>
> (Updated March 25, 2019, 3:09 p.m.)
>
>
> Review request for mesos, Gilbert Song, Joseph Wu, and Till Toenshoff.
>
>
> Bugs: MESOS-9677
> https://issues.apache.org/jira/browse/MESOS-9677
>
>
> Repository: mesos
>
>
> Description
> -------
>
> We enable this flag since with it disabled certain public functions
> are not available making it hard to e.g., write modules against this
> version of Mesos.
>
> While launcher sealing depends on a recent kernel, the platform we
> build RPMs for already satisfies the requirements.
>
>
> Diffs
> -----
>
> support/packaging/centos/mesos.spec de905fda3366be5904a0aec8249abcc53b0af9a0
>
>
> Diff: https://reviews.apache.org/r/70295/diff/1/
>
>
> Testing
> -------
>
> `make check`
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 70295: Enabled launcher sealing for RPM packages.
Posted by James DeFelice <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70295/#review214282
-----------------------------------------------------------
support/packaging/centos/mesos.spec
Lines 94 (patched)
<https://reviews.apache.org/r/70295/#comment300497>
```
%{!?el6:%define launcher_sealing --enable-launcher-sealing}
% configure %{launcher_sealing} \
...
```
- James DeFelice
On March 25, 2019, 3:09 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70295/
> -----------------------------------------------------------
>
> (Updated March 25, 2019, 3:09 p.m.)
>
>
> Review request for mesos, Gilbert Song, Joseph Wu, and Till Toenshoff.
>
>
> Bugs: MESOS-9677
> https://issues.apache.org/jira/browse/MESOS-9677
>
>
> Repository: mesos
>
>
> Description
> -------
>
> We enable this flag since with it disabled certain public functions
> are not available making it hard to e.g., write modules against this
> version of Mesos.
>
> While launcher sealing depends on a recent kernel, the platform we
> build RPMs for already satisfies the requirements.
>
>
> Diffs
> -----
>
> support/packaging/centos/mesos.spec de905fda3366be5904a0aec8249abcc53b0af9a0
>
>
> Diff: https://reviews.apache.org/r/70295/diff/1/
>
>
> Testing
> -------
>
> `make check`
>
>
> Thanks,
>
> Benjamin Bannier
>
>