Skip to content

Commit

Permalink
Only selects the highest matching requirement if multiple match
Browse files Browse the repository at this point in the history
  • Loading branch information
@laeubi
laeubi committed Jan 20, 2024
1 parent 2a52f8b commit 6b9cefe
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 32 deletions.
43 changes: 30 additions & 13 deletions tycho-core/src/main/java/org/eclipse/tycho/p2tools/TychoMirrorApplication.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.net.URISyntaxException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
Expand All @@ -32,8 +33,6 @@
import java.util.stream.Stream;

import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.equinox.internal.p2.director.PermissiveSlicer;
import org.eclipse.equinox.internal.p2.director.Slicer;
import org.eclipse.equinox.internal.p2.metadata.IRequiredCapability;
import org.eclipse.equinox.internal.p2.metadata.InstallableUnit;
import org.eclipse.equinox.internal.p2.metadata.RequiredCapability;
Expand All @@ -57,10 +56,11 @@
import org.eclipse.equinox.p2.repository.metadata.IMetadataRepository;
import org.eclipse.equinox.p2.repository.metadata.IMetadataRepositoryManager;
import org.eclipse.tycho.TargetPlatform;
import org.eclipse.tycho.p2.repository.ListCompositeMetadataRepository;
import org.eclipse.tycho.p2.tools.DestinationRepositoryDescriptor;
import org.eclipse.tycho.p2.tools.RepositoryReference;
import org.eclipse.tycho.p2maven.ListCompositeArtifactRepository;
import org.eclipse.tycho.p2tools.copiedfromp2.PermissiveSlicer;
import org.eclipse.tycho.p2tools.copiedfromp2.Slicer;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -106,16 +106,6 @@ public IArtifactRepository getCompositeArtifactRepository() {
return repository;
}

@Override
public IMetadataRepository getCompositeMetadataRepository() {
IMetadataRepository repository = super.getCompositeMetadataRepository();
if (targetPlatform != null) {
return new ListCompositeMetadataRepository(List.of(repository, targetPlatform.getMetadataRepository()),
agent);
}
return repository;
}

@Override
protected Slicer createSlicer(SlicingOptions options) {
List<Map<String, String>> filters = getContextFilters();
Expand Down Expand Up @@ -206,6 +196,33 @@ public IQueryable<IInstallableUnit> slice(Collection<IInstallableUnit> ius, IPro
}
return slice;
}

@Override
protected Stream<IInstallableUnit> selectIUsForRequirement(IQueryable<IInstallableUnit> queryable,
IRequirement req) {
Stream<IInstallableUnit> stream = super.selectIUsForRequirement(queryable, req);
if (targetPlatform == null) {
return stream;
}
List<IInstallableUnit> list = stream.toList();
if (list.isEmpty() && req.getMin() > 0) {
// It is possible that an IU is not visible to the slicer (e.g. dynamic category produced by a categorx.xml)
// In such case we additionally try to query the full target platform here.
return selectHighestIUsForRequirement(targetPlatform.getMetadataRepository(), req);
}
return list.stream();
}

protected Stream<IInstallableUnit> selectHighestIUsForRequirement(IQueryable<IInstallableUnit> queryable,
IRequirement req) {
//first group by ID
Map<String, List<IInstallableUnit>> groupById = queryable
.query(QueryUtil.createMatchQuery(req.getMatches()), null).stream().filter(this::isApplicable)
.collect(Collectors.groupingBy(IInstallableUnit::getId));
//now select the max of items in each group with the same id
return groupById.values().stream().flatMap(list -> list.stream()
.sorted(Comparator.comparing(IInstallableUnit::getVersion).reversed()).limit(req.getMax()));
}
};
}

