Switch to the correct BuildContext API

[gnodet]

This avoids rebuilding the jar when nothing has changed, and in projects with subproject, avoids downstream project recompilations.

[bjhargrave]

Seems like this capability should be part of the build context rather than requiring all plugins to do something like this :-)

[cstamas]

This build context (the one you use)? https://github.com/sonatype/sisu-build-api

[cstamas]

No, my mistake, sorry, the used one is this https://github.com/codehaus-plexus/plexus-build-api

[bjhargrave]

We use this one:

bnd/maven-plugins/bnd-maven-plugin/src/main/java/aQute/bnd/maven/plugin/AbstractBndMavenPlugin.java

Lines 169 to 170 in a644f2d

	@Component
	BuildContext buildContext;

It is injected by maven.

[gnodet]

I think we're on the wrong track. The default build context should already cache the stream, see https://github.com/codehaus-plexus/plexus-build-api/blob/903ed6c3e9dc767da4f9ac6cf5a22f0324400bbf/src/main/java/org/codehaus/plexus/build/DefaultBuildContext.java#L96-L101

However, multiple successive runs of bad generate different output, for example:

< Export-Package: org.eclipse.aether;uses:="org.eclipse.aether.artifact,
<  org.eclipse.aether.collection,org.eclipse.aether.deployment,org.eclip
<  se.aether.graph,org.eclipse.aether.installation,org.eclipse.aether.me
<  tadata,org.eclipse.aether.repository,org.eclipse.aether.resolution,or
<  g.eclipse.aether.scope,org.eclipse.aether.transfer";version="2.0.11",
---
> Export-Package: org.eclipse.aether;version="2.0.11";uses:="org.eclipse
>  .aether.artifact,org.eclipse.aether.collection,org.eclipse.aether.dep
>  loyment,org.eclipse.aether.graph,org.eclipse.aether.installation,org.
>  eclipse.aether.metadata,org.eclipse.aether.repository,org.eclipse.aet
>  her.resolution,org.eclipse.aether.scope,org.eclipse.aether.transfer",

is there a way to have a consistent manifest written ?

[chrisrueger]

lack of build idempotency

Not sure if I interpret "idempotency" correctly, but if you are referring to reproducible build outputs maybe have a look at https://bnd.bndtools.org/instructions/reproducible.html and https://bnd.bndtools.org/instructions/noextraheaders.html (also see https://github.com/bndtools/bnd/blob/master/maven-plugins/bnd-maven-plugin/README.md#reproducible-builds )

This is mainly about timestamps.. but I will have a look if it is used for something else.

Question: Whic bnd version are you using here?

[chrisrueger]

@gnodet @cstamas I just remembered a past issue which might be related to your problem (not sure):
#6221 (comment) and the PR #6222

I believe the #6222 should be in bnd 7.1.0 https://github.com/bndtools/bnd/releases/tag/7.1.0

Docs: https://github.com/bndtools/bnd/blob/master/maven-plugins/bnd-maven-plugin/README.md#configuration-parameters-1

see the deleteExistingManifest option

[gnodet]

Sure, but that looks like a work-around. The input looks the same, just the order of clause seems different. Given that has no effect in OSGi, a fix order should be used somehow I think.

[chrisrueger]

@gnodet

I had a quick look and maaaybe a suspicion:

I noticed that in your debug output above the uses: directive comes first in example A, and then at the end in Example B.
So this means the attributes (Attrs) of the Export-Package parameter (or any) are ordered differently... potentially in insertion-order.

bnd/biz.aQute.bndlib/src/aQute/bnd/osgi/Analyzer.java

Lines 2465 to 2469 in a644f2d

	override = override.substring(1);
	if (override.isEmpty()) {
	clause.remove(USES_DIRECTIVE);
	} else {
	clause.put(USES_DIRECTIVE, override);

And given the comment in

bnd/biz.aQute.bndlib/src/aQute/bnd/osgi/Analyzer.java

Line 2426 in a644f2d

// Check if someone already set the uses: directive

It looks like this could be your case where "someone else" is your previous build.

Attrs.java

bnd/biz.aQute.bndlib/src/aQute/bnd/header/Attrs.java

Lines 86 to 88 in a644f2d

	public Attrs() {
	this(new LinkedHashMap<>(), new HashMap<>());
	}

is initialized with a LinkedHashMap which indicates insertion order.

Maybe we should think about using a Treemap instead so that attributes are consistently ordered by key.

Thoughts @bjhargrave @pkriens ?

[bjhargrave]

Or fix where emitted. Isn't that already done?

[gnodet]

I repurposed this PR to switch to the correct library for BuildContext.
That one does indeed support using caching streams.

[chrisrueger]

Given BJ's 👍 and since this change became minimal I think we can give this a try. Thanks a lot for the PR.