Use the updated p2 metadata once the project is packed

Currently Tycho always uses the INITIAL dependency metadata to compute the preliminary target platform. As this metadata can change once the project is packed (e.g. headers added / removed) this can lead to problems in plugins that depend on the changed plugin because P2 sees the updated metadata afterwards and fails. This now distinguish both cases and used the SEED metadata if the project was already packed. Fix #3824
eclipse-tycho · Jun 8, 2024 · fc91332 · fc91332
1 parent 6394de5
commit fc91332
Show file tree

Hide file tree

Showing 6 changed files with 88 additions and 30 deletions.
diff --git a/p2-maven-plugin/src/main/java/org/eclipse/tycho/p2maven/InstallableUnitGenerator.java b/p2-maven-plugin/src/main/java/org/eclipse/tycho/p2maven/InstallableUnitGenerator.java
@@ -86,6 +86,8 @@ public class InstallableUnitGenerator {
 
 	private static final String KEY_UNITS = "InstallableUnitGenerator.units";
 
+	private static final String KEY_ARTIFACT_FILE = "InstallableUnitGenerator.artifactFile";
+
 	@Requirement
 	private IProvisioningAgent provisioningAgent;
 
@@ -165,28 +167,37 @@ public Collection<IInstallableUnit> getInstallableUnits(MavenProject project, Ma
 		Objects.requireNonNull(session);
 		log.debug("Computing installable units for " + project + ", force update = " + forceUpdate);
 		synchronized (project) {
-			if (!forceUpdate) {
-				Object contextValue = project.getContextValue(KEY_UNITS);
-				if (contextValue instanceof Collection<?>) {
-					Collection<IInstallableUnit> collection = (Collection<IInstallableUnit>) contextValue;
-					if (isCompatible(collection)) {
-						log.debug("Using cached value for " + project);
-						return collection;
-					} else {
-						log.debug("Cannot use cached value for " + project
-								+ " because of incompatible classloaders, update is forced");
-					}
-				}
-			}
 			File basedir = project.getBasedir();
 			if (basedir == null || !basedir.isDirectory()) {
 				log.warn("No valid basedir for " + project + " found");
 				return Collections.emptyList();
 			}
+			File projectArtifact = getProjectArtifact(project);
+			if (!forceUpdate) {
+				// first check if the packed state might has changed...
+				if (Objects.equals(project.getContextValue(KEY_ARTIFACT_FILE), projectArtifact)) {
+					Object contextValue = project.getContextValue(KEY_UNITS);
+					if (contextValue instanceof Collection<?>) {
+						// now check if we are classlaoder compatible...
+						Collection<IInstallableUnit> collection = (Collection<IInstallableUnit>) contextValue;
+						if (isCompatible(collection)) {
+							log.debug("Using cached value for " + project);
+							return collection;
+						} else {
+							log.debug("Cannot use cached value for " + project
+									+ " because of incompatible classloaders, update is forced");
+						}
+					}
+				} else {
+					log.info("Cannot use cached value for " + project
+							+ " because project artifact has changed, update is forced");
+				}
+			}
 			String packaging = project.getPackaging();
 			String version = project.getVersion();
 			String artifactId = project.getArtifactId();
-			List<IPublisherAction> actions = getPublisherActions(packaging, basedir, version, artifactId);
+			List<IPublisherAction> actions = getPublisherActions(packaging, basedir, projectArtifact, version,
+					artifactId);
 			Collection<IInstallableUnit> publishedUnits = publisher.publishMetadata(actions);
 			for (InstallableUnitProvider unitProvider : getProvider(project, session)) {
 				log.debug("Asking " + unitProvider + " for additional units for " + project);
@@ -207,24 +218,38 @@ public Collection<IInstallableUnit> getInstallableUnits(MavenProject project, Ma
 				log.debug("Cannot generate any InstallableUnit for packaging type '" + packaging + "' for " + project);
 			}
 			project.setContextValue(KEY_UNITS, result);
+			project.setContextValue(KEY_ARTIFACT_FILE, projectArtifact);
 			return result;
 
 		}
 	}
 
-	private List<IPublisherAction> getPublisherActions(String packaging, File basedir, String version,
-			String artifactId) throws CoreException {
+	private static File getProjectArtifact(MavenProject project) {
+		Artifact artifact = project.getArtifact();
+		if (artifact != null) {
+			File file = artifact.getFile();
+			if (file != null && file.exists()) {
+				return file;
+			}
+		}
+		return null;
+	}
+
+	private List<IPublisherAction> getPublisherActions(String packaging, File basedir, File projectArtifact,
+			String version, String artifactId) throws CoreException {
 		List<IPublisherAction> actions = new ArrayList<>();
 		switch (packaging) {
 		case PackagingType.TYPE_ECLIPSE_TEST_PLUGIN:
 		case PackagingType.TYPE_ECLIPSE_PLUGIN: {
-			actions.add(new BundlesAction(new File[] { basedir }));
+			File bundleFile = Objects.requireNonNullElse(projectArtifact, basedir);
+			actions.add(new BundlesAction(new File[] { bundleFile }));
 			break;
 		}
 		case PackagingType.TYPE_ECLIPSE_FEATURE: {
 			FeatureParser parser = new FeatureParser();
-			Feature feature = parser.parse(basedir);
-			feature.setLocation(basedir.getAbsolutePath());
+			File featureFile = Objects.requireNonNullElse(projectArtifact, basedir);
+			Feature feature = parser.parse(featureFile);
+			feature.setLocation(featureFile.getAbsolutePath());
 			FeatureDependenciesAction action = new FeatureDependenciesAction(feature);
 			actions.add(action);
 			break;
@@ -353,32 +378,31 @@ public synchronized Collection<IInstallableUnit> getUnits(Artifact artifact) {
 					if (PackagingType.TYPE_ECLIPSE_PLUGIN.equals(type)
 							|| PackagingType.TYPE_ECLIPSE_TEST_PLUGIN.equals(type) || "bundle".equals(type)) {
 						List<IPublisherAction> actions = getPublisherActions(PackagingType.TYPE_ECLIPSE_PLUGIN, file,
-								artifact.getVersion(), artifact.getArtifactId());
+								file, artifact.getVersion(), artifact.getArtifactId());
 						return units = publisher.publishMetadata(actions);
 					} else if (PackagingType.TYPE_ECLIPSE_FEATURE.equals(type)) {
 						List<IPublisherAction> actions = getPublisherActions(PackagingType.TYPE_ECLIPSE_FEATURE, file,
-								artifact.getVersion(), artifact.getArtifactId());
+								file, artifact.getVersion(), artifact.getArtifactId());
 						return units = publisher.publishMetadata(actions);
 					} else {
 						boolean isBundle = false;
 						boolean isFeature = false;
 						try (JarFile jarFile = new JarFile(file)) {
 							Manifest manifest = jarFile.getManifest();
 							isBundle = manifest != null
-									&& manifest.getMainAttributes()
-									.getValue(Constants.BUNDLE_SYMBOLICNAME) != null;
+									&& manifest.getMainAttributes().getValue(Constants.BUNDLE_SYMBOLICNAME) != null;
 							isFeature = jarFile.getEntry("feature.xml") != null;
 						} catch (IOException e) {
 							// can't determine the type then...
 						}
 						if (isBundle) {
 							List<IPublisherAction> actions = getPublisherActions(PackagingType.TYPE_ECLIPSE_PLUGIN,
-									file, artifact.getVersion(), artifact.getArtifactId());
+									file, file, artifact.getVersion(), artifact.getArtifactId());
 							return units = publisher.publishMetadata(actions);
 						}
 						if (isFeature) {
 							List<IPublisherAction> actions = getPublisherActions(PackagingType.TYPE_ECLIPSE_FEATURE,
-									file, artifact.getVersion(), artifact.getArtifactId());
+									file, file, artifact.getVersion(), artifact.getArtifactId());
 							return units = publisher.publishMetadata(actions);
 						}
 					}

diff --git a/tycho-api/src/main/java/org/eclipse/tycho/ReactorProjectIdentities.java b/tycho-api/src/main/java/org/eclipse/tycho/ReactorProjectIdentities.java
@@ -46,4 +46,10 @@ public final int hashCode() {
         return Objects.hash(getArtifactId(), getGroupId(), getVersion());
     }
 
+    @Override
+    public String toString() {
+        return "ReactorProjectIdentities [GroupId=" + getGroupId() + ", ArtifactId=" + getArtifactId() + ", Version="
+                + getVersion() + ", Basedir=" + getBasedir() + "]";
+    }
+
 }
diff --git a/tycho-core/src/main/java/org/eclipse/tycho/p2resolver/P2GeneratorImpl.java b/tycho-core/src/main/java/org/eclipse/tycho/p2resolver/P2GeneratorImpl.java
@@ -451,8 +451,7 @@ public Map<String, IP2Artifact> generateMetadata(MavenProject project, boolean g
         PublisherOptions options = new PublisherOptions();
         options.setGenerateDownloadStats(generateDownloadStatsProperty);
         options.setGenerateChecksums(generateChecksums);
-        Map<String, IP2Artifact> generatedMetadata = generateMetadata(artifacts, options, targetDir);
-        return generatedMetadata;
+        return generateMetadata(artifacts, options, targetDir);
     }
 
     @Override

diff --git a/tycho-core/src/main/java/org/eclipse/tycho/p2resolver/TargetPlatformFactoryImpl.java b/tycho-core/src/main/java/org/eclipse/tycho/p2resolver/TargetPlatformFactoryImpl.java
@@ -561,9 +561,9 @@ private Map<IInstallableUnit, ReactorProjectIdentities> getPreliminaryReactorPro
         Map<IInstallableUnit, Set<File>> duplicateReactorUIs = new HashMap<>();
 
         for (ReactorProject project : reactorProjects) {
-            Set<IInstallableUnit> projectIUs = project.getDependencyMetadata(DependencyMetadataType.INITIAL);
 
-            if (projectIUs == null) {
+            Set<IInstallableUnit> projectIUs = getPreliminaryReactorProjectUIs(project);
+            if (projectIUs == null || projectIUs.isEmpty()) {
                 continue;
             }
             for (IInstallableUnit iu : projectIUs) {
@@ -589,6 +589,19 @@ private Map<IInstallableUnit, ReactorProjectIdentities> getPreliminaryReactorPro
         return reactorUIs;
     }
 
+    private Set<IInstallableUnit> getPreliminaryReactorProjectUIs(ReactorProject project) {
+        String packaging = project.getPackaging();
+        if (PackagingType.TYPE_ECLIPSE_PLUGIN.equals(packaging) || PackagingType.TYPE_ECLIPSE_FEATURE.equals(packaging)
+                || PackagingType.TYPE_ECLIPSE_TEST_PLUGIN.equals(packaging)) {
+            File artifact = project.getArtifact();
+            if (artifact != null && artifact.isFile()) {
+                //the project was already build, use the seed units as they include anything maybe updated by p2-metadata mojo
+                return project.getDependencyMetadata(DependencyMetadataType.SEED);
+            }
+        }
+        return project.getDependencyMetadata(DependencyMetadataType.INITIAL);
+    }
+
     private void applyFilters(TargetPlatformFilterEvaluator filter, Collection<IInstallableUnit> collectionToModify,
             Set<IInstallableUnit> reactorProjectUIs, ExecutionEnvironmentResolutionHints eeResolutionHints,
             Set<IInstallableUnit> shadowedIus) {

diff --git a/tycho-its/projects/eeProfile.resolution.fragments/org.eclipse.swt.gtk.linux.x86/pom.xml b/tycho-its/projects/eeProfile.resolution.fragments/org.eclipse.swt.gtk.linux.x86/pom.xml
@@ -13,5 +13,20 @@
 	<artifactId>org.eclipse.swt.gtk.linux.x86</artifactId>
 	<version>3.102.0-SNAPSHOT</version>
 	<packaging>eclipse-plugin</packaging>
-
+	<build>
+        <plugins>
+            <plugin>
+                <groupId>org.eclipse.tycho</groupId>
+                <artifactId>target-platform-configuration</artifactId>
+                <configuration>
+                    <dependency-resolution>
+                        <optionalDependencies>ignore</optionalDependencies>
+                        <profileProperties>
+                            <org.eclipse.swt.buildtime>true</org.eclipse.swt.buildtime>
+                        </profileProperties>
+                    </dependency-resolution>
+                </configuration>
+            </plugin>
+        </plugins>
+    </build>
 </project>
diff --git a/tycho-its/projects/tycho-ds-dependency/.mvn/maven.config b/tycho-its/projects/tycho-ds-dependency/.mvn/maven.config
@@ -0,0 +1 @@
+-Dtycho.localArtifacts=ignore