Skip to content

Add classpath_depth to allow controlling transitive dependency depth #503

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Jul 19, 2024
Merged

Add classpath_depth to allow controlling transitive dependency depth #503

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
1beb5db
Add import_depth to allow controlling transative dependency depth
brian-mcnamara Jun 27, 2024
b2a04ce
Some refactoring
brian-mcnamara Jun 28, 2024
7e4701e
Store unloaded dependencies for use in runtime
brian-mcnamara Jul 1, 2024
7eb1b4e
Refactor
brian-mcnamara Jul 2, 2024
5e55f34
Ensure loaded and unloaded are correct
brian-mcnamara Jul 3, 2024
95d88a8
Allow import depth of 0
brian-mcnamara Jul 3, 2024
bbd6c32
Refactor importDepth to runtimeImportDepth
brian-mcnamara Jul 8, 2024
d2f127c
Rename import depth flag
brian-mcnamara Jul 15, 2024
4426bfd
Don't maintain order and move flag inside settings
brian-mcnamara Jul 16, 2024
e32a36f
Apply suggestions from code review
brian-mcnamara Jul 18, 2024
a9c87e6
Review comments
brian-mcnamara Jul 18, 2024
0e36caa
Merge remote-tracking branch 'origin/main' into perf
brian-mcnamara Jul 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 4 additions & 18 deletions .../bazel/eclipse/core/model/discovery/BuildFileAndVisibilityDrivenProvisioningStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import static java.util.stream.Collectors.groupingBy;
import static java.util.stream.Collectors.joining;
import static java.util.stream.Collectors.toCollection;
import static java.util.stream.Collectors.toSet;
import static org.eclipse.core.resources.IResource.DEPTH_ZERO;

