-
Notifications
You must be signed in to change notification settings - Fork 403
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Polish outdated configs #831
Conversation
Follow up 1a57c16.
tasks.withType(JavaCompile).configureEach { | ||
// This will be the default in Gradle 5.0 | ||
if (!options.compilerArgs.contains("-processor")) { | ||
options.compilerArgs << '-proc:none' | ||
} | ||
} | ||
|
||
tasks.withType(GroovyCompile).configureEach { | ||
// This will be the default in Gradle 5.0 | ||
if (!options.compilerArgs.contains("-processor")) { | ||
options.compilerArgs << '-proc:none' | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems we can remove these from buildSrc
too?
// Remove the gradleApi so it isn't merged into the jar file. | ||
configurations.named(JavaPlugin.API_CONFIGURATION_NAME) { | ||
dependencies.remove(project.dependencies.gradleApi()) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
compileOnly
should be enough
./gradlew assemble
diffuse diff --jar shadow-8.1.0-SNAPSHOT-before.jar shadow-8.1.0-SNAPSHOT-after.jar
OLD: shadow-8.1.0-SNAPSHOT-before.jar
NEW: shadow-8.1.0-SNAPSHOT-after.jar
JAR │ old │ new │ diff
───────┼───────────┼───────────┼──────
class │ 700.9 KiB │ 700.9 KiB │ 0 B
other │ 10.6 KiB │ 10.6 KiB │ 0 B
───────┼───────────┼───────────┼──────
total │ 711.5 KiB │ 711.5 KiB │ 0 B
CLASSES │ old │ new │ diff
─────────┼──────┼──────┼───────────
classes │ 146 │ 146 │ 0 (+0 -0)
methods │ 1813 │ 1813 │ 0 (+0 -0)
fields │ 746 │ 746 │ 0 (+0 -0)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This appears to have broken shadowJar
itself in this project. I think you're only comparing the output of the jar
command above. However, the shadowJar
tasks has been running for 18m on my machine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, so applying the Gradle Plugin plugin adds the gradleApi()
to the api
configuration...https://github.com/gradle/gradle/blob/master/subprojects/plugin-development/src/main/java/org/gradle/plugin/devel/plugins/JavaGradlePluginPlugin.java#L162
And this is what this block was doing before.
So the compileOnly
change works, but this block needs to be kept to remove this behavior (as there is no way to exclude the dependency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've read-ed this line in 8.1.1. I'm not sure if that affects the configuration caching in an way.
gradle/docs.gradle
Outdated
archiveClassifier.set('javadoc') | ||
from 'build/docs/javadoc' | ||
} | ||
|
||
tasks.register('sourcesJar', Jar) { | ||
def sourcesJar = tasks.register('sourcesJar', Jar) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can drop these tasks entirely and instead use
java {
withJavadocJar()
withSourcesJar()
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch!
|
||
plugins { | ||
id 'groovy' | ||
id 'project-report' | ||
id 'idea' | ||
id 'java-gradle-plugin' | ||
// id 'signing' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I will re-add signing
as plugin-publish
supports it natively. I had turned this off when troubleshooting errors with publishing the 8.1.0 plugin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll put it back, believe you just set signing.required = false
will disable the sign tasks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// See https://github.com/johnrengelman/shadow/pull/831#discussion_r1119012328 | ||
required = false && gradle.taskGraph.hasTask("artifactoryPublish") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disabled for now.
de0c61d
to
65595ff
Compare
Follow up 1a57c16.