-
Notifications
You must be signed in to change notification settings - Fork 25.1k
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
Make forbidden apis check cacheable and cc compatible #101217
Conversation
Pinging @elastic/es-delivery (Team:Delivery) |
6a1f6e8
to
8ddbfee
Compare
f0c6a65
to
6c374c6
Compare
@@ -88,8 +88,6 @@ public class DependencyLicensesTask extends DefaultTask { | |||
|
|||
private final Logger logger = Logging.getLogger(getClass()); | |||
|
|||
private static final String SHA_EXTENSION = ".sha1"; |
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.
not used; removed while been there
@@ -65,60 +52,36 @@ public TaskProvider<? extends Task> createTask(Project project) { | |||
String sourceSetTaskName = sourceSet.getTaskName(FORBIDDEN_APIS_TASK_NAME, null); | |||
var sourceSetTask = project.getTasks().register(sourceSetTaskName, CheckForbiddenApisTask.class, t -> { | |||
t.setDescription("Runs forbidden-apis checks on '${sourceSet.name}' classes."); | |||
t.setResourcesDir(resourcesDir); | |||
t.getOutputs().upToDateWhen(Specs.SATISFIES_ALL); | |||
t.setClassesDirs(sourceSet.getOutput().getClassesDirs()); | |||
t.dependsOn(resourcesTask); | |||
t.setClasspath(sourceSet.getRuntimeClasspath().plus(sourceSet.getCompileClasspath()).plus(sourceSet.getOutput())); |
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.
Why all the plus
nonsense here? Shouldn't the runtime classpath include all that stuff?
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 recall I explicitly added the getOutput() to this quite a while ago. I don't recall why that is needed. The compileclasspath is required as we have a lot of compileOnly dependencies declared and we need those to run forbiddenapis check successfully.
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.
Fair enough.
This work was related to #57918 |
Also ported the implementation to use the gradle worker api that allows us to expand our parallalism of task execution within a single project. Example build scan: https://gradle-enterprise.elastic.co/s/non7hf532jrlg/timeline?sort=longest&type=org.elasticsearch.gradle.internal.precommit.CheckForbiddenApisTask