Tasks that use jdeps analyzer #983

esword · 2019-10-17T18:32:11Z

Before this PR

Maven dependency analyzer has several shortcomings, including those that lead to:
#847
#614

Plus, the current analyzer tasks cache everything which can lead to OoMs with large projects.

After this PR

==COMMIT_MSG==
New dependency analyzer yields cleaner results and tasks store output in files so better
caching and analysis can be reused for other purposes
==COMMIT_MSG==

Possible downsides?

Possibly slightly slower since don't cache contents of all jars across projects.

changelog-app · 2019-10-17T18:32:16Z

Generate changelog in `changelog/@unreleased`

Type

Description

New dependency analyzer yields cleaner results and tasks store output in files so better
caching and analysis can be reused for other purposes

Check the box to generate changelog(s)

Generate changelog entry

esword · 2019-10-17T18:33:34Z

I have not moved over the existing tasks to use the new analysis file yet. The file is useful on it's own if we want to make that a separate PR, or can do as part of this one.

unfortunately, it doesn't list full path to dependency files, so is useless for determining other projects that deps came from. Committing purely so have record of how to do this should I come back to it.

also do some cleanup for task generation

The gradle APIs that use resolved items will be deprecated eventually. so best not to use them. This also reduces memory overhead.

Gradle task validation would fail if the directory already existed with the other annotation.

the classpath configuration has "runtime" varients for artifacts that allow it to look at classes directories rather than jars. This avoids errors because project-dependencies may not always have jars generated.

Now generates a stub output file so that downstream tasks don't bomb.

Looks like ABSOLUTE is relative to the root project

will merge in Analyzer to the task to avoid duplication

CRogers

I didn't manage to allocated as much time as I wanted to review this, still need to test it out locally.

CRogers · 2019-10-30T16:51:36Z

gradle-baseline-java/build.gradle

@@ -58,6 +61,10 @@ gradlePlugin {
            id = 'com.palantir.baseline-exact-dependencies'
            implementationClass = 'com.palantir.baseline.plugins.BaselineExactDependencies'
        }
