You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Bernardo Gomez Palacio <be...@gmail.com> on 2014/05/03 02:51:19 UTC
Review Request 21051: [MESOS-1260]: Protobuf shading for Mesos 0.16.x
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21051/
-----------------------------------------------------------
Review request for mesos and Mesos ReviewBot.
Bugs: MESOS-1260
https://issues.apache.org/jira/browse/MESOS-1260
Repository: mesos-git
Description
-------
This commit adds support for protobuf shading to 0.16.0.
As with RR https://reviews.apache.org/r/21044/ I ran into some trouble submitting this review and the result is a patch that bundles what should really be three. The original intention was to base this patch on b7e27cfac378c9e0a81596081fa4c65c7cea301a:"Minor fix to make Slave::detected() public for testing." but I kept getting errors from RB. The solution, not perfect, was to base this patch on 4207dccfb9cecfddc689cee08367c0e803306825.
All said this patch can be applied directly to b7e27cfac378c9e0a81596081fa4c65c7cea301a. I would assume that a branch that targets 0.16.1 will be created and will include this patch.
====
[MESOS-1212]: Use maven to compile and package Mesos' Java files.
The Mesos Jar files are now generated through Maven. This update also
adds a _shaded protobuf jar_ that contains the _protobuf_ version _shaded_
inside the `org.apache.mesos.protobuf` namespace.
List of Java Artifacts.
* mesos-<Mesos Version>-javadoc.jar
* mesos-<Mesos Version>-shaded-protobuf.jar
* mesos-<Mesos Version>-sources.jar
* mesos-<Mesos Version>.jar
Where `mesos-<Mesos Version>-shaded-protobuf.jar` will be the dependency
that a _framework_ should use to avoid any conflicts of _protobuf_
versions that it might have with other _dependencies_. Not that
`shaded-protobuf` is the _dependency qualifier_.
Applied patch from:
https://reviews.apache.org/r/20402/
ticket : https://issues.apache.org/jira/browse/MESOS-1212
[MESOS-1252]: Support ENV MAVEN_HOME on the build.
The build now supports the `MAVEN_HOME` environment variable which is
used to resolve the `mvn` executable. If no `MAVEN_HOME` is specified we
will use the one available in the `PATH`.
ticket: https://issues.apache.org/jira/browse/MESOS-1252
[MESOS-1259]: POM & Fixing Javadocs for Maven Package.
The `protobuf` namespace is defined as `org.apache.mesos.protobuf`
The Maven _package_ task will depends on the _javadoc_ task. The
_javadoc_ task fails if there are _Classes_ which expose _public_ fields
or methods without proper _Java Documentation_. Note that my proper I am
referring to the structure validation that _Javadoc_ executes and not a
proper narrative of such documentation.
This change addresses the issues that caused failures on the Maven
Javadoc plugin while attempting to add meaningful content to such
documentation.
Diffs
-----
configure.ac ba4ec1d930d2e74a5dad39bcd5f8ad5aec4411c0
src/Makefile.am abef3d2c43314e040e205f33086ec550812812cc
src/java/mesos.pom.in f817c944216274ebb3dac88304629d1adce20c13
src/java/src/org/apache/mesos/Executor.java 8f4eaeeea0f96927e67b746e506310555e0e9a05
src/java/src/org/apache/mesos/ExecutorDriver.java f7d288a322b96641fdafd4ff935722882ec00cc0
src/java/src/org/apache/mesos/Log.java d27e4f9e226f1aa5588cf75cb8c0a117dbf3015d
src/java/src/org/apache/mesos/MesosExecutorDriver.java ce146cdb33b3c80ea43866d304b3f1ff1a3e7c00
src/java/src/org/apache/mesos/MesosSchedulerDriver.java ed4b4a31e55c5cc108150a8059cf08436c8b846e
src/java/src/org/apache/mesos/Scheduler.java 55c4706c86ed68f13a9a33168381165900f7cfcf
src/java/src/org/apache/mesos/SchedulerDriver.java 5b0ca391730b34f4d62c92d0e1b50193c0fa03ad
src/java/src/org/apache/mesos/state/State.java dccb0ff0fa9f0e456e3ff2657520aee2ae2a7f5e
src/java/src/org/apache/mesos/state/Variable.java bedb74b5aaafa7e2bb304f44e3bb972445edceca
src/java/src/org/apache/mesos/state/ZooKeeperState.java f3d939f73192d2cec470ca1a7ff35e7b0a9b93a6
Diff: https://reviews.apache.org/r/21051/diff/
Testing
-------
1. ./bootstrap && mkdir build && cd build && ../configure && make && make check
2. Ensure the existence of
build/src/java/target/mesos-0.16.0-javadoc.jar
build/src/java/target/mesos-0.16.0-shaded-protobuf.jar
build/src/java/target/mesos-0.16.0-sources.jar
build/src/java/target/mesos-0.16.0.jar
build/src/java/target/protobuf-java-2.4.1.jar
3. do a `jar tvf build/src/java/target/mesos-0.16.0-shaded-protobuf.jar | grep protobuf | grep .class` and you should see the protobuf classes inside the org.apache.mesos.protobuf namespace.
Thanks,
Bernardo Gomez Palacio
Re: Review Request 21051: [MESOS-1260]: Protobuf shading for Mesos 0.16.x
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21051/#review42081
-----------------------------------------------------------
Bad patch!
Reviews applied: [21051]
Failed command: git apply --index 21051.patch
Error:
error: patch failed: configure.ac:152
error: configure.ac: patch does not apply
error: patch failed: src/Makefile.am:474
error: src/Makefile.am: patch does not apply
error: patch failed: src/java/mesos.pom.in:28
error: src/java/mesos.pom.in: patch does not apply
error: patch failed: src/java/src/org/apache/mesos/Executor.java:29
error: src/java/src/org/apache/mesos/Executor.java: patch does not apply
error: patch failed: src/java/src/org/apache/mesos/ExecutorDriver.java:31
error: src/java/src/org/apache/mesos/ExecutorDriver.java: patch does not apply
error: patch failed: src/java/src/org/apache/mesos/Scheduler.java:43
error: src/java/src/org/apache/mesos/Scheduler.java: patch does not apply
error: patch failed: src/java/src/org/apache/mesos/SchedulerDriver.java:34
error: src/java/src/org/apache/mesos/SchedulerDriver.java: patch does not apply
error: patch failed: src/java/src/org/apache/mesos/state/State.java:36
error: src/java/src/org/apache/mesos/state/State.java: patch does not apply
error: patch failed: src/java/src/org/apache/mesos/state/Variable.java:23
error: src/java/src/org/apache/mesos/state/Variable.java: patch does not apply
error: patch failed: src/java/src/org/apache/mesos/state/ZooKeeperState.java:45
error: src/java/src/org/apache/mesos/state/ZooKeeperState.java: patch does not apply
- Mesos ReviewBot
On May 3, 2014, 12:51 a.m., Bernardo Gomez Palacio wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21051/
> -----------------------------------------------------------
>
> (Updated May 3, 2014, 12:51 a.m.)
>
>
> Review request for mesos and Mesos ReviewBot.
>
>
> Bugs: MESOS-1260
> https://issues.apache.org/jira/browse/MESOS-1260
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> This commit adds support for protobuf shading to 0.16.0.
>
> As with RR https://reviews.apache.org/r/21044/ I ran into some trouble submitting this review and the result is a patch that bundles what should really be three. The original intention was to base this patch on b7e27cfac378c9e0a81596081fa4c65c7cea301a:"Minor fix to make Slave::detected() public for testing." but I kept getting errors from RB. The solution, not perfect, was to base this patch on 4207dccfb9cecfddc689cee08367c0e803306825.
>
>
> All said this patch can be applied directly to b7e27cfac378c9e0a81596081fa4c65c7cea301a. I would assume that a branch that targets 0.16.1 will be created and will include this patch.
>
>
> ====
>
> [MESOS-1212]: Use maven to compile and package Mesos' Java files.
>
> The Mesos Jar files are now generated through Maven. This update also
> adds a _shaded protobuf jar_ that contains the _protobuf_ version _shaded_
> inside the `org.apache.mesos.protobuf` namespace.
>
> List of Java Artifacts.
>
> * mesos-<Mesos Version>-javadoc.jar
> * mesos-<Mesos Version>-shaded-protobuf.jar
> * mesos-<Mesos Version>-sources.jar
> * mesos-<Mesos Version>.jar
>
> Where `mesos-<Mesos Version>-shaded-protobuf.jar` will be the dependency
> that a _framework_ should use to avoid any conflicts of _protobuf_
> versions that it might have with other _dependencies_. Not that
> `shaded-protobuf` is the _dependency qualifier_.
>
> Applied patch from:
> https://reviews.apache.org/r/20402/
>
> ticket : https://issues.apache.org/jira/browse/MESOS-1212
>
> [MESOS-1252]: Support ENV MAVEN_HOME on the build.
>
> The build now supports the `MAVEN_HOME` environment variable which is
> used to resolve the `mvn` executable. If no `MAVEN_HOME` is specified we
> will use the one available in the `PATH`.
>
> ticket: https://issues.apache.org/jira/browse/MESOS-1252
>
> [MESOS-1259]: POM & Fixing Javadocs for Maven Package.
>
> The `protobuf` namespace is defined as `org.apache.mesos.protobuf`
>
> The Maven _package_ task will depends on the _javadoc_ task. The
> _javadoc_ task fails if there are _Classes_ which expose _public_ fields
> or methods without proper _Java Documentation_. Note that my proper I am
> referring to the structure validation that _Javadoc_ executes and not a
> proper narrative of such documentation.
>
> This change addresses the issues that caused failures on the Maven
> Javadoc plugin while attempting to add meaningful content to such
> documentation.
>
>
> Diffs
> -----
>
> configure.ac ba4ec1d930d2e74a5dad39bcd5f8ad5aec4411c0
> src/Makefile.am abef3d2c43314e040e205f33086ec550812812cc
> src/java/mesos.pom.in f817c944216274ebb3dac88304629d1adce20c13
> src/java/src/org/apache/mesos/Executor.java 8f4eaeeea0f96927e67b746e506310555e0e9a05
> src/java/src/org/apache/mesos/ExecutorDriver.java f7d288a322b96641fdafd4ff935722882ec00cc0
> src/java/src/org/apache/mesos/Log.java d27e4f9e226f1aa5588cf75cb8c0a117dbf3015d
> src/java/src/org/apache/mesos/MesosExecutorDriver.java ce146cdb33b3c80ea43866d304b3f1ff1a3e7c00
> src/java/src/org/apache/mesos/MesosSchedulerDriver.java ed4b4a31e55c5cc108150a8059cf08436c8b846e
> src/java/src/org/apache/mesos/Scheduler.java 55c4706c86ed68f13a9a33168381165900f7cfcf
> src/java/src/org/apache/mesos/SchedulerDriver.java 5b0ca391730b34f4d62c92d0e1b50193c0fa03ad
> src/java/src/org/apache/mesos/state/State.java dccb0ff0fa9f0e456e3ff2657520aee2ae2a7f5e
> src/java/src/org/apache/mesos/state/Variable.java bedb74b5aaafa7e2bb304f44e3bb972445edceca
> src/java/src/org/apache/mesos/state/ZooKeeperState.java f3d939f73192d2cec470ca1a7ff35e7b0a9b93a6
>
> Diff: https://reviews.apache.org/r/21051/diff/
>
>
> Testing
> -------
>
> 1. ./bootstrap && mkdir build && cd build && ../configure && make && make check
> 2. Ensure the existence of
> build/src/java/target/mesos-0.16.0-javadoc.jar
> build/src/java/target/mesos-0.16.0-shaded-protobuf.jar
> build/src/java/target/mesos-0.16.0-sources.jar
> build/src/java/target/mesos-0.16.0.jar
> build/src/java/target/protobuf-java-2.4.1.jar
> 3. do a `jar tvf build/src/java/target/mesos-0.16.0-shaded-protobuf.jar | grep protobuf | grep .class` and you should see the protobuf classes inside the org.apache.mesos.protobuf namespace.
>
>
> Thanks,
>
> Bernardo Gomez Palacio
>
>
Re: Review Request 21051: [MESOS-1260]: Protobuf shading for Mesos 0.16.x
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21051/#review42090
-----------------------------------------------------------
Hey Bernardo, we've created bug fix releases in the past by cherry-picking the commits we want (into a local branch) modifying them as necessary so that they apply (which you've effectively done here, just all in one patch). The advantage of applying them one at a time is that we have a nice paper trail for how the bug fix release was constructed. While you cherry-pick each commit you can also update JIRA to mark the associated tickets with the another 'Fix Version'.
You can read more about this here: https://github.com/apache/mesos/blob/master/docs/release-guide.md. I'd be happy to work with you on doing these bug fix releases too!
- Benjamin Hindman
On May 3, 2014, 12:51 a.m., Bernardo Gomez Palacio wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21051/
> -----------------------------------------------------------
>
> (Updated May 3, 2014, 12:51 a.m.)
>
>
> Review request for mesos and Mesos ReviewBot.
>
>
> Bugs: MESOS-1260
> https://issues.apache.org/jira/browse/MESOS-1260
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> This commit adds support for protobuf shading to 0.16.0.
>
> As with RR https://reviews.apache.org/r/21044/ I ran into some trouble submitting this review and the result is a patch that bundles what should really be three. The original intention was to base this patch on b7e27cfac378c9e0a81596081fa4c65c7cea301a:"Minor fix to make Slave::detected() public for testing." but I kept getting errors from RB. The solution, not perfect, was to base this patch on 4207dccfb9cecfddc689cee08367c0e803306825.
>
>
> All said this patch can be applied directly to b7e27cfac378c9e0a81596081fa4c65c7cea301a. I would assume that a branch that targets 0.16.1 will be created and will include this patch.
>
>
> ====
>
> [MESOS-1212]: Use maven to compile and package Mesos' Java files.
>
> The Mesos Jar files are now generated through Maven. This update also
> adds a _shaded protobuf jar_ that contains the _protobuf_ version _shaded_
> inside the `org.apache.mesos.protobuf` namespace.
>
> List of Java Artifacts.
>
> * mesos-<Mesos Version>-javadoc.jar
> * mesos-<Mesos Version>-shaded-protobuf.jar
> * mesos-<Mesos Version>-sources.jar
> * mesos-<Mesos Version>.jar
>
> Where `mesos-<Mesos Version>-shaded-protobuf.jar` will be the dependency
> that a _framework_ should use to avoid any conflicts of _protobuf_
> versions that it might have with other _dependencies_. Not that
> `shaded-protobuf` is the _dependency qualifier_.
>
> Applied patch from:
> https://reviews.apache.org/r/20402/
>
> ticket : https://issues.apache.org/jira/browse/MESOS-1212
>
> [MESOS-1252]: Support ENV MAVEN_HOME on the build.
>
> The build now supports the `MAVEN_HOME` environment variable which is
> used to resolve the `mvn` executable. If no `MAVEN_HOME` is specified we
> will use the one available in the `PATH`.
>
> ticket: https://issues.apache.org/jira/browse/MESOS-1252
>
> [MESOS-1259]: POM & Fixing Javadocs for Maven Package.
>
> The `protobuf` namespace is defined as `org.apache.mesos.protobuf`
>
> The Maven _package_ task will depends on the _javadoc_ task. The
> _javadoc_ task fails if there are _Classes_ which expose _public_ fields
> or methods without proper _Java Documentation_. Note that my proper I am
> referring to the structure validation that _Javadoc_ executes and not a
> proper narrative of such documentation.
>
> This change addresses the issues that caused failures on the Maven
> Javadoc plugin while attempting to add meaningful content to such
> documentation.
>
>
> Diffs
> -----
>
> configure.ac ba4ec1d930d2e74a5dad39bcd5f8ad5aec4411c0
> src/Makefile.am abef3d2c43314e040e205f33086ec550812812cc
> src/java/mesos.pom.in f817c944216274ebb3dac88304629d1adce20c13
> src/java/src/org/apache/mesos/Executor.java 8f4eaeeea0f96927e67b746e506310555e0e9a05
> src/java/src/org/apache/mesos/ExecutorDriver.java f7d288a322b96641fdafd4ff935722882ec00cc0
> src/java/src/org/apache/mesos/Log.java d27e4f9e226f1aa5588cf75cb8c0a117dbf3015d
> src/java/src/org/apache/mesos/MesosExecutorDriver.java ce146cdb33b3c80ea43866d304b3f1ff1a3e7c00
> src/java/src/org/apache/mesos/MesosSchedulerDriver.java ed4b4a31e55c5cc108150a8059cf08436c8b846e
> src/java/src/org/apache/mesos/Scheduler.java 55c4706c86ed68f13a9a33168381165900f7cfcf
> src/java/src/org/apache/mesos/SchedulerDriver.java 5b0ca391730b34f4d62c92d0e1b50193c0fa03ad
> src/java/src/org/apache/mesos/state/State.java dccb0ff0fa9f0e456e3ff2657520aee2ae2a7f5e
> src/java/src/org/apache/mesos/state/Variable.java bedb74b5aaafa7e2bb304f44e3bb972445edceca
> src/java/src/org/apache/mesos/state/ZooKeeperState.java f3d939f73192d2cec470ca1a7ff35e7b0a9b93a6
>
> Diff: https://reviews.apache.org/r/21051/diff/
>
>
> Testing
> -------
>
> 1. ./bootstrap && mkdir build && cd build && ../configure && make && make check
> 2. Ensure the existence of
> build/src/java/target/mesos-0.16.0-javadoc.jar
> build/src/java/target/mesos-0.16.0-shaded-protobuf.jar
> build/src/java/target/mesos-0.16.0-sources.jar
> build/src/java/target/mesos-0.16.0.jar
> build/src/java/target/protobuf-java-2.4.1.jar
> 3. do a `jar tvf build/src/java/target/mesos-0.16.0-shaded-protobuf.jar | grep protobuf | grep .class` and you should see the protobuf classes inside the org.apache.mesos.protobuf namespace.
>
>
> Thanks,
>
> Bernardo Gomez Palacio
>
>