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

[Build] Unify bnd-maven-plugin and maven-jar-plugin invocations #509

Merged
merged 1 commit into from
Jun 21, 2024

Conversation

HannesWell
Copy link
Contributor

Keep the 'bnd' configuration for the bnd-maven-plugin at plugin level to simplify overwrites in sub-modules (avoid the need to re-specify the execution there).

Additionally update to bnd-maven-plugin 6.4.0.

If this would be applied before #505, it would reduce the 'noise' in that commit due to necessary re-configuraiton.

@cstamas and @gnodet if you think this change is fine, I can also forward port it to master. I also left out a JIRA ticket for such non-functional 'clean-up', let me know if it is still necessary.

@gnodet
Copy link
Contributor

gnodet commented Jun 20, 2024

I had a build failure locally, will see what GH says about it...

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-jar-plugin:3.4.0:jar (default-jar) on project maven-resolver-demo-snippets: Error assembling JAR: Manifest file: /Users/gnodet/work/git/maven-resolver/maven-resolver-demos/maven-resolver-demo-snippets/target/classes/META-INF/MANIFEST.MF does not exist. -> [Help 1]

@HannesWell
Copy link
Contributor Author

I had a build failure locally, will see what GH says about it...

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-jar-plugin:3.4.0:jar (default-jar) on project maven-resolver-demo-snippets: Error assembling JAR: Manifest file: /Users/gnodet/work/git/maven-resolver/maven-resolver-demos/maven-resolver-demo-snippets/target/classes/META-INF/MANIFEST.MF does not exist. -> [Help 1]

Can reproduce that with a mvn clean verify (only run mvn verify before).

I think it can be fixed by either clearing the archive element in the demo-snippets like it is done in the demo-plugin with:

<archive combine.self="override" />

Or just skip the jar and also install mojo execution for all demo plugins. I assume they don't have to be installed (in the build?) anyways?

@gnodet
Copy link
Contributor

gnodet commented Jun 20, 2024

I had a build failure locally, will see what GH says about it...

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-jar-plugin:3.4.0:jar (default-jar) on project maven-resolver-demo-snippets: Error assembling JAR: Manifest file: /Users/gnodet/work/git/maven-resolver/maven-resolver-demos/maven-resolver-demo-snippets/target/classes/META-INF/MANIFEST.MF does not exist. -> [Help 1]

Can reproduce that with a mvn clean verify (only run mvn verify before).

I think it can be fixed by either clearing the archive element in the demo-snippets like it is done in the demo-plugin with:

<archive combine.self="override" />

Or just skip the jar and also install mojo execution for all demo plugins. I assume they don't have to be installed (in the build?) anyways?

@cstamas any preference ?

Keep the 'bnd' configuration for the bnd-maven-plugin at plugin level to
simplify overwrites in sub-modules (avoid the need to re-specify the
execution there).

Additionally update to bnd-maven-plugin 6.4.0
@HannesWell
Copy link
Contributor Author

HannesWell commented Jun 20, 2024

I didn't found a simple skip property for the jar-plugin and therefore went with the first approach.
This also made me reconsider the change of the m-jar-plugin config in the root pom which now lead to less changes.

A mvn clean verify now passed locally for me.

@gnodet gnodet merged commit 7466cae into apache:maven-resolver-1.9.x Jun 21, 2024
11 checks passed
@gnodet
Copy link
Contributor

gnodet commented Jun 21, 2024

@HannesWell could you create a similar PR for master please ?

@rfscholte
Copy link
Contributor

fyi, this broke #508

@HannesWell
Copy link
Contributor Author

Thanks for your review and submitting this.

@HannesWell could you create a similar PR for master please ?

I was about to do that but saw, you already did it. Thanks for that.

HannesWell pushed a commit to HannesWell/apache.maven-resolver that referenced this pull request Jun 21, 2024
@gnodet gnodet added this to the 2.0.0 milestone Jul 1, 2024
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.

4 participants