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

Install a specific version of Maven #32

Merged
merged 1 commit into from
Oct 4, 2024

Conversation

jonesbusy
Copy link
Contributor

This fixes occurrence of

[ERROR] The build could not read 1 project -> [Help 1]
org.apache.maven.project.ProjectBuildingException: Some problems were encountered while processing the POMs:
[ERROR] Unknown packaging: hpi @ io.jenkins.plugins:flyway-api:${revision}-${changelist}, /home/runner/work/flyway-api-plugin/flyway-api-plugin/pom.xml, line 13, column 14
at org.apache.maven.project.DefaultProjectBuilder.build (DefaultProjectBuilder.java:397)
at org.apache.maven.graph.DefaultGraphBuilder.collectProjects (DefaultGraphBuilder.java:414)

Seen in https://github.com/jenkinsci/flyway-api-plugin/pull/78/checks for example

Confirmed this work by running this adapted workflow on https://github.com/jenkinsci/flyway-api-plugin/pull/99/checks

Except the maven version upgrade, the rest remain outouched

Possible caused by recent changes https://github.com/jenkinsci/maven-hpi-plugin ?

I can easily reproduce locally with Maven 3.8.8.

@daniel-beck
Copy link
Contributor

jenkinsci/maven-hpi-plugin#668 is another reason to ensure a recent Maven.

@basil
Copy link

basil commented Sep 27, 2024

See actions/setup-java#685 (a different fix for the same issue) for comparison. I am not implying that one solution is better or worse than another, but rather I am mentioning the alternative for completeness.

@jonesbusy
Copy link
Contributor Author

See actions/setup-java#685 (a different fix for the same issue) for comparison. I am not implying that one solution is better or worse than another, but rather I am mentioning the alternative for completeness.

Thanks I saw also this page. To be honest I don't have a strong opinion about the approach. As long is consistent across Jenkins reusable workflows

I just choose this action s4u/setup-maven-action because it looks well maintained, and less code to maintains on the workflow

Let me know and I can adapt the PR with the choose approach

Copy link
Contributor

@daniel-beck daniel-beck left a comment

Choose a reason for hiding this comment

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

It's unclear what the benefit of s4u/setup-maven-action is compared to stCarolas/setup-maven (which it uses internally).

If we're not doing the download manually ourselves, having a smaller dependency seems better.

If we want consistency between workflows, I'll defer to the CD workflow maintainers, that's the more important one.

.github/workflows/jenkins-security-scan.yaml Outdated Show resolved Hide resolved
@jonesbusy
Copy link
Contributor Author

Once decided I will update this PR with the choose approach

Copy link

@basil basil left a comment

Choose a reason for hiding this comment

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

jenkins-infra/github-reusable-workflows#37 was merged, so can we update this PR to match for consistency?

@jonesbusy
Copy link
Contributor Author

Sure I will update tomorrow by doing the maven download instead of composite action

@jonesbusy
Copy link
Contributor Author

I've applied the same change here.

Ensured the maven version is correct by running test workflow here https://github.com/jenkinsci/flyway-api-plugin/pull/109/checks

@jonesbusy jonesbusy changed the title Use composite setup-maven-action to allow using recent maven version Install a specific version of Maven Oct 4, 2024
Copy link

@basil basil left a comment

Choose a reason for hiding this comment

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

@timja timja merged commit dfdc0c6 into jenkins-infra:main Oct 4, 2024
@jonesbusy jonesbusy deleted the feature/recent-maven branch October 4, 2024 19:06
@timja
Copy link
Member

timja commented Oct 4, 2024

@basil
Copy link

basil commented Oct 4, 2024

Thank you very much!

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.

5 participants