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

[bnd-maven-plugin] Running bnd-process without mvn clean causes different behavior #6221

Closed
Marcono1234 opened this issue Aug 17, 2024 · 13 comments

Comments

@Marcono1234
Copy link

Version

  • 6.4.0
  • 7.1.0-SNAPSHOT

Description

It looks like the behavior of bnd-maven-plugin's bnd-process differs when the Maven build is executed a second time, without mvn clean, respectively when target/classes/META-INF/MANIFEST.MF exists from a previous run.

Reproduction steps

  1. Download the attached bnd-test.zip and extract it
  2. Run mvn process-classes
  3. Create a copy of target/classes/META-INF/MANIFEST.MF named clean-MANIFEST.MF
    (but don't delete MANIFEST.MF)
  4. Run mvn process-classes again

Now compare clean-MANIFEST.MF with MANIFEST.MF, you should see the following:
(clean-MANIFEST.MF being old, MANIFEST.MF being new)

  Manifest-Version: 1.0
  Bundle-ManifestVersion: 2
  Bundle-Name: my-app
  Bundle-SymbolicName: my-app
  Bundle-Version: 1.0.0.SNAPSHOT
- Export-Package: example.a;uses:="example.b";version="1.0.0",example.b;
+ Export-Package: example.a;version="1.0.0";uses:="example.b",example.b;
   version="1.0.0",example.c;version="1.0.0"
- Import-Package: example.b,example.c;version="${range;[==,=+)}"
+ Import-Package: example.b;version="[1.0,2)",example.c;version="[1.0,1.
+  1)"
  Require-Capability: osgi.ee;filter:="(&(osgi.ee=JavaSE)(version=1.8))"
