Skip to content

Conversation

ptziegler
Copy link
Contributor

Using the Import-Header entries allows consumers to contribute their own Lucene bundles, without having to match the symbolic name defined by Orbit.

@ptziegler
Copy link
Contributor Author

ptziegler commented Sep 1, 2025

For context: We maintain three releases of our application, the oldest one based on the Eclipse 2023-12. Because of CVE-2024-45772, we need to update to a newer Lucene version. But because the Help plugin expects a specific bundle name, this became slightly more frustrating.

@eclipse-platform-bot
Copy link
Contributor

eclipse-platform-bot commented Sep 1, 2025

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

terminal/bundles/org.eclipse.terminal.control/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 11ec50b005c14267c4027877494bc31270aedaea Mon Sep 17 00:00:00 2001
From: Eclipse Platform Bot <[email protected]>
Date: Mon, 1 Sep 2025 18:28:59 +0000
Subject: [PATCH] Version bump(s) for 4.38 stream


diff --git a/terminal/bundles/org.eclipse.terminal.control/META-INF/MANIFEST.MF b/terminal/bundles/org.eclipse.terminal.control/META-INF/MANIFEST.MF
index cc66d1a667..00dd09516b 100644
--- a/terminal/bundles/org.eclipse.terminal.control/META-INF/MANIFEST.MF
+++ b/terminal/bundles/org.eclipse.terminal.control/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %pluginName
 Bundle-SymbolicName: org.eclipse.terminal.control; singleton:=true
-Bundle-Version: 1.0.0.qualifier
+Bundle-Version: 1.0.100.qualifier
 Bundle-Activator: org.eclipse.terminal.internal.control.impl.TerminalPlugin
 Bundle-Vendor: %providerName
 Bundle-Localization: plugin
@@ -11,14 +11,14 @@ Require-Bundle: org.eclipse.core.runtime;bundle-version="[3.33.0,4.0.0)",
 Bundle-ActivationPolicy: lazy
 Eclipse-LazyStart: true
 Bundle-RequiredExecutionEnvironment: JavaSE-17
-Export-Package: org.eclipse.terminal.connector;version="1.0.0",
- org.eclipse.terminal.connector.provider;version="1.0.0",
- org.eclipse.terminal.control;version="1.0.0",
+Export-Package: org.eclipse.terminal.connector;version="1.0.100",
+ org.eclipse.terminal.connector.provider;version="1.0.100",
+ org.eclipse.terminal.control;version="1.0.100",
  org.eclipse.terminal.internal.connector;x-internal:=true,
  org.eclipse.terminal.internal.control.impl;x-internal:=true,
  org.eclipse.terminal.internal.emulator;x-internal:=true,
  org.eclipse.terminal.internal.model;x-internal:=true,
  org.eclipse.terminal.internal.preferences;x-internal:=true,
  org.eclipse.terminal.internal.textcanvas;x-internal:=true,
- org.eclipse.terminal.model;version="1.0.0"
+ org.eclipse.terminal.model;version="1.0.100"
 Automatic-Module-Name: org.eclipse.terminal.control
-- 
2.51.0

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

Copy link
Contributor

@merks merks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a better design.

Copy link
Contributor

@laeubi laeubi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks resonable

Copy link
Contributor

github-actions bot commented Sep 1, 2025

Test Results

 1 947 files  ±0   1 947 suites  ±0   1h 44m 3s ⏱️ + 12m 49s
 4 720 tests ±0   4 696 ✅ ±0   24 💤 ±0  0 ❌ ±0 
14 160 runs  ±0  13 993 ✅ ±0  167 💤 ±0  0 ❌ ±0 

Results for commit 06be955. ± Comparison against base commit cba5e6c.

♻️ This comment has been updated with latest results.

@ptziegler
Copy link
Contributor Author

I've udated the PR to handle the failing test cases. Following import needed to be added as well: org.apache.lucene.analysis.cn.smart.

Which raises a potential problem I'm not sure how to handle. Users can contribute their own analyzers using the org.eclipse.help.base.luceneAnalyzer extension point. And given that Lucene has somehow decided to define a package for each language, all of those packages need to be imported, so that they are visible to the classloader.

Or to rephrase: Using Import-Package seems really scary, now that I have a closer look...

@laeubi
Copy link
Contributor

laeubi commented Sep 1, 2025

@ptziegler

Users can contribute their own analyzers using the org.eclipse.help.base.luceneAnalyzer extension point

That seems reasonable and I don't see an immediate problem here?

And given that Lucene has somehow decided to define a package for each language, all of those packages need to be imported, so that they are visible to the classloader

Can you explain this more especially how require-bundle makes the situation better? We then maybe will find a solution out here. Maybe Dynamic-ImportPackage org.apache.lucene.analysis.*;version="[10.0.0,11.0.0)" would be a solution?

But I'm not completely sure why the imports are needed for a custom Analyzer, should not the custom one then make sure all packages are properly imported?

@ptziegler
Copy link
Contributor Author

@laeubi

Assume someone contributes the following analyzer:

  <analyzer
        class="org.apache.lucene.analysis.ta.TamilAnalyzer"
        locale="ta">
  </analyzer>

Because the org.apache.lucene.analysis.ta package is not imported, this leads to a ClassNotFoundException:

java.lang.ClassNotFoundException: org.apache.lucene.analysis.ta.TamilAnalyzer cannot be found by org.eclipse.help.base_4.5.300.qualifier
	at org.eclipse.osgi.internal.loader.BundleLoader.generateException(BundleLoader.java:567)
	at org.eclipse.osgi.internal.loader.BundleLoader.findClass0(BundleLoader.java:562)
	at org.eclipse.osgi.internal.loader.BundleLoader.findClass(BundleLoader.java:438)
	at org.eclipse.osgi.internal.loader.ModuleClassLoader.loadClass(ModuleClassLoader.java:195)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:526)
	at org.eclipse.osgi.internal.framework.EquinoxBundle.loadClass(EquinoxBundle.java:652)
	at org.eclipse.core.internal.registry.osgi.RegistryStrategyOSGI.createExecutableExtension(RegistryStrategyOSGI.java:226)
	at org.eclipse.core.internal.registry.ExtensionRegistry.createExecutableExtension(ExtensionRegistry.java:1034)
	at org.eclipse.core.internal.registry.ConfigurationElement.createExecutableExtension(ConfigurationElement.java:286)
	at org.eclipse.core.internal.registry.ConfigurationElementHandle.createExecutableExtension(ConfigurationElementHandle.java:65)
	at org.eclipse.help.internal.search.AnalyzerDescriptor.createAnalyzer(AnalyzerDescriptor.java:113)
	at org.eclipse.help.internal.search.AnalyzerDescriptor.<init>(AnalyzerDescriptor.java:40)

You wouldn't have this issue when using Require-Bundle, because all packages are visible.

But I'm not completely sure why the imports are needed for a custom Analyzer, should not the custom one then make sure all packages are properly imported?

The problem is that the classloader for org.eclipse.help.base is used, not the one from the contributing bundle. Otherwise you would be correct.

@laeubi
Copy link
Contributor

laeubi commented Sep 1, 2025

The problem is that the classloader for org.eclipse.help.base is used, not the one from the contributing bundle.

This sounds strange as it would mean that the user of the extension point would need to import all requirements what should not be the case, so it it ensured that the provider has a proper import package or was it just omitted because it "worked anyways"?

I'm asking because the code in question explicitly says it loads the class from the contributing bundle:

RegistryStrategyOSGI.java

// load the requested class from this bundle
Class<?> classInstance = null;
try {
	classInstance = contributingBundle.loadClass(className);
} catch (Exception | LinkageError e1) {
	throwException(
			NLS.bind(RegistryMessages.plugin_loadClassError, contributingBundle.getSymbolicName(), className),
			e1);
}

@laeubi
Copy link
Contributor

laeubi commented Sep 1, 2025

@ptziegler I now found that the extensions are directly provided by the org.eclipse.help.base bundle, so contributor == consumer.

In this case I would recommend to use the @Referenced(... list of implicitly required class names ...) for example at the AnalyzerDescriptor (alternatively a package-info.class can be used) to make this explicit and visible. Otherwise the imported packages will be assumed to be unused sooner or later!

@ptziegler
Copy link
Contributor Author

I'm asking because the code in question explicitly says it loads the class from the contributing bundle:

Yes, you are correct. I got misled by the stacktrace.

I've extended the UA test with the Tamil analyzer, which is also where I contributed to the extension point. After I added the additional package to the test bundle, the stack trace went away.

So that is indeed a non-issue.

@ptziegler
Copy link
Contributor Author

In this case I would recommend to use the @referenced(... list of implicitly required class names ...) for example at the AnalyzerDescriptor (alternatively a package-info.class can be used) to make this explicit and visible. Otherwise the imported packages will be assumed to be unused sooner or later!

Done

ptziegler and others added 2 commits September 2, 2025 20:56
Using the Import-Header entries allows consumers to contribute their own
Lucene bundles, without having to match the symbolic name defined by
Orbit.
@ptziegler
Copy link
Contributor Author

Merging, as the only issue seems to be my misunderstanding of which classes are loaded by which classloader.

@ptziegler ptziegler merged commit 89544a1 into eclipse-platform:master Sep 2, 2025
18 checks passed
@ptziegler ptziegler deleted the lucene-dependency branch September 2, 2025 20:14
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