Skip to content

Commit

Permalink
Only rely on context IUs for filtering project dependencies
Browse files Browse the repository at this point in the history 
Currently the context IUs are not used to filter project requirements,
this can result in dependencies that are actually disabled to be
included in the project dependencies.

This now removes the previous special handling of fragments by fully
depend on only the context IUs and maximum requirements, for this
purpose the filtering has to be made on project specific contextIUs and
can't just store one collection of filtered items for all callers.

Fix #3197
  • Loading branch information
@laeubi
laeubi committed Dec 12, 2023
1 parent e92d7b6 commit 1bf7c48
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 69 deletions.
83 changes: 21 additions & 62 deletions p2-maven-plugin/src/main/java/org/eclipse/tycho/p2maven/MavenProjectDependencyProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
Expand All @@ -29,6 +26,7 @@
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.Stream;

Expand Down Expand Up @@ -57,8 +55,7 @@
@Component(role = MavenProjectDependencyProcessor.class)
public class MavenProjectDependencyProcessor {

private static final ProjectDependencies EMPTY_DEPENDENCIES = new ProjectDependencies(Collections.emptyList(),
Collections.emptyList(), Map.of(), Set.of());
private static final ProjectDependencies EMPTY_DEPENDENCIES = new ProjectDependencies(Map.of(), Set.of());

private static final boolean DUMP_DATA = Boolean.getBoolean("tycho.p2.dump")
|| Boolean.getBoolean("tycho.p2.dump.dependencies");
Expand Down Expand Up @@ -112,9 +109,11 @@ public ProjectDependencies getProjectDependecies(MavenProject mavenProject) {
}

@Override
public Stream<Entry<MavenProject, Collection<IInstallableUnit>>> dependencies() {
public Stream<Entry<MavenProject, Collection<IInstallableUnit>>> dependencies(
Function<MavenProject, Collection<IInstallableUnit>> contextIuSupplier) {
return projectDependenciesMap.entrySet().stream()
.map(pd -> new SimpleEntry<>(pd.getKey(), pd.getValue().dependencies));
.map(pd -> new SimpleEntry<>(pd.getKey(),
pd.getValue().getDependencies(contextIuSupplier.apply(pd.getKey()))));
}

@Override
Expand Down Expand Up @@ -158,7 +157,8 @@ private Map<MavenProject, ProjectDependencies> computeProjectDependencies(Collec
if (DUMP_DATA) {
File file = new File(project.getBasedir(), "project-dependencies.xml");
try {
new MetadataIO().writeXML(Collections.unmodifiableCollection(projectDependencies.dependencies),
new MetadataIO().writeXML(
Collections.unmodifiableCollection(projectDependencies.getDependencies(List.of())),
file);
} catch (IOException e) {
}
Expand Down Expand Up @@ -197,29 +197,7 @@ private ProjectDependencies computeProjectDependencies(Set<IInstallableUnit> pro
}
Map<IRequirement, Collection<IInstallableUnit>> dependencies = slicer.computeDirectDependencies(projectUnits,
avaiableIUs);
// first collect the maximum desired number
Set<IInstallableUnit> resolved = new LinkedHashSet<>();
for (Entry<IRequirement, Collection<IInstallableUnit>> entry : dependencies.entrySet()) {
IRequirement requirement = entry.getKey();
Collection<IInstallableUnit> units = entry.getValue();
List<IInstallableUnit> limit = units.stream().filter(unit -> !projectUnits.contains(unit))
.limit(requirement.getMax()).toList();
resolved.addAll(limit);
}
// now we need to filter all fragments that we are a host!
// for example SWT creates an explicit requirement to its fragments and we don't
// want them included directly
// TODO reevaluate, this is where filters come into place we probabbly no longer
// need this part!
Set<IInstallableUnit> projectFragments = new HashSet<>();
for (Iterator<IInstallableUnit> iterator = resolved.iterator(); iterator.hasNext();) {
IInstallableUnit unit = iterator.next();
if (hasAnyHost(unit, projectUnits)) {
projectFragments.add(unit);
iterator.remove();
}
}
return new ProjectDependencies(resolved, projectFragments, dependencies, projectUnits);
return new ProjectDependencies(dependencies, projectUnits);
}

private static boolean hasAnyHost(IInstallableUnit unit, Iterable<IInstallableUnit> collection) {
Expand Down Expand Up @@ -267,27 +245,15 @@ private static boolean isMatch(IRequirement requirement, Collection<IInstallable

public static final class ProjectDependencies {

private final Collection<IInstallableUnit> dependencies;
private final Collection<IInstallableUnit> fragments;
private Map<IRequirement, Collection<IInstallableUnit>> requirementsMap;
private Set<IInstallableUnit> projectUnits;
private final Map<IRequirement, Collection<IInstallableUnit>> requirementsMap;
private final Set<IInstallableUnit> projectUnits;

ProjectDependencies(Collection<IInstallableUnit> dependencies, Collection<IInstallableUnit> fragments,
Map<IRequirement, Collection<IInstallableUnit>> requirementsMap, Set<IInstallableUnit> projectUnits) {
this.dependencies = dependencies;
this.fragments = fragments;
ProjectDependencies(Map<IRequirement, Collection<IInstallableUnit>> requirementsMap,
Set<IInstallableUnit> projectUnits) {
this.requirementsMap = requirementsMap;
this.projectUnits = projectUnits;
}

public Collection<IInstallableUnit> getDependencies() {
return dependencies;
}

public Collection<IInstallableUnit> getFragments() {
return fragments;
}

public Collection<IInstallableUnit> getDependencies(Collection<IInstallableUnit> contextIUs) {
return requirementsMap.entrySet().stream().filter(entry -> isMatch(entry.getKey(), contextIUs))
.flatMap(entry -> entry.getValue().stream().filter(unit -> !projectUnits.contains(unit))
Expand Down Expand Up @@ -319,31 +285,24 @@ public static interface ProjectDependencyClosure {
/**
* @return a stream of all contained maven projects with dependecies
*/
Stream<Entry<MavenProject, Collection<IInstallableUnit>>> dependencies();
Stream<Entry<MavenProject, Collection<IInstallableUnit>>> dependencies(
Function<MavenProject, Collection<IInstallableUnit>> contextIuSupplier);

/**
* Given a maven project returns all other maven projects this one (directly)
* depends on
*
* @param mavenProject
* @param mavenProject the maven project for which all direct dependent projects
* should be collected
* @param contextIUs the context IUs to filter dependencies
* @return the collection of projects this maven project depend on in this
* closure
*/
default Collection<MavenProject> getDependencyProjects(MavenProject mavenProject) {
default Collection<MavenProject> getDependencyProjects(MavenProject mavenProject,
Collection<IInstallableUnit> contextIUs) {
ProjectDependencies projectDependecies = getProjectDependecies(mavenProject);
List<MavenProject> list = projectDependecies.getDependencies().stream()
return projectDependecies.getDependencies(contextIUs).stream()
.flatMap(dependency -> getProject(dependency).stream()).distinct().toList();
if (isFragment(mavenProject)) {
return list;
}
return list.stream().flatMap(project -> {
ProjectDependencies dependecies = getProjectDependecies(project);
if (dependecies.getFragments().isEmpty()) {
return Stream.of(project);
}
return Stream.concat(Stream.of(project),
dependecies.getFragments().stream().flatMap(dependency -> getProject(dependency).stream()));
}).distinct().toList();
}

/**
Expand Down
15 changes: 11 additions & 4 deletions tycho-build/src/main/java/org/eclipse/tycho/build/TychoGraphBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package org.eclipse.tycho.build;

import java.util.ArrayList;
import java.util.Collection;
import java.util.Comparator;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -161,11 +162,14 @@ public Result<ProjectDependencyGraph> build(MavenSession session) {
if (DEBUG) {
for (MavenProject project : projects) {
ProjectDependencies depends = dependencyClosure.getProjectDependecies(project);
if (depends.getDependencies().isEmpty()) {
// we fetch all dependencies here without filtering, because the goal is to find
// as many projects that are maybe required
Collection<IInstallableUnit> dependencies = depends.getDependencies(List.of());
if (dependencies.isEmpty()) {
continue;
}
log.info("[[ project " + project.getName() + " depends on: ]]");
for (IInstallableUnit dependency : depends.getDependencies()) {
for (IInstallableUnit dependency : dependencies) {
Optional<MavenProject> mavenProject = dependencyClosure.getProject(dependency);
if (mavenProject.isEmpty()) {
log.info(" IU: " + dependency);
Expand All @@ -186,7 +190,10 @@ public Result<ProjectDependencyGraph> build(MavenSession session) {
ProjectRequest projectRequest = queue.poll();
if (selectedProjects.add(projectRequest.mavenProject)) {
if (projectRequest.addDependencies) {
dependencyClosure.getDependencyProjects(projectRequest.mavenProject).forEach(project -> {
// we fetch all dependencies here without filtering for the context, because the
// goal is to find as many projects that are might be required
dependencyClosure.getDependencyProjects(projectRequest.mavenProject, List.of())
.forEach(project -> {
if (DEBUG) {
log.info(" + add dependency project '" + project.getId() + "' of project '"
+ projectRequest.mavenProject.getId() + "'");
Expand All @@ -196,7 +203,7 @@ public Result<ProjectDependencyGraph> build(MavenSession session) {
});
}
if (projectRequest.addRequires) {
dependencyClosure.dependencies()//
dependencyClosure.dependencies(always -> List.of())//
.filter(entry -> entry.getValue().stream()//
.flatMap(dependency -> dependencyClosure.getProject(dependency).stream())//
.anyMatch(projectRequest::matches))//
Expand Down
2 changes: 1 addition & 1 deletion tycho-core/src/main/java/org/eclipse/tycho/core/TychoProjectManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ public void readExecutionEnvironmentConfiguration(ReactorProject project, Execut
}
}

public List<IInstallableUnit> getContextIUs(MavenProject project) {
public Collection<IInstallableUnit> getContextIUs(MavenProject project) {
TargetPlatformConfiguration configuration = getTargetPlatformConfiguration(project);
return configuration.getEnvironments().stream().map(env -> getProfileProperties(env, configuration))
.map(InstallableUnit::contextIU).toList();
Expand Down
6 changes: 4 additions & 2 deletions tycho-core/src/main/java/org/eclipse/tycho/core/maven/TychoMavenLifecycleParticipant.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,8 @@ public void afterProjectsRead(MavenSession session) throws MavenExecutionExcepti
//do not inject additional dependencies for non Tycho managed projects!
continue;
}
Collection<MavenProject> dependencyProjects = closure.getDependencyProjects(project);
Collection<MavenProject> dependencyProjects = closure.getDependencyProjects(project,
projectManager.getContextIUs(project));
MavenDependencyInjector.injectMavenProjectDependencies(project, dependencyProjects);
if (DUMP_DATA) {
try {
Expand Down Expand Up @@ -198,7 +199,8 @@ private void dumpProjectRequirements(MavenProject project, BufferedWriter writer
writer.write(indent2 + "provides " + satIU + " that satisfies " + requirement + "\r\n");
}
}
dumpProjectRequirements(dependency, writer, closure, closure.getDependencyProjects(dependency), indent2,
dumpProjectRequirements(dependency, writer, closure,
closure.getDependencyProjects(dependency, projectManager.getContextIUs(project)), indent2,
visited);
}
}
Expand Down

0 comments on commit 1bf7c48

Please sign in to comment.