Skip to content
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

Move publishing configuration to a separate plugin #56727

Merged
merged 8 commits into from
May 15, 2020

Conversation

rjernst
Copy link
Member

@rjernst rjernst commented May 13, 2020

This is another part of the breakup of the massive BuildPlugin. This PR
moves the code for configuring publications to a separate plugin. Most
of the time these publications are jar files, but this also supports the
zip publication we have for integ tests.

This is another part of the breakup of the massive BuildPlugin. This PR
moves the code for configuring publications to a separate plugin. Most
of the time these publications are jar files, but this also supports the
zip publication we have for integ tests.
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (:Core/Infra/Build)

@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label May 13, 2020
Copy link
Contributor

@mark-vieira mark-vieira left a comment

Choose a reason for hiding this comment

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

Left a few comments but otherwise LGTM. Thanks, Ryan!

scmNode.appendNode("url", BuildParams.getGitOrigin());
}

private static void configureJavadoc(Project project) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should better live in the ElasticsearchJavaPlugin. This is dealing with the actual generation of javadocs, not creating an artifact to publish. Every java project gets the javadoc task created for it, so due to that, I think we should have consistent behavior there, even if we rarely if ever generate javadocs for modules that aren't also published.

javadocOptions.addBooleanOption("html5", true);
});
// ensure javadoc task is run with 'check'
project.getTasks()
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps this bit we leave in the publish plugin, as we don't necessarily want to fail builds for javadoc errors on unpublished modules.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we do still want this in those cases. If we are writing javadocs, we should make sure they are valid.

private static void configurePomGeneration(Project project) {

TaskProvider<Task> generatePomTask = project.getTasks().register("generatePom");
// generatePomTask.configure(t -> t.dependsOn("generatePomFileForNebulaPublication"));
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be commented out?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope this can be deleted. It's handled by the last block for all publishing tasks.

publication.getPom().withXml(PublishPlugin::addScmInfo);

// have to defer this until archivesBaseName is set
project.afterEvaluate(p -> publication.setArtifactId(getArchivesBaseName(project)));
Copy link
Contributor

Choose a reason for hiding this comment

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

I see we no longer do this explicitly after applying the shadow plugin. I put that there to workaround some very specific configuration ordering issues. Please confirm that the artifact id for the poms generated for the HLRC and JDBC drive are correct. They should be elasticsearch-rest-high-level-client and x-pack-sql-jdbc, respectively.

To be even more thorough its probably worth doing a diff on all POMs, given we fuck something up every time we touch publishing code. When I was testing this I ran ./gradlew generatePom copied all the files to a folder, applied my changes, generated again, then did a diff to manually confirm my PR didn't change how POMs were generated.

Copy link
Member Author

Choose a reason for hiding this comment

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

I double checked both those poms and they are correct. I will do a diff as you suggested.

} catch (IOException e) {
throw new UncheckedIOException(e);
}
javadoc.setClasspath(javadoc.getClasspath().filter(f -> classes.contains(f) == false));
Copy link
Contributor

Choose a reason for hiding this comment

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

We can simplify this a lot. If all we want is the compile class minus the source set output we can just use the main source set's getCompileClasspath().

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh I see what you mean now. Done.

@rjernst rjernst merged commit c0ee68b into elastic:master May 15, 2020
@rjernst rjernst deleted the buildsplit6 branch May 15, 2020 01:59
rjernst added a commit that referenced this pull request May 15, 2020
This is another part of the breakup of the massive BuildPlugin. This PR
moves the code for configuring publications to a separate plugin. Most
of the time these publications are jar files, but this also supports the
zip publication we have for integ tests.
rjernst added a commit that referenced this pull request May 15, 2020
This is another part of the breakup of the massive BuildPlugin. This PR
moves the code for configuring publications to a separate plugin. Most
of the time these publications are jar files, but this also supports the
zip publication we have for integ tests.
rjernst added a commit that referenced this pull request May 15, 2020
This is another part of the breakup of the massive BuildPlugin. This PR
moves the code for configuring publications to a separate plugin. Most
of the time these publications are jar files, but this also supports the
zip publication we have for integ tests.
@mark-vieira mark-vieira added Team:Delivery Meta label for Delivery team and removed Team:Core/Infra Meta label for core/infra team labels Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants