Skip to content

Commit

Permalink
Rework the pomDependencies=consider mode
Browse files Browse the repository at this point in the history 
With the new resolver mode (`-Dtycho.resolver.classic=false`) pom
dependencies are considered by default, also the way how they are
handled have slightly changed:

Previously all units where always added to the full target resolution
result. This has often lead to undesired effects, especially when there
are large (transitive) dependency chains
as things can easily slip in.

From now on the target platform is always queried first for a unit
fulfilling the requirement and only if not found the pom dependencies
are queried for an alternative.

If you want to use so called mixed-reactor setups, that is you have
bundles build by other techniques than Tycho (e.g.
bnd/felix-maven-plugin) mixed with ones build by Tycho,
previously this was only possible with enabling an incomplete resolver
mode and using `pomDependencies=consider`.

From now on such setups require the use of the new resolver mode
(`-Dtycho.resolver.classic=false`) supporting the usual resolver mode
and thus incomplete resolver mode was removed completely.

Fix #1555
  • Loading branch information
@laeubi
laeubi committed Nov 17, 2022
1 parent 64a6f8a commit 16c8dd8
Show file tree
Hide file tree
Showing 48 changed files with 740 additions and 1,045 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/maven.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ jobs:
uses: actions/cache@v3
with:
path: ~/.m2/repository
key: ${{ runner.os }}-maven-${{ matrix.os }}-${{ hashFiles('**/pom.xml', '**/*.target') }}
key: ${{ runner.os }}-tycho4-${{ matrix.os }}-${{ hashFiles('**/pom.xml', '**/*.target') }}
restore-keys: |
${{ runner.os }}-maven-${{ matrix.os }}-
${{ runner.os }}-tycho4-${{ matrix.os }}-
- name: Set up Maven
uses: stCarolas/[email protected]
with:
Expand Down
16 changes: 16 additions & 0 deletions RELEASE_NOTES.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,22 @@ This page describes the noteworthy improvements provided by each release of Ecli

### Migration guide 3.x > 4.x

#### mixed reactor setups require the new resolver now

If you want to use so called mixed-reactor setups, that is you have bundles build by other techniques than Tycho (e.g. bnd/felix-maven-plugin) mixed with ones build by Tycho,
previously this was only possible with enabling an incomplete resolver mode and using `pomDependencies=consider`.

From now on such setups require the use of the new resolver mode (`-Dtycho.resolver.classic=false`) supporting the usual resolver mode and thus incomplete resolver mode was removed completely.

#### pom declared dependencies handling has slightly changed

With the new resolver mode (`-Dtycho.resolver.classic=false`) pom dependencies are considered by default, also the way how they are handled have slightly changed:

Previously all units where always added to the full target resolution result. This has often lead to undesired effects, especially when there are large (transitive) dependency chains
as things can easily slip in.

From now on the target platform is always queried first for a unit fulfilling the requirement and only if not found the pom dependencies are queried for an alternative.

#### pom declared dependencies are considered for compile

Previously dependencies declared in the pom are ignored by Tycho completely and even though one could enable these to be considered in target platform
Expand Down
188 changes: 136 additions & 52 deletions p2-maven-plugin/src/main/java/org/eclipse/tycho/p2maven/InstallableUnitGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.jar.JarFile;

import org.apache.maven.artifact.Artifact;
import org.apache.maven.artifact.handler.manager.ArtifactHandlerManager;
import org.apache.maven.execution.MavenSession;
import org.apache.maven.plugin.descriptor.PluginDescriptor;
import org.apache.maven.project.MavenProject;
Expand Down Expand Up @@ -57,6 +60,7 @@
import org.eclipse.tycho.p2maven.actions.ProductFile2;
import org.eclipse.tycho.p2maven.helper.PluginRealmHelper;
import org.eclipse.tycho.p2maven.io.MetadataIO;
import org.osgi.framework.Constants;
import org.xml.sax.SAXException;

/**
Expand Down Expand Up @@ -89,6 +93,11 @@ public class InstallableUnitGenerator {
@Requirement
private PlexusContainer plexus;

@Requirement
ArtifactHandlerManager artifactHandlerManager;

private Map<Artifact, ArtifactUnits> artifactUnitMap = new ConcurrentHashMap<>();

/**
* Computes the {@link IInstallableUnit}s for a collection of projects.
*
Expand All @@ -97,8 +106,7 @@ public class InstallableUnitGenerator {
* @throws CoreException if computation for any project failed
*/
public Map<MavenProject, Collection<IInstallableUnit>> getInstallableUnits(Collection<MavenProject> projects,
MavenSession session)
throws CoreException {
MavenSession session) throws CoreException {
init();
Objects.requireNonNull(session);
List<CoreException> errors = new CopyOnWriteArrayList<>();
Expand Down Expand Up @@ -144,8 +152,7 @@ private void init() {
*/
@SuppressWarnings("unchecked")
public Collection<IInstallableUnit> getInstallableUnits(MavenProject project, MavenSession session,
boolean forceUpdate)
throws CoreException {
boolean forceUpdate) throws CoreException {
init();
Objects.requireNonNull(session);
log.debug("Computing installable units for " + project + ", force update = " + forceUpdate);
Expand All @@ -163,58 +170,15 @@ public Collection<IInstallableUnit> getInstallableUnits(MavenProject project, Ma
}
}
}
List<IPublisherAction> actions = new ArrayList<>();
File basedir = project.getBasedir();
if (basedir == null || !basedir.isDirectory()) {
log.warn("No valid basedir for " + project + "!");
return Collections.emptyList();
}
String packaging = project.getPackaging();
switch (packaging) {
case PackagingType.TYPE_ECLIPSE_TEST_PLUGIN:
case PackagingType.TYPE_ECLIPSE_PLUGIN: {
actions.add(new BundlesAction(new File[] { basedir }));
break;
}
case PackagingType.TYPE_ECLIPSE_FEATURE: {
FeatureParser parser = new FeatureParser();
Feature feature = parser.parse(basedir);
feature.setLocation(basedir.getAbsolutePath());
FeatureDependenciesAction action = new FeatureDependenciesAction(feature);
actions.add(action);
break;
}
case PackagingType.TYPE_ECLIPSE_REPOSITORY: {
File categoryFile = new File(basedir, "category.xml");
if (categoryFile.exists()) {
try (InputStream stream = new FileInputStream(categoryFile)) {
SiteModel siteModel = new CategoryParser(null).parse(stream);
actions.add(new CategoryDependenciesAction(siteModel, project.getArtifactId(),
project.getVersion()));
} catch (IOException | SAXException e) {
throw new CoreException(Status.error("Error reading " + categoryFile.getAbsolutePath()));
}
}
for (File f : basedir.listFiles(File::isFile)) {
if (f.getName().endsWith(".product") && !f.getName().startsWith(".polyglot")) {
try {
IProductDescriptor productDescriptor = new ProductFile2(f.getAbsolutePath());
actions.add(new ProductDependenciesAction(productDescriptor));
} catch (CoreException e) {
throw e;
} catch (Exception e) {
throw new CoreException(Status.error("Error reading " + f.getAbsolutePath() + ": " + e, e));
}
}
}
break;
}
case PackagingType.TYPE_P2_IU: {
actions.add(new AuthoredIUAction(basedir));
break;
}
default:
}
String version = project.getVersion();
String artifactId = project.getArtifactId();
List<IPublisherAction> actions = getPublisherActions(packaging, basedir, 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 @@ -240,10 +204,63 @@ public Collection<IInstallableUnit> getInstallableUnits(MavenProject project, Ma
}
}

private List<IPublisherAction> getPublisherActions(String packaging, File basedir, 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 }));
break;
}
case PackagingType.TYPE_ECLIPSE_FEATURE: {
FeatureParser parser = new FeatureParser();
Feature feature = parser.parse(basedir);
feature.setLocation(basedir.getAbsolutePath());
FeatureDependenciesAction action = new FeatureDependenciesAction(feature);
actions.add(action);
break;
}
case PackagingType.TYPE_ECLIPSE_REPOSITORY: {
File categoryFile = new File(basedir, "category.xml");
if (categoryFile.exists()) {
try (InputStream stream = new FileInputStream(categoryFile)) {
SiteModel siteModel = new CategoryParser(null).parse(stream);
actions.add(new CategoryDependenciesAction(siteModel, artifactId, version));
} catch (IOException | SAXException e) {
throw new CoreException(Status.error("Error reading " + categoryFile.getAbsolutePath()));
}
}
for (File f : basedir.listFiles(File::isFile)) {
if (f.getName().endsWith(".product") && !f.getName().startsWith(".polyglot")) {
try {
IProductDescriptor productDescriptor = new ProductFile2(f.getAbsolutePath());
actions.add(new ProductDependenciesAction(productDescriptor));
} catch (CoreException e) {
throw e;
} catch (Exception e) {
throw new CoreException(Status.error("Error reading " + f.getAbsolutePath() + ": " + e, e));
}
}
}
break;
}
case PackagingType.TYPE_P2_IU: {
actions.add(new AuthoredIUAction(basedir));
break;
}
default:
}
return actions;
}

