You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by revans2 <gi...@git.apache.org> on 2015/09/15 21:36:47 UTC
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
GitHub user revans2 opened a pull request:
https://github.com/apache/storm/pull/736
STORM-1012 STORM-967 STORM-922 STORM-1042 Shaded everything that was not already shaded, and cleaned up build s…
…o the dependency-reduced pom.xml is used correctly.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/revans2/incubator-storm STORM-1042
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/736.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #736
----
commit 2e0597b16cd431290fed065463997d8e5412bc84
Author: Robert (Bobby) Evans <ev...@yahoo-inc.com>
Date: 2015-09-11T16:56:34Z
Shaded everything that was not already shaded, and cleaned up build so the dependency-reduced pom.xml is used correctly.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/736#issuecomment-140786708
Yes, I would love to see maven do the right thing from the beginning. But it just is not setup to be able to do this type of thing. I don't even know if it is possible. Essentially all we really need to do is to be able to change the storm-core pom.xml dynamically during the build to switch it over to the dependency reduced version. But I have no idea how to make that happen.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 [DO-...
Posted by ptgoetz <gi...@git.apache.org>.
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/736#issuecomment-145139265
+1
Thanks for your patience @revans2, and for bearing with me. :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/736#discussion_r39567827
--- Diff: dev-tools/travis/travis-script.sh ---
@@ -31,7 +31,7 @@ cd ${STORM_SRC_ROOT_DIR}
export STORM_TEST_TIMEOUT_MS=100000
# We now lean on Travis CI's implicit behavior, ```mvn clean install -DskipTests``` before running script
-mvn test -fae -Pnative
+mvn test -fae -Pnative -Pstorm-core && mvn test -fae -Pnative -Pstorm-more
--- End diff --
We could remove -Pnative when running test with -Pstorm-more.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/736#issuecomment-144106731
@ptgoetz If you are OK with doing a manual process instead of using the fancy plugin I think we can make it work.
```
mvn versions:set
mvn clean install -P storm-core -DskipTests
mvn clean install -P storm-more -DskipTests
mvn clean install -P dist -DskipTests
mvn clean deploy -P dist,storm-core,storm-more
```
Then you need to do the git tagging/branching manually as well.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by knusbaum <gi...@git.apache.org>.
Github user knusbaum commented on a diff in the pull request:
https://github.com/apache/storm/pull/736#discussion_r39576725
--- Diff: DEVELOPER.md ---
@@ -274,7 +261,8 @@ You can create a _distribution_ (like what you can download from Apache) as foll
do not use the Maven release plugin because creating an official release is the task of our release manager.
# First, build the code.
- $ mvn clean install # you may skip tests with `-DskipTests=true` to save time
+ $ mvn clean install -Pstorm-core # you may skip tests with `-DskipTests=true` to save time
+ $ mvn clean install -Pstorm-more # you may skip tests with `-DskipTests=true` to save time
--- End diff --
Duplicate?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by erikdw <gi...@git.apache.org>.
Github user erikdw commented on a diff in the pull request:
https://github.com/apache/storm/pull/736#discussion_r39771342
--- Diff: storm-core/pom.xml ---
@@ -385,168 +427,205 @@
<include>com.googlecode.json-simple:json-simple</include>
<include>org.clojure:math.numeric-tower</include>
<include>org.clojure:tools.cli</include>
+ <include>org.clojure:tools.logging</include>
<include>org.clojure:tools.macro</include>
+ <include>org.clojure:java.jmx</include>
<include>joda-time:joda-time</include>
<include>org.eclipse.jetty:*</include>
<include>com.fasterxml.jackson.core:*</include>
<include>com.fasterxml.jackson.dataformat:*</include>
+ <include>clout:clout</include>
+ <include>org.clojure:tools.namespace</include>
+ <include>cheshire:cheshire</include>
+ <include>org.clojure:core.incubator</include>
</includes>
</artifactSet>
-
<relocations>
<relocation>
- <pattern>com.fasterxml.jackson.core</pattern>
- <shadedPattern>com.fasterxml.jackson.storm.core</shadedPattern>
+ <pattern>cheshire</pattern>
+ <shadedPattern>org.apache.storm.shade.cheshire</shadedPattern>
</relocation>
<relocation>
- <pattern>com.fasterxml.jackson.dataformat</pattern>
- <shadedPattern>com.fasterxml.jackson.storm.dataformat</shadedPattern>
+ <pattern>clojure.tools.logging</pattern>
+ <shadedPattern>org.apache.storm.shade.clojure.tools.logging</shadedPattern>
</relocation>
<relocation>
- <pattern>org.apache.thrift</pattern>
- <shadedPattern>org.apache.thrift7</shadedPattern>
+ <pattern>clojure.core.incubator</pattern>
+ <shadedPattern>org.apache.storm.shade.clojure.core.incubator</shadedPattern>
</relocation>
<relocation>
- <pattern>org.jboss.netty</pattern>
- <shadedPattern>org.apache.storm.netty</shadedPattern>
+ <pattern>clojure.tools.namespace</pattern>
+ <shadedPattern>org.apache.storm.shade.clojure.tools.namespace</shadedPattern>
</relocation>
<relocation>
- <pattern>com.google.common</pattern>
- <shadedPattern>org.apache.storm.guava</shadedPattern>
+ <pattern>clout</pattern>
+ <shadedPattern>org.apache.storm.shade.clout</shadedPattern>
</relocation>
<relocation>
- <pattern>com.google.thirdparty</pattern>
- <shadedPattern>org.apache.storm.guava.thirdparty</shadedPattern>
+ <pattern>compojure</pattern>
+ <shadedPattern>org.apache.storm.shade.compojure</shadedPattern>
</relocation>
<relocation>
- <pattern>org.apache.http</pattern>
- <shadedPattern>org.apache.storm.http</shadedPattern>
+ <pattern>ns_tracker</pattern>
+ <shadedPattern>org.apache.storm.shade.ns_tracker</shadedPattern>
</relocation>
<relocation>
- <pattern>org.apache.zookeeper</pattern>
- <shadedPattern>org.apache.storm.zookeeper</shadedPattern>
+ <pattern>ns-tracker</pattern>
+ <shadedPattern>org.apache.storm.shade.ns-tracker</shadedPattern>
</relocation>
<relocation>
- <pattern>org.apache.curator</pattern>
- <shadedPattern>org.apache.storm.curator</shadedPattern>
+ <pattern>hiccup</pattern>
+ <shadedPattern>org.apache.storm.shade.hiccup</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>ring</pattern>
+ <shadedPattern>org.apache.storm.shade.ring</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>clj_time</pattern>
+ <shadedPattern>org.apache.storm.shade.clj_time</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>clj-time</pattern>
+ <shadedPattern>org.apache.storm.shade.clj-time</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>com.fasterxml</pattern>
+ <shadedPattern>org.apache.storm.shade.com.fasterxml</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>org.apache.thrift</pattern>
+ <!-- This pattern is inconsistent for backwards compatibility purposes. -->
+ <shadedPattern>org.apache.thrift7</shadedPattern>
</relocation>
<relocation>
<pattern>org.jboss.netty</pattern>
- <shadedPattern>org.apache.storm.netty</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.jboss.netty</shadedPattern>
</relocation>
<relocation>
<pattern>com.google.common</pattern>
- <shadedPattern>org.apache.storm.guava</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.com.google.common</shadedPattern>
</relocation>
<relocation>
<pattern>com.google.thirdparty</pattern>
- <shadedPattern>org.apache.storm.guava.thirdparty</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.com.google.thirdparty</shadedPattern>
</relocation>
<relocation>
<pattern>org.apache.http</pattern>
- <shadedPattern>org.apache.storm.http</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.apache.http</shadedPattern>
</relocation>
<relocation>
- <pattern>org.apache.zookeeper</pattern>
- <shadedPattern>org.apache.storm.zookeeper</shadedPattern>
+ <pattern>org.apache.curator</pattern>
+ <shadedPattern>org.apache.storm.shade.org.apache.curator</shadedPattern>
</relocation>
<relocation>
- <pattern>org.apache.jute</pattern>
- <shadedPattern>org.apache.storm.jute</shadedPattern>
+ <pattern>org.apache.zookeeper</pattern>
+ <shadedPattern>org.apache.storm.shade.org.apache.zookeeper</shadedPattern>
</relocation>
<relocation>
- <pattern>org.apache.curator</pattern>
- <shadedPattern>org.apache.storm.curator</shadedPattern>
+ <pattern>org.apache.jute</pattern>
+ <shadedPattern>org.apache.storm.shade.org.apache.jute</shadedPattern>
</relocation>
<relocation>
<pattern>carbonite</pattern>
- <shadedPattern>org.apache.storm.carbonite</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.carbonite</shadedPattern>
</relocation>
<relocation>
<pattern>com.twitter.chill</pattern>
- <shadedPattern>org.apache.storm.chill</shadedPattern>
- </relocation>
- <relocation>
- <pattern>org.objenesis</pattern>
- <shadedPattern>org.apache.storm.objenesis</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.com.twitter.chill</shadedPattern>
</relocation>
<relocation>
<pattern>org.tukaani.xz</pattern>
- <shadedPattern>org.apache.storm.xz</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.tukaani.xz</shadedPattern>
</relocation>
<relocation>
<pattern>org.yaml.snakeyaml</pattern>
- <shadedPattern>org.apache.storm.snakeyaml</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.yaml.snakeyaml</shadedPattern>
</relocation>
<relocation>
<pattern>org.jgrapht</pattern>
- <shadedPattern>org.apache.storm.jgrapht</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.jgrapht</shadedPattern>
</relocation>
<relocation>
<pattern>org.fusesource</pattern>
- <shadedPattern>org.apache.storm.fusesource</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.fusesource</shadedPattern>
</relocation>
<relocation>
<pattern>com.metamx.http.client</pattern>
- <shadedPattern>org.apache.storm.metamx.http.client</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.com.metamx.http.client</shadedPattern>
</relocation>
<relocation>
<pattern>org.apache.commons.io</pattern>
- <shadedPattern>org.apache.storm.commons.io</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.apache.commons.io</shadedPattern>
</relocation>
<relocation>
<pattern>org.apache.commons.codec</pattern>
- <shadedPattern>org.apache.storm.commons.codec</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.apache.commons.codec</shadedPattern>
</relocation>
<relocation>
<pattern>org.apache.commons.fileupload</pattern>
- <shadedPattern>org.apache.storm.commons.fileupload</shadedPattern>
- </relocation>
- <relocation>
- <pattern>org.apache.commons.compress</pattern>
- <shadedPattern>org.apache.storm.commons.compress</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.apache.commons.fileupload</shadedPattern>
</relocation>
<relocation>
<pattern>org.apache.commons.exec</pattern>
- <shadedPattern>org.apache.storm.commons.exec</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.apache.commons.exec</shadedPattern>
</relocation>
<relocation>
<pattern>org.apache.commons.lang</pattern>
- <shadedPattern>org.apache.storm.commons.lang</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.apache.commons.lang</shadedPattern>
</relocation>
<relocation>
<pattern>org.json.simple</pattern>
- <shadedPattern>org.apache.storm.json.simple</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.json.simple</shadedPattern>
</relocation>
<relocation>
<pattern>clojure.math</pattern>
- <shadedPattern>org.apache.storm.clojure.math</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.clojure.math</shadedPattern>
</relocation>
<relocation>
<pattern>clojure.tools.cli</pattern>
- <shadedPattern>org.apache.storm.clojure.tools.cli</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.clojure.tools.cli</shadedPattern>
</relocation>
<relocation>
<pattern>cljs.tools.cli</pattern>
- <shadedPattern>org.apache.storm.cljs.tools.cli</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.cljs.tools.cli</shadedPattern>
</relocation>
<relocation>
<pattern>clojure.tools.macro</pattern>
- <shadedPattern>org.apache.storm.clojure.tools.macro</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.clojure.tools.macro</shadedPattern>
</relocation>
<relocation>
<pattern>org.joda.time</pattern>
- <shadedPattern>org.apache.storm.joda.time</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.joda.time</shadedPattern>
</relocation>
<relocation>
<pattern>org.eclipse.jetty</pattern>
- <shadedPattern>org.apache.storm.jetty</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.eclipse.jetty</shadedPattern>
</relocation>
</relocations>
<transformers>
<transformer implementation="org.apache.storm.maven.shade.clojure.ClojureTransformer" />
</transformers>
<filters>
+ <!-- Several of these filters remove the .clj files from the shaded dependencies, even though only .clj files are in these jars.
+ The reason for this is a bit complex, but intentional. Durring the build process all of the dependency .clj files are
+ compiled down into .class files, and included in storm-core.jar. The regular shade thransformer handles these in
+ the majority of cases correctly. However, the ClojureTransformer does not shade everything correctly all the
+ time. Instead of spending a lot of time to get the ClojureTransformer to parse clojrue correctly we opted to remove
--- End diff --
typo: clojrue
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by knusbaum <gi...@git.apache.org>.
Github user knusbaum commented on the pull request:
https://github.com/apache/storm/pull/736#issuecomment-140810082
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on the pull request:
https://github.com/apache/storm/pull/736#issuecomment-140766502
> It would be nice to have a makefile or somesuch to implement basic targets like compile, install, and test, since maven's commands have gotten so complicated.
Yeah, I agree with the idea that we should be able to issue one command line and have that build everything correctly. I think layering something else like Make on top of Maven would complicate things. Maven should be capable of building a complex setup like this correctly without needing to wrap it in some other build tool or some script. Several devs use IDEs and other tools that are Maven-aware, and it would be good to keep things working smoothly.
I think this should be something for another Jira Issue, as it could take a lot of work to figure out how to tell Maven what we want to do.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/736#discussion_r39574688
--- Diff: storm-dist/binary/src/main/assembly/binary.xml ---
@@ -30,38 +30,6 @@
<useProjectArtifact>false</useProjectArtifact>
<outputDirectory>lib</outputDirectory>
<unpack>false</unpack>
- <excludes>
--- End diff --
That was one thing what I didn't understand, and it was a sign that we didn't shade libraries properly.
(Because assembly can see those packages...)
Do I understand correctly?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by erikdw <gi...@git.apache.org>.
Github user erikdw commented on a diff in the pull request:
https://github.com/apache/storm/pull/736#discussion_r39771292
--- Diff: storm-core/pom.xml ---
@@ -385,168 +427,205 @@
<include>com.googlecode.json-simple:json-simple</include>
<include>org.clojure:math.numeric-tower</include>
<include>org.clojure:tools.cli</include>
+ <include>org.clojure:tools.logging</include>
<include>org.clojure:tools.macro</include>
+ <include>org.clojure:java.jmx</include>
<include>joda-time:joda-time</include>
<include>org.eclipse.jetty:*</include>
<include>com.fasterxml.jackson.core:*</include>
<include>com.fasterxml.jackson.dataformat:*</include>
+ <include>clout:clout</include>
+ <include>org.clojure:tools.namespace</include>
+ <include>cheshire:cheshire</include>
+ <include>org.clojure:core.incubator</include>
</includes>
</artifactSet>
-
<relocations>
<relocation>
- <pattern>com.fasterxml.jackson.core</pattern>
- <shadedPattern>com.fasterxml.jackson.storm.core</shadedPattern>
+ <pattern>cheshire</pattern>
+ <shadedPattern>org.apache.storm.shade.cheshire</shadedPattern>
</relocation>
<relocation>
- <pattern>com.fasterxml.jackson.dataformat</pattern>
- <shadedPattern>com.fasterxml.jackson.storm.dataformat</shadedPattern>
+ <pattern>clojure.tools.logging</pattern>
+ <shadedPattern>org.apache.storm.shade.clojure.tools.logging</shadedPattern>
</relocation>
<relocation>
- <pattern>org.apache.thrift</pattern>
- <shadedPattern>org.apache.thrift7</shadedPattern>
+ <pattern>clojure.core.incubator</pattern>
+ <shadedPattern>org.apache.storm.shade.clojure.core.incubator</shadedPattern>
</relocation>
<relocation>
- <pattern>org.jboss.netty</pattern>
- <shadedPattern>org.apache.storm.netty</shadedPattern>
+ <pattern>clojure.tools.namespace</pattern>
+ <shadedPattern>org.apache.storm.shade.clojure.tools.namespace</shadedPattern>
</relocation>
<relocation>
- <pattern>com.google.common</pattern>
- <shadedPattern>org.apache.storm.guava</shadedPattern>
+ <pattern>clout</pattern>
+ <shadedPattern>org.apache.storm.shade.clout</shadedPattern>
</relocation>
<relocation>
- <pattern>com.google.thirdparty</pattern>
- <shadedPattern>org.apache.storm.guava.thirdparty</shadedPattern>
+ <pattern>compojure</pattern>
+ <shadedPattern>org.apache.storm.shade.compojure</shadedPattern>
</relocation>
<relocation>
- <pattern>org.apache.http</pattern>
- <shadedPattern>org.apache.storm.http</shadedPattern>
+ <pattern>ns_tracker</pattern>
+ <shadedPattern>org.apache.storm.shade.ns_tracker</shadedPattern>
</relocation>
<relocation>
- <pattern>org.apache.zookeeper</pattern>
- <shadedPattern>org.apache.storm.zookeeper</shadedPattern>
+ <pattern>ns-tracker</pattern>
+ <shadedPattern>org.apache.storm.shade.ns-tracker</shadedPattern>
</relocation>
<relocation>
- <pattern>org.apache.curator</pattern>
- <shadedPattern>org.apache.storm.curator</shadedPattern>
+ <pattern>hiccup</pattern>
+ <shadedPattern>org.apache.storm.shade.hiccup</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>ring</pattern>
+ <shadedPattern>org.apache.storm.shade.ring</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>clj_time</pattern>
+ <shadedPattern>org.apache.storm.shade.clj_time</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>clj-time</pattern>
+ <shadedPattern>org.apache.storm.shade.clj-time</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>com.fasterxml</pattern>
+ <shadedPattern>org.apache.storm.shade.com.fasterxml</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>org.apache.thrift</pattern>
+ <!-- This pattern is inconsistent for backwards compatibility purposes. -->
+ <shadedPattern>org.apache.thrift7</shadedPattern>
</relocation>
<relocation>
<pattern>org.jboss.netty</pattern>
- <shadedPattern>org.apache.storm.netty</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.jboss.netty</shadedPattern>
</relocation>
<relocation>
<pattern>com.google.common</pattern>
- <shadedPattern>org.apache.storm.guava</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.com.google.common</shadedPattern>
</relocation>
<relocation>
<pattern>com.google.thirdparty</pattern>
- <shadedPattern>org.apache.storm.guava.thirdparty</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.com.google.thirdparty</shadedPattern>
</relocation>
<relocation>
<pattern>org.apache.http</pattern>
- <shadedPattern>org.apache.storm.http</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.apache.http</shadedPattern>
</relocation>
<relocation>
- <pattern>org.apache.zookeeper</pattern>
- <shadedPattern>org.apache.storm.zookeeper</shadedPattern>
+ <pattern>org.apache.curator</pattern>
+ <shadedPattern>org.apache.storm.shade.org.apache.curator</shadedPattern>
</relocation>
<relocation>
- <pattern>org.apache.jute</pattern>
- <shadedPattern>org.apache.storm.jute</shadedPattern>
+ <pattern>org.apache.zookeeper</pattern>
+ <shadedPattern>org.apache.storm.shade.org.apache.zookeeper</shadedPattern>
</relocation>
<relocation>
- <pattern>org.apache.curator</pattern>
- <shadedPattern>org.apache.storm.curator</shadedPattern>
+ <pattern>org.apache.jute</pattern>
+ <shadedPattern>org.apache.storm.shade.org.apache.jute</shadedPattern>
</relocation>
<relocation>
<pattern>carbonite</pattern>
- <shadedPattern>org.apache.storm.carbonite</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.carbonite</shadedPattern>
</relocation>
<relocation>
<pattern>com.twitter.chill</pattern>
- <shadedPattern>org.apache.storm.chill</shadedPattern>
- </relocation>
- <relocation>
- <pattern>org.objenesis</pattern>
- <shadedPattern>org.apache.storm.objenesis</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.com.twitter.chill</shadedPattern>
</relocation>
<relocation>
<pattern>org.tukaani.xz</pattern>
- <shadedPattern>org.apache.storm.xz</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.tukaani.xz</shadedPattern>
</relocation>
<relocation>
<pattern>org.yaml.snakeyaml</pattern>
- <shadedPattern>org.apache.storm.snakeyaml</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.yaml.snakeyaml</shadedPattern>
</relocation>
<relocation>
<pattern>org.jgrapht</pattern>
- <shadedPattern>org.apache.storm.jgrapht</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.jgrapht</shadedPattern>
</relocation>
<relocation>
<pattern>org.fusesource</pattern>
- <shadedPattern>org.apache.storm.fusesource</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.fusesource</shadedPattern>
</relocation>
<relocation>
<pattern>com.metamx.http.client</pattern>
- <shadedPattern>org.apache.storm.metamx.http.client</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.com.metamx.http.client</shadedPattern>
</relocation>
<relocation>
<pattern>org.apache.commons.io</pattern>
- <shadedPattern>org.apache.storm.commons.io</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.apache.commons.io</shadedPattern>
</relocation>
<relocation>
<pattern>org.apache.commons.codec</pattern>
- <shadedPattern>org.apache.storm.commons.codec</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.apache.commons.codec</shadedPattern>
</relocation>
<relocation>
<pattern>org.apache.commons.fileupload</pattern>
- <shadedPattern>org.apache.storm.commons.fileupload</shadedPattern>
- </relocation>
- <relocation>
- <pattern>org.apache.commons.compress</pattern>
- <shadedPattern>org.apache.storm.commons.compress</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.apache.commons.fileupload</shadedPattern>
</relocation>
<relocation>
<pattern>org.apache.commons.exec</pattern>
- <shadedPattern>org.apache.storm.commons.exec</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.apache.commons.exec</shadedPattern>
</relocation>
<relocation>
<pattern>org.apache.commons.lang</pattern>
- <shadedPattern>org.apache.storm.commons.lang</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.apache.commons.lang</shadedPattern>
</relocation>
<relocation>
<pattern>org.json.simple</pattern>
- <shadedPattern>org.apache.storm.json.simple</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.json.simple</shadedPattern>
</relocation>
<relocation>
<pattern>clojure.math</pattern>
- <shadedPattern>org.apache.storm.clojure.math</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.clojure.math</shadedPattern>
</relocation>
<relocation>
<pattern>clojure.tools.cli</pattern>
- <shadedPattern>org.apache.storm.clojure.tools.cli</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.clojure.tools.cli</shadedPattern>
</relocation>
<relocation>
<pattern>cljs.tools.cli</pattern>
- <shadedPattern>org.apache.storm.cljs.tools.cli</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.cljs.tools.cli</shadedPattern>
</relocation>
<relocation>
<pattern>clojure.tools.macro</pattern>
- <shadedPattern>org.apache.storm.clojure.tools.macro</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.clojure.tools.macro</shadedPattern>
</relocation>
<relocation>
<pattern>org.joda.time</pattern>
- <shadedPattern>org.apache.storm.joda.time</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.joda.time</shadedPattern>
</relocation>
<relocation>
<pattern>org.eclipse.jetty</pattern>
- <shadedPattern>org.apache.storm.jetty</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.eclipse.jetty</shadedPattern>
</relocation>
</relocations>
<transformers>
<transformer implementation="org.apache.storm.maven.shade.clojure.ClojureTransformer" />
</transformers>
<filters>
+ <!-- Several of these filters remove the .clj files from the shaded dependencies, even though only .clj files are in these jars.
+ The reason for this is a bit complex, but intentional. Durring the build process all of the dependency .clj files are
+ compiled down into .class files, and included in storm-core.jar. The regular shade thransformer handles these in
--- End diff --
typo: thransformer
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/storm/pull/736#issuecomment-140570127
May be a beginner questions, I have some questions regarding changeset.
1. Seems like we're excluding "clj" files from shaded clojure library dependencies which doesn't have any "class" files in jar. Could you explain how it works?
2. I don't know what splitting build steps changes build result. I'm seeing two dependency-reduced-pom.xml but can't find major differences except additional shading. Could you explain it, too?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by knusbaum <gi...@git.apache.org>.
Github user knusbaum commented on a diff in the pull request:
https://github.com/apache/storm/pull/736#discussion_r39577652
--- Diff: DEVELOPER.md ---
@@ -274,7 +261,8 @@ You can create a _distribution_ (like what you can download from Apache) as foll
do not use the Maven release plugin because creating an official release is the task of our release manager.
# First, build the code.
- $ mvn clean install # you may skip tests with `-DskipTests=true` to save time
+ $ mvn clean install -Pstorm-core # you may skip tests with `-DskipTests=true` to save time
+ $ mvn clean install -Pstorm-more # you may skip tests with `-DskipTests=true` to save time
--- End diff --
Ahh. *more* and *core*
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/736#issuecomment-144491158
I filed https://issues.apache.org/jira/browse/MSHADE-206 to see if there is something we can do as part of the shade plugin to address this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by knusbaum <gi...@git.apache.org>.
Github user knusbaum commented on the pull request:
https://github.com/apache/storm/pull/736#issuecomment-140578252
Compiles/runs fine for me.
It would be nice to have a makefile or somesuch to implement basic targets like compile, install, and test, since maven's commands have gotten so complicated.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by knusbaum <gi...@git.apache.org>.
Github user knusbaum commented on a diff in the pull request:
https://github.com/apache/storm/pull/736#discussion_r39576902
--- Diff: storm-core/pom.xml ---
@@ -385,168 +427,199 @@
<include>com.googlecode.json-simple:json-simple</include>
<include>org.clojure:math.numeric-tower</include>
<include>org.clojure:tools.cli</include>
+ <include>org.clojure:tools.logging</include>
<include>org.clojure:tools.macro</include>
+ <include>org.clojure:java.jmx</include>
<include>joda-time:joda-time</include>
<include>org.eclipse.jetty:*</include>
<include>com.fasterxml.jackson.core:*</include>
<include>com.fasterxml.jackson.dataformat:*</include>
+ <include>clout:clout</include>
+ <include>org.clojure:tools.namespace</include>
+ <include>cheshire:cheshire</include>
+ <include>org.clojure:core.incubator</include>
</includes>
</artifactSet>
-
<relocations>
<relocation>
- <pattern>com.fasterxml.jackson.core</pattern>
- <shadedPattern>com.fasterxml.jackson.storm.core</shadedPattern>
+ <pattern>cheshire</pattern>
+ <shadedPattern>org.apache.storm.shade.cheshire</shadedPattern>
</relocation>
<relocation>
- <pattern>com.fasterxml.jackson.dataformat</pattern>
- <shadedPattern>com.fasterxml.jackson.storm.dataformat</shadedPattern>
+ <pattern>clojure.tools.logging</pattern>
+ <shadedPattern>org.apache.storm.shade.clojure.tools.logging</shadedPattern>
</relocation>
<relocation>
- <pattern>org.apache.thrift</pattern>
- <shadedPattern>org.apache.thrift7</shadedPattern>
+ <pattern>clojure.core.incubator</pattern>
+ <shadedPattern>org.apache.storm.shade.clojure.core.incubator</shadedPattern>
</relocation>
<relocation>
- <pattern>org.jboss.netty</pattern>
- <shadedPattern>org.apache.storm.netty</shadedPattern>
+ <pattern>clojure.tools.namespace</pattern>
+ <shadedPattern>org.apache.storm.shade.clojure.tools.namespace</shadedPattern>
</relocation>
<relocation>
- <pattern>com.google.common</pattern>
- <shadedPattern>org.apache.storm.guava</shadedPattern>
+ <pattern>clout</pattern>
+ <shadedPattern>org.apache.storm.shade.clout</shadedPattern>
</relocation>
<relocation>
- <pattern>com.google.thirdparty</pattern>
- <shadedPattern>org.apache.storm.guava.thirdparty</shadedPattern>
+ <pattern>compojure</pattern>
+ <shadedPattern>org.apache.storm.shade.compojure</shadedPattern>
</relocation>
<relocation>
- <pattern>org.apache.http</pattern>
- <shadedPattern>org.apache.storm.http</shadedPattern>
+ <pattern>ns_tracker</pattern>
+ <shadedPattern>org.apache.storm.shade.ns_tracker</shadedPattern>
</relocation>
<relocation>
- <pattern>org.apache.zookeeper</pattern>
- <shadedPattern>org.apache.storm.zookeeper</shadedPattern>
+ <pattern>ns-tracker</pattern>
+ <shadedPattern>org.apache.storm.shade.ns-tracker</shadedPattern>
</relocation>
<relocation>
- <pattern>org.apache.curator</pattern>
- <shadedPattern>org.apache.storm.curator</shadedPattern>
+ <pattern>hiccup</pattern>
+ <shadedPattern>org.apache.storm.shade.hiccup</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>ring</pattern>
+ <shadedPattern>org.apache.storm.shade.ring</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>clj_time</pattern>
+ <shadedPattern>org.apache.storm.shade.clj_time</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>clj-time</pattern>
+ <shadedPattern>org.apache.storm.shade.clj-time</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>com.fasterxml</pattern>
+ <shadedPattern>org.apache.storm.shade.com.fasterxml</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>org.apache.thrift</pattern>
+ <!-- This pattern is inconsistent for backwards compatability purposes. -->
--- End diff --
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/736#issuecomment-140513112
I am working on a version for 0.10.0 too, This is against master
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by ptgoetz <gi...@git.apache.org>.
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/736#issuecomment-144190863
@revans2 That seems reasonable to me. Let me make sure that works for deploying to Nexus and get back to you. If it works then I'm fine with these changes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by erikdw <gi...@git.apache.org>.
Github user erikdw commented on a diff in the pull request:
https://github.com/apache/storm/pull/736#discussion_r39771236
--- Diff: storm-core/pom.xml ---
@@ -385,168 +427,205 @@
<include>com.googlecode.json-simple:json-simple</include>
<include>org.clojure:math.numeric-tower</include>
<include>org.clojure:tools.cli</include>
+ <include>org.clojure:tools.logging</include>
<include>org.clojure:tools.macro</include>
+ <include>org.clojure:java.jmx</include>
<include>joda-time:joda-time</include>
<include>org.eclipse.jetty:*</include>
<include>com.fasterxml.jackson.core:*</include>
<include>com.fasterxml.jackson.dataformat:*</include>
+ <include>clout:clout</include>
+ <include>org.clojure:tools.namespace</include>
+ <include>cheshire:cheshire</include>
+ <include>org.clojure:core.incubator</include>
</includes>
</artifactSet>
-
<relocations>
<relocation>
- <pattern>com.fasterxml.jackson.core</pattern>
- <shadedPattern>com.fasterxml.jackson.storm.core</shadedPattern>
+ <pattern>cheshire</pattern>
+ <shadedPattern>org.apache.storm.shade.cheshire</shadedPattern>
</relocation>
<relocation>
- <pattern>com.fasterxml.jackson.dataformat</pattern>
- <shadedPattern>com.fasterxml.jackson.storm.dataformat</shadedPattern>
+ <pattern>clojure.tools.logging</pattern>
+ <shadedPattern>org.apache.storm.shade.clojure.tools.logging</shadedPattern>
</relocation>
<relocation>
- <pattern>org.apache.thrift</pattern>
- <shadedPattern>org.apache.thrift7</shadedPattern>
+ <pattern>clojure.core.incubator</pattern>
+ <shadedPattern>org.apache.storm.shade.clojure.core.incubator</shadedPattern>
</relocation>
<relocation>
- <pattern>org.jboss.netty</pattern>
- <shadedPattern>org.apache.storm.netty</shadedPattern>
+ <pattern>clojure.tools.namespace</pattern>
+ <shadedPattern>org.apache.storm.shade.clojure.tools.namespace</shadedPattern>
</relocation>
<relocation>
- <pattern>com.google.common</pattern>
- <shadedPattern>org.apache.storm.guava</shadedPattern>
+ <pattern>clout</pattern>
+ <shadedPattern>org.apache.storm.shade.clout</shadedPattern>
</relocation>
<relocation>
- <pattern>com.google.thirdparty</pattern>
- <shadedPattern>org.apache.storm.guava.thirdparty</shadedPattern>
+ <pattern>compojure</pattern>
+ <shadedPattern>org.apache.storm.shade.compojure</shadedPattern>
</relocation>
<relocation>
- <pattern>org.apache.http</pattern>
- <shadedPattern>org.apache.storm.http</shadedPattern>
+ <pattern>ns_tracker</pattern>
+ <shadedPattern>org.apache.storm.shade.ns_tracker</shadedPattern>
</relocation>
<relocation>
- <pattern>org.apache.zookeeper</pattern>
- <shadedPattern>org.apache.storm.zookeeper</shadedPattern>
+ <pattern>ns-tracker</pattern>
+ <shadedPattern>org.apache.storm.shade.ns-tracker</shadedPattern>
</relocation>
<relocation>
- <pattern>org.apache.curator</pattern>
- <shadedPattern>org.apache.storm.curator</shadedPattern>
+ <pattern>hiccup</pattern>
+ <shadedPattern>org.apache.storm.shade.hiccup</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>ring</pattern>
+ <shadedPattern>org.apache.storm.shade.ring</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>clj_time</pattern>
+ <shadedPattern>org.apache.storm.shade.clj_time</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>clj-time</pattern>
+ <shadedPattern>org.apache.storm.shade.clj-time</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>com.fasterxml</pattern>
+ <shadedPattern>org.apache.storm.shade.com.fasterxml</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>org.apache.thrift</pattern>
+ <!-- This pattern is inconsistent for backwards compatibility purposes. -->
+ <shadedPattern>org.apache.thrift7</shadedPattern>
</relocation>
<relocation>
<pattern>org.jboss.netty</pattern>
- <shadedPattern>org.apache.storm.netty</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.jboss.netty</shadedPattern>
</relocation>
<relocation>
<pattern>com.google.common</pattern>
- <shadedPattern>org.apache.storm.guava</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.com.google.common</shadedPattern>
</relocation>
<relocation>
<pattern>com.google.thirdparty</pattern>
- <shadedPattern>org.apache.storm.guava.thirdparty</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.com.google.thirdparty</shadedPattern>
</relocation>
<relocation>
<pattern>org.apache.http</pattern>
- <shadedPattern>org.apache.storm.http</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.apache.http</shadedPattern>
</relocation>
<relocation>
- <pattern>org.apache.zookeeper</pattern>
- <shadedPattern>org.apache.storm.zookeeper</shadedPattern>
+ <pattern>org.apache.curator</pattern>
+ <shadedPattern>org.apache.storm.shade.org.apache.curator</shadedPattern>
</relocation>
<relocation>
- <pattern>org.apache.jute</pattern>
- <shadedPattern>org.apache.storm.jute</shadedPattern>
+ <pattern>org.apache.zookeeper</pattern>
+ <shadedPattern>org.apache.storm.shade.org.apache.zookeeper</shadedPattern>
</relocation>
<relocation>
- <pattern>org.apache.curator</pattern>
- <shadedPattern>org.apache.storm.curator</shadedPattern>
+ <pattern>org.apache.jute</pattern>
+ <shadedPattern>org.apache.storm.shade.org.apache.jute</shadedPattern>
</relocation>
<relocation>
<pattern>carbonite</pattern>
- <shadedPattern>org.apache.storm.carbonite</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.carbonite</shadedPattern>
</relocation>
<relocation>
<pattern>com.twitter.chill</pattern>
- <shadedPattern>org.apache.storm.chill</shadedPattern>
- </relocation>
- <relocation>
- <pattern>org.objenesis</pattern>
- <shadedPattern>org.apache.storm.objenesis</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.com.twitter.chill</shadedPattern>
</relocation>
<relocation>
<pattern>org.tukaani.xz</pattern>
- <shadedPattern>org.apache.storm.xz</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.tukaani.xz</shadedPattern>
</relocation>
<relocation>
<pattern>org.yaml.snakeyaml</pattern>
- <shadedPattern>org.apache.storm.snakeyaml</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.yaml.snakeyaml</shadedPattern>
</relocation>
<relocation>
<pattern>org.jgrapht</pattern>
- <shadedPattern>org.apache.storm.jgrapht</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.jgrapht</shadedPattern>
</relocation>
<relocation>
<pattern>org.fusesource</pattern>
- <shadedPattern>org.apache.storm.fusesource</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.fusesource</shadedPattern>
</relocation>
<relocation>
<pattern>com.metamx.http.client</pattern>
- <shadedPattern>org.apache.storm.metamx.http.client</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.com.metamx.http.client</shadedPattern>
</relocation>
<relocation>
<pattern>org.apache.commons.io</pattern>
- <shadedPattern>org.apache.storm.commons.io</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.apache.commons.io</shadedPattern>
</relocation>
<relocation>
<pattern>org.apache.commons.codec</pattern>
- <shadedPattern>org.apache.storm.commons.codec</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.apache.commons.codec</shadedPattern>
</relocation>
<relocation>
<pattern>org.apache.commons.fileupload</pattern>
- <shadedPattern>org.apache.storm.commons.fileupload</shadedPattern>
- </relocation>
- <relocation>
- <pattern>org.apache.commons.compress</pattern>
- <shadedPattern>org.apache.storm.commons.compress</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.apache.commons.fileupload</shadedPattern>
</relocation>
<relocation>
<pattern>org.apache.commons.exec</pattern>
- <shadedPattern>org.apache.storm.commons.exec</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.apache.commons.exec</shadedPattern>
</relocation>
<relocation>
<pattern>org.apache.commons.lang</pattern>
- <shadedPattern>org.apache.storm.commons.lang</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.apache.commons.lang</shadedPattern>
</relocation>
<relocation>
<pattern>org.json.simple</pattern>
- <shadedPattern>org.apache.storm.json.simple</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.json.simple</shadedPattern>
</relocation>
<relocation>
<pattern>clojure.math</pattern>
- <shadedPattern>org.apache.storm.clojure.math</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.clojure.math</shadedPattern>
</relocation>
<relocation>
<pattern>clojure.tools.cli</pattern>
- <shadedPattern>org.apache.storm.clojure.tools.cli</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.clojure.tools.cli</shadedPattern>
</relocation>
<relocation>
<pattern>cljs.tools.cli</pattern>
- <shadedPattern>org.apache.storm.cljs.tools.cli</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.cljs.tools.cli</shadedPattern>
</relocation>
<relocation>
<pattern>clojure.tools.macro</pattern>
- <shadedPattern>org.apache.storm.clojure.tools.macro</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.clojure.tools.macro</shadedPattern>
</relocation>
<relocation>
<pattern>org.joda.time</pattern>
- <shadedPattern>org.apache.storm.joda.time</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.joda.time</shadedPattern>
</relocation>
<relocation>
<pattern>org.eclipse.jetty</pattern>
- <shadedPattern>org.apache.storm.jetty</shadedPattern>
+ <shadedPattern>org.apache.storm.shade.org.eclipse.jetty</shadedPattern>
</relocation>
</relocations>
<transformers>
<transformer implementation="org.apache.storm.maven.shade.clojure.ClojureTransformer" />
</transformers>
<filters>
+ <!-- Several of these filters remove the .clj files from the shaded dependencies, even though only .clj files are in these jars.
+ The reason for this is a bit complex, but intentional. Durring the build process all of the dependency .clj files are
--- End diff --
typo: durring
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on the pull request:
https://github.com/apache/storm/pull/736#issuecomment-140829978
+1 Looks OK to me.
Built with an empty maven repo & ran all tests following the instructions:
`mvn clean install -Pstorm-core -DskipTests && mvn clean install -Pstorm-more -DskipTests`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/736#issuecomment-143232892
@ptgoetz
What exactly is the release process? I don't see it documented anywhere? Our internally release process is mostly.
```
mvn -Pstorm-core,storm-more versions:set -DnewVersion=${NEW_VERSION}
mvn -Pstorm-core,storm-more versions:update-child-modules
mvn -Pnative -Pstorm-core -Dworker-launcher.conf.dir=${EXEC_CONF_DIR} install
mvn -Pstorm-more install
```
we typically will package thins a little differently here because we don't use the .tgz or .zip directly but the following works fine.
```
cd storm-dist/binary
mvn clean package
```
We typically don't check in the version changes so revert them with
```
mvn versions:revert
mvn versions:update-child-modules
```
and just tag the base. But checking in the specific version to a new branch is helpful too.
How exactly are you pushing the finished jars/poms to the maven repository?
Is it really that hard to manually use git manually instead of relying on the release plugin?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by ptgoetz <gi...@git.apache.org>.
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/736#issuecomment-143268083
@revans2 The release process for pushing maven artifacts is:
```
mvn release:prepare -P dist
mvn release:perform -P dist
```
That prepares and signs all the maven artifacts (including javadoc, etc.) and pushes them to repository.apache.org. It also tags the release in git and rewrites the version numbers across all poms and commits those changes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/736#discussion_r39569716
--- Diff: storm-core/pom.xml ---
@@ -385,168 +427,199 @@
<include>com.googlecode.json-simple:json-simple</include>
<include>org.clojure:math.numeric-tower</include>
<include>org.clojure:tools.cli</include>
+ <include>org.clojure:tools.logging</include>
<include>org.clojure:tools.macro</include>
+ <include>org.clojure:java.jmx</include>
<include>joda-time:joda-time</include>
<include>org.eclipse.jetty:*</include>
<include>com.fasterxml.jackson.core:*</include>
<include>com.fasterxml.jackson.dataformat:*</include>
+ <include>clout:clout</include>
+ <include>org.clojure:tools.namespace</include>
+ <include>cheshire:cheshire</include>
+ <include>org.clojure:core.incubator</include>
</includes>
</artifactSet>
-
<relocations>
<relocation>
- <pattern>com.fasterxml.jackson.core</pattern>
- <shadedPattern>com.fasterxml.jackson.storm.core</shadedPattern>
+ <pattern>cheshire</pattern>
+ <shadedPattern>org.apache.storm.shade.cheshire</shadedPattern>
</relocation>
<relocation>
- <pattern>com.fasterxml.jackson.dataformat</pattern>
- <shadedPattern>com.fasterxml.jackson.storm.dataformat</shadedPattern>
+ <pattern>clojure.tools.logging</pattern>
+ <shadedPattern>org.apache.storm.shade.clojure.tools.logging</shadedPattern>
</relocation>
<relocation>
- <pattern>org.apache.thrift</pattern>
- <shadedPattern>org.apache.thrift7</shadedPattern>
+ <pattern>clojure.core.incubator</pattern>
+ <shadedPattern>org.apache.storm.shade.clojure.core.incubator</shadedPattern>
</relocation>
<relocation>
- <pattern>org.jboss.netty</pattern>
- <shadedPattern>org.apache.storm.netty</shadedPattern>
+ <pattern>clojure.tools.namespace</pattern>
+ <shadedPattern>org.apache.storm.shade.clojure.tools.namespace</shadedPattern>
</relocation>
<relocation>
- <pattern>com.google.common</pattern>
- <shadedPattern>org.apache.storm.guava</shadedPattern>
+ <pattern>clout</pattern>
+ <shadedPattern>org.apache.storm.shade.clout</shadedPattern>
</relocation>
<relocation>
- <pattern>com.google.thirdparty</pattern>
- <shadedPattern>org.apache.storm.guava.thirdparty</shadedPattern>
+ <pattern>compojure</pattern>
+ <shadedPattern>org.apache.storm.shade.compojure</shadedPattern>
</relocation>
<relocation>
- <pattern>org.apache.http</pattern>
- <shadedPattern>org.apache.storm.http</shadedPattern>
+ <pattern>ns_tracker</pattern>
+ <shadedPattern>org.apache.storm.shade.ns_tracker</shadedPattern>
</relocation>
<relocation>
- <pattern>org.apache.zookeeper</pattern>
- <shadedPattern>org.apache.storm.zookeeper</shadedPattern>
+ <pattern>ns-tracker</pattern>
+ <shadedPattern>org.apache.storm.shade.ns-tracker</shadedPattern>
</relocation>
<relocation>
- <pattern>org.apache.curator</pattern>
- <shadedPattern>org.apache.storm.curator</shadedPattern>
+ <pattern>hiccup</pattern>
+ <shadedPattern>org.apache.storm.shade.hiccup</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>ring</pattern>
+ <shadedPattern>org.apache.storm.shade.ring</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>clj_time</pattern>
+ <shadedPattern>org.apache.storm.shade.clj_time</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>clj-time</pattern>
+ <shadedPattern>org.apache.storm.shade.clj-time</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>com.fasterxml</pattern>
+ <shadedPattern>org.apache.storm.shade.com.fasterxml</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>org.apache.thrift</pattern>
+ <!-- This pattern is inconsistent for backwards compatability purposes. -->
--- End diff --
Trivial typo here, compatability -> compatibility
Btw, we're always having possibilities of breaking backward compatibility in point of users view when more libraries would be shaded/relocated.
I mean users should include additional dependencies to their topology and rebuild topology package.
So IMO, with explanation we can fix the thrift shaded pattern now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/736#issuecomment-144487889
@ptgoetz if it does work for you, I'll file a follow on JIRA to look at how we can clean this entire process up. To me it feels like a bug in the shade plugin, but I don't know for sure.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/736#issuecomment-142949954
@ptgoetz @HeartSaVioR are there any objections to merging this in? How about #741 for 0.10.X.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/736#discussion_r39577362
--- Diff: DEVELOPER.md ---
@@ -274,7 +261,8 @@ You can create a _distribution_ (like what you can download from Apache) as foll
do not use the Maven release plugin because creating an official release is the task of our release manager.
# First, build the code.
- $ mvn clean install # you may skip tests with `-DskipTests=true` to save time
+ $ mvn clean install -Pstorm-core # you may skip tests with `-DskipTests=true` to save time
+ $ mvn clean install -Pstorm-more # you may skip tests with `-DskipTests=true` to save time
--- End diff --
No, its profiles are different. It shows that build steps need to be split into two.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 [DO-...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/736#issuecomment-145117649
Turns out that this is a bug in maven itself, along with @ptgoetz's comment. If you want this to work correctly you need to use version maven 3.2.5 or below. Anything above that will package dependencies incorrectly.
I will revert my changes to split the build into two, revert #621, update the documentation accordingly, and see what I can do about making sure travis gets the right version of maven.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by ptgoetz <gi...@git.apache.org>.
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/736#issuecomment-140895741
@revans2 @d2r @knusbaum Can you guys summarize all the issues this addresses? It looks like it covers a lot. I noticed that at some point an issue with shading was introduced in master (manifest as the fat jar in flux-examples not including its dependencies), but this PR seems to cover a lot more.
I need a little time to review this before we merge it in. I mainly want to make sure it doesn't cause any issues with the maven release plugin (which is a painfully fickle beast ;) ).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by knusbaum <gi...@git.apache.org>.
Github user knusbaum commented on the pull request:
https://github.com/apache/storm/pull/736#issuecomment-140782564
Another Jira is fine. I definitely agree we should aim to have Maven behave properly.
As you said, though, it could take a lot of work to figure out. I'm assuming many are going to write their own build scripts or makefiles around Maven anyway, so an *official* makefile or build script might be a convenient stop-gap.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 [DO-...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/736#issuecomment-145139772
@ptgoetz I totally get it. the split build was a really ugly hack, I am very glad that I understand now why I was getting messed up jars.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/736#discussion_r40823287
--- Diff: DEVELOPER.md ---
@@ -222,6 +222,11 @@ To pull in a merge request you should generally follow the command line instruct
# Build the code and run the tests
+Storm has some oddities with packaging and plugin dependencies. We build our own plugins and shade storm-core,
+but have sub-modules that depend on storm-core. Because of this the build needs to be split into two parts.
--- End diff --
Essentially what happens is that if you build a multi-module project where one module depends on another module that is a part of the same build, maven tries to be smart and will use the pom.xml and .jar or .class directory from the module that is a dependency.
So if we have a project set up like the following where we have b depends on a, and a is shaded. b will see different versions of the code and pom.xml from a depending on what maven command is run.
- base/
- pom.xml
- a/
- pom.xml
- dependency-reduce-pom.xml
- target/
- classes/
- a.jar (shaded)
- a.orig.jar (no-shaded)
- b/
- pom.xml (depends on a)
If I run `mvn clean compile` the classpath for compiling b will include `base/a/target/classes` which is not shaded, and if b is written to use a shaded class it will fail to compile. b will also see the full set of non-shaded dependencies from a, so if b is creating an uber jar, it will assume that all of those dependencies will be provided by a, when in reality they will not be.
If I run `mvn clean package` or `mvn clean install` b will see the shaded a.jar on its classpath, but it will still see the full non-shaded set of a's dependencies.
The only way I found to make b use the dependency-reduced-pom.xml from a, is to first build/install a without b, and then build/install b without a. That way a will install the shaded jar and dependency-reduced-pom.xml into the local repository. Then b will get its dependencies from the local repo instead of trying to rebuild a.
I'll try to add this to the documentation.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on the pull request:
https://github.com/apache/storm/pull/736#issuecomment-142948581
Still +1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/736#discussion_r39639639
--- Diff: storm-core/pom.xml ---
@@ -385,168 +427,199 @@
<include>com.googlecode.json-simple:json-simple</include>
<include>org.clojure:math.numeric-tower</include>
<include>org.clojure:tools.cli</include>
+ <include>org.clojure:tools.logging</include>
<include>org.clojure:tools.macro</include>
+ <include>org.clojure:java.jmx</include>
<include>joda-time:joda-time</include>
<include>org.eclipse.jetty:*</include>
<include>com.fasterxml.jackson.core:*</include>
<include>com.fasterxml.jackson.dataformat:*</include>
+ <include>clout:clout</include>
+ <include>org.clojure:tools.namespace</include>
+ <include>cheshire:cheshire</include>
+ <include>org.clojure:core.incubator</include>
</includes>
</artifactSet>
-
<relocations>
<relocation>
- <pattern>com.fasterxml.jackson.core</pattern>
- <shadedPattern>com.fasterxml.jackson.storm.core</shadedPattern>
+ <pattern>cheshire</pattern>
+ <shadedPattern>org.apache.storm.shade.cheshire</shadedPattern>
</relocation>
<relocation>
- <pattern>com.fasterxml.jackson.dataformat</pattern>
- <shadedPattern>com.fasterxml.jackson.storm.dataformat</shadedPattern>
+ <pattern>clojure.tools.logging</pattern>
+ <shadedPattern>org.apache.storm.shade.clojure.tools.logging</shadedPattern>
</relocation>
<relocation>
- <pattern>org.apache.thrift</pattern>
- <shadedPattern>org.apache.thrift7</shadedPattern>
+ <pattern>clojure.core.incubator</pattern>
+ <shadedPattern>org.apache.storm.shade.clojure.core.incubator</shadedPattern>
</relocation>
<relocation>
- <pattern>org.jboss.netty</pattern>
- <shadedPattern>org.apache.storm.netty</shadedPattern>
+ <pattern>clojure.tools.namespace</pattern>
+ <shadedPattern>org.apache.storm.shade.clojure.tools.namespace</shadedPattern>
</relocation>
<relocation>
- <pattern>com.google.common</pattern>
- <shadedPattern>org.apache.storm.guava</shadedPattern>
+ <pattern>clout</pattern>
+ <shadedPattern>org.apache.storm.shade.clout</shadedPattern>
</relocation>
<relocation>
- <pattern>com.google.thirdparty</pattern>
- <shadedPattern>org.apache.storm.guava.thirdparty</shadedPattern>
+ <pattern>compojure</pattern>
+ <shadedPattern>org.apache.storm.shade.compojure</shadedPattern>
</relocation>
<relocation>
- <pattern>org.apache.http</pattern>
- <shadedPattern>org.apache.storm.http</shadedPattern>
+ <pattern>ns_tracker</pattern>
+ <shadedPattern>org.apache.storm.shade.ns_tracker</shadedPattern>
</relocation>
<relocation>
- <pattern>org.apache.zookeeper</pattern>
- <shadedPattern>org.apache.storm.zookeeper</shadedPattern>
+ <pattern>ns-tracker</pattern>
+ <shadedPattern>org.apache.storm.shade.ns-tracker</shadedPattern>
</relocation>
<relocation>
- <pattern>org.apache.curator</pattern>
- <shadedPattern>org.apache.storm.curator</shadedPattern>
+ <pattern>hiccup</pattern>
+ <shadedPattern>org.apache.storm.shade.hiccup</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>ring</pattern>
+ <shadedPattern>org.apache.storm.shade.ring</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>clj_time</pattern>
+ <shadedPattern>org.apache.storm.shade.clj_time</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>clj-time</pattern>
+ <shadedPattern>org.apache.storm.shade.clj-time</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>com.fasterxml</pattern>
+ <shadedPattern>org.apache.storm.shade.com.fasterxml</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>org.apache.thrift</pattern>
+ <!-- This pattern is inconsistent for backwards compatability purposes. -->
--- End diff --
I left it the same because it is a different level of compatibility. This change just requires a recompile and explicitly calling out dependencies. If I change the package that changes some APIs that we expose because we expose a TException in a few places. That is why I didn't shade disruptor or kryo because both of them have pluggable interfaces exposed through configs, so I didn't want to touch anything that was directly exposed to the end user. If we want to make the change we can, but I would like to see a separate JIRA for that. This is to fix the builds so shading is working properly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by kishorvpatil <gi...@git.apache.org>.
Github user kishorvpatil commented on the pull request:
https://github.com/apache/storm/pull/736#issuecomment-142948399
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by ptgoetz <gi...@git.apache.org>.
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/736#issuecomment-142992324
@revans2 let me quickly check to make sure the release process works with this. Be aware that there will be some commits as a result that I will revert.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 [DO-...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/736#issuecomment-145130723
@ptgoetz I have the build updated. Shading is working if you use maven 3.2.5 or below. Please take a look again, and I will update the pull request for 0.10.x
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 [DO-...
Posted by ptgoetz <gi...@git.apache.org>.
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/736#issuecomment-145092203
I did some more digging and figured out that the following commit:
https://github.com/apache/storm/pull/621
is what broke the packaging for flux. Specifically, minimizing the shaded jar for flux-examples excludes classes that Maven doesn't think are used -- when in fact they are used, but used via reflection.
The warnings mentioned in the above pull request can be eliminated by alternate methods, but minimizing a flux-enabled jar will exclude classes that are in fact needed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/736#discussion_r39639834
--- Diff: storm-dist/binary/src/main/assembly/binary.xml ---
@@ -30,38 +30,6 @@
<useProjectArtifact>false</useProjectArtifact>
<outputDirectory>lib</outputDirectory>
<unpack>false</unpack>
- <excludes>
--- End diff --
Yes, the issue was partly that we didn't shade the libraries properly, but also we had excludes in there that were not needed. The build would print out lots of warning that we were excluding things that were not a part of the dependency list.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by ptgoetz <gi...@git.apache.org>.
Github user ptgoetz commented on the pull request:
https://github.com/apache/storm/pull/736#issuecomment-143020062
Unfortunately, I'm -1 at this point since it breaks the release process (Maven release plugin) and I could not find a workaround.
The main problem is the two step build using profiles, since the release plugin expects the project to be built all at once. I can release with the `storm-core` profile, but when I go to release with `storm-more` maven goes into an infinite loop of resolving snapshot dependencies (because the poms for `storm-core` have a new version).
If we eliminate the separate profiles so we can build with one maven command, the release plugin should be happy. But in its current form, this patch would make it impossible to release maven artifacts.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/736#discussion_r39567664
--- Diff: dev-tools/travis/travis-install.sh ---
@@ -26,12 +26,23 @@ TRAVIS_SCRIPT_DIR=$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )
cd ${STORM_SRC_ROOT_DIR}
-python ${TRAVIS_SCRIPT_DIR}/save-logs.py "install.txt" mvn clean install -DskipTests -Pnative
+python ${TRAVIS_SCRIPT_DIR}/save-logs.py "install-storm-core.txt" mvn clean install -DskipTests -Pnative -Pstorm-core
BUILD_RET_VAL=$?
if [[ "$BUILD_RET_VAL" != "0" ]];
then
- cat "install.txt"
+ cat "install-storm-core.txt"
+ exit ${BUILD_RET_VAL}
+fi
+
+exit ${BUILD_RET_VAL}
--- End diff --
This line should be removed to continue building storm-more.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/736#issuecomment-140787382
I just pushed an update addressing the review comments. The big thing is an explanation in storm-core/pom.xml explaining about the filtering of the .clj files and what is happening there.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by erikdw <gi...@git.apache.org>.
Github user erikdw commented on a diff in the pull request:
https://github.com/apache/storm/pull/736#discussion_r40755399
--- Diff: DEVELOPER.md ---
@@ -222,6 +222,11 @@ To pull in a merge request you should generally follow the command line instruct
# Build the code and run the tests
+Storm has some oddities with packaging and plugin dependencies. We build our own plugins and shade storm-core,
+but have sub-modules that depend on storm-core. Because of this the build needs to be split into two parts.
--- End diff --
Can you please explain in a bit more detail why this split is needed, and how the change accomplishes that?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 [DO-...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/736#issuecomment-145040001
Good news on MSHADE-206 the author thinks that is a bug in maven itself and thinks they have tracked it down to a specific commit that broke this. I am going to test this and possibly update things. We may have top require a very specific version of maven to get the build to work properly though.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/736#issuecomment-143270080
@ptgoetz I will play around with it and see if I can come up with a release strategy that works.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/736#issuecomment-141142255
@ptgoetz There were a number of issues.
We were shading dependencies of dependencies without shading the direct dependency. This resulted in shaded classes leaking into user facing APIs.
We were not using the storm-core dependency reduced pom to build other components (flux most notably). This resulted in flux assuming that dependencies would be there when they were not.
We also were partially shading some of our clojure dependencies. Our dependency clojure code was being compiled into .class files placed in our jar. The packages for these dependencies were being shaded, but the maven dependency was not listed, so it was not removed from the dependency reduced pom.
There are also some issues with the ClojureTransformer where it would not correctly shade clojure code when the package name is split up in the .clj file, so to work around that we removed the majority of .clj files from the uber jar.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/736
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: STORM-1012 STORM-967 STORM-922 STORM-1042 Shad...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/736#issuecomment-140805769
The travis failure is unrelated looks like another test did not release the socket fast enough and the test could not bind to it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---