Organize manifest: do not remove reexported optional dependency

[HeikoKlare]

When a bundle has an optional dependency to A and an optional dependency to B while A depends on B, the "Organize manifest" tool removes the optional dependency to B. While this makes sense when the dependencies are not optional (as A is transitively required by B), this is not reasonable when the dependency to A is optional. In that case, A may be missing at runtime, still it might be reasonable to require B (optional or non-optional) to allow the usage of functionality that just depends on B.

This change enhances the cleanup logic to not remove transitive dependencies of optional dependencies.

It avoids the error that was introduced with this automated cleanup and readds the dependency that was erroneously removed:

• Clean Code for ui/org.eclipse.pde.runtime #2235

That cleanup effectively reverted this change:

• Add missing dependency for PDE Runtime to PDE Core #2213

With this change, the org.eclipse.pde.core dependency is not removed on manifest cleanup whereas is was removed without this change:

[eclipse-pde-bot]

This pull request changes some projects for the first time in this development cycle.
Therefore the following files need a version increment:

ui/org.eclipse.pde.runtime/META-INF/MANIFEST.MF

An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch.

Git patch

From 2713e83fc9cec68629253a60e16a58a80a26148d Mon Sep 17 00:00:00 2001
From: Eclipse PDE Bot <pde-bot@eclipse.org>
Date: Fri, 24 Jul 2026 14:06:51 +0000
Subject: [PATCH] Version bump(s) for 4.41 stream


diff --git a/ui/org.eclipse.pde.runtime/META-INF/MANIFEST.MF b/ui/org.eclipse.pde.runtime/META-INF/MANIFEST.MF
index 947eee4a75..1e07201a24 100644
--- a/ui/org.eclipse.pde.runtime/META-INF/MANIFEST.MF
+++ b/ui/org.eclipse.pde.runtime/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %name
 Bundle-SymbolicName: org.eclipse.pde.runtime; singleton:=true
-Bundle-Version: 3.9.100.qualifier
+Bundle-Version: 3.9.200.qualifier
 Bundle-Activator: org.eclipse.pde.internal.runtime.PDERuntimePlugin
 Bundle-Vendor: %provider-name
 Bundle-Localization: plugin
-- 
2.54.0

Further information are available in Common Build Issues - Missing version increments.

[github-actions]

Test Results

  129 files  ±0    129 suites  ±0   38m 38s ⏱️ -24s
3 522 tests ±0  3 468 ✅ ±0   54 💤 ±0  0 ❌ ±0 
9 369 runs  ±0  9 239 ✅ ±0  130 💤 ±0  0 ❌ ±0 

Results for commit 7a1e1ff. ± Comparison against base commit 91d8f89.

♻️ This comment has been updated with latest results.

[laeubi]

@HeikoKlare I do not think this holds true. It is a common misunderstanding that optional is anything special.

It just means the the resolver is allowed to resolve the bundle even though this requirement is not fulfill able and the code is well capable of handling this, what complete contradicts statement "We can't remove it because it is needed".

Optional requirements are not meant to pull in arbitrary thing that are actually needed! For this OSGi already offers other ways and I therefore do not think this is a valid change.

[merks]

Like @laeubi I'm not sure I fully understand what's really being fixed.

I tried removing these optional UI things:

Then there were these errors.

So yes, some of there are related to (provided by) pde.core. But I think only org.eclipse.pde.internal.runtime.spy.SpyFormToolkit could function without pde.ui. So the goal here is actually for the runtime to support some spy things (I think) when pde.ui isn't present and the cleanup in harming that goal? I can imagine that to be the case, but I specifically don't understand exactly what's "broken" by the removal of pde.core.

I wonder too the purpose of this file:

Maybe it's completely bogus because one could mark dependencies greedy in the MANIFEST.MF?

[HeikoKlare]

I should have been more precise in the PR description and repeated what we had discussed in the linked PR where we had an agreement of having these dependencies, which this change restores. Like Ed pointed to already, it's about the Plug-in Selection Spy being usable when having pde.core and pde.runtime in your runtime without the need to have pde.ui. With pde.runtime having an optional dependency to only pde.ui, you cannot use that spy if you only have pde.core available, whereas with pde.core as additional optional dependency it's possible.

I am not fully sure what has changed since the agreement for having these dependencies you posted here:

• Add missing dependency for PDE Runtime to PDE Core #2213 (comment)

To me, the change I proposed still make sense to me with your argumentation: I may have optional dependencies to two other bundles, with my bundle working well without either of them, but in some way enhancing functionality when one or both of them are available. If now one of those dependencies depends on the other and re-exports it, why should I be forced to only be able to pull in either both optional dependencies or none, because the cleanup always removes one of them? It could well be that just resolving the one optional dependency if available is already a desirable thing.

If you don't think this is reasonable: do you have an alternative proposal for dealing with the situation? Should we split up pde.runtime for cleaner dependencies and allowing the usage of the spy without pde.ui?