Expand Down
12 changes: 5 additions & 7 deletions tycho-core/src/main/java/org/eclipse/tycho/p2tools/copiedfromp2/MirrorApplication.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@
import org.eclipse.equinox.app.IApplication;
import org.eclipse.equinox.app.IApplicationContext;
import org.eclipse.equinox.internal.p2.core.helpers.LogHelper;
import org.eclipse.equinox.internal.p2.director.PermissiveSlicer;
import org.eclipse.equinox.internal.p2.director.Slicer;
import org.eclipse.equinox.internal.p2.repository.Transport;
import org.eclipse.equinox.internal.p2.repository.helpers.RepositoryHelper;
import org.eclipse.equinox.p2.core.IProvisioningAgent;
Expand Down Expand Up @@ -280,7 +278,7 @@ protected Mirroring getMirroring(Collection<IInstallableUnit> ius, IProgressMoni
* Collect all artifacts from the IUs that should be mirrored
*
* @param ius
* the IUs that are selected for mirroring
* the IUs that are selected for mirroring
* @return a (modifiable) list of {@link IArtifactKey}s that must be mirrored
*/
protected List<IArtifactKey> collectArtifactKeys(Collection<IInstallableUnit> ius, IProgressMonitor monitor)
Expand Down Expand Up @@ -315,7 +313,7 @@ private void mirrorMetadata(Collection<IInstallableUnit> units, IProgressMonitor
* Collect all IUS from the slice that should be mirrored
*
* @param slice
* the slice for mirroring
* the slice for mirroring
* @return a (modifiable) set of {@link IInstallableUnit}s that must be mirrored
* @throws ProvisionException
*/
Expand All @@ -327,9 +325,9 @@ protected Set<IInstallableUnit> collectUnits(IQueryable<IInstallableUnit> slice,
}

/*
* Ensure all mandatory parameters have been set. Throw an exception if there are any missing. We
* don't require the user to specify the artifact repository here, we will default to the ones
* already registered in the manager. (callers are free to add more if they wish)
* Ensure all mandatory parameters have been set. Throw an exception if there are any missing.
* We don't require the user to specify the artifact repository here, we will default to the
* ones already registered in the manager. (callers are free to add more if they wish)
*/
private void validate() throws ProvisionException {
if (sourceRepositories.isEmpty())
Expand Down
1 change: 0 additions & 1 deletion tycho-core/src/main/java/org/eclipse/tycho/p2tools/copiedfromp2/PermissiveSlicer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

import java.util.Map;

import org.eclipse.equinox.internal.p2.director.Slicer;
import org.eclipse.equinox.internal.p2.metadata.RequiredCapability;
import org.eclipse.equinox.p2.metadata.IInstallableUnit;
import org.eclipse.equinox.p2.metadata.IRequirement;
Expand Down
18 changes: 9 additions & 9 deletions tycho-core/src/main/java/org/eclipse/tycho/p2tools/copiedfromp2/Slicer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.util.Map;
import java.util.Queue;
import java.util.Set;
import java.util.stream.Stream;

import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.core.runtime.IStatus;
Expand All @@ -44,7 +45,6 @@
import org.eclipse.equinox.p2.metadata.IRequirementChange;
import org.eclipse.equinox.p2.metadata.Version;
import org.eclipse.equinox.p2.metadata.expression.IMatchExpression;
import org.eclipse.equinox.p2.query.IQueryResult;
import org.eclipse.equinox.p2.query.IQueryable;
import org.eclipse.equinox.p2.query.QueryUtil;
import org.eclipse.osgi.util.NLS;
Expand Down Expand Up @@ -210,19 +210,14 @@ private void expandRequirement(IInstallableUnit iu, IRequirement req) {
if (req.getMax() == 0) {
return;
}
IQueryResult<IInstallableUnit> matches = possibilites.query(QueryUtil.createMatchQuery(req.getMatches()), null);
int validMatches = 0;
for (IInstallableUnit match : matches) {
if (!isApplicable(match)) {
continue;
}
validMatches++;
List<IInstallableUnit> selected = selectIUsForRequirement(possibilites, req).toList();
for (IInstallableUnit match : selected) {
Map<Version, IInstallableUnit> iuSlice = slice.get(match.getId());
if ((iuSlice == null || !iuSlice.containsKey(match.getVersion())) && considered.add(match)) {
toProcess.add(match);
}
}
if (validMatches == 0) {
if (selected.isEmpty()) {
if (req.getMin() == 0) {
if (DEBUG) {
System.out.println("No IU found to satisfy optional dependency of " + iu + " on req " + req); //$NON-NLS-1$//$NON-NLS-2$
Expand All @@ -233,6 +228,11 @@ private void expandRequirement(IInstallableUnit iu, IRequirement req) {
}
}

protected Stream<IInstallableUnit> selectIUsForRequirement(IQueryable<IInstallableUnit> queryable,
IRequirement req) {
return queryable.query(QueryUtil.createMatchQuery(req.getMatches()), null).stream().filter(this::isApplicable);
}

Set<IInstallableUnit> getNonGreedyIUs() {
return nonGreedyIUs;
}
Expand Down
4 changes: 3 additions & 1 deletion .../java/org/eclipse/tycho/test/p2Repository/RepoRefLocationP2RepositoryIntegrationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.io.File;
import java.util.List;
import java.util.function.Consumer;
import java.util.stream.Collectors;

import org.apache.maven.it.VerificationException;
import org.apache.maven.it.Verifier;
Expand Down Expand Up @@ -77,7 +78,8 @@ public void testAdditionOfOnlyProvidingRepos() throws Exception {
"/p2Repository.repositoryRef.filter.providing", c -> {
});

assertEquals(4, allRepositoryReferences.size());
assertEquals(allRepositoryReferences.stream().map(rr -> rr.uri()).collect(Collectors.joining(", ")), 4,
allRepositoryReferences.size());
assertThat(allRepositoryReferences, containsInAnyOrder( //
new RepositoryReference("https://download.eclipse.org/eclipse/updates/4.29", TYPE_ARTIFACT, ENABLED),
new RepositoryReference("https://download.eclipse.org/eclipse/updates/4.29", TYPE_METADATA, ENABLED),
Expand Down
5 changes: 4 additions & 1 deletion tycho-testing-harness/src/main/java/org/eclipse/tycho/test/AbstractTychoIntegrationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,10 @@ protected File[] assertFileExists(File baseDir, String pattern) {
DirectoryScanner ds = scan(baseDir, pattern);
File[] includedFiles = Arrays.stream(ds.getIncludedFiles()).map(file -> new File(baseDir, file))
.toArray(File[]::new);
assertEquals(baseDir.getAbsolutePath() + "/" + pattern, 1, includedFiles.length);
assertEquals(
baseDir.getAbsolutePath() + "/" + pattern + System.lineSeparator() + Arrays.stream(includedFiles)
.map(f -> f.getName()).collect(Collectors.joining(System.lineSeparator())),
1, includedFiles.length);
assertTrue(baseDir.getAbsolutePath() + "/" + pattern, includedFiles[0].canRead());
return includedFiles;
}
Expand Down

0 comments on commit 6b9cefe

Please sign in to comment.