From 73bb2ed11feaffef0ca970c91d00567d6869752a Mon Sep 17 00:00:00 2001 From: Michael Haubenwallner Date: Wed, 8 Mar 2023 15:35:59 +0100 Subject: [PATCH 1/2] ClasspathComputer: add tests for classpath computation Failing tests for these issues: * erroneous exception "Build path contains duplicate entry" * updating source attachment changed unrelated attributes * failed to preserve some existing attributes * failed to preserve entries of kind="var" * failed to preserve existing order * UpdateClasspathJob returned CANCEL on success Additional tests for existing behavior: * from scratch, create lib entries with exported="true" * from scratch, create classpath in some specific order * set TEST attributes when plugin name matches test_plugin_pattern --- .../ClasspathUpdaterTest.java | 209 ++++++++++++++++++ .../projects/classpathupdater/.classpath | 23 ++ .../tests/projects/classpathupdater/.project | 29 +++ .../.settings/org.eclipse.jdt.core.prefs | 9 + .../tests/projects/classpathupdater/A.jar | Bin 0 -> 2171 bytes .../tests/projects/classpathupdater/Asrc.zip | Bin 0 -> 2171 bytes .../classpathupdater/META-INF/MANIFEST.MF | 9 + .../classpathupdater/build.properties | 21 ++ .../projects/classpathupdater/library.jar | Bin 0 -> 2171 bytes 9 files changed, 300 insertions(+) create mode 100644 ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/classpathupdater/ClasspathUpdaterTest.java create mode 100644 ui/org.eclipse.pde.ui.tests/tests/projects/classpathupdater/.classpath create mode 100644 ui/org.eclipse.pde.ui.tests/tests/projects/classpathupdater/.project create mode 100644 ui/org.eclipse.pde.ui.tests/tests/projects/classpathupdater/.settings/org.eclipse.jdt.core.prefs create mode 100644 ui/org.eclipse.pde.ui.tests/tests/projects/classpathupdater/A.jar create mode 100644 ui/org.eclipse.pde.ui.tests/tests/projects/classpathupdater/Asrc.zip create mode 100644 ui/org.eclipse.pde.ui.tests/tests/projects/classpathupdater/META-INF/MANIFEST.MF create mode 100644 ui/org.eclipse.pde.ui.tests/tests/projects/classpathupdater/build.properties create mode 100644 ui/org.eclipse.pde.ui.tests/tests/projects/classpathupdater/library.jar diff --git a/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/classpathupdater/ClasspathUpdaterTest.java b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/classpathupdater/ClasspathUpdaterTest.java new file mode 100644 index 0000000000..9809960e8a --- /dev/null +++ b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/classpathupdater/ClasspathUpdaterTest.java @@ -0,0 +1,209 @@ +package org.eclipse.pde.ui.tests.classpathupdater; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +import java.util.Arrays; +import java.util.Map; +import java.util.Optional; +import java.util.function.Function; +import java.util.function.Predicate; + +import org.eclipse.core.resources.IProject; +import org.eclipse.core.runtime.IPath; +import org.eclipse.jdt.core.IClasspathAttribute; +import org.eclipse.jdt.core.IClasspathEntry; +import org.eclipse.jdt.core.IJavaProject; +import org.eclipse.jdt.core.JavaCore; +import org.eclipse.jdt.core.JavaModelException; +import org.eclipse.pde.core.plugin.IPluginModelBase; +import org.eclipse.pde.core.plugin.PluginRegistry; +import org.eclipse.pde.internal.core.ClasspathComputer; +import org.eclipse.pde.internal.core.ICoreConstants; +import org.eclipse.pde.internal.core.PDECore; +import org.eclipse.pde.internal.ui.wizards.tools.UpdateClasspathJob; +import org.eclipse.pde.ui.tests.util.ProjectUtils; +import org.junit.After; +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.rules.TestRule; + +public class ClasspathUpdaterTest { + + private static IProject project; + private static IJavaProject jProject; + + private static IClasspathEntry[] originalClasspath; + private static String originalTestPluginPattern; + private static Boolean expectIsTest; + @ClassRule + public static final TestRule CLEAR_WORKSPACE = ProjectUtils.DELETE_ALL_WORKSPACE_PROJECTS_BEFORE_AND_AFTER; + + @BeforeClass + public static void setUpBeforeClass() throws Exception { + project = ProjectUtils.importTestProject("tests/projects/classpathupdater"); + assertTrue("Project was not created", project.exists()); + jProject = JavaCore.create(project); + originalClasspath = jProject.getRawClasspath(); + originalTestPluginPattern = PDECore.getDefault().getPreferencesManager() + .getString(ICoreConstants.TEST_PLUGIN_PATTERN); + expectIsTest = null; + } + + @After + public void restoreOriginalClasspath() throws Exception { + jProject.setRawClasspath(originalClasspath, null); + } + + @After + public void restoreOriginalTestPluginPattern() { + PDECore.getDefault().getPreferencesManager().setValue(ICoreConstants.TEST_PLUGIN_PATTERN, + originalTestPluginPattern); + expectIsTest = null; + } + + private void setTestPluginPattern() { + PDECore.getDefault().getPreferencesManager().setValue(ICoreConstants.TEST_PLUGIN_PATTERN, project.getName()); + expectIsTest = Boolean.TRUE; + } + + @Test + public void test_PreserveAttributes() throws Exception { + runUpdateClasspathJob(); + assertClasspathAttribute("JavaSE-17", IClasspathAttribute.MODULE, true, Boolean::valueOf); + assertClasspathAttribute("library.jar", IClasspathAttribute.TEST, true, Boolean::valueOf); + assertClasspathProperty("library.jar", "exported=true", e -> e.isExported()); + assertClasspathProperty("library.jar", "no source", e -> e.getSourceAttachmentPath() == null); + assertClasspathAttribute("A.jar", IClasspathAttribute.TEST, null, Boolean::valueOf); + assertClasspathProperty("A.jar", "exported=false", e -> !e.isExported()); + assertClasspathProperty("A.jar", "default source", + e -> "Asrc.zip".equals(nullOr(e.getSourceAttachmentPath(), IPath::lastSegment))); + assertClasspathProperty("src", "exported=false", e -> !e.isExported()); + assertClasspathAttribute("src", IClasspathAttribute.IGNORE_OPTIONAL_PROBLEMS, true, Boolean::valueOf); + assertClasspathAttribute("src", IClasspathAttribute.TEST, true, Boolean::valueOf); + assertClasspathProperty("tests", "exported=false", e -> !e.isExported()); + assertClasspathAttribute("tests", IClasspathAttribute.IGNORE_OPTIONAL_PROBLEMS, null, Boolean::valueOf); + assertClasspathAttribute("tests", IClasspathAttribute.TEST, expectIsTest, Boolean::valueOf); + assertClasspathOrder("A.jar", "src", "org.eclipse.pde.core.requiredPlugins", "JavaSE-17", "SOMEVAR", + "library.jar", "tests"); + } + + @Test + public void test_PreserveAttributes_WithTestPluginName() throws Exception { + setTestPluginPattern(); + test_PreserveAttributes(); + } + + @Test + public void test_CreateFromScratch() throws Exception { + jProject.setRawClasspath(new IClasspathEntry[0], null); + + runUpdateClasspathJob(); + assertClasspathAttribute("JavaSE-17", IClasspathAttribute.MODULE, null, Boolean::valueOf); + assertClasspathAttribute("library.jar", IClasspathAttribute.TEST, null, Boolean::valueOf); + assertClasspathProperty("library.jar", "exported=true", e -> e.isExported()); + assertClasspathProperty("library.jar", "no source", e -> e.getSourceAttachmentPath() == null); + assertClasspathAttribute("A.jar", IClasspathAttribute.TEST, null, Boolean::valueOf); + assertClasspathProperty("A.jar", "exported=true", e -> e.isExported()); + assertClasspathProperty("A.jar", "default source", + e -> "Asrc.zip".equals(nullOr(e.getSourceAttachmentPath(), IPath::lastSegment))); + assertClasspathProperty("src", "exported=false", e -> !e.isExported()); + assertClasspathAttribute("src", IClasspathAttribute.IGNORE_OPTIONAL_PROBLEMS, null, Boolean::valueOf); + assertClasspathAttribute("src", IClasspathAttribute.TEST, expectIsTest, Boolean::valueOf); + assertClasspathProperty("tests", "exported=false", e -> !e.isExported()); + assertClasspathAttribute("tests", IClasspathAttribute.IGNORE_OPTIONAL_PROBLEMS, null, Boolean::valueOf); + assertClasspathAttribute("tests", IClasspathAttribute.TEST, expectIsTest, Boolean::valueOf); + assertClasspathOrder("JavaSE-17", "org.eclipse.pde.core.requiredPlugins", "library.jar", "A.jar", "src", + "tests"); + } + + @Test + public void test_CreateFromScatch_WithTestPluginName() throws Exception { + setTestPluginPattern(); + test_CreateFromScratch(); + } + + @Test + public void test_ChangeSourceAttachment() throws Exception { + Map sourceMap = Map.of( // + "A.jar", IPath.fromOSString("library.jar"), // + "library.jar", IPath.fromOSString("A.jar")); + + IPluginModelBase model = PluginRegistry.findModel(project.getProject()); + + IClasspathEntry[] cp = ClasspathComputer.getClasspath(project, model, sourceMap, false, false); + jProject.setRawClasspath(cp, null); + + assertClasspathAttribute("JavaSE-17", IClasspathAttribute.MODULE, true, Boolean::valueOf); + assertClasspathAttribute("library.jar", IClasspathAttribute.TEST, true, Boolean::valueOf); + assertClasspathProperty("library.jar", "exported=true", e -> e.isExported()); + assertClasspathProperty("library.jar", "overridden source", + e -> "A.jar".equals(nullOr(e.getSourceAttachmentPath(), IPath::lastSegment))); + assertClasspathAttribute("A.jar", IClasspathAttribute.TEST, null, Boolean::valueOf); + assertClasspathProperty("A.jar", "exported=false", e -> !e.isExported()); + assertClasspathProperty("A.jar", "overridden source", + e -> "library.jar".equals(nullOr(e.getSourceAttachmentPath(), IPath::lastSegment))); + assertClasspathProperty("src", "exported=false", e -> !e.isExported()); + assertClasspathAttribute("src", IClasspathAttribute.IGNORE_OPTIONAL_PROBLEMS, true, Boolean::valueOf); + assertClasspathAttribute("src", IClasspathAttribute.TEST, true, Boolean::valueOf); + assertClasspathProperty("tests", "exported=false", e -> !e.isExported()); + assertClasspathAttribute("tests", IClasspathAttribute.IGNORE_OPTIONAL_PROBLEMS, null, Boolean::valueOf); + assertClasspathAttribute("tests", IClasspathAttribute.TEST, expectIsTest, Boolean::valueOf); + assertClasspathOrder("A.jar", "src", "org.eclipse.pde.core.requiredPlugins", "JavaSE-17", "SOMEVAR", + "library.jar", "tests"); + } + + @Test + public void test_ChangeSourceAttachment_WithTestPluginName() throws Exception { + setTestPluginPattern(); + test_ChangeSourceAttachment(); + } + + private void runUpdateClasspathJob() throws InterruptedException { + IPluginModelBase model = PluginRegistry.findModel(project.getProject()); + UpdateClasspathJob job = new UpdateClasspathJob(new IPluginModelBase[] { model }); + job.schedule(); + job.join(); + assertTrue("Update Classpath Job failed", job.getResult().isOK()); + } + + private void assertClasspathAttribute(String entryName, String attrName, T expectedValue, + Function parser) throws JavaModelException { + IClasspathEntry entry = findClasspathEntry(entryName); + assertNotNull("Classpath entry for " + entryName + " is missing", entry); + String attrValue = findClasspathAttributeValue(entry, attrName); + T current = attrValue != null ? parser.apply(attrValue) : null; // null: attribute not set + assertEquals("Classpath entry for '" + entry.getPath().lastSegment() + "': attribute '" + attrName + + "' is not '" + expectedValue + "'", expectedValue, current); + } + + private String findClasspathAttributeValue(IClasspathEntry entry, String name) { + return Arrays.stream(entry.getExtraAttributes()) // + .filter(a -> name.equals(a.getName())).map(IClasspathAttribute::getValue) // + .findFirst().orElse(null); + } + + private void assertClasspathProperty(String entryName, String expectedValue, Predicate checker) + throws JavaModelException { + IClasspathEntry entry = findClasspathEntry(entryName); + assertTrue("Classpath entry for '" + entryName + "' has not set '" + expectedValue + "'", checker.test(entry)); + } + + private void assertClasspathOrder(String... names) throws Exception { + var actualNames = Arrays.stream(jProject.getRawClasspath()).map(e -> e.getPath().lastSegment()).toList(); + assertEquals(Arrays.asList(names), actualNames); + } + + private IClasspathEntry findClasspathEntry(String name) throws JavaModelException { + Optional entry = Arrays.stream(jProject.getRawClasspath()) + .filter(e -> name.equals(e.getPath().lastSegment())).findFirst(); + assertTrue("Classpath entry for " + name + " is missing", entry.isPresent()); + return entry.get(); + } + + private static R nullOr(T obj, Function f) { + return obj == null ? null : f.apply(obj); + } +} diff --git a/ui/org.eclipse.pde.ui.tests/tests/projects/classpathupdater/.classpath b/ui/org.eclipse.pde.ui.tests/tests/projects/classpathupdater/.classpath new file mode 100644 index 0000000000..668576bd95 --- /dev/null +++ b/ui/org.eclipse.pde.ui.tests/tests/projects/classpathupdater/.classpath @@ -0,0 +1,23 @@ + + + + + + + + + + + + + + + + + + + + + + + diff --git a/ui/org.eclipse.pde.ui.tests/tests/projects/classpathupdater/.project b/ui/org.eclipse.pde.ui.tests/tests/projects/classpathupdater/.project new file mode 100644 index 0000000000..1012ffa778 --- /dev/null +++ b/ui/org.eclipse.pde.ui.tests/tests/projects/classpathupdater/.project @@ -0,0 +1,29 @@ + + + classpathupdater + + + + + + org.eclipse.jdt.core.javabuilder + + + + + org.eclipse.pde.ManifestBuilder + + + + + org.eclipse.pde.SchemaBuilder + + + + + + org.eclipse.pde.PluginNature + org.eclipse.jdt.core.javanature + org.eclipse.pde.ui.tests.testNature + + diff --git a/ui/org.eclipse.pde.ui.tests/tests/projects/classpathupdater/.settings/org.eclipse.jdt.core.prefs b/ui/org.eclipse.pde.ui.tests/tests/projects/classpathupdater/.settings/org.eclipse.jdt.core.prefs new file mode 100644 index 0000000000..62ef3488cc --- /dev/null +++ b/ui/org.eclipse.pde.ui.tests/tests/projects/classpathupdater/.settings/org.eclipse.jdt.core.prefs @@ -0,0 +1,9 @@ +eclipse.preferences.version=1 +org.eclipse.jdt.core.compiler.codegen.targetPlatform=17 +org.eclipse.jdt.core.compiler.compliance=17 +org.eclipse.jdt.core.compiler.problem.assertIdentifier=error +org.eclipse.jdt.core.compiler.problem.enablePreviewFeatures=disabled +org.eclipse.jdt.core.compiler.problem.enumIdentifier=error +org.eclipse.jdt.core.compiler.problem.reportPreviewFeatures=warning +org.eclipse.jdt.core.compiler.release=enabled +org.eclipse.jdt.core.compiler.source=17 diff --git a/ui/org.eclipse.pde.ui.tests/tests/projects/classpathupdater/A.jar b/ui/org.eclipse.pde.ui.tests/tests/projects/classpathupdater/A.jar new file mode 100644 index 0000000000000000000000000000000000000000..74d8b4490d2c9f1d076ca89a9e262bb31bb1517e GIT binary patch literal 2171 zcma)72{fBo8xAVS#MVTKXc_w&jeSdrN+=ScOq~*ygc@5$?WUC~Lp3yvy{bWC9qm}! zno4b{r46b$mbPZlnxfUx1}**7{K?GxLwnAg^S}4pbM86!d+xp8`@GNl5ZuJ}K>?D2 zQ5K$N2-p))z&-#DYZV1ui%WCYXeOE(rhP0vW+BS`(@1h)Ik^P$Eb%F_DBp*B%Eb zD!;!yJ2LXdswL4I(_o;e@U;=hQLb_<+3TWBo_3ylHLVB%_1_es6(s6-zq)s!Z3wkE z)Rxn@n&SbD$|M2@8_LJ06)B;qNND@_(@qz-R*>f0#r1DD1Hv2cS0v=wu8@HlVh0Z# zm&)ncL;OGSSaR%0p;LLV&6Ded( zAzy%y?`3C5UB9=pqo0c?-b@OzeojUt=(5bVl3m04o;23^6wNdt&7-2H)eS$InK$cb zD_gAH!Ccle8l12lq5a8)q`w%9AOZj^3+BIbA@vs*yEKR)p{SRX8dF9qUbXYo983RX z+&oKtJXH;>?#+30s}fsdYLZXiB?1D9C#hL}V8%pKsh#JKbmiXvI$nu??KcQ~NVC|2 z51wqxuI5x!!D64DP0nz%gn?u+z6g*mvOo-$b(LoYeR)t;gF7d|OL0zSx)MFOshnci zQDnuTNZMWdL%m-E4|qz6hhvXf*e+m`04*6`HuffKG99o zQx3&$Sf=`wo}N8bR0s=}MayC<&~kBJS6{bN5FUwm* z$++M(f5{2xb|#-m@` z;qj=hHyE#}`I<_clwNTH<@l@ZCqHjkd6NTK5Ae@iF+Q$8A;1Z?yC`77* zQ%I+!3Mk;JvUaPLx##l+X~4Of9jP+hk}+3TEQ|ecLVikWIDSJqf1*P%sGN*vuxadf zX3jeO7U2ryLYRBD0)v9-{=T0~d#O;ZDq>q?@ffk?)~lSQ*(84-S3p4^XZmH9;6k42 z5632+5pbj~x>qErUozx3Ji_Y9Cl}sWtUg zRRuKKawnwA{5{)qnH&WH*Xk*OcvB=OepXkB+EE+G#Q&UXXs8)k;$m?9ED2gRJ1djA znP#sf0b*FfKF%{$FXkC%Ev(zLYxFLc5u&<@wUao~HBm(P2J^6G#P4%K&D!fKAka(v z4dQa=Cr4upT+6Bjx*ltr-f|hFB0i{*?H|*P7oYx*XHUDU;a;~b+ z^knP_tcpE2U>Ho3QH3vF^YNP^|(ruijy zZ_g37_1$?D2 zQ5K$N2-p))z&-#DYZV1ui%WCYXeOE(rhP0vW+BS`(@1h)Ik^P$Eb%F_DBp*B%Eb zD!;!yJ2LXdswL4I(_o;e@U;=hQLb_<+3TWBo_3ylHLVB%_1_es6(s6-zq)s!Z3wkE z)Rxn@n&SbD$|M2@8_LJ06)B;qNND@_(@qz-R*>f0#r1DD1Hv2cS0v=wu8@HlVh0Z# zm&)ncL;OGSSaR%0p;LLV&6Ded( zAzy%y?`3C5UB9=pqo0c?-b@OzeojUt=(5bVl3m04o;23^6wNdt&7-2H)eS$InK$cb zD_gAH!Ccle8l12lq5a8)q`w%9AOZj^3+BIbA@vs*yEKR)p{SRX8dF9qUbXYo983RX z+&oKtJXH;>?#+30s}fsdYLZXiB?1D9C#hL}V8%pKsh#JKbmiXvI$nu??KcQ~NVC|2 z51wqxuI5x!!D64DP0nz%gn?u+z6g*mvOo-$b(LoYeR)t;gF7d|OL0zSx)MFOshnci zQDnuTNZMWdL%m-E4|qz6hhvXf*e+m`04*6`HuffKG99o zQx3&$Sf=`wo}N8bR0s=}MayC<&~kBJS6{bN5FUwm* z$++M(f5{2xb|#-m@` z;qj=hHyE#}`I<_clwNTH<@l@ZCqHjkd6NTK5Ae@iF+Q$8A;1Z?yC`77* zQ%I+!3Mk;JvUaPLx##l+X~4Of9jP+hk}+3TEQ|ecLVikWIDSJqf1*P%sGN*vuxadf zX3jeO7U2ryLYRBD0)v9-{=T0~d#O;ZDq>q?@ffk?)~lSQ*(84-S3p4^XZmH9;6k42 z5632+5pbj~x>qErUozx3Ji_Y9Cl}sWtUg zRRuKKawnwA{5{)qnH&WH*Xk*OcvB=OepXkB+EE+G#Q&UXXs8)k;$m?9ED2gRJ1djA znP#sf0b*FfKF%{$FXkC%Ev(zLYxFLc5u&<@wUao~HBm(P2J^6G#P4%K&D!fKAka(v z4dQa=Cr4upT+6Bjx*ltr-f|hFB0i{*?H|*P7oYx*XHUDU;a;~b+ z^knP_tcpE2U>Ho3QH3vF^YNP^|(ruijy zZ_g37_1$?D2 zQ5K$N2-p))z&-#DYZV1ui%WCYXeOE(rhP0vW+BS`(@1h)Ik^P$Eb%F_DBp*B%Eb zD!;!yJ2LXdswL4I(_o;e@U;=hQLb_<+3TWBo_3ylHLVB%_1_es6(s6-zq)s!Z3wkE z)Rxn@n&SbD$|M2@8_LJ06)B;qNND@_(@qz-R*>f0#r1DD1Hv2cS0v=wu8@HlVh0Z# zm&)ncL;OGSSaR%0p;LLV&6Ded( zAzy%y?`3C5UB9=pqo0c?-b@OzeojUt=(5bVl3m04o;23^6wNdt&7-2H)eS$InK$cb zD_gAH!Ccle8l12lq5a8)q`w%9AOZj^3+BIbA@vs*yEKR)p{SRX8dF9qUbXYo983RX z+&oKtJXH;>?#+30s}fsdYLZXiB?1D9C#hL}V8%pKsh#JKbmiXvI$nu??KcQ~NVC|2 z51wqxuI5x!!D64DP0nz%gn?u+z6g*mvOo-$b(LoYeR)t;gF7d|OL0zSx)MFOshnci zQDnuTNZMWdL%m-E4|qz6hhvXf*e+m`04*6`HuffKG99o zQx3&$Sf=`wo}N8bR0s=}MayC<&~kBJS6{bN5FUwm* z$++M(f5{2xb|#-m@` z;qj=hHyE#}`I<_clwNTH<@l@ZCqHjkd6NTK5Ae@iF+Q$8A;1Z?yC`77* zQ%I+!3Mk;JvUaPLx##l+X~4Of9jP+hk}+3TEQ|ecLVikWIDSJqf1*P%sGN*vuxadf zX3jeO7U2ryLYRBD0)v9-{=T0~d#O;ZDq>q?@ffk?)~lSQ*(84-S3p4^XZmH9;6k42 z5632+5pbj~x>qErUozx3Ji_Y9Cl}sWtUg zRRuKKawnwA{5{)qnH&WH*Xk*OcvB=OepXkB+EE+G#Q&UXXs8)k;$m?9ED2gRJ1djA znP#sf0b*FfKF%{$FXkC%Ev(zLYxFLc5u&<@wUao~HBm(P2J^6G#P4%K&D!fKAka(v z4dQa=Cr4upT+6Bjx*ltr-f|hFB0i{*?H|*P7oYx*XHUDU;a;~b+ z^knP_tcpE2U>Ho3QH3vF^YNP^|(ruijy zZ_g37_1$ Date: Mon, 17 Apr 2023 12:22:02 +0200 Subject: [PATCH 2/2] ClasspathComputer: overhaul classpath computation Issues fixed: * erroneous exception "Build path contains duplicate entry" * updating source attachment changed unrelated attributes * failed to preserve some existing attributes * failed to preserve entries of kind="var" * failed to preserve existing order * UpdateClasspathJob returned CANCEL on success * drop now unused yet public method createEntryUsingPreviousEntry Tests now succeed, and run as part of AllPDEMinimalTests suite --- .../pde/internal/core/ClasspathComputer.java | 236 +++++++++--------- .../org/eclipse/pde/ui/tests/AllPDETests.java | 2 + .../ui/wizards/tools/UpdateClasspathJob.java | 2 +- 3 files changed, 122 insertions(+), 118 deletions(-) diff --git a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/ClasspathComputer.java b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/ClasspathComputer.java index 26500ca2fc..e86f41c952 100644 --- a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/ClasspathComputer.java +++ b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/ClasspathComputer.java @@ -15,12 +15,15 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; import java.util.Iterator; +import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.regex.Pattern; +import java.util.stream.Collectors; import java.util.stream.Stream; import org.eclipse.core.resources.IFile; @@ -28,7 +31,6 @@ import org.eclipse.core.resources.IResource; import org.eclipse.core.runtime.CoreException; import org.eclipse.core.runtime.IPath; -import org.eclipse.jdt.core.IAccessRule; import org.eclipse.jdt.core.IClasspathAttribute; import org.eclipse.jdt.core.IClasspathEntry; import org.eclipse.jdt.core.IJavaModelStatus; @@ -53,6 +55,11 @@ public class ClasspathComputer { + private record ClasspathConfiguration(IPluginModelBase model, IJavaProject javaProject, + IClasspathAttribute[] defaultAttrs, Map originalByPath, + List reloaded) { + } + private static Map fSeverityTable = null; private static final int SEVERITY_ERROR = 3; private static final int SEVERITY_WARNING = 2; @@ -65,79 +72,91 @@ public static void setClasspath(IProject project, IPluginModelBase model) throws public static IClasspathEntry[] getClasspath(IProject project, IPluginModelBase model, Map sourceLibraryMap, boolean clear, boolean overrideCompliance) throws CoreException { IJavaProject javaProject = JavaCore.create(project); - ArrayList result = new ArrayList<>(); - IBuild build = getBuild(project); + List originalClasspath = clear ? List.of() : Arrays.asList(javaProject.getRawClasspath()); + ClasspathConfiguration context = new ClasspathConfiguration(model, javaProject, + getClasspathAttributes(project, model), mapFirstSeenByPath(originalClasspath.stream()), + new ArrayList<>()); // add JRE and set compliance options String ee = getExecutionEnvironment(model.getBundleDescription()); - result.add(createEntryUsingPreviousEntry(javaProject, ee, PDECore.JRE_CONTAINER_PATH)); - setComplianceOptions(JavaCore.create(project), ee, overrideCompliance); + addContainerEntry(getEEPath(ee), context); + setComplianceOptions(javaProject, ee, overrideCompliance); // add pde container - result.add(createEntryUsingPreviousEntry(javaProject, ee, PDECore.REQUIRED_PLUGINS_CONTAINER_PATH)); + addContainerEntry(PDECore.REQUIRED_PLUGINS_CONTAINER_PATH, context); // add own libraries/source - addSourceAndLibraries(project, model, build, clear, sourceLibraryMap, result); + addSourceAndLibraries(sourceLibraryMap != null ? sourceLibraryMap : Collections.emptyMap(), context); - IClasspathEntry[] entries = result.toArray(new IClasspathEntry[result.size()]); + boolean isTestPlugin = hasTestPluginName(project); + IClasspathEntry[] entries = collectInOriginalOrder(originalClasspath, context.reloaded, isTestPlugin); IJavaModelStatus validation = JavaConventions.validateClasspath(javaProject, entries, javaProject.getOutputLocation()); if (!validation.isOK()) { PDECore.logErrorMessage(validation.getMessage()); throw new CoreException(validation); } - return result.toArray(new IClasspathEntry[result.size()]); + return entries; } - private static void addSourceAndLibraries(IProject project, IPluginModelBase model, IBuild build, boolean clear, - Map sourceLibraryMap, ArrayList result) throws CoreException { - boolean isTestPlugin = hasTestPluginName(project); - HashSet paths = new HashSet<>(); - - // keep existing source folders - if (!clear) { - IClasspathEntry[] entries = JavaCore.create(project).getRawClasspath(); - for (IClasspathEntry entry : entries) { - if (entry.getPath() != null ) { - if (PDECore.JRE_CONTAINER_PATH.isPrefixOf(entry.getPath()) - || PDECore.REQUIRED_PLUGINS_CONTAINER_PATH.equals(entry.getPath())) { - continue; - } - } - if (entry.getEntryKind() == IClasspathEntry.CPE_SOURCE - || entry.getEntryKind() == IClasspathEntry.CPE_PROJECT - || entry.getEntryKind() == IClasspathEntry.CPE_LIBRARY - || entry.getEntryKind() == IClasspathEntry.CPE_CONTAINER) { - if (paths.add(entry.getPath())) { - result.add(updateTestAttribute(isTestPlugin, entry)); - } - } - } + private static IClasspathEntry[] collectInOriginalOrder(List originalClasspath, + List reloaded, boolean isTestPlugin) { + // preserve original entries which eventually weren't reloaded + Map resultingReloadedByPath = mapFirstSeenByPath(reloaded.stream()); + List result = new ArrayList<>(originalClasspath); + result.replaceAll(e -> { + IClasspathEntry replacement = resultingReloadedByPath.remove(pathWithoutEE(e.getPath())); + return replacement != null ? replacement : e; + }); + // using the order of reloading, append new entries (in the map still) + result.addAll(resultingReloadedByPath.values()); + if (isTestPlugin) { + // don't remove existing TEST attributes, but set if advised + result.replaceAll(e -> updateTestAttribute(true, e)); } + return result.toArray(IClasspathEntry[]::new); + } - IClasspathAttribute[] attrs = getClasspathAttributes(project, model); - IPluginLibrary[] libraries = model.getPluginBase().getLibraries(); + private static Map mapFirstSeenByPath(Stream entryStream) { + return entryStream.collect( + Collectors.toMap(e -> pathWithoutEE(e.getPath()), e -> e, (first, dupe) -> first, LinkedHashMap::new)); + } + + private static IPath pathWithoutEE(IPath path) { + // The path member of IClasspathEntry for JRE_CONTAINER_PATH may + // also declare an Execution Environment, which is an attribute. + return PDECore.JRE_CONTAINER_PATH.isPrefixOf(path) ? PDECore.JRE_CONTAINER_PATH : path; + } + + private static void addContainerEntry(IPath path, ClasspathConfiguration context) { + IClasspathEntry original = context.originalByPath.get(pathWithoutEE(path)); + context.reloaded.add(JavaCore.newContainerEntry(path, // + original != null ? original.getAccessRules() : null, + original != null ? original.getExtraAttributes() : context.defaultAttrs, + original != null ? original.isExported() : false)); + } + + private static void addSourceAndLibraries(Map sourceLibraryMap, ClasspathConfiguration context) + throws CoreException { + IPluginLibrary[] libraries = context.model.getPluginBase().getLibraries(); + IBuild build = getBuild(context.javaProject.getProject()); for (IPluginLibrary library : libraries) { IBuildEntry buildEntry = build == null ? null : build.getEntry("source." + library.getName()); //$NON-NLS-1$ if (buildEntry != null) { - addSourceFolder(buildEntry, project, paths, result, isTestPlugin); + addSourceFolders(buildEntry, context); + } else if (library.getName().equals(".")) { //$NON-NLS-1$ + addJARdPlugin(".", sourceLibraryMap, context); //$NON-NLS-1$ } else { - IPath sourceAttachment = sourceLibraryMap != null ? sourceLibraryMap.get(library.getName()) : null; - if (library.getName().equals(".")) { //$NON-NLS-1$ - addJARdPlugin(project, ClasspathUtilCore.getFilename(model), sourceAttachment, attrs, result); - } else { - addLibraryEntry(project, library, sourceAttachment, attrs, result); - } + addLibraryEntry(library, sourceLibraryMap, context); } } if (libraries.length == 0) { if (build != null) { IBuildEntry buildEntry = build.getEntry("source.."); //$NON-NLS-1$ if (buildEntry != null) { - addSourceFolder(buildEntry, project, paths, result, isTestPlugin); + addSourceFolders(buildEntry, context); } - } else if (ClasspathUtilCore.hasBundleStructure(model)) { - IPath sourceAttachment = sourceLibraryMap != null ? (IPath) sourceLibraryMap.get(".") : null; //$NON-NLS-1$ - addJARdPlugin(project, ClasspathUtilCore.getFilename(model), sourceAttachment, attrs, result); + } else if (ClasspathUtilCore.hasBundleStructure(context.model)) { + addJARdPlugin(".", sourceLibraryMap, context); //$NON-NLS-1$ } } } @@ -202,28 +221,27 @@ private static IClasspathAttribute[] getClasspathAttributes(IProject project, IP return attributes; } - private static void addSourceFolder(IBuildEntry buildEntry, IProject project, HashSet paths, - ArrayList result, boolean isTestPlugin) throws CoreException { + private static void addSourceFolders(IBuildEntry buildEntry, ClasspathConfiguration context) + throws CoreException { String[] folders = buildEntry.getTokens(); + IProject project = context.javaProject.getProject(); for (String folder : folders) { IPath path = project.getFullPath().append(folder); - if (paths.add(path)) { - if (project.findMember(folder) == null) { - CoreUtility.createFolder(project.getFolder(folder)); - } else { - IPackageFragmentRoot root = JavaCore.create(project).getPackageFragmentRoot(path.toString()); - if (root.exists() && root.getKind() == IPackageFragmentRoot.K_BINARY) { - result.add(root.getRawClasspathEntry()); - continue; - } - } - if (isTestPlugin) { - result.add(JavaCore.newSourceEntry(path, null, null, null, new IClasspathAttribute[] { - JavaCore.newClasspathAttribute(IClasspathAttribute.TEST, "true") })); //$NON-NLS-1$ - } else { - result.add(JavaCore.newSourceEntry(path)); + IClasspathEntry orig = context.originalByPath.get(pathWithoutEE(path)); + if (orig != null) { + context.reloaded.add(orig); + continue; + } + if (project.findMember(folder) == null) { + CoreUtility.createFolder(project.getFolder(folder)); + } else { + IPackageFragmentRoot root = context.javaProject.getPackageFragmentRoot(path.toString()); + if (root.exists() && root.getKind() == IPackageFragmentRoot.K_BINARY) { + context.reloaded.add(root.getRawClasspathEntry()); + continue; } } + context.reloaded.add(JavaCore.newSourceEntry(path)); } } @@ -237,45 +255,59 @@ protected static IBuild getBuild(IProject project) throws CoreException { return (buildModel != null) ? buildModel.getBuild() : null; } - private static void addLibraryEntry(IProject project, IPluginLibrary library, IPath sourceAttachment, IClasspathAttribute[] attrs, ArrayList result) throws JavaModelException { + private static void addLibraryEntry(IPluginLibrary library, Map sourceLibraryMap, + ClasspathConfiguration context) throws JavaModelException { String name = ClasspathUtilCore.expandLibraryName(library.getName()); - IResource jarFile = project.findMember(name); + IResource jarFile = context.javaProject.getProject().findMember(name); if (jarFile == null) { return; } - IPackageFragmentRoot root = JavaCore.create(project).getPackageFragmentRoot(jarFile); + IPath sourceAttachment = sourceLibraryMap.get(library.getName()); + boolean isExported = library.isExported(); + + IPackageFragmentRoot root = context.javaProject.getPackageFragmentRoot(jarFile); if (root.exists() && root.getKind() == IPackageFragmentRoot.K_BINARY) { IClasspathEntry oldEntry = root.getRawClasspathEntry(); - // If we have the same binary root but new or different source, we should recreate the entry - if ((sourceAttachment == null && oldEntry.getSourceAttachmentPath() != null) || (sourceAttachment != null && sourceAttachment.equals(oldEntry.getSourceAttachmentPath()))) { - if (!result.contains(oldEntry)) { - result.add(oldEntry); - return; - } + // If we have the same binary root but new or different source, we + // should recreate the entry. That is when the source attachment: + // - is not defined: the default could be available now, or + // - is overridden with a different value. + if ((sourceAttachment == null && oldEntry.getSourceAttachmentPath() != null) + || (sourceAttachment != null && sourceAttachment.equals(oldEntry.getSourceAttachmentPath()))) { + context.reloaded.add(oldEntry); + return; } + isExported = oldEntry.isExported(); } - - IClasspathEntry entry = createClasspathEntry(project, jarFile, name, sourceAttachment, attrs, library.isExported()); - if (!result.contains(entry)) { - result.add(entry); - } + reloadClasspathEntry(jarFile, name, sourceAttachment, isExported, context); } - private static void addJARdPlugin(IProject project, String filename, IPath sourceAttachment, IClasspathAttribute[] attrs, ArrayList result) { + private static void addJARdPlugin(String libraryName, Map sourceLibraryMap, + ClasspathConfiguration context) { + String filename = ClasspathUtilCore.getFilename(context.model); String name = ClasspathUtilCore.expandLibraryName(filename); - IResource jarFile = project.findMember(name); + IResource jarFile = context.javaProject.getProject().findMember(name); if (jarFile != null) { - IClasspathEntry entry = createClasspathEntry(project, jarFile, filename, sourceAttachment, attrs, true); - if (!result.contains(entry)) { - result.add(entry); - } + IPath sourceAttachment = sourceLibraryMap.get(libraryName); + reloadClasspathEntry(jarFile, filename, sourceAttachment, true, context); } } - private static IClasspathEntry createClasspathEntry(IProject project, IResource library, String fileName, IPath sourceAttachment, IClasspathAttribute[] attrs, boolean isExported) { - IResource resource = sourceAttachment != null ? project.findMember(sourceAttachment) : project.findMember(ClasspathUtilCore.getSourceZipName(fileName)); - return JavaCore.newLibraryEntry(library.getFullPath(), resource == null ? null : resource.getFullPath(), null, new IAccessRule[0], attrs, isExported); + private static void reloadClasspathEntry(IResource library, String fileName, IPath sourceAttachment, + boolean isExported, ClasspathConfiguration context) { + IClasspathEntry orig = context.originalByPath.get(pathWithoutEE(library.getFullPath())); + if (orig != null && sourceAttachment == null) { + sourceAttachment = orig.getSourceAttachmentPath(); + } + IProject project = context.javaProject.getProject(); + IResource source = sourceAttachment != null ? project.findMember(sourceAttachment) + : project.findMember(ClasspathUtilCore.getSourceZipName(fileName)); + sourceAttachment = source == null ? null : source.getFullPath(); + context.reloaded.add(JavaCore.newLibraryEntry(library.getFullPath(), sourceAttachment, null, + orig != null ? orig.getAccessRules() : null, // + orig != null ? orig.getExtraAttributes() : context.defaultAttrs, + orig != null ? orig.isExported() : isExported)); } private static String getExecutionEnvironment(BundleDescription bundleDescription) { @@ -398,36 +430,6 @@ private static void setMinimumCompliance(Map map, String key, St } } - /** - * Returns a new classpath container entry for the given execution environment. If the given java project - * has an existing JRE/EE classpath entry, the access rules, extra attributes and isExported settings of - * the existing entry will be added to the new execution entry. - * - * @param javaProject project to check for existing classpath entries - * @param ee id of the execution environment to create an entry for - * @param path id of the container to create an entry for - * - * @return new classpath container entry - * @throws CoreException if there is a problem accessing the classpath entries of the project - */ - public static IClasspathEntry createEntryUsingPreviousEntry(IJavaProject javaProject, String ee, IPath path) throws CoreException { - IClasspathEntry[] entries = javaProject.getRawClasspath(); - for (IClasspathEntry entry : entries) { - if (path.isPrefixOf(entry.getPath()) && path.equals(PDECore.JRE_CONTAINER_PATH)) { - return JavaCore.newContainerEntry(getEEPath(ee), entry.getAccessRules(), entry.getExtraAttributes(), entry.isExported()); - } - if (entry.getPath().equals(path)) { - return JavaCore.newContainerEntry(path, entry.getAccessRules(), entry.getExtraAttributes(), entry.isExported()); - } - } - - if (path.equals(PDECore.JRE_CONTAINER_PATH)) { - return createJREEntry(ee); - } - - return JavaCore.newContainerEntry(path); - } - /** * Returns a classpath container entry for the given execution environment. * @param ee id of the execution environment or null diff --git a/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/AllPDETests.java b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/AllPDETests.java index 9d29fa50ab..4070f42e72 100644 --- a/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/AllPDETests.java +++ b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/AllPDETests.java @@ -20,6 +20,7 @@ import org.eclipse.pde.ui.tests.build.properties.AllValidatorTests; import org.eclipse.pde.ui.tests.classpathcontributor.ClasspathContributorTest; import org.eclipse.pde.ui.tests.classpathresolver.ClasspathResolverTest; +import org.eclipse.pde.ui.tests.classpathupdater.ClasspathUpdaterTest; import org.eclipse.pde.ui.tests.ee.ExportBundleTests; import org.eclipse.pde.ui.tests.imports.AllImportTests; import org.eclipse.pde.ui.tests.launcher.AllLauncherTests; @@ -58,6 +59,7 @@ BundleRootTests.class, // PluginRegistryTests.class, // ClasspathResolverTest.class, // + ClasspathUpdaterTest.class, // PDESchemaHelperTest.class, // ClasspathContributorTest.class, // DynamicPluginProjectReferencesTest.class, // diff --git a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/wizards/tools/UpdateClasspathJob.java b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/wizards/tools/UpdateClasspathJob.java index 8e167933fb..ba76d63867 100644 --- a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/wizards/tools/UpdateClasspathJob.java +++ b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/wizards/tools/UpdateClasspathJob.java @@ -75,7 +75,7 @@ class UpdateClasspathWorkspaceRunnable implements IWorkspaceRunnable { @Override public void run(IProgressMonitor monitor) throws CoreException { - fCanceled = doUpdateClasspath(monitor, fModels); + fCanceled = !doUpdateClasspath(monitor, fModels); } public boolean isCanceled() {