Skip to content

Commit

Permalink
Use the updated p2 metadata once the project is packed
Browse files Browse the repository at this point in the history
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 eclipse-tycho#3824
  • Loading branch information
laeubi committed Jun 8, 2024
1 parent 829568d commit b913061
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -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);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() + "]";
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
1 change: 1 addition & 0 deletions tycho-its/projects/tycho-ds-dependency/.mvn/maven.config
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
-Dtycho.localArtifacts=ignore

0 comments on commit b913061

Please sign in to comment.