Skip to content

Add pom validation#55272

Merged
rjernst merged 7 commits intoelastic:masterfrom
rjernst:pom_test
Apr 16, 2020
Merged

Add pom validation#55272
rjernst merged 7 commits intoelastic:masterfrom
rjernst:pom_test

Conversation

@rjernst
Copy link
Copy Markdown
Member

@rjernst rjernst commented Apr 15, 2020

The pom files for our published artifacts are sent to maven central
during Elastic's release process, but we may not found out until then
that we have inadvertently broken the pom structure, as has happened
several times before. This commit adds validation of the pom file
specifically for the rules required by maven central.

The pom files for our published artifacts are sent to maven central
during Elastic's release process, but we may not found out until then
that we have inadvertently broken the pom structure, as has happened
several times before. This commit adds validation of the pom file
specifically for the rules required by maven central.
@elasticmachine
Copy link
Copy Markdown
Collaborator

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

@rjernst rjernst requested a review from mark-vieira April 15, 2020 22:45
Copy link
Copy Markdown
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 couple of comments, otherwise LGTM.


@Override
public void apply(Project project) {
project.getTasks().withType(GenerateMavenPom.class, generateMavenPom -> {
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.

FWIW, doing withType here is going to eagerly create those generate pom tasks. But we are in a bit of a pickle becuase using configureEach is going to barf here as you can't register a task in a lazy task config block.

Perhaps we can key this off of the publications themselves instead of the generate pom task. We know for each publication there's going to be a generate pom task, maybe do that instead? So something like publishing.publications.all { // create validate task }?

I realize this is being a bit pedantic about not doing eager task creation but this seems like a challenge you're up to 😉

import java.util.function.Consumer;
import java.util.function.Predicate;

public class PomValidationTask extends DefaultTask {
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.

Should we make this extend PrecommitTask so it creates a marker file and thus could be UP-TO-DATE when the pom doesn't change? Right now this task will always run because it produces no output.

@TaskAction
public void checkPom() throws Exception {
MavenXpp3Reader reader = new MavenXpp3Reader();
Model model = reader.read(new FileReader(pomFile.getAsFile().get()));
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.

We should probably put this in a try-with-resources block.

validateNonNull("scm", model.getScm(), () -> validateString("scm.url", model.getScm().getUrl()));

if (foundError) {
throw new GradleException("Pom file [" + pomFile.getAsFile().get() + "] failed validation");
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.

Maybe add a message like "see console log for details".

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.

Also, let's use one of these verification failure prefixes so that when we search for failures in Gradle Enterprise, this kinds of failures show up as a "verification" failure, which is what it is.

@rjernst
Copy link
Copy Markdown
Member Author

rjernst commented Apr 16, 2020

@elasticmachine run elasticsearch-ci/docs

@rjernst rjernst merged commit b2c9d68 into elastic:master Apr 16, 2020
@rjernst rjernst deleted the pom_test branch April 16, 2020 22:58
rjernst added a commit to rjernst/elasticsearch that referenced this pull request Apr 16, 2020
The pom files for our published artifacts are sent to maven central
during Elastic's release process, but we may not found out until then
that we have inadvertently broken the pom structure, as has happened
several times before. This commit adds validation of the pom file
specifically for the rules required by maven central.
rjernst added a commit that referenced this pull request Apr 17, 2020
The pom files for our published artifacts are sent to maven central
during Elastic's release process, but we may not found out until then
that we have inadvertently broken the pom structure, as has happened
several times before. This commit adds validation of the pom file
specifically for the rules required by maven central.
rjernst added a commit that referenced this pull request Apr 17, 2020
The pom files for our published artifacts are sent to maven central
during Elastic's release process, but we may not found out until then
that we have inadvertently broken the pom structure, as has happened
several times before. This commit adds validation of the pom file
specifically for the rules required by maven central.
@mark-vieira mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Delivery/Build Build or test infrastructure >enhancement Team:Delivery Meta label for Delivery team v7.7.1 v7.8.0 v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants