GitHub user NicoK opened a pull request:

https://github.com/apache/flink/pull/3959

FLINK-6654 let 'flink-dist' properly depend on 'flink-shaded-hadoop2-uber'

Since applying FLINK-6514, flink-dist includes `flink-shaded-hadoop2-uber-*.jar` but without giving this dependency in its `pom.xml`. This may lead to concurrency issues during builds but also fails building the flink-dist module only (with dependencies) as in

`mvn clean install -pl flink-dist -am`

This PR adds the missing dependency.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/NicoK/flink flink-6654

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/flink/pull/3959.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 #3959

Github user NicoK commented on the issue:

https://github.com/apache/flink/pull/3959

sorry, wrong branch to submit to

Github user NicoK closed the pull request at:

https://github.com/apache/flink/pull/3959

GitHub user NicoK opened a pull request:

https://github.com/apache/flink/pull/3960

FLINK-6654[build] let 'flink-dist' properly depend on 'flink-shaded…

Since applying FLINK-6514, flink-dist includes `flink-shaded-hadoop2-uber-*.jar` but without giving this dependency in its `pom.xml`. This may lead to concurrency issues during builds but also fails building the flink-dist module only (with dependencies) as in

`mvn clean install -pl flink-dist -am`

This PR adds the missing dependency.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/NicoK/flink flink-6654

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/flink/pull/3960.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 #3960

commit 1b40449ac5373e93f8ddb4b3cf46dca2bcc3a900
Author: Nico Kruber <nico@data-artisans.com>
Date: 2017-05-22T10:14:59Z

FLINK-6654[build] let 'flink-dist' properly depend on 'flink-shaded-hadoop2-uber'

I think the dependency is not included on purpose. Stephan Ewen, could you please chime in?

Github user StephanEwen commented on the issue:

https://github.com/apache/flink/pull/3960

Thanks. The exclusion should not be necessary, since the `flink-shaded-hadoop2-uber` dependency shades everything into one jar and has no further dependencies.

Github user NicoK commented on the issue:

https://github.com/apache/flink/pull/3960

I wasn't quite sure and went the safe way - can remove the exclusion though...

Github user rmetzger commented on the issue:

https://github.com/apache/flink/pull/3960

Not sure about the initial issue. For me the command worked (without yoru changes on the 1.3 branch)

```
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 10:12 min
[INFO] Finished at: 2017-05-23T16:50:31+02:00
[INFO] Final Memory: 131M/441M
[INFO] ------------------------------------------------------------------------
mvn clean install -pl flink-dist -am -DskipTests
```

Github user NicoK commented on the issue:

https://github.com/apache/flink/pull/3960

you probably need to clean up your whole source with `mvn clean` and try from there - I'm not quite sure, my initial command will clean up everything that is not in the dependencies of the flink-dist project

Github user rmetzger commented on the issue:

https://github.com/apache/flink/pull/3960

Okay, I was able to reproduce the issue.

Github user rmetzger commented on the issue:

https://github.com/apache/flink/pull/3960

I'm not sure if the proposed fix is correct, because it will add the shaded hadoop uber project's dependencies into Flink dist.
IIRC, this was the whole point of Stephan's recent refactoring of the dependencies.

I did a quick check, just based on the flink-dist file size and content, and with the current rel 1.3 branch, we have the following file sizes:
```
rw-rr- 1 robert robert 68M May 24 09:47 flink-dist_2.10-1.3-SNAPSHOT.jar
rw-rr- 1 robert robert 99K May 24 09:40 flink-python_2.10-1.3-SNAPSHOT.jar
rw-rr- 1 robert robert 23M May 24 09:47 flink-shaded-hadoop2-uber-1.3-SNAPSHOT.jar
rw-rr- 1 robert robert 479K Nov 17 2015 log4j-1.2.17.jar
rw-rr- 1 robert robert 8.7K Nov 17 2015 slf4j-log4j12-1.7.7.jar
```

after the change, the flink dist file grows significantly:
```
rw-rr- 1 robert robert 87M May 24 09:51 flink-dist_2.10-1.3-SNAPSHOT.jar
rw-rr- 1 robert robert 99K May 24 09:40 flink-python_2.10-1.3-SNAPSHOT.jar
rw-rr- 1 robert robert 23M May 24 09:47 flink-shaded-hadoop2-uber-1.3-SNAPSHOT.jar
rw-rr- 1 robert robert 479K Nov 17 2015 log4j-1.2.17.jar
rw-rr- 1 robert robert 8.7K Nov 17 2015 slf4j-log4j12-1.7.7.jar
```
I quickly opened the file and I found all `org.apache.hadoop` classes in the dist again.

I'm against doing a "quick fix" before the release here.

Github user StephanEwen commented on the issue:

https://github.com/apache/flink/pull/3960

I think this looks correct now. Merging for `master` and `release-1.3`...

Github user asfgit closed the pull request at:

https://github.com/apache/flink/pull/3960