For the shadowJar issue see https://issues.apache.org/jira/browse/BEAM-4402. There are 280 test failures, but many of them are the same issue.
I believe there is significant work on both, but I think there is significant value as well. I suppose there should be a vote on applying these rules to the whole project and a fix it day to make it happen. Andrew On Fri, Jun 1, 2018 at 9:39 AM Kenneth Knowles <k...@google.com> wrote: > Spotless is slightly off topic and less serious than test coverage > problems, but see https://issues.apache.org/jira/browse/BEAM-4394. > > On Fri, Jun 1, 2018 at 9:30 AM Scott Wegner <sweg...@google.com> wrote: > >> Andrew, it sounds like you're suggesting testShadowJar and enableSpotless >> should also be the default for all modules? Do you have an idea of how much >> effort is required for migrating? For failOnWarning / ErrorProne, the >> migration is pretty straightforward, so I created starter bugs for each >> module [1] with step-by-step instructions that anybody could pick up, and >> so far it's been fairly successful-- only 13 out of 47 left to go. Do we >> have anything tracking the work for testShadowJar / spotless? >> >> [1] https://issues.apache.org/jira/issues/?jql=labels+%3D+errorprone >> >> >> On Thu, May 31, 2018 at 9:20 AM Andrew Pilloud <apill...@google.com> >> wrote: >> >>> There is now a testShadowJar option on applyJavaNature, which allows you >>> to run your tests against the shaded jar. Everyone should turn it on for >>> their module along with failOnWarning and enableSpotless for maximum >>> testing. >>> >>> On Thu, May 24, 2018 at 1:24 PM Andrew Pilloud <apill...@google.com> >>> wrote: >>> >>>> I've make the SQL PR able to be merged standalone so I can get back to >>>> work. I've also opened issues to track the things I found and dumped my >>>> work in progress into a PR. >>>> >>>> https://issues.apache.org/jira/browse/BEAM-4402 >>>> https://issues.apache.org/jira/browse/BEAM-4403 >>>> https://github.com/apache/beam/pull/5471 >>>> >>>> Andrew >>>> >>>> On Thu, May 24, 2018 at 11:54 AM Kenneth Knowles <k...@google.com> >>>> wrote: >>>> >>>>> If it is taking days of work we should definitely put it behind a flag >>>>> and do it incrementally, find a way to share the work. >>>>> >>>>> If our tests aren't running on the actual artifacts, then we don't >>>>> really have assurance that they work. That should interest just about >>>>> everyone. (though it is also status quo relative to mvn) >>>>> >>>>> Kenn >>>>> >>>>> On Thu, May 24, 2018 at 10:17 AM Andrew Pilloud <apill...@google.com> >>>>> wrote: >>>>> >>>>>> I think I'm down to 11 packages with failures, some of which might be >>>>>> precommits that don't run outside of Jenkins. Its not an issue of >>>>>> figuring >>>>>> them out, it is an issue of time spent doing so. >>>>>> >>>>>> On Thu, May 24, 2018 at 9:31 AM Lukasz Cwik <lc...@google.com> wrote: >>>>>> >>>>>>> Were you able to figure out how to fix them or still having problems? >>>>>>> >>>>>>> On Thu, May 24, 2018 at 9:27 AM Andrew Pilloud <apill...@google.com> >>>>>>> wrote: >>>>>>> >>>>>>>> I spent all of yesterday investigating and fixing dependency issues >>>>>>>> outside of SQL. I really regret the decision to write a test for this. >>>>>>>> Would it be acceptable for us to put testing with the output jar >>>>>>>> behind a >>>>>>>> flag like we do for failOnWarning? >>>>>>>> >>>>>>>> On Wed, May 23, 2018 at 5:21 PM Kenneth Knowles <k...@google.com> >>>>>>>> wrote: >>>>>>>> >>>>>>>>> What's the status of moving it forward? Is it a ton of work / too >>>>>>>>> much to do quickly? >>>>>>>>> >>>>>>>>> On Wed, May 23, 2018 at 9:11 AM Andrew Pilloud < >>>>>>>>> apill...@google.com> wrote: >>>>>>>>> >>>>>>>>>> To loop the list in on discussions going on in >>>>>>>>>> https://github.com/apache/beam/pull/5443: our normal tests don't >>>>>>>>>> run against the shaded jars. Gradle can run the tests against the >>>>>>>>>> shaded >>>>>>>>>> jars, but a bunch fail due to dependency issues. It's not just SQL. >>>>>>>>>> >>>>>>>>>> Andrew >>>>>>>>>> >>>>>>>>>> On Mon, May 21, 2018 at 11:35 AM Lukasz Cwik <lc...@google.com> >>>>>>>>>> wrote: >>>>>>>>>> >>>>>>>>>>> Shading requires two pieces of information: >>>>>>>>>>> 1) Which dependencies should be part of the shaded jar >>>>>>>>>>> (controlled by includes/excludes) >>>>>>>>>>> 2) How to relocate code within those dependencies (controlled by >>>>>>>>>>> relocations) >>>>>>>>>>> >>>>>>>>>>> The reason why the exclude(".*") exists is because typically it >>>>>>>>>>> is an error to produce a shaded package with dependencies which are >>>>>>>>>>> not >>>>>>>>>>> relocated. When libraries do this, it causes a lot of >>>>>>>>>>> NoClassFound/NoMethodFound errors for users since a user can't know >>>>>>>>>>> which >>>>>>>>>>> version of a dependency they are actually getting (the one that was >>>>>>>>>>> bundled >>>>>>>>>>> part of your jar or the one they depend on as a library). Only >>>>>>>>>>> applications >>>>>>>>>>> should ever really do this, libraries should always repackage all >>>>>>>>>>> their >>>>>>>>>>> code to prevent such errors. >>>>>>>>>>> >>>>>>>>>>> Note that in the SQL package, you can provide your own >>>>>>>>>>> shadowClosure to the applyJavaNature() which means that the default >>>>>>>>>>> won't >>>>>>>>>>> apply. For example: >>>>>>>>>>> https://github.com/apache/beam/blob/a3ba6a0e8de3ae72b8fc6fc6038eb9dc725f092e/sdks/java/harness/build.gradle#L20 >>>>>>>>>>> and remove the 'DEFAULT_SHADOW_CLOSURE <<' >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> On Mon, May 21, 2018 at 10:26 AM Andrew Pilloud < >>>>>>>>>>> apill...@google.com> wrote: >>>>>>>>>>> >>>>>>>>>>>> The issue SQL is seeing is caused by a default dependency of >>>>>>>>>>>> exclude(".*") added in build_rules.gradle. This breaks the normal >>>>>>>>>>>> method of >>>>>>>>>>>> building shadow jars as everything must be explicitly included. SQL >>>>>>>>>>>> explicitly added calcite to the jar, but not calcite's >>>>>>>>>>>> dependencies. I've >>>>>>>>>>>> been told this is the desired behavior as we want to ensure >>>>>>>>>>>> everything >>>>>>>>>>>> included is relocated. >>>>>>>>>>>> >>>>>>>>>>>> I don't know much about gradle, but this seems fragile. Is it >>>>>>>>>>>> possible to have all dependencies automatically relocated so we >>>>>>>>>>>> don't need >>>>>>>>>>>> the exclude(".*") rule? >>>>>>>>>>>> >>>>>>>>>>>> Andrew >>>>>>>>>>>> >>>>>>>>>>>> On Thu, May 17, 2018 at 7:41 PM Andrew Pilloud < >>>>>>>>>>>> apill...@google.com> wrote: >>>>>>>>>>>> >>>>>>>>>>>>> Yep, I added the issue as a blocker. >>>>>>>>>>>>> https://issues.apache.org/jira/projects/BEAM/issues/BEAM-4357 >>>>>>>>>>>>> >>>>>>>>>>>>> On Thu, May 17, 2018, 6:05 PM Kenneth Knowles <k...@google.com> >>>>>>>>>>>>> wrote: >>>>>>>>>>>>> >>>>>>>>>>>>>> This sounds like a release blocker. Can you add it to the >>>>>>>>>>>>>> list? (Assign fix version on jira) >>>>>>>>>>>>>> >>>>>>>>>>>>>> Kenn >>>>>>>>>>>>>> >>>>>>>>>>>>>> On Thu, May 17, 2018, 17:30 Lukasz Cwik <lc...@google.com> >>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>> >>>>>>>>>>>>>>> Typically we have a test block which uses a configuration >>>>>>>>>>>>>>> that has the shadow/shadowTest configurations on the classpath >>>>>>>>>>>>>>> instead of >>>>>>>>>>>>>>> the compile/testCompile configurations. The most common >>>>>>>>>>>>>>> examples are >>>>>>>>>>>>>>> validates runner/integration tests for example: >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> https://github.com/apache/beam/blob/0c5ebc449554a02cae5e4fd01afb07ecdb0bbaea/runners/direct-java/build.gradle#L84 >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> On Thu, May 17, 2018 at 3:59 PM Andrew Pilloud < >>>>>>>>>>>>>>> apill...@google.com> wrote: >>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> I decided to try our new JDBC support with sqlline and >>>>>>>>>>>>>>>> discovered that our SQL shaded jar is completely broken. As >>>>>>>>>>>>>>>> in java.lang.NoClassDefFoundError all over the place. How are >>>>>>>>>>>>>>>> we testing >>>>>>>>>>>>>>>> the output jars from other beam packages? Is there an example >>>>>>>>>>>>>>>> I can follow >>>>>>>>>>>>>>>> to make our integration tests run against the release >>>>>>>>>>>>>>>> artifacts? >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Andrew >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>