+        baselineDependencyPluginv2 {
+            id = 'com.palantir.baseline-dependencies-v2'
+            implementationClass = 'com.palantir.baseline.plugins.BaselineDependencyPluginv2'


We should just replace it entirely if we have confidence it's good. If we have lower confidence we should take a gradle property to excavate it out rather than an entirely new task (this allows us to track adoption and easily excavate the changes).

CRogers · 2019-10-30T16:55:09Z

...ne-java/src/main/groovy/com/palantir/baseline/tasks/dependencies/DependencyAnalysisTask.java

+        return report;
+    }
+
+    public static final class ReportContent {


I would use immutables here as it will build this class for you and handle the yaml dumping by generating the correct jackson classes as well, meaning we don't need snakeyaml.

CRogers · 2019-10-30T16:57:14Z

...seline-java/src/main/groovy/com/palantir/baseline/tasks/dependencies/DependencyAnalyzer.java

+    }
+
+    /**
+     * Dependencies that are declared in one of the given configurations but not used in byte code.  This will only


Does it detect things like Class.forName(...) or other dynamic class loading?

CRogers · 2019-10-30T16:58:55Z

...-baseline-java/src/main/groovy/com/palantir/baseline/tasks/dependencies/DependencyUtils.java

+import org.slf4j.LoggerFactory;
+import org.yaml.snakeyaml.Yaml;
+
+public final class DependencyUtils {


I think most of these public classes should be package-private.

CRogers · 2019-10-30T16:59:29Z

...-baseline-java/src/main/groovy/com/palantir/baseline/tasks/dependencies/DependencyUtils.java

+    }
+
+    static void writeDepReport(File file, DependencyAnalysisTask.ReportContent content) {
+        Yaml yaml = new Yaml();


We should try to use jackson rather than snakeyaml here to be consistent with everything else that uses jackson.

CRogers · 2019-11-05T17:55:53Z

gradle-baseline-java/build.gradle

@@ -17,6 +17,9 @@ dependencies {
    compile 'org.github.ngbinh.scalastyle:gradle-scalastyle-plugin_2.11'
    implementation 'com.palantir.javaformat:palantir-java-format-spi'
    implementation 'com.palantir.javaformat:gradle-palantir-java-format'
+    implementation 'com.paypal.digraph:digraph-parser'
+    implementation 'org.zeroturnaround:zt-exec'


We should avoid taking dependencies on extra stuff if it's not necessary. The java process API may be nasty but it's workable. Since we put everything into gradle buildscript internally there's great capacity to have breaking changes between gradle plugins wreak, especially from something as widely used as baseline.

CRogers · 2019-11-05T17:56:11Z

gradle-baseline-java/build.gradle

@@ -17,6 +17,9 @@ dependencies {
    compile 'org.github.ngbinh.scalastyle:gradle-scalastyle-plugin_2.11'
    implementation 'com.palantir.javaformat:palantir-java-format-spi'
    implementation 'com.palantir.javaformat:gradle-palantir-java-format'
+    implementation 'com.paypal.digraph:digraph-parser'
+    implementation 'org.zeroturnaround:zt-exec'
+    implementation 'org.yaml:snakeyaml'


Can we avoid snakeyaml and use jackson everywhere?

CRogers · 2019-11-05T17:57:31Z

...-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineDependencyPluginv2.java

+                    .getSourceSets();
+
+            createTasksForSourceSet(project, sourceSets.getByName(SourceSet.MAIN_SOURCE_SET_NAME));
+            createTasksForSourceSet(project, sourceSets.getByName(SourceSet.TEST_SOURCE_SET_NAME));


What about people using custom integrationTest or integTest source sets?

I guess we can perhaps just ignore them.

CRogers · 2019-11-05T18:12:28Z

...-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineDependencyPluginv2.java

+            t.getIgnored().add("javax.annotation:javax.annotation-api");
+            //Projects may apply this in compileOnly to work around
+            //https://github.com/immutables/immutables/issues/291
+            t.getIgnored().add("org.immutables:value::annotations");


It might make sense to ignore all dependencies which are in compileOnly and also annotationProcessor as this happens to more libraries than just immutables, iirc.

correct for bug where jdeps doesn't report dependency if referenced class in same package as current class, even if it's in a different module.

stale · 2019-11-20T02:52:21Z

This PR has been automatically marked as stale because it has not been touched in the last 14 days. If you'd like to keep it open, please leave a comment or add the 'long-lived' label, otherwise it'll be closed in 7 days.

iamdanfox · 2019-12-12T00:49:40Z

I think we may want to resuscitate this because the maven-dependency-analyzer can't parse bytecode produced by java13.

stale · 2019-12-26T01:29:18Z

This PR has been automatically marked as stale because it has not been touched in the last 14 days. If you'd like to keep it open, please leave a comment or add the 'long-lived' label, otherwise it'll be closed in 7 days.

Tasks that use jdeps analyzer

b1f9907

policy-bot bot requested a review from CRogers October 17, 2019 18:32

esword added 26 commits October 17, 2019 16:27

add in api deps reporting

ec02197

style fixes

d29a4a6

more style fixes

91f01f7

convert implicit dep task to use dep report

bd1541d

more style fixes

5d2b259

temp hack to verify this is what's causing circle to hang

81c4b9c

Merge branch 'develop' into feature/jdeps-analyzer

36cfbb6

move unused dependency check to reading from dep report

3c626b7

undo temp hack that was checking timeouts on circle

00dbc5b

style fixes

a80586e

adding logging to see why timing out on circle

045cc9e

fix test name

68a4504

plugin validation fixes

0c1dd8b

fix back compat with gradle 5.4

1a67669

temp ignore test until I come back and handle source-only deps

ddd21c1

revert changes to existing dependency plugin classes

83d2579

use classpath with jdeps to get more info

b325062

unfortunately, it doesn't list full path to dependency files, so is useless for determining other projects that deps came from. Committing purely so have record of how to do this should I come back to it.

add in source-only filtering

1e4dd86

plugin validation fixes

40fb18d

tweak plugin id to be more appropriate

b974a99

get newer implicit and unused tasks working

da5bfc9

also do some cleanup for task generation

fix tests

7e27a4d

javadoc cleanup

cffc1b2

expose util class for getting task names for other plugins

067e248

handle when source directory isn't present

4ce6d5a

fix bug where wrong dot file was selected and improve name cleanup

f0fbcf9

esword added 15 commits October 28, 2019 14:41

works with exact set to of configs analyze

81069bd

pass around dependency IDs rather than resolved artifacts where possible

5404892

The gradle APIs that use resolved items will be deprecated eventually. so best not to use them. This also reduces memory overhead.

handle the immutables annotation situation automatically

d07272e

Merge branch 'develop' into feature/jdeps-analyzer

db157ba

add change log

d151211

quick mod to handle projects with more than one classes dir

ba2aa2f

correct annotation for report dir.

dd69323

Gradle task validation would fail if the directory already existed with the other annotation.

deconflict task name with the one in core gradle

87c6f06

use full classpath configuration to work around errors in gradle 5.x

5721b30

the classpath configuration has "runtime" varients for artifacts that allow it to look at classes directories rather than jars. This avoids errors because project-dependencies may not always have jars generated.

better handling of no sources.

26c9475

Now generates a stub output file so that downstream tasks don't bomb.

fix pathsensitivity for inputs

81e6c34

Looks like ABSOLUTE is relative to the root project

avoid spurious exceptions

47b117e

properly detect dependency inputs so can detect when task is dirty.

d701e72

will merge in Analyzer to the task to avoid duplication

combine analyzer into task

481bd85

turning off extra test for circle

693d395

CRogers reviewed Nov 5, 2019

View reviewed changes

fix bug with classes in same package

30e357c

correct for bug where jdeps doesn't report dependency if referenced class in same package as current class, even if it's in a different module.

stale bot added the stale label Nov 20, 2019

stale bot closed this Nov 27, 2019

iamdanfox reopened this Dec 12, 2019

stale bot removed the stale label Dec 12, 2019

stale bot added the stale label Dec 26, 2019

stale bot closed this Jan 2, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tasks that use jdeps analyzer #983

Tasks that use jdeps analyzer #983

esword commented Oct 17, 2019

changelog-app bot commented Oct 17, 2019

esword commented Oct 17, 2019

CRogers left a comment

CRogers Oct 30, 2019

CRogers Oct 30, 2019

CRogers Oct 30, 2019

CRogers Oct 30, 2019

CRogers Oct 30, 2019

CRogers Nov 5, 2019

CRogers Nov 5, 2019

CRogers Nov 5, 2019

CRogers Nov 5, 2019

CRogers Nov 5, 2019

stale bot commented Nov 20, 2019

iamdanfox commented Dec 12, 2019

stale bot commented Dec 26, 2019

Tasks that use jdeps analyzer #983

Tasks that use jdeps analyzer #983

Conversation

esword commented Oct 17, 2019

Before this PR

After this PR

Possible downsides?

changelog-app bot commented Oct 17, 2019

Generate changelog in changelog/@unreleased

esword commented Oct 17, 2019

CRogers left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stale bot commented Nov 20, 2019

iamdanfox commented Dec 12, 2019

stale bot commented Dec 26, 2019

Generate changelog in `changelog/@unreleased`