Skip to content

Commit 6866e29

Browse files
iloveeclipseiloveeclipse
committed
Regression test for RequiredPluginsClasspathContainer
Shows the regression caused by #2218 / 89b00be The test will fail on master after 89b00be and pass before this commit. Since ~20 years PDE never allowed not imported/required bundles be on the classpath of a bundle. From PDE point of view, bundle code should only see references to the code from other bundles on project classpath if: - the bundle requires another bundle - or the bundle imports a package from other bundle Transitive dependencies are only added to the classpath if the required bundle reexports one of its dependencies. This has a simple rationale: 1) Developers can't compile code in the IDE which would be not working at runtime, saving time for development by showing the errors before starting application. 2) The IDE can quickly recompile one line code change in a workspace with hundreds of bundles, because the classpath only has the dependencies mentioned above. With 89b00be above is broken. All transitively used projects are now on the classpath, code that can compile (but uses illegal references) will not work at runtime and the incremental build is significantly slower in a multi-project workspace. See #2244
1 parent 38e5986 commit 6866e29

65 files changed

Lines changed: 758 additions & 0 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/core/tests/internal/classpath/ClasspathResolutionTest2.java‎

Lines changed: 192 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,192 @@
1+
/*******************************************************************************
2+
* Copyright (c) 2026 Andrey Loskutov <loskutov@gmx.de> and others.
3+
*
4+
* This program and the accompanying materials
5+
* are made available under the terms of the Eclipse Public License 2.0
6+
* which accompanies this distribution, and is available at
7+
* https://www.eclipse.org/legal/epl-2.0/
8+
*
9+
* SPDX-License-Identifier: EPL-2.0
10+
*
11+
* Contributors:
12+
* Andrey Loskutov <loskutov@gmx.de> - initial API and implementation
13+
*******************************************************************************/
14+
package org.eclipse.pde.core.tests.internal.classpath;
15+
16+
import static org.assertj.core.api.Assertions.assertThat;
17+
import static org.junit.Assert.assertFalse;
18+
import static org.junit.Assert.assertTrue;
19+
import static org.junit.Assert.fail;
20+
21+
import java.util.ArrayList;
22+
import java.util.Arrays;
23+
import java.util.List;
24+
import java.util.stream.Stream;
25+
26+
import org.eclipse.core.resources.IMarker;
27+
import org.eclipse.core.resources.IProject;
28+
import org.eclipse.core.resources.IResource;
29+
import org.eclipse.core.resources.IncrementalProjectBuilder;
30+
import org.eclipse.core.resources.ResourcesPlugin;
31+
import org.eclipse.core.runtime.CoreException;
32+
import org.eclipse.core.runtime.IPath;
33+
import org.eclipse.core.runtime.NullProgressMonitor;
34+
import org.eclipse.jdt.core.IClasspathEntry;
35+
import org.eclipse.pde.core.plugin.IPluginModelBase;
36+
import org.eclipse.pde.internal.core.ClasspathComputer;
37+
import org.eclipse.pde.internal.core.PDECore;
38+
import org.eclipse.pde.ui.tests.runtime.TestUtils;
39+
import org.eclipse.pde.ui.tests.util.ProjectUtils;
40+
import org.junit.BeforeClass;
41+
import org.junit.ClassRule;
42+
import org.junit.Rule;
43+
import org.junit.Test;
44+
import org.junit.rules.TestRule;
45+
46+
/**
47+
* Regression test for classpath resolution of plugin projects. Tests that only
48+
* the expected bundles are on the classpath, and that errors are reported for
49+
* missing packages from not accessible bundles, but not for missing packages.
50+
*
51+
* See https://github.com/eclipse-pde/eclipse.pde/issues/2244.
52+
*/
53+
public class ClasspathResolutionTest2 {
54+
55+
@ClassRule
56+
public static final TestRule CLEAR_WORKSPACE = ProjectUtils.DELETE_ALL_WORKSPACE_PROJECTS_BEFORE_AND_AFTER;
57+
58+
@Rule
59+
public final TestRule deleteCreatedTestProjectsAfter = ProjectUtils.DELETE_CREATED_WORKSPACE_PROJECTS_AFTER;
60+
61+
private static IProject projectA;
62+
63+
private static IClasspathEntry[] classpathEntriesA;
64+
65+
static final List<String> expectedAccessibleBundles = List.of("B", "G");
66+
static final List<String> expectedApiPackages = apiPackagesFor(expectedAccessibleBundles);
67+
static final List<String> expectedInternalPackages = internalPackagesFor(expectedAccessibleBundles);
68+
69+
static final List<String> notAccessibleBundles = List.of("C", "D", "E", "F", "H");
70+
static final List<String> notAccessibleApiPackages = apiPackagesFor(notAccessibleBundles);
71+
static final List<String> notAccessibleInternalPackages = internalPackagesFor(notAccessibleBundles);
72+
73+
static final List<String> allOtherProjects = Stream
74+
.concat(expectedAccessibleBundles.stream(), notAccessibleBundles.stream()).sorted().toList();
75+
76+
@BeforeClass
77+
public static void setupBeforeClass() throws Exception {
78+
List<IProject> importedProjects = new ArrayList<>();
79+
80+
for (String name : allOtherProjects) {
81+
IProject project = ProjectUtils.importTestProject("tests/projects/" + name);
82+
importedProjects.add(project);
83+
}
84+
85+
// Build all projects in reversed order to ensure that dependencies are
86+
// built before dependents
87+
for (IProject project : importedProjects.reversed()) {
88+
project.open(new NullProgressMonitor());
89+
project.build(IncrementalProjectBuilder.FULL_BUILD, new NullProgressMonitor());
90+
}
91+
TestUtils.processUIEvents(100);
92+
93+
// Now import and build project A, which depends on all other projects.
94+
projectA = ProjectUtils.importTestProject("tests/projects/A");
95+
projectA.open(new NullProgressMonitor());
96+
projectA.build(IncrementalProjectBuilder.FULL_BUILD, new NullProgressMonitor());
97+
98+
IPluginModelBase modelA = PDECore.getDefault().getModelManager().findModel(projectA);
99+
classpathEntriesA = ClasspathComputer.computeClasspathEntries(modelA, projectA);
100+
101+
TestUtils.processUIEvents(100);
102+
}
103+
104+
/**
105+
* Check that the classpath of plugin A contains exactly the expected
106+
* bundles. Checks that "missing type" compilation errors are reported for
107+
* all references form not accessible bundles. This bundle classpath is
108+
* computed by RequiredPluginsClasspathContainer.
109+
*/
110+
@Test
111+
public void testRequiredPluginsClasspathContainerContract() throws Exception {
112+
// Check every project except A - they should build without errors
113+
List<IProject> otherProjects = allOtherProjects.stream().map(ClasspathResolutionTest2::getProject).toList();
114+
for (IProject project : otherProjects) {
115+
IMarker[] markers = project.findMarkers(IMarker.PROBLEM, true, IResource.DEPTH_INFINITE);
116+
for (IMarker marker : markers) {
117+
if (marker.getAttribute(IMarker.SEVERITY, -1) == IMarker.SEVERITY_ERROR) {
118+
fail("Unexpected error in project " + project.getName() + ": "
119+
+ marker.getAttribute(IMarker.MESSAGE, ""));
120+
}
121+
}
122+
}
123+
124+
// Check that project A has errors, and that all errors are related to
125+
// missing packages from not accessible bundles, and that no error is
126+
// related to missing packages from expected accessible bundles
127+
IMarker[] markers = projectA.findMarkers(IMarker.PROBLEM, true, IResource.DEPTH_INFINITE);
128+
129+
List<String> errorMessages = Arrays.asList(markers).stream()
130+
.filter(marker -> marker.getAttribute(IMarker.SEVERITY, -1) == IMarker.SEVERITY_ERROR)
131+
.map(marker -> marker.getAttribute(IMarker.MESSAGE, "")).toList();
132+
133+
for (String message : errorMessages) {
134+
// Check that no error is related to missing packages from expected
135+
// accessible bundles
136+
boolean isRelatedToExpectedAccessibleBundle = false;
137+
for (String bundle : expectedAccessibleBundles) {
138+
String pack = bundle.toLowerCase();
139+
if (message.contains(pack + " cannot be resolved to a type")) {
140+
isRelatedToExpectedAccessibleBundle = true;
141+
break;
142+
}
143+
}
144+
assertFalse("Unexpected error in project A: " + message, isRelatedToExpectedAccessibleBundle);
145+
146+
// and that all errors are related to missing packages from not
147+
// accessible bundles
148+
boolean isRelatedToNotAccessibleBundle = false;
149+
for (String bundle : notAccessibleBundles) {
150+
String pack = bundle.toLowerCase();
151+
if (message.contains(pack + " cannot be resolved to a type")) {
152+
isRelatedToNotAccessibleBundle = true;
153+
break;
154+
}
155+
}
156+
assertTrue("Unexpected error in project A: " + message, isRelatedToNotAccessibleBundle);
157+
}
158+
159+
// There must be at least one error, otherwise the test would not be
160+
// meaningful
161+
assertFalse("Expected errors in project A, but found none!", errorMessages.isEmpty());
162+
163+
// Check that all expected accessible bundles are on the classpath, and
164+
// only those
165+
List<String> projectNames = Arrays.asList(classpathEntriesA).stream()
166+
.map(entry -> entry.getPath().lastSegment()).toList();
167+
168+
assertThat(projectNames).containsExactlyInAnyOrderElementsOf(expectedAccessibleBundles);
169+
170+
// Same check using the API of PDECore and ClasspathComputer instead
171+
List<String> classpathEntries = getRequiredPluginContainerEntries(projectA);
172+
assertThat(classpathEntries).containsExactlyElementsOf(expectedAccessibleBundles);
173+
}
174+
175+
static IProject getProject(String name) {
176+
return ResourcesPlugin.getWorkspace().getRoot().getProject(name);
177+
}
178+
179+
private static List<String> apiPackagesFor(List<String> expectedAccessibleBundles) {
180+
return expectedAccessibleBundles.stream().flatMap(bundle -> List.of(bundle + ".api").stream()).toList();
181+
}
182+
183+
private static List<String> internalPackagesFor(List<String> expectedAccessibleBundles) {
184+
return expectedAccessibleBundles.stream().flatMap(bundle -> List.of(bundle + ".internal").stream()).toList();
185+
}
186+
187+
private List<String> getRequiredPluginContainerEntries(IProject project) throws CoreException {
188+
IPluginModelBase model = PDECore.getDefault().getModelManager().findModel(project);
189+
IClasspathEntry[] computeClasspathEntries = ClasspathComputer.computeClasspathEntries(model, project);
190+
return Arrays.stream(computeClasspathEntries).map(IClasspathEntry::getPath).map(IPath::lastSegment).toList();
191+
}
192+
}

