You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by James Peach <jp...@apache.org> on 2018/09/18 17:39:17 UTC
Review Request 68749: Fixed the CSI protobuf build dependencies.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68749/
-----------------------------------------------------------
Review request for mesos, Benjamin Bannier, Joseph Wu, and Vinod Kone.
Bugs: MESOS-9240
https://issues.apache.org/jira/browse/MESOS-9240
Repository: mesos
Description
-------
Generating the CSI protobufs depends on the `$(builddir)/include/csi`
directory being created at configuration time. This only happens
when automake build dependencies are enabled, however. In some
distributions, rpmbuild will pass `--disable-dependency-tracking`,
which will disable automake dependency tracking and prevent this
directory being created. The fix is to simply ensure the CSI protobuf
generation rules create the output directory is necessary.
Diffs
-----
Makefile.am 25028a949b9a3420bf3a379d65bcbae8e028aeaa
src/Makefile.am b0d63e2feef4bbbb186443ff9620a96473799a9f
Diff: https://reviews.apache.org/r/68749/diff/1/
Testing
-------
Verified that `./support/docker-build.sh` fails after updating `AM_DISTCHECK_CONFIGURE_FLAGS` and then succeeds.
Thanks,
James Peach
Re: Review Request 68749: Fixed the CSI protobuf build dependencies.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68749/#review208721
-----------------------------------------------------------
PASS: Mesos patch 68749 was successfully built and tested.
Reviews applied: `['68749']`
All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2350/mesos-review-68749
- Mesos Reviewbot Windows
On Sept. 18, 2018, 5:39 p.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68749/
> -----------------------------------------------------------
>
> (Updated Sept. 18, 2018, 5:39 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Joseph Wu, and Vinod Kone.
>
>
> Bugs: MESOS-9240
> https://issues.apache.org/jira/browse/MESOS-9240
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Generating the CSI protobufs depends on the `$(builddir)/include/csi`
> directory being created at configuration time. This only happens
> when automake build dependencies are enabled, however. In some
> distributions, rpmbuild will pass `--disable-dependency-tracking`,
> which will disable automake dependency tracking and prevent this
> directory being created. The fix is to simply ensure the CSI protobuf
> generation rules create the output directory is necessary.
>
>
> Diffs
> -----
>
> Makefile.am 25028a949b9a3420bf3a379d65bcbae8e028aeaa
> src/Makefile.am b0d63e2feef4bbbb186443ff9620a96473799a9f
>
>
> Diff: https://reviews.apache.org/r/68749/diff/1/
>
>
> Testing
> -------
>
> Verified that `./support/docker-build.sh` fails after updating `AM_DISTCHECK_CONFIGURE_FLAGS` and then succeeds.
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 68749: Fixed the CSI protobuf build dependencies.
Posted by James Peach <jp...@apache.org>.
> On Sept. 18, 2018, 8:14 p.m., Benjamin Bannier wrote:
> > src/Makefile.am
> > Lines 483 (patched)
> > <https://reviews.apache.org/r/68749/diff/1/?file=2090121#file2090121line486>
> >
> > Can we use `$(MKDIR_P) $(@D)` for consistency and easier maintenance? The only difference between this rule and e.g., the Java protobuf rules below seems to be that this one uses multiple targets.
I verified that when you have multiple targets `$(@D)` gives you the dirname of the first target.
- James
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68749/#review208728
-----------------------------------------------------------
On Sept. 18, 2018, 8:57 p.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68749/
> -----------------------------------------------------------
>
> (Updated Sept. 18, 2018, 8:57 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Joseph Wu, and Vinod Kone.
>
>
> Bugs: MESOS-9240
> https://issues.apache.org/jira/browse/MESOS-9240
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Generating the CSI protobufs depends on the `$(builddir)/include/csi`
> directory being created at configuration time. This only happens
> when automake build dependencies are enabled, however. In some
> distributions, rpmbuild will pass `--disable-dependency-tracking`,
> which will disable automake dependency tracking and prevent this
> directory being created. The fix is to simply ensure the CSI protobuf
> generation rules create the output directory as necessary.
>
>
> Diffs
> -----
>
> Makefile.am 25028a949b9a3420bf3a379d65bcbae8e028aeaa
> src/Makefile.am b0d63e2feef4bbbb186443ff9620a96473799a9f
>
>
> Diff: https://reviews.apache.org/r/68749/diff/2/
>
>
> Testing
> -------
>
> Verified that `./support/docker-build.sh` fails after updating `AM_DISTCHECK_CONFIGURE_FLAGS` and then succeeds.
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 68749: Fixed the CSI protobuf build dependencies.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68749/#review208728
-----------------------------------------------------------
Fix it, then Ship it!
Makefile.am
Lines 55 (patched)
<https://reviews.apache.org/r/68749/#comment292877>
`s/we don't have/we do not/`
src/Makefile.am
Lines 483 (patched)
<https://reviews.apache.org/r/68749/#comment292878>
Can we use `$(MKDIR_P) $(@D)` for consistency and easier maintenance? The only difference between this rule and e.g., the Java protobuf rules below seems to be that this one uses multiple targets.
src/Makefile.am
Lines 487 (patched)
<https://reviews.apache.org/r/68749/#comment292879>
Ditto.
- Benjamin Bannier
On Sept. 18, 2018, 7:39 p.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68749/
> -----------------------------------------------------------
>
> (Updated Sept. 18, 2018, 7:39 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Joseph Wu, and Vinod Kone.
>
>
> Bugs: MESOS-9240
> https://issues.apache.org/jira/browse/MESOS-9240
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Generating the CSI protobufs depends on the `$(builddir)/include/csi`
> directory being created at configuration time. This only happens
> when automake build dependencies are enabled, however. In some
> distributions, rpmbuild will pass `--disable-dependency-tracking`,
> which will disable automake dependency tracking and prevent this
> directory being created. The fix is to simply ensure the CSI protobuf
> generation rules create the output directory is necessary.
>
>
> Diffs
> -----
>
> Makefile.am 25028a949b9a3420bf3a379d65bcbae8e028aeaa
> src/Makefile.am b0d63e2feef4bbbb186443ff9620a96473799a9f
>
>
> Diff: https://reviews.apache.org/r/68749/diff/1/
>
>
> Testing
> -------
>
> Verified that `./support/docker-build.sh` fails after updating `AM_DISTCHECK_CONFIGURE_FLAGS` and then succeeds.
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 68749: Fixed the CSI protobuf build dependencies.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68749/#review208738
-----------------------------------------------------------
PASS: Mesos patch 68749 was successfully built and tested.
Reviews applied: `['68749']`
All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2353/mesos-review-68749
- Mesos Reviewbot Windows
On Sept. 18, 2018, 8:57 p.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68749/
> -----------------------------------------------------------
>
> (Updated Sept. 18, 2018, 8:57 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Joseph Wu, and Vinod Kone.
>
>
> Bugs: MESOS-9240
> https://issues.apache.org/jira/browse/MESOS-9240
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Generating the CSI protobufs depends on the `$(builddir)/include/csi`
> directory being created at configuration time. This only happens
> when automake build dependencies are enabled, however. In some
> distributions, rpmbuild will pass `--disable-dependency-tracking`,
> which will disable automake dependency tracking and prevent this
> directory being created. The fix is to simply ensure the CSI protobuf
> generation rules create the output directory as necessary.
>
>
> Diffs
> -----
>
> Makefile.am 25028a949b9a3420bf3a379d65bcbae8e028aeaa
> src/Makefile.am b0d63e2feef4bbbb186443ff9620a96473799a9f
>
>
> Diff: https://reviews.apache.org/r/68749/diff/2/
>
>
> Testing
> -------
>
> Verified that `./support/docker-build.sh` fails after updating `AM_DISTCHECK_CONFIGURE_FLAGS` and then succeeds.
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 68749: Fixed the CSI protobuf build dependencies.
Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68749/
-----------------------------------------------------------
(Updated Sept. 18, 2018, 8:57 p.m.)
Review request for mesos, Benjamin Bannier, Joseph Wu, and Vinod Kone.
Bugs: MESOS-9240
https://issues.apache.org/jira/browse/MESOS-9240
Repository: mesos
Description (updated)
-------
Generating the CSI protobufs depends on the `$(builddir)/include/csi`
directory being created at configuration time. This only happens
when automake build dependencies are enabled, however. In some
distributions, rpmbuild will pass `--disable-dependency-tracking`,
which will disable automake dependency tracking and prevent this
directory being created. The fix is to simply ensure the CSI protobuf
generation rules create the output directory as necessary.
Diffs (updated)
-----
Makefile.am 25028a949b9a3420bf3a379d65bcbae8e028aeaa
src/Makefile.am b0d63e2feef4bbbb186443ff9620a96473799a9f
Diff: https://reviews.apache.org/r/68749/diff/2/
Changes: https://reviews.apache.org/r/68749/diff/1-2/
Testing
-------
Verified that `./support/docker-build.sh` fails after updating `AM_DISTCHECK_CONFIGURE_FLAGS` and then succeeds.
Thanks,
James Peach