Skip to content

Fixed NPE when api project dependency has a version number.#477

Merged
johnrengelman merged 1 commit intoGradleUp:masterfrom
kelemen:master
Jun 29, 2019
Merged

Fixed NPE when api project dependency has a version number.#477
johnrengelman merged 1 commit intoGradleUp:masterfrom
kelemen:master

Conversation

@kelemen
Copy link
Copy Markdown
Contributor

@kelemen kelemen commented Mar 29, 2019

If an API project dependency has a version number, then the check for the dependency name was wrong (i.e., it won't end with myproject.jar but say myproject-1.0.jar). I tried to create a change which is reasonably compatible with the previous behaviour in other cases.

@sormuras
Copy link
Copy Markdown
Contributor

sormuras commented Apr 2, 2019

I duplicated this PR in #478, methinks.

Copy link
Copy Markdown
Contributor

@sormuras sormuras left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me -- and like it better then my #478 PR.

@johnrengelman ... please review, merge and release? ;-)

} else if (dep instanceof SelfResolvingDependency) {
apiJars.addAll(dep.resolve())
} else {
addJar(runtimeConfiguration, dep, apiJars)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this and the next line? Ain't some duplication going on then?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assumed that the intent is essentially the same in both location (i.e., add only the dependency jar). Although, arguably the isProjectDependencyFile name makes less sense this way. If you mean the duplicate call to addJar, then it is difficult to not repeat something with the middle condition in the way.

Copy link
Copy Markdown
Contributor

@sormuras sormuras left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR (and also #478) produce the following exception:

Caused by: java.lang.IllegalArgumentException: neither file nor directory
	at shadow.org.vafer.jdependency.Clazzpath.addClazzpathUnit(Clazzpath.java:122)
	at shadow.org.vafer.jdependency.Clazzpath.addClazzpathUnit(Clazzpath.java:95)
	at shadow.org.vafer.jdependency.Clazzpath.addClazzpathUnit(Clazzpath.java:86)
	at shadow.org.vafer.jdependency.Clazzpath$addClazzpathUnit.call(Unknown Source)
	at com.github.jengelman.gradle.plugins.shadow.internal.UnusedTracker$_closure2.doCall(UnusedTracker.groovy:22)

when used via @jitpack on the @junit-team JUnit 5 code base.

@kelemen
Copy link
Copy Markdown
Contributor Author

kelemen commented Apr 3, 2019

I'm almost certain that, that issue is a completely different issue and as such it would be better to fix it in a separate PR (and only manifests here because so far the code didn't even get to that point).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants