Guava was the only thing that we shaded everywhere but the original intent was for us to shade more and more by default until we decided to do vendoring (which is a better solution).
So yes, this really only removed shading of Guava, we still have shading in all these other places: model/* sdks/java/core sdks/java/extensions/kryo sdks/java/extensions/sql sdks/java/extensions/sql/jdbc sdks/java/harness runners/spark/job-server runners/direct-java runners/samza/job-server runners/google-cloud-dataflow-java/worker runners/google-cloud-dataflow-java/worker/legacy-worker runners/google-cloud-dataflow-java/worker/windmill vendor/* On Fri, Jun 7, 2019 at 1:05 AM Ismaël Mejía <ieme...@gmail.com> wrote: > This is fantastic. Took a look at the PR and did not see anything that > jump to my eyes and also validated with two external projects with > today’s snapshots (after merge) without issues so far. Great that we > finally tackle this on, thanks Luke! > > Have one minor comment because the title of the thread may be > confusing, after checking sdks-java-core I noticed we are still > shading other dependencies: protobuf, bytebuddy, antlr, apache > commons, so I suppose this was mostly around shading guava, isn’t it? > > On Wed, Jun 5, 2019 at 10:09 PM Lukasz Cwik <lc...@google.com> wrote: > > > > I am able to pass several runners validates runner tests and the Java > PostCommit. > > > > I also was able to publish a release into the staging repository[1] and > compared the newly generated poms artifact-2.14.0-20190605.*-30.pom against > the previously nightly snapshot of artifact-2.14.0-20190605.*-28.pom for > the following projects as a spot check and found no differences in those > poms: > > beam-sdks-java-core > > beam-sdks-java-fn-execution > > beam-runners-spark > > > > I believe my PR is now ready for review. > > > > 1: > https://repository.apache.org/content/groups/snapshots/org/apache/beam/ > > > > On Tue, Jun 4, 2019 at 7:18 PM Kenneth Knowles <k...@apache.org> wrote: > >> > >> Nice! This is a huge step. One thing that showed up in the last big > gradle change was needing to check the generated poms. > >> > >> Kenn > >> > >> On Tue, Jun 4, 2019 at 5:07 PM Lukasz Cwik <lc...@google.com> wrote: > >>> > >>> Since we have been migrating to using vendoring instead of shading[1] > and due to previous efforts in vendoring[2, 3] I have opened up PR 8762[4] > which migrates all projects that weren't doing anything shading wise to not > perform any shading. This required me to fix up all intra project > dependencies and release publishing. > >>> > >>> The following is a list of all project paths which are still using > shading for some reason: > >>> model/* > >>> sdks/java/core > >>> sdks/java/extensions/kryo > >>> sdks/java/extensions/sql > >>> sdks/java/extensions/sql/jdbc > >>> sdks/java/harness > >>> runners/spark/job-server > >>> runners/direct-java > >>> runners/samza/job-server > >>> runners/google-cloud-dataflow-java/worker > >>> runners/google-cloud-dataflow-java/worker/legacy-worker > >>> runners/google-cloud-dataflow-java/worker/windmill > >>> vendor/* > >>> > >>> Out of the list above, migrating sdks/java/core and > runners/direct-java (in that order) would provide the most benefit to > moving away from shading within our project. Many of the others are either > shaded proto classes or applications (e.g. job-servers, harness, sql jdbc) > and either require shading to be compatible with vendoring or aren't meant > to be used as dependencies. > >>> > >>> Since this is a larger change that cuts across so many projects there > is risk for breakage. I'm looking for people to help test the change and > validate any scenarios that they are specifically interested in. I'm > planning to run several of the postcommits on my PR and check that we can > build a release in addition to any efforts others provide before looking to > have the change merged. > >>> > >>> The following guidance should help those who edit Gradle build files > (after this change is merged): > >>> * For projects that don't perform any shading, those projects have > been migrated to use the default configurations that the Gradle Java plugin > uses[5]. Note that the default configurations we use have been deprecated. > >>> * For projects that depend on another project that isn't shaded, the > intra project configuration has been swapped to use compile / testRuntime > instead of shadow and shadowTest > >>> * Existing projects that are still shaded should use the shadow and > shadowTest configurations as before. > >>> > >>> 1: > https://lists.apache.org/thread.html/4c12db35b40a6d56e170cd6fc8bb0ac4c43a99aa3cb7dbae54176815@%3Cdev.beam.apache.org%3E > >>> 2: > https://lists.apache.org/thread.html/4c12db35b40a6d56e170cd6fc8bb0ac4c43a99aa3cb7dbae54176815@%3Cdev.beam.apache.org%3E > >>> 3: > https://lists.apache.org/thread.html/972b5175641f4eaf7ec92870cc0ff72fa52e6f0bbaccc384a3814e45@%3Cdev.beam.apache.org%3E > >>> 4: https://github.com/apache/beam/pull/8762 > >>> 5: > https://docs.gradle.org/current/userguide/java_plugin.html#sec:java_plugin_and_dependency_management >