[laeubi]

@HeikoKlare it seems I somehow misunderstood the full scope here and as it was placed up before the loop I thought it would apply to all optional dependencies while now I think it is more targeting reexported dependencies (your description is not fully clear) as only those can be transitively provided?

[merks]

Ah yes, there is a nasty reeport to further complicate things.

So indeed one might determine that the direct require of org.eclipse.pde.core isn't needed because it's "provided" by org.eclipse.pde.ui which is already required, but with optional thrown into the mix, that's not a good determination.

[HeikoKlare]

it seems I somehow misunderstood the full scope here and as it was placed up before the loop I thought it would apply to all optional dependencies while now I think it is more targeting reexported dependencies (your description is not fully clear) as only those can be transitively provided?

Yes, my description was quite incomplete, as it did not even mention that it's about reexported optional dependencies. I am sorry for that. The change affects the calculation step that minimizes dependencies by checking for reexports. I see that it's a bit misleading as the check is placed before the loop. This is just correct because the whole loop is only about reexports. But I could still pull that check inside the loop so that it's more clear that is about the combination of reexported and optional dependency (accepting the slight performance impact of the unnecessary loop iterations for the sake of comprehensibility).
In any case, I will adapt the commit message accordingly, so that it properly reflects the motivation in the Git history.

[laeubi]

Yes, my description was quite incomplete, as it did not even mention that it's about reexported optional dependencies. I am sorry for that. The change affects the calculation step that minimizes dependencies by checking for reexports.

My opinion on that matter: Actually it should work the other way round! Relying on re-exported dependencies is actually worse and I wonder if we therefore should remove that function all together (independent of optional).

[HeikoKlare]

Relying on re-exported dependencies is actually worse and I wonder if we therefore should remove that function all together (independent of optional).

Do I understand correctly that you propose to completely eliminate this step of minimizing dependencies, i.e., always preserve dependencies to bundles that are reexported by other bundles I depend on, no matter if either of them is declared as optional?

I would be fine with doing it that way as well, but I personally have no strong opinion if this cleanup step should be removed completely or only for optional dependencies. I just want to mention that the feature was originally discussed here as desired behavior: https://bugs.eclipse.org/bugs/show_bug.cgi?id=128683

[merks]

Imagine having require bundle B with a specific range; a high lower bound because you need something new from B. But some other bundle you require re-exports B, but that bundle has a lower lower bound. In that case, regardless of optional, the required on bundle B should not be removed...

[HeikoKlare]

That sounds like a good example. Actually, in your example, if you define a range for the reexporting bundle, you do not even know what range of bundle B any of the version of that bundle inside the defined range may require. So to be safe, you have to leave the dependency in anyway.
A have just simulated that with the pde.core, pde.ui, pde.runtime example (high lower bound for pde.core in pde.runtime, lower lower bound for pde.core in pde.ui, none optional anymore) and, of course, the pde.core dependency is removed (even with the change proposed here).

In pde.runtime:

In pde.ui:

So should we conclude that this kind of cleanup for reexports should be removed completely?

[merks]

So should we conclude that this kind of cleanup for reexports should be removed completely?

I think so. I think maybe @laeubi is suggesting that as well. It seems best not to rely on reexport and it seems best to specify accurate lower bounds, so removal generally works in the opposite direction of both those things...

[laeubi]

I just want to mention that the feature was originally discussed here as desired behavior: https://bugs.eclipse.org/bugs/show_bug.cgi?id=128683

Sadly the bug is not accessible (for me) it just gave an error "Oops, something went wrong and
we're completely broken down."

So should we conclude that this kind of cleanup for reexports should be removed completely?

I would vote for that, re-exports are more a way to mitigate a package being moved from one bundle to the other. So if now we do not let people actually migrate how would such thing worn out ever - instead we now let even more people depend on the re-export.

I even would like the other way round: PDE warns you if you depend on a reexport and the user can possibly migrate (at best to import package).

[HeikoKlare]

Sadly the bug is not accessible (for me) it just gave an error "Oops, something went wrong and
we're completely broken down."

The thread is from 2006 and it said:

I was using the organize manifest to clean up org.eclipse.jface.txt/META-INF/MANIFEST.MF rev. 1.10 and it correctly remove the SWT bundle in the required section. However, it leaves in the commands bundle which is also re-exported by JFace.

We will obviously revert the fix for that expected behavior based on what we discussed.

I would vote for that, re-exports are more a way to mitigate a package being moved from one bundle to the other. So if now we do not let people actually migrate how would such thing worn out ever - instead we now let even more people depend on the re-export.

Agreed. To avoid confusion in this PR, I created a new one for that change and would close this one in favor of the other:

• Organize manifest: do not remove reexported dependencies #2404

I even would like the other way round: PDE warns you if you depend on a reexport and the user can possibly migrate (at best to import package).

That sounds reasonable. I would keep that for a potential follow-up enhancement and have the immediate change focussed on fixing the "organize manifest" behavior.