Skip to content

Polish outdated configs#831

Merged
johnrengelman merged 5 commits intoGradleUp:masterfrom
Goooler:more
Mar 20, 2023
Merged

Polish outdated configs#831
johnrengelman merged 5 commits intoGradleUp:masterfrom
Goooler:more

Conversation

@Goooler
Copy link
Copy Markdown
Member

@Goooler Goooler commented Feb 27, 2023

Follow up 1a57c16.

Comment on lines -56 to -68
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'
}
}
Copy link
Copy Markdown
Member Author

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?

Comment on lines -14 to -17
// Remove the gradleApi so it isn't merged into the jar file.
configurations.named(JavaPlugin.API_CONFIGURATION_NAME) {
dependencies.remove(project.dependencies.gradleApi())
}
Copy link
Copy Markdown
Member Author

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) 

Copy link
Copy Markdown
Collaborator

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.

Copy link
Copy Markdown
Collaborator

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.

Copy link
Copy Markdown
Collaborator

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.

}

tasks.register('sourcesJar', Jar) {
def sourcesJar = tasks.register('sourcesJar', Jar) {
Copy link
Copy Markdown
Collaborator

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()
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good catch!

id 'project-report'
id 'idea'
id 'java-gradle-plugin'
// id 'signing'
Copy link
Copy Markdown
Collaborator

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.

Copy link
Copy Markdown
Member Author

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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Comment on lines +45 to +46
// See https://github.com/johnrengelman/shadow/pull/831#discussion_r1119012328
required = false && gradle.taskGraph.hasTask("artifactoryPublish")
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Disabled for now.

@Goooler Goooler changed the title Remove outdated configs Polish outdated configs Feb 27, 2023
@Goooler Goooler force-pushed the more branch 3 times, most recently from de0c61d to 65595ff Compare March 2, 2023 16:15
@johnrengelman johnrengelman added this to the 8.1.1 milestone Mar 20, 2023
@johnrengelman johnrengelman merged commit 7f7a26a into GradleUp:master Mar 20, 2023
@Goooler Goooler deleted the more branch March 21, 2023 01:24
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.

2 participants