import java.util.ArrayList;
Expand Down Expand Up @@ -248,29 +247,17 @@ public Map<BazelProject, Collection<ClasspathEntry>> computeClasspaths(Collectio

// query for rdeps to find classpath exclusions
var projectTargets = activeTargetsPerProject.get(bazelProject);
var rdeps = workspace.getCommandExecutor()
.runQueryWithoutLock(
new BazelQueryForLabelsCommand(
workspaceRoot,
format(
"kind(java_library, rdeps(//..., %s))",
projectTargets.stream().map(BazelLabel::toString).collect(joining(" + "))),
true,
format(
"Querying for reverse dependencies of '%s' for classpath computation",
bazelProject.getName())))
.stream()
.map(BazelLabel::new)
.collect(toSet());

var targets = projectTargets.stream().map(BazelLabel::toString).collect(joining(" + "));
// get all accessible targets based on visibility
Set<BazelLabel> allVisibleTargets = workspace.getCommandExecutor()
.runQueryWithoutLock(
new BazelQueryForLabelsCommand(
workspaceRoot,
format(
"kind(java_library, visible(%s, //...))",
projectTargets.stream().map(BazelLabel::toString).collect(joining(" + "))),
"kind(java_library, visible(%s, //...)) except kind(java_library, rdeps(//..., %s))",
targets,
targets),
true,
format(
"Querying for Java targets visibile to '%s' for classpath computation",
Expand All @@ -295,7 +282,6 @@ public Map<BazelProject, Collection<ClasspathEntry>> computeClasspaths(Collectio

// get the remaining list of visible deps based on workspace dependency graph
var visibleDeps = new ArrayList<>(allVisibleTargets);
visibleDeps.removeAll(rdeps); // exclude reverse deps
visibleDeps.removeAll(projectTargets); // exclude project targets (avoid dependencies on itself)

// compute the classpath
Expand Down
11 changes: 10 additions & 1 deletion ....core/src/com/salesforce/bazel/eclipse/core/model/discovery/JavaAspectsClasspathInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
import com.salesforce.bazel.eclipse.core.model.discovery.classpath.AccessRule;
import com.salesforce.bazel.eclipse.core.model.discovery.classpath.ClasspathEntry;
import com.salesforce.bazel.sdk.command.BazelBuildWithIntelliJAspectsCommand;
import com.salesforce.bazel.sdk.model.BazelLabel;

/**
* Holds information for computing Java classpath configuration of a target or a package.
Expand Down Expand Up @@ -132,10 +133,13 @@ static boolean isJavaProtoTarget(TargetIdeInfo target) {

/** set of exports (insertion order is not relevant) */
final Set<Label> exports = new HashSet<>();
final Set<BazelLabel> availableDependencies;

public JavaAspectsClasspathInfo(JavaAspectsInfo aspectsInfo, BazelWorkspace bazelWorkspace) throws CoreException {
public JavaAspectsClasspathInfo(JavaAspectsInfo aspectsInfo, BazelWorkspace bazelWorkspace,
Set<BazelLabel> availableDependencies) throws CoreException {
super(bazelWorkspace);
this.aspectsInfo = aspectsInfo;
this.availableDependencies = availableDependencies;
}

private void addDirectDependency(Dependency directDependency) {
Expand Down Expand Up @@ -363,6 +367,10 @@ public Collection<ClasspathEntry> compute() throws CoreException {

// Collect jars referenced by runtime deps
for (TargetKey targetKey : runtimeDeps) {
if ((availableDependencies.size() > 0)
&& !availableDependencies.contains(new BazelLabel(targetKey.getLabel().toString()))) {
continue;
}
var entries = resolveDependency(targetKey);
for (ClasspathEntry entry : entries) {
// runtime dependencies are only visible to tests
Expand Down Expand Up @@ -514,6 +522,7 @@ protected Collection<ClasspathEntry> resolveDependency(TargetKey targetKey) thro
var exported = exports.contains(targetKey.getLabel());
var result = new LinkedHashSet<ClasspathEntry>();
for (BlazeJarLibrary library : jars) {

var jarEntry = resolveJar(library.libraryArtifact);
if (jarEntry == null) {
if (LOG.isDebugEnabled()) {
Expand Down
35 changes: 34 additions & 1 deletion .../salesforce/bazel/eclipse/core/model/discovery/ProjectPerPackageProvisioningStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import static java.util.stream.Collectors.groupingBy;
import static java.util.stream.Collectors.joining;
import static java.util.stream.Collectors.toList;
import static java.util.stream.Collectors.toSet;

import java.util.ArrayList;
import java.util.Collection;
Expand Down Expand Up @@ -48,6 +49,7 @@
import com.salesforce.bazel.sdk.aspects.intellij.IntellijAspects;
import com.salesforce.bazel.sdk.aspects.intellij.IntellijAspects.OutputGroup;
import com.salesforce.bazel.sdk.command.BazelBuildWithIntelliJAspectsCommand;
import com.salesforce.bazel.sdk.command.BazelQueryForLabelsCommand;
import com.salesforce.bazel.sdk.model.BazelLabel;

/**
Expand Down Expand Up @@ -80,6 +82,34 @@ public Map<BazelProject, Collection<ClasspathEntry>> computeClasspaths(Collectio
try {
var monitor =
TracingSubMonitor.convert(progress, "Computing Bazel project classpaths", 1 + bazelProjects.size());

var importDepth = workspace.getBazelProjectView().importDepth();
Set<BazelLabel> availableDependencies = Set.of();
if (importDepth > 0) {
List<BazelTarget> targets = new ArrayList<>();
for (BazelProject project : bazelProjects) {
monitor.checkCanceled();
targets.addAll(project.getBazelTargets());
}
var targetLabels =
targets.stream().map(BazelTarget::getLabel).map(BazelLabel::toString).collect(joining(" + "));
availableDependencies = workspace.getCommandExecutor()
.runQueryWithoutLock(
new BazelQueryForLabelsCommand(
workspace.getLocation().toPath(),
format(
"kind(java_library, deps(%s, %d)) + kind(java_import, deps(%s, %d))",
targetLabels,
importDepth,
targetLabels,
importDepth),
true,
format("Querying for depdendencies for projects: %s", targetLabels)))
.stream()
.map(BazelLabel::new)
.collect(toSet());
}

monitor.subTask("Collecting shards...");

Map<BazelProject, Collection<BazelTarget>> activeTargetsPerProject = new HashMap<>();
Expand Down Expand Up @@ -137,6 +167,7 @@ public Map<BazelProject, Collection<ClasspathEntry>> computeClasspaths(Collectio
var currentShardCount = 0;
for (Map<BazelProject, Collection<BazelTarget>> shard : shardsToBuild) {
currentShardCount++;

var targetsToBuild = shard.values()
.stream()
.flatMap(Collection::stream)
Expand Down Expand Up @@ -181,7 +212,7 @@ public Map<BazelProject, Collection<ClasspathEntry>> computeClasspaths(Collectio
subMonitor.checkCanceled();

// build index of classpath info
var classpathInfo = new JavaAspectsClasspathInfo(aspectsInfo, workspace);
var classpathInfo = new JavaAspectsClasspathInfo(aspectsInfo, workspace, availableDependencies);
var buildPathProblems = new ArrayList<IStatus>();

// add the targets
Expand Down Expand Up @@ -235,6 +266,7 @@ public Map<BazelProject, Collection<ClasspathEntry>> computeClasspaths(Collectio
}

return classpathsByProject;

} finally {
if (progress != null) {
progress.done();
Expand Down Expand Up @@ -438,6 +470,7 @@ yield requireNonNull(additionalJavaLikeRules, "additional java like rules not in
.contains(ruleName);
}
};

}

private void logSourceFilesWithoutCommonRoot(BazelProject project, JavaSourceInfo sourceInfo, boolean isTestSources)
Expand Down
26 changes: 25 additions & 1 deletion ...m/salesforce/bazel/eclipse/core/model/discovery/ProjectPerTargetProvisioningStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

import static java.lang.String.format;
import static java.nio.file.Files.isRegularFile;
import static java.util.stream.Collectors.joining;
import static java.util.stream.Collectors.toList;
import static java.util.stream.Collectors.toSet;
import static org.eclipse.core.runtime.SubMonitor.SUPPRESS_ALL_LABELS;

import java.util.ArrayList;
Expand Down Expand Up @@ -32,6 +34,7 @@
import com.salesforce.bazel.sdk.aspects.intellij.IntellijAspects;
import com.salesforce.bazel.sdk.aspects.intellij.IntellijAspects.OutputGroup;
import com.salesforce.bazel.sdk.command.BazelBuildWithIntelliJAspectsCommand;
import com.salesforce.bazel.sdk.command.BazelQueryForLabelsCommand;
import com.salesforce.bazel.sdk.model.BazelLabel;

/**
Expand Down Expand Up @@ -78,6 +81,27 @@ public Map<BazelProject, Collection<ClasspathEntry>> computeClasspaths(Collectio

var workspaceRoot = workspace.getLocation().toPath();

var importDepth = workspace.getBazelProjectView().importDepth();
Set<BazelLabel> availableDependencies = Set.of();
if (importDepth > 0) {
var targetLabels = targetsToBuild.stream().map(BazelLabel::toString).collect(joining(" + "));
availableDependencies = workspace.getCommandExecutor()
.runQueryWithoutLock(
new BazelQueryForLabelsCommand(
workspace.getLocation().toPath(),
format(
"kind(java_library, deps(%s, %d)) + kind(java_import, deps(%s, %d))",
targetLabels,
importDepth,
targetLabels,
importDepth),
true,
format("Querying for depdendencies for projects: %s", targetLabels)))
.stream()
.map(BazelLabel::new)
.collect(toSet());
}

// run the aspect to compute all required information
var aspects = workspace.getParent().getModelManager().getIntellijAspects();
var languages = Set.of(LanguageClass.JAVA);
Expand Down Expand Up @@ -114,7 +138,7 @@ public Map<BazelProject, Collection<ClasspathEntry>> computeClasspaths(Collectio
monitor.checkCanceled();

// build index of classpath info
var classpathInfo = new JavaAspectsClasspathInfo(aspectsInfo, workspace);
var classpathInfo = new JavaAspectsClasspathInfo(aspectsInfo, workspace, availableDependencies);

// remove old marker
deleteClasspathContainerProblems(bazelProject);
Expand Down
8 changes: 7 additions & 1 deletion ...clipse.core/src/com/salesforce/bazel/eclipse/core/projectview/BazelProjectFileReader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ public void close() {
final LinkedHashSet<String> externalJarsFilters = new LinkedHashSet<>();
int targetShardSize = 500;
boolean shardSync = true;
int importDepth = 0;

public BazelProjectView build() throws IllegalStateException {
// check mandatory parameters
Expand Down Expand Up @@ -147,7 +148,8 @@ public BazelProjectView build() throws IllegalStateException {
discoverAllExternalAndWorkspaceJars,
new GlobSetMatcher(externalJarsFilters),
shardSync,
targetShardSize);
targetShardSize,
importDepth);
}

public ImportHandle startImporting(Path bazelProjectViewFile) throws IOException {
Expand Down Expand Up @@ -404,6 +406,10 @@ private void parseProjectFile(Path bazelProjectFile, BazelProjectViewBuilder bui
// ignore deprecated
break;
}
case "import_depth": {
builder.importDepth = parseSectionBodyAsInt(rawSection, builder.importDepth);
break;
}
default:
LOG.warn("Unexpected section '{}' while reading '{}'", rawSection.getName(), bazelProjectFile);
break;
Expand Down
3 changes: 2 additions & 1 deletion ...azel.eclipse.core/src/com/salesforce/bazel/eclipse/core/projectview/BazelProjectView.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ public record BazelProjectView(
boolean discoverAllExternalAndWorkspaceJars,
GlobSetMatcher externalJarsDiscoveryFilter,
boolean shardSync,
int targetShardSize) {
int targetShardSize,
int importDepth) {

public BazelProjectView {
directoriesToImport = unmodifiableCollection(directoriesToImport);
Expand Down
Loading