clean-MANIFEST.MF MANIFEST.MF
Export-Package first lists uses then version first lists version then uses
Import-Package range macro was not expanded (separate bug?) range macro was expanded, as desired
Import-Package example.b has no version range (see also #6220) example.b has a version range
@chrisrueger
Copy link
Contributor

I spoted a comment in the code and wonder if that might be related:

// However, if extensions for this plugin are not enabled

// However, if extensions for this plugin are not enabled
// the maven-(jar|war)-plugin will also execute, resulting in
// conflicting results

and some outOfDate() checks in

private void writeManifest(Jar jar, File manifestPath) throws Exception {
and
private void attachArtifactToProject(Jar bndJar) throws Exception {

Without anymore background knowledge I am wondering:

  • bndJar.write(os); and / or jar.writeManifest(manifestOut) are inside these outofdate checks

Is it possible that some other part is creating the Manifest in one case so that it does not go through bnd?

@Marcono1234
Copy link
Author

Marcono1234 commented Aug 18, 2024

Is it possible that some other part is creating the Manifest in one case so that it does not go through bnd?

Is that a question to me, or the other maintainers of this project?

Though with the bnd-test.zip provided above, in both cases it definitely goes (to some extent?) through bnd, because:

  • bnd configuration is only specified in <configuration><bnd> of the plugin; there is no pre-existing MANIFEST.MF
  • bnd is in both cases generating all the Export-Package and Import-Package, and the other bundle attributes (but with the differences mentioned above)

With Maven debug logging the only difference for the second run seems to be this additional line:

[DEBUG] collecting Contracts  from classes

It looks like this comes from here:

contracts.collectContracts(jar.getName(), pcs);

So it seems bnd is in some way reading the manifest it generated previously. But I would still expect that the results are then (ideally) the same. Besides the functionality-wise differences mentioned above this otherwise can also break reproducible builds.

It appears a hacky workaround is then to specify a custom bnd-maven-plugin <configuration><manifestPath> to prevent bnd from picking up its own manifest in the second run, for example:

<manifestPath>${project.build.outputDirectory}/META-INF/bnd-MANIFEST.MF</manifestPath>

(and then accordingly specifying maven-jar-plugin's manifestFile)

@chrisrueger
Copy link
Contributor

It was more a thought out loud for anyone reading this😃 but i also did some debugging and found differences in Analyzer.java i will post details later when I'm back.

@chrisrueger
Copy link
Contributor

chrisrueger commented Aug 18, 2024

First of all thanks for the reproducer zip file .
Just for sake of completeness I will link the docu of the bnd-maven-plugin, _range macro and Version Policy for further reference.

I did some debugging. Here are some thoughts:

  1. With "clean" exportVersion is null in Analyzer.augmentImports() so it goes into the branch
    in here https://github.com/bndtools/bnd/blob/master/biz.aQute.bndlib/src/aQute/bnd/osgi/Analyzer.java#L2055

In this branch Analyzer.applyVersionPolicy() is not called.

importAttributes.put(VERSION_ATTRIBUTE, importRange);

This branch was touched in e12204d#diff-a526f72b59db4b5fb903e0254a07d2c143fb11a3ce160df375348a0399f7cf0bR2031-R2033

  1. without "clean" it goes into the other branch where exportVersion==1.0.0 and the importVersion range is calculated based on that https://github.com/bndtools/bnd/blob/master/biz.aQute.bndlib/src/aQute/bnd/osgi/Analyzer.java#L2079

In this branch Analyzer.applyVersionPolicy() is called and applies processing of the _range macro

importRange = applyVersionPolicy(exportVersion, importRange, provider);

So it seems bnd is in some way reading the manifest it generated previously.

It looks like it. Analyzer.getManifestInfoFromClasspath() is getting a non-null Manifest in the 2nd "no-clean" case.

@pkriens thoughts?

@chrisrueger
Copy link
Contributor

chrisrueger commented Aug 18, 2024

@Marcono1234

I could get consistent results (clean, and no clean) when changing

<configuration>
    <bnd><![CDATA[
        -exportcontents: example.a, example.b, example.c
        Import-Package: example.c;version="${range;[==,=+)}", *
    ]]></bnd>
</configuration>

to

<configuration>
    <bnd><![CDATA[
        Export-Package: example.a;version=1.0.0, example.b;version=1.0.0, example.c;version=1.0.0
        Import-Package: example.c;version="${range;[==,=+)}", *
    ]]></bnd>
</configuration>

always results in this Manifest.mf

Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: my-app
Bundle-SymbolicName: my-app
Bundle-Version: 1.0.0.SNAPSHOT
Export-Package: example.a;version="1.0.0";uses:="example.b",example.b;
 version="1.0.0",example.c;version="1.0.0"
Import-Package: example.b;version="[1.0,2)",example.c;version="[1.0,1.
 1)"
Require-Capability: osgi.ee;filter:="(&(osgi.ee=JavaSE)(version=1.8))"


Update: also works with:

-exportcontents: example.a;version=1.0.0, example.b;version=1.0.0, example.c;version=1.0.0

and also

Export-Package: *;version=1.0.0

but the latter that could be too broad. I think it is generally better to be specific.

I don't have a good explanation for the behavior. Just found out about it e.g. in #1284

@chrisrueger
Copy link
Contributor

Here is the same project as a bnd-workspace.

bnd-test-bndworkspace.zip

Maybe it helps debugging to track down things, in case something needs to be done.

@chrisrueger
Copy link
Contributor

One thing I noticed during my experiments:

I tried placing a packageinfo file in the packages as an alternative to versioning the Exports in Manifest.
I noticed that pakageinfo is ignored when running mvn process-classes or mvn clean process-classes. But those files were not found to the target folder, thus were not found by bnd.

is this expected behavior? @pkriens

In my bnd-workspace case (see comment above) those packageinfo files did get picked up and I got the same result as putting versions in the Export-Package instruction.

@pkriens
Copy link
Member

pkriens commented Aug 19, 2024

What seems wrong is that it picks up the manifest in the target/META-INF folder. This is output and should not be picked up.

Since there are no versions set, the macro has nothing to base its work on so getting the macro is ok, although I am surprised it does not generate a warning. This is why you set the versions explicitly it is ok.

But the fact that bnd sees is the target folder's Manifest is concerning.

@chrisrueger
Copy link
Contributor

But the fact that bnd sees is the target folder's Manifest is concerning.

https://github.com/bndtools/bnd/blob/master/maven-plugins/bnd-maven-plugin/src/main/java/aQute/bnd/maven/plugin/AbstractBndMavenPlugin.java#L261

Might the code around here be a reason? @pkriens

@pkriens
Copy link
Member

pkriens commented Aug 19, 2024

Yes, that seems to be the culprint ... I guess we need to delete the manifest if it is in the target/classes directory.

@chrisrueger
Copy link
Contributor

chrisrueger commented Aug 19, 2024

Thanks.

Do you have an idea to my other observation that the packageinfo files are not considered?

I tried placing a packageinfo file in the packages as an alternative to versioning the Exports in Manifest.
I noticed that pakageinfo is ignored when running mvn process-classes or mvn clean process-classes. But those files were not found to the target folder, thus were not found by bnd.

I would have expected them in the target Folder. Not sure if this is within the responsibility of the bnd-maven plugin.

@chrisrueger
Copy link
Contributor

@Marcono1234 I have added a fix in #6222 which should be available in next bnd 7.1 snapshot (once reviewed and merged). This shoud fix at least the problem you describe initially with the inconsistent results with repeated mvn process-classes (without mvn clean)

@chrisrueger
Copy link
Contributor

Fixed via #6222

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