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

BndConfiguration.getConfiguration(List<Plugin>) does not take into account global configuration #6390

Open
kwin opened this issue Dec 5, 2024 · 7 comments

Comments

@kwin
Copy link
Contributor

kwin commented Dec 5, 2024

The code at

private Optional<Xpp3Dom> getConfiguration(List<Plugin> plugins) {
does not take the global configuration into account.

For example if the parent pom has something like

...
<plugin>
    <groupId>biz.aQute.bnd</groupId>
    <artifactId>bnd-maven-plugin</artifactId>
    <executions>
        <execution>
            <id>bnd-process</id>
            <goals>
                <goal>bnd-process</goal>
            </goals>
            <configuration>
                <bnd>Bundle-Category: Something</bnd>
           </configuration>
      </execution>
  </executions>
</plugin>

and the child pom has

<plugin>
                <groupId>biz.aQute.bnd</groupId>
                <artifactId>bnd-maven-plugin</artifactId>
                <configuration>
                    <bnd>Fragment-Host: some-bsn</bnd>
                </configuration>
            </plugin>

The configuration being returned by

private Optional<Xpp3Dom> getConfiguration(List<Plugin> plugins) {
for the plugin of the child pom is just containing the configuration of the parent POM.

The logic is missing some merge logic between execution configuration and global configuration (compare also with https://stackoverflow.com/questions/33908315/what-is-the-difference-between-executions-and-configurations-in-a-maven-plugin).

@stbischof
Copy link
Contributor

Would you be able to do a PR?

@kwin
Copy link
Contributor Author

kwin commented Dec 5, 2024

I am still trying to work out the inner Maven logic. We need to reproduce it in bnd due to the bnd instruction inheritance which deviates a bit from the Maven Plugin configuration inheritance

I found https://github.com/apache/maven/blob/maven-3.9.x/maven-model-builder/src/main/java/org/apache/maven/model/plugin/DefaultPluginConfigurationExpander.java, but not sure yet when this kicks in.

I opened https://issues.apache.org/jira/browse/MNGSITE-544 to document the status quo a bit better.

@laeubi
Copy link
Contributor

laeubi commented Dec 11, 2024

I think this is actually expected and nothing bnd should/need to handle because maven already merges the configurations appropriately..

In your child you provide a configuration that would apply to all executions but in your specific execution you override this so it is merged with the plugin one and effectively overriding what is configured in the child. This would be different if you specify an additional execution without configuration, then your general configuration will take precedence.

To accomplish what you described your child configuration must look like this:

<plugin>
    <groupId>biz.aQute.bnd</groupId>
    <artifactId>bnd-maven-plugin</artifactId>
    <executions>
        <execution>
            <id>bnd-process</id>
            <configuration>
                <bnd>Fragment-Host: some-bsn</bnd>
           </configuration>
      </execution>
  </executions>
</plugin>

@kwin
Copy link
Contributor Author

kwin commented Dec 11, 2024

The merging mechanism in Maven and Bnd differs! bnd-maven-plugin deliberately deviates from Maven standards to allow merging of bnd configurations (which wouldn't be possible in Maven). Therefore also the same kind of merging should happen on all levels.

  1. Parent POMs
  2. Plugin Mgmt -> Plugins
  3. Plugin -> Plugin Executions

Currently the bnd specific merging is only implemented for 2.

@laeubi
Copy link
Contributor

laeubi commented Dec 11, 2024

Currently the bnd specific merging is only implemented for 2.

bnd-maven already considers the parent pom so have you tried my suggestion?

@kwin
Copy link
Contributor Author

kwin commented Dec 11, 2024

parent pom is not correctly considered: for example having multiple parent levels with each having bnd plugin parameter will just be merged by Maven logic, i.e. just one value wins. It is not merged one level below (within the bnd instructions)!

@laeubi
Copy link
Contributor

laeubi commented Dec 11, 2024

Can you probably give a runnable example (or even better, provide a testcase here)?

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

No branches or pull requests

3 participants