From acfec2f7fb2dbc7fcb03ce7d0518fa452b55159f Mon Sep 17 00:00:00 2001 From: Gunnar Wagenknecht Date: Thu, 28 Nov 2024 16:29:57 +0100 Subject: [PATCH] Modify contract for more flexibility Instead of using BazelPackage and BazelTarget the TargetDiscoveryStrategy now uses more flexible Bazel IJ classes. This allows an optimization for a future strategy that does ignore the targets (to save one additional query operation). --- .../core/model/SynchronizeProjectViewJob.java | 28 +++++---- .../discovery/BaseProvisioningStrategy.java | 34 +++++++++- .../discovery/BazelQueryTargetDiscovery.java | 63 +++++++++---------- .../discovery/TargetDiscoveryStrategy.java | 10 +-- .../discovery/TargetProvisioningStrategy.java | 7 +-- 5 files changed, 86 insertions(+), 56 deletions(-) diff --git a/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/SynchronizeProjectViewJob.java b/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/SynchronizeProjectViewJob.java index 95665d779..707dbf86b 100644 --- a/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/SynchronizeProjectViewJob.java +++ b/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/SynchronizeProjectViewJob.java @@ -19,6 +19,7 @@ import static org.eclipse.core.resources.IResource.DEPTH_ONE; import static org.eclipse.core.resources.IResource.FORCE; import static org.eclipse.core.resources.IResource.NEVER_DELETE_PROJECT_CONTENT; +import static org.eclipse.core.runtime.IPath.forPosix; import java.io.FileInputStream; import java.io.IOException; @@ -62,6 +63,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.google.idea.blaze.base.model.primitives.Label; import com.google.idea.blaze.base.model.primitives.TargetExpression; import com.google.idea.blaze.base.model.primitives.WorkspacePath; import com.google.idea.blaze.base.model.primitives.WorkspaceRoot; @@ -76,7 +78,6 @@ import com.salesforce.bazel.eclipse.core.util.trace.TraceGraphDumper; import com.salesforce.bazel.eclipse.core.util.trace.TracingSubMonitor; import com.salesforce.bazel.sdk.command.BazelQueryForLabelsCommand; -import com.salesforce.bazel.sdk.model.BazelLabel; import com.salesforce.bazel.sdk.projectview.ImportRoots; /** @@ -270,11 +271,11 @@ public ISchedulingRule detectMissingRule() { return null; } - private Set detectTargetsToMaterializeInEclipse(IProject workspaceProject, TracingSubMonitor monitor, - int work) throws CoreException { + private Set detectTargetsToMaterializeInEclipse(IProject workspaceProject, + TracingSubMonitor monitor, int work) throws CoreException { monitor = monitor.split(work, "Detecting targets"); - Set result = new HashSet<>(); + Set result = new HashSet<>(); if (projectView.deriveTargetsFromDirectories()) { // use strategy configured for workspace @@ -299,7 +300,7 @@ private Set detectTargetsToMaterializeInEclipse(IProject workspaceP // filter packages to remove excludes bazelPackages = bazelPackages.stream().filter(bazelPackage -> { // filter packages based in includes - var directory = bazelPackage.getWorkspaceRelativePath(); + var directory = forPosix(bazelPackage.relativePath()); if (!includeEverything && !findPathOrAnyParentInSet(directory, allowedDirectories)) { return false; } @@ -315,8 +316,8 @@ private Set detectTargetsToMaterializeInEclipse(IProject workspaceP var bazelTargets = targetDiscoveryStrategy.discoverTargets(workspace, bazelPackages, monitor.slice(1)); // add only targets not explicitly excluded - for (BazelTarget t : bazelTargets) { - if (!importRoots.targetInProject(t.getLabel().toPrimitive())) { + for (TargetExpression t : bazelTargets) { + if (t instanceof Label l && !importRoots.targetInProject(l)) { if (LOG.isDebugEnabled()) { LOG.debug("Excluding target '{}' per project view exclusion", t); } @@ -336,8 +337,13 @@ private Set detectTargetsToMaterializeInEclipse(IProject workspaceP "Identifying manual specified targets to synchronize"); Collection labels = workspace.getCommandExecutor().runQueryWithoutLock(queryCommand); for (String label : labels) { - var bazelLabel = new BazelLabel(label.toString()); - result.add(workspace.getBazelTarget(bazelLabel)); + var targetExpression = Label.validate(label) == null ? TargetExpression.fromStringSafe(label) + : TargetExpression.fromStringSafe(label); + if (targetExpression != null) { + result.add(targetExpression); + } + //var bazelLabel = new BazelLabel(label.toString()); + //result.add(workspace.getBazelTarget(bazelLabel)); } } @@ -529,8 +535,8 @@ private void logSyncStats(String workspaceName, Duration duration, int projectsC lines.stream().collect(joining(System.lineSeparator()))); } - private List provisionProjectsForTarget(Set targets, TracingSubMonitor monitor, int work) - throws CoreException { + private List provisionProjectsForTarget(Set targets, TracingSubMonitor monitor, + int work) throws CoreException { return getTargetProvisioningStrategy() .provisionProjectsForSelectedTargets(targets, workspace, monitor.split(work, "Provisioning Projects")); } diff --git a/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/BaseProvisioningStrategy.java b/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/BaseProvisioningStrategy.java index e27619bd4..457533657 100644 --- a/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/BaseProvisioningStrategy.java +++ b/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/BaseProvisioningStrategy.java @@ -48,6 +48,7 @@ import java.util.Map; import java.util.Map.Entry; import java.util.NoSuchElementException; +import java.util.Objects; import java.util.Properties; import java.util.Set; import java.util.function.Predicate; @@ -88,7 +89,9 @@ import org.slf4j.LoggerFactory; import com.google.idea.blaze.base.model.primitives.Label; +import com.google.idea.blaze.base.model.primitives.TargetExpression; import com.google.idea.blaze.base.model.primitives.TargetName; +import com.google.idea.blaze.base.model.primitives.WildcardTargetPattern; import com.google.idea.blaze.base.model.primitives.WorkspacePath; import com.salesforce.bazel.eclipse.core.BazelCoreSharedContstants; import com.salesforce.bazel.eclipse.core.model.BazelElement; @@ -1666,13 +1669,38 @@ protected void linkSourcesIntoProject(BazelProject project, JavaProjectInfo java } @Override - public List provisionProjectsForSelectedTargets(Collection targets, + public List provisionProjectsForSelectedTargets(Collection targetsOrPackages, BazelWorkspace workspace, IProgressMonitor progress) throws CoreException { try { var monitor = TracingSubMonitor.convert(progress, "Provisioning projects", 3); - // load all packages to be provisioned - workspace.open(targets.stream().map(BazelTarget::getBazelPackage).distinct().toList()); + // open all packages at once + monitor.subTask("Loading packages"); + var bazelPackages = targetsOrPackages.parallelStream().map(e -> { + if (e instanceof Label l) { + return new BazelLabel(l.toString()); + } + var w = WildcardTargetPattern.stripWildcardSuffix(e.toString()); + if (w != null) { + return new BazelLabel(w); + } + return null; + }).filter(Predicate.not(Objects::isNull)).map(workspace::getBazelPackage).distinct().toList(); + workspace.open(bazelPackages); + + // collect targets + monitor.subTask("Collecting targets"); + List targets = new ArrayList<>(); + for (TargetExpression targetExpression : targetsOrPackages) { + if (targetExpression instanceof Label l) { + // we don't check for no-ide tag here because we assume this was done already when discovering targets + targets.add(workspace.getBazelTarget(new BazelLabel(l.toString()))); + } else { + LOG.warn( + "Ignoring target expression '{}' for provisioning because this is not supported by the current implementation.", + targetExpression); + } + } // ensure there is a mapper fileSystemMapper = new BazelProjectFileSystemMapper(workspace); diff --git a/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/BazelQueryTargetDiscovery.java b/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/BazelQueryTargetDiscovery.java index f93b4a140..e837570ae 100644 --- a/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/BazelQueryTargetDiscovery.java +++ b/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/BazelQueryTargetDiscovery.java @@ -1,10 +1,13 @@ package com.salesforce.bazel.eclipse.core.model.discovery; import static com.salesforce.bazel.eclipse.core.model.BazelWorkspace.findWorkspaceFile; +import static java.lang.String.format; +import static java.util.function.Predicate.not; +import static java.util.stream.Collectors.joining; import java.util.ArrayList; import java.util.Collection; -import java.util.List; +import java.util.Objects; import org.eclipse.core.runtime.CoreException; import org.eclipse.core.runtime.IPath; @@ -14,9 +17,11 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.salesforce.bazel.eclipse.core.model.BazelPackage; -import com.salesforce.bazel.eclipse.core.model.BazelTarget; +import com.google.idea.blaze.base.model.primitives.Label; +import com.google.idea.blaze.base.model.primitives.TargetExpression; +import com.google.idea.blaze.base.model.primitives.WorkspacePath; import com.salesforce.bazel.eclipse.core.model.BazelWorkspace; +import com.salesforce.bazel.sdk.command.BazelQueryForLabelsCommand; import com.salesforce.bazel.sdk.command.BazelQueryForPackagesCommand; /** @@ -34,7 +39,7 @@ public class BazelQueryTargetDiscovery implements TargetDiscoveryStrategy { private static Logger LOG = LoggerFactory.getLogger(BazelQueryTargetDiscovery.class); @Override - public Collection discoverPackages(BazelWorkspace bazelWorkspace, IProgressMonitor progress) + public Collection discoverPackages(BazelWorkspace bazelWorkspace, IProgressMonitor progress) throws CoreException { var monitor = SubMonitor.convert(progress, 100); @@ -50,7 +55,7 @@ public Collection discoverPackages(BazelWorkspace bazelWorkspace, monitor.worked(1); monitor.setWorkRemaining(labels.size()); - var result = new ArrayList(); + var result = new ArrayList(); for (String label : labels) { monitor.worked(1); @@ -72,41 +77,31 @@ public Collection discoverPackages(BazelWorkspace bazelWorkspace, continue; } - var bazelPackage = bazelWorkspace.getBazelPackage(packagePath); - if (bazelPackage.exists()) { - result.add(bazelPackage); - } - + result.add(new WorkspacePath(packagePath.toString())); } return result; } @Override - public Collection discoverTargets(BazelWorkspace bazelWorkspace, - Collection bazelPackages, IProgressMonitor progress) throws CoreException { + public Collection discoverTargets(BazelWorkspace bazelWorkspace, + Collection bazelPackages, IProgressMonitor progress) throws CoreException { try { - var monitor = SubMonitor.convert(progress, "Querying targets", 1 + bazelPackages.size()); - - // open all packages at once - monitor.subTask("Loading package info"); - bazelWorkspace.open(bazelPackages); - - // collect targets - monitor.subTask("Collecting targets"); - List targets = new ArrayList<>(); - for (BazelPackage bazelPackage : bazelPackages) { - monitor.checkCanceled(); - if (LOG.isDebugEnabled()) { - LOG.debug( - "Discovered targets in package '{}': {}", - bazelPackage.getLabel(), - bazelPackage.getBazelTargets()); - } - bazelPackage.getBazelTargets().stream().filter(BazelTarget::isVisibleToIde).forEach(targets::add); - monitor.worked(1); - } - - return targets; + var monitor = SubMonitor.convert(progress, "Discovering targets", 1 + bazelPackages.size()); + + monitor.subTask("Querying for targets"); + Collection labels = bazelWorkspace.getCommandExecutor() + .runQueryWithoutLock( + new BazelQueryForLabelsCommand( + bazelWorkspace.getLocation().toPath(), + format( + "let all_target = kind(.*rule, %s) in $all_target - attr(tags, 'no-ide', $all_target)", + bazelPackages.stream() + .map(p -> "//" + p.relativePath().toString() + ":all") + .collect(joining(" + "))), + true, + "Querying for targets to synchronize")); + + return labels.parallelStream().map(Label::fromStringSafe).filter(not(Objects::isNull)).toList(); } finally { if (progress != null) { progress.done(); diff --git a/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/TargetDiscoveryStrategy.java b/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/TargetDiscoveryStrategy.java index 5ba2a0dc6..aab0ffae6 100644 --- a/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/TargetDiscoveryStrategy.java +++ b/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/TargetDiscoveryStrategy.java @@ -6,8 +6,9 @@ import org.eclipse.core.runtime.CoreException; import org.eclipse.core.runtime.IProgressMonitor; +import com.google.idea.blaze.base.model.primitives.TargetExpression; +import com.google.idea.blaze.base.model.primitives.WorkspacePath; import com.salesforce.bazel.eclipse.core.model.BazelPackage; -import com.salesforce.bazel.eclipse.core.model.BazelTarget; import com.salesforce.bazel.eclipse.core.model.BazelWorkspace; import com.salesforce.bazel.eclipse.core.model.SynchronizeProjectViewJob; @@ -35,7 +36,8 @@ public interface TargetDiscoveryStrategy { *

* This method is typically called by {@link SynchronizeProjectViewJob} before * {@link #discoverTargets(BazelWorkspace, Collection, IProgressMonitor)} is called. However, the list of packages - * may be further reduced. + * may be further reduced by the caller. Therefore implementors must not expect the return value to be passed to + * {@link #discoverTargets(BazelWorkspace, Collection, IProgressMonitor)}. *

* * @param bazelWorkspace @@ -45,7 +47,7 @@ public interface TargetDiscoveryStrategy { * not need to call {@link IProgressMonitor#done()} * @return the found packages (never null) */ - Collection discoverPackages(BazelWorkspace bazelWorkspace, IProgressMonitor progress) + Collection discoverPackages(BazelWorkspace bazelWorkspace, IProgressMonitor progress) throws CoreException; /** @@ -70,6 +72,6 @@ Collection discoverPackages(BazelWorkspace bazelWorkspace, IProgre * * @return the found targets (never null) */ - Collection discoverTargets(BazelWorkspace bazelWorkspace, Collection bazelPackages, + Collection discoverTargets(BazelWorkspace bazelWorkspace, Collection bazelPackages, IProgressMonitor progress) throws CoreException; } diff --git a/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/TargetProvisioningStrategy.java b/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/TargetProvisioningStrategy.java index 1bf294011..ed7da31ca 100644 --- a/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/TargetProvisioningStrategy.java +++ b/bundles/com.salesforce.bazel.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/TargetProvisioningStrategy.java @@ -11,10 +11,10 @@ import org.eclipse.core.runtime.IStatus; import org.eclipse.jdt.core.IClasspathEntry; +import com.google.idea.blaze.base.model.primitives.TargetExpression; import com.salesforce.bazel.eclipse.core.classpath.BazelClasspathScope; import com.salesforce.bazel.eclipse.core.classpath.CompileAndRuntimeClasspath; import com.salesforce.bazel.eclipse.core.model.BazelProject; -import com.salesforce.bazel.eclipse.core.model.BazelTarget; import com.salesforce.bazel.eclipse.core.model.BazelWorkspace; import com.salesforce.bazel.eclipse.core.model.SynchronizeProjectViewJob; @@ -128,7 +128,6 @@ Map computeClasspaths(Collectionnull) * @return a list of provisioned projects (never null) */ - List provisionProjectsForSelectedTargets(Collection targets, BazelWorkspace workspace, - IProgressMonitor progress) throws CoreException; - + List provisionProjectsForSelectedTargets(Collection targets, + BazelWorkspace workspace, IProgressMonitor progress) throws CoreException; }