public Collection<IInstallableUnit> getInstallableUnits(Artifact artifact) {
return artifactUnitMap.computeIfAbsent(artifact, x -> new ArtifactUnits()).getUnits(artifact);
}

private Collection<InstallableUnitProvider> getProvider(MavenProject project, MavenSession mavenSession)
throws CoreException {
Set<InstallableUnitProvider> unitProviders = new HashSet<>(
additionalUnitProviders.values());
Set<InstallableUnitProvider> unitProviders = new HashSet<>(additionalUnitProviders.values());
try {
pluginRealmHelper.execute(mavenSession, project, () -> {
try {
Expand Down Expand Up @@ -282,4 +299,71 @@ private static boolean hasPluginDependency(PluginDescriptor pluginDescriptor) {
.filter(dep -> P2Plugin.ARTIFACT_ID.equals(dep.getArtifactId())).findAny().isPresent();
}

private final class ArtifactUnits {

private Collection<IInstallableUnit> units;
private long lastModified;

public synchronized Collection<IInstallableUnit> getUnits(Artifact artifact) {
if (units != null && !hasChanges(artifact)) {
return units;
}
try {
// TODO in case of "java-source" type, we might want to generate the source IU
// based on the parent artifact!
File file = artifact.getFile();
if (isValidFile(file)) {
lastModified = file.lastModified();
String type = artifact.getType();
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());
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());
return units = publisher.publishMetadata(actions);
} else {
boolean isBundle = false;
boolean isFeature = false;
try (JarFile jarFile = new JarFile(file)) {
isBundle = jarFile.getManifest().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());
return units = publisher.publishMetadata(actions);
}
if (isFeature) {
List<IPublisherAction> actions = getPublisherActions(PackagingType.TYPE_ECLIPSE_FEATURE,
file, artifact.getVersion(), artifact.getArtifactId());
return units = publisher.publishMetadata(actions);
}
}
}
} catch (CoreException e) {
// can't generate one then...
}
return units = Collections.emptyList();
}

private boolean isValidFile(File file) {
return file != null && file.getName().toLowerCase().endsWith(".jar") && file.exists();
}

private boolean hasChanges(Artifact artifact) {
File file = artifact.getFile();
if (isValidFile(file)) {
return file.lastModified() != lastModified;
}
return false;
}

}

}
55 changes: 53 additions & 2 deletions p2-maven-plugin/src/main/java/org/eclipse/tycho/p2maven/InstallableUnitPublisher.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,26 @@
package org.eclipse.tycho.p2maven;

import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.Map;
import java.util.Map.Entry;

import org.codehaus.plexus.component.annotations.Component;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IStatus;
import org.eclipse.core.runtime.NullProgressMonitor;
import org.eclipse.equinox.internal.p2.artifact.repository.simple.SimpleArtifactDescriptor;
import org.eclipse.equinox.p2.metadata.IInstallableUnit;
import org.eclipse.equinox.p2.publisher.IPublisherAction;
import org.eclipse.equinox.p2.publisher.IPublisherAdvice;
import org.eclipse.equinox.p2.publisher.IPublisherInfo;
import org.eclipse.equinox.p2.publisher.PublisherInfo;
import org.eclipse.equinox.p2.publisher.PublisherResult;
import org.eclipse.equinox.p2.publisher.actions.IPropertyAdvice;
import org.eclipse.equinox.p2.query.IQueryResult;
import org.eclipse.equinox.p2.query.QueryUtil;
import org.eclipse.equinox.p2.repository.artifact.IArtifactDescriptor;

/**
* Component that helps publishing units using publisher actions
Expand All @@ -36,24 +44,67 @@ public class InstallableUnitPublisher {
* perform the provided {@link IPublisherAction}s and return a (modifiable)
* collection of the resulting {@link IInstallableUnit}s
*
* @param actions the actions to perform
* @param actions the {@link IPublisherAction}s to perform
* @return the result of the publishing operation
* @throws CoreException if publishing of an action failed
*/
public Collection<IInstallableUnit> publishMetadata(Collection<? extends IPublisherAction> actions)
throws CoreException {
return publishMetadata(actions, Collections.emptyList());
}

/**
* perform the provided {@link IPublisherAction}s and return a (modifiable)
* collection of the resulting {@link IInstallableUnit}s
*
* @param actions the {@link IPublisherAction}s to perform
* @param advices additional {@link IPublisherAdvice}s for the operation
* @return the result of the publishing operation
* @throws CoreException if publishing of an action failed
*/
public Collection<IInstallableUnit> publishMetadata(Collection<? extends IPublisherAction> actions,
Collection<? extends IPublisherAdvice> advices)
throws CoreException {
if (actions.isEmpty()) {
return new HashSet<>();
}
PublisherInfo publisherInfo = new PublisherInfo();
publisherInfo.setArtifactOptions(IPublisherInfo.A_INDEX);
for (IPublisherAdvice advice : advices) {
publisherInfo.addAdvice(advice);
}
PublisherResult results = new PublisherResult();
for (IPublisherAction action : actions) {
IStatus status = action.perform(publisherInfo, results, new NullProgressMonitor());
if (status.matches(IStatus.ERROR)) {
throw new CoreException(status);
}
}
return results.query(QueryUtil.ALL_UNITS, null).toSet();
IQueryResult<IInstallableUnit> units = results.query(QueryUtil.ALL_UNITS, null);
return units.toSet();
}

public void applyAdvices(Collection<IInstallableUnit> units, IArtifactDescriptor descriptor,
Collection<? extends IPublisherAdvice> advices) {
PublisherInfo publisherInfo = new PublisherInfo();
publisherInfo.setArtifactOptions(IPublisherInfo.A_INDEX);
for (IPublisherAdvice advice : advices) {
publisherInfo.addAdvice(advice);
}
for (IInstallableUnit unit : units) {
Collection<IPropertyAdvice> advice = publisherInfo.getAdvice(null, false, unit.getId(), unit.getVersion(),
IPropertyAdvice.class);
for (IPropertyAdvice entry : advice) {
Map<String, String> props = entry.getArtifactProperties(unit, descriptor);
if (props == null)
continue;
if (descriptor instanceof SimpleArtifactDescriptor simpleArtifactDescriptor) {
for (Entry<String, String> pe : props.entrySet()) {
simpleArtifactDescriptor.setRepositoryProperty(pe.getKey(), pe.getValue());
}
}
}
}

}
}
2 changes: 1 addition & 1 deletion ...cho/p2/publisher/MavenChecksumAdvice.java → .../p2maven/advices/MavenChecksumAdvice.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
* Contributors:
* Christoph Läubrich - initial API and implementation
*******************************************************************************/
package org.eclipse.tycho.p2.publisher;
package org.eclipse.tycho.p2maven.advices;

import java.io.File;
import java.io.IOException;
Expand Down
Loading

0 comments on commit 16c8dd8

Please sign in to comment.