‎ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/AllPDETests.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
import org.eclipse.pde.core.tests.internal.AllPDECoreTests;
1717
import org.eclipse.pde.core.tests.internal.classpath.ClasspathResolutionTest;
18+
import org.eclipse.pde.core.tests.internal.classpath.ClasspathResolutionTest2;
1819
import org.eclipse.pde.core.tests.internal.core.builders.BundleErrorReporterTest;
1920
import org.eclipse.pde.core.tests.internal.util.PDESchemaHelperTest;
2021
import org.eclipse.pde.ui.tests.build.properties.AllValidatorTests;
@@ -63,6 +64,7 @@
6364
ClasspathContributorTest.class, //
6465
DynamicPluginProjectReferencesTest.class, //
6566
ClasspathResolutionTest.class, //
67+
ClasspathResolutionTest2.class, //
6668
BundleErrorReporterTest.class, //
6769
AllPDECoreTests.class, //
6870
ProjectSmartImportTest.class, //

‎ui/org.eclipse.pde.ui.tests/tests/projects/A/.classpath‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<classpath>
3+
<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-21"/>
4+
<classpathentry kind="con" path="org.eclipse.pde.core.requiredPlugins"/>
5+
<classpathentry kind="src" path="src"/>
6+
<classpathentry kind="output" path="bin"/>
7+
</classpath>

‎ui/org.eclipse.pde.ui.tests/tests/projects/A/.project‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<projectDescription>
3+
<name>A</name>
4+
<comment></comment>
5+
<projects>
6+
</projects>
7+
<buildSpec>
8+
<buildCommand>
9+
<name>org.eclipse.jdt.core.javabuilder</name>
10+
<arguments>
11+
</arguments>
12+
</buildCommand>
13+
<buildCommand>
14+
<name>org.eclipse.pde.ManifestBuilder</name>
15+
<arguments>
16+
</arguments>
17+
</buildCommand>
18+
<buildCommand>
19+
<name>org.eclipse.pde.SchemaBuilder</name>
20+
<arguments>
21+
</arguments>
22+
</buildCommand>
23+
</buildSpec>
24+
<natures>
25+
<nature>org.eclipse.pde.PluginNature</nature>
26+
<nature>org.eclipse.jdt.core.javanature</nature>
27+
</natures>
28+
</projectDescription>

‎ui/org.eclipse.pde.ui.tests/tests/projects/A/.settings/org.eclipse.core.resources.prefs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
eclipse.preferences.version=1
2+
encoding/<project>=UTF-8

‎ui/org.eclipse.pde.ui.tests/tests/projects/A/.settings/org.eclipse.jdt.core.prefs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
eclipse.preferences.version=1
2+
org.eclipse.jdt.core.compiler.codegen.targetPlatform=21
3+
org.eclipse.jdt.core.compiler.compliance=21
4+
org.eclipse.jdt.core.compiler.release=enabled
5+
org.eclipse.jdt.core.compiler.source=21
6+
org.eclipse.jdt.core.compiler.problem.forbiddenReference=warning

‎ui/org.eclipse.pde.ui.tests/tests/projects/A/META-INF/MANIFEST.MF‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
Manifest-Version: 1.0
2+
Bundle-ManifestVersion: 2
3+
Bundle-Name: A
4+
Bundle-SymbolicName: A
5+
Bundle-Version: 1.0.0.qualifier
6+
Export-Package: a.api
7+
Import-Package: b.api;version="1.0.0",
8+
g.api;version="1.0.0"
9+
Automatic-Module-Name: A
10+
Bundle-RequiredExecutionEnvironment: JavaSE-21

‎ui/org.eclipse.pde.ui.tests/tests/projects/A/build.properties‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
source.. = src/
2+
output.. = bin/
3+
bin.includes = META-INF/,\
4+
.

‎ui/org.eclipse.pde.ui.tests/tests/projects/A/src/a/api/AClass.java‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
package a.api;
2+
public class AClass {
3+
// bundle B b.api package imported by A
4+
public Object objectFromB_allowed = new b.api.MyObject();
5+
public Object objectFromB_restricted = new b.internal.MyObject();
6+
7+
// bundle G reexported via B, g.api package is imported by A
8+
public Object objectFromG_allowed = new g.api.MyObject();
9+
public Object objectFromG_restricted = new g.internal.MyObject();
10+
11+
/*
12+
* All references below never compilable before https://github.com/eclipse-pde/eclipse.pde/pull/2218
13+
*
14+
* bundles C, D, E, F, H not required by A, neither package is imported by A
15+
* bundles C, D, E, F, H only referenced in different ways from bundle B or G and not reexported
16+
*/
17+
/*
18+
* Regression introduced: all (transitive) dependencies from B or G are now accessible from A,
19+
* even if not reexported by B or G and not imported by A.
20+
*/
21+
22+
// Bundle C directly required by B, but not reexported by B
23+
public Object objectFromC_not_accessible1 = new c.api.MyObject();
24+
public Object objectFromC_not_accessible2 = new c.internal.MyObject();
25+
26+
// Bundle D package imported by B, but not reexported by B
27+
public Object objectFromD_not_accessible1 = new d.api.MyObject();
28+
public Object objectFromD_not_accessible2 = new d.internal.MyObject();
29+
30+
// Optionally required or imported by B, but not reexported by B
31+
public Object objectFromE_not_accessible1 = new e.api.MyObject();
32+
public Object objectFromE_not_accessible2 = new e.internal.MyObject();
33+
public Object objectFromF_not_accessible1 = new f.api.MyObject();
34+
public Object objectFromF_not_accessible2 = new f.internal.MyObject();
35+
36+
// Bundle H package optionally imported by G, not reexported by anyone
37+
public Object objectFromH_not_accessible1 = new h.api.MyObject();
38+
public Object objectFromH_not_accessible2 = new h.internal.MyObject();
39+
}

‎ui/org.eclipse.pde.ui.tests/tests/projects/B/.classpath‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<classpath>
3+
<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-21"/>
4+
<classpathentry kind="con" path="org.eclipse.pde.core.requiredPlugins"/>
5+
<classpathentry kind="src" path="src"/>
6+
<classpathentry kind="output" path="bin"/>
7+
</classpath>

0 commit comments

Comments
 (0)