Skip to content

Commit

Permalink
Close gaps when aspects failed to gather library details
Browse files Browse the repository at this point in the history 
This is a bit more tricky. I noticed ijars on the classpath when I
didn't expect them to be there. This is because we try to eliminate them
as much as possible. They don't work in IDEs nicely. Turns out that with
a little bit of code we are able to turn missing libraries into more
complete objects. These objects (especially the target label) allow a
proper translation later to source project references instead of jars
where applicable.
  • Loading branch information
@guw
guw committed Nov 27, 2024
1 parent 4d04548 commit facf206
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 30 deletions.
53 changes: 41 additions & 12 deletions ....core/src/com/salesforce/bazel/eclipse/core/model/discovery/JavaAspectsClasspathInfo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,30 +89,47 @@ static record JdepsDependency(
private static Logger LOG = LoggerFactory.getLogger(JavaAspectsClasspathInfo.class);

/**
* Uses a filename heuristic to guess the location of a source jar corresponding to the given output jar.
* Uses a filename heuristic to guess the location of a class jar corresponding to the given output jar.
*/
private static ArtifactLocation guessSrcJarLocation(ArtifactLocation outputJar) {
// copied from BlazeJavaWorkspaceImporter
var srcJarRelPath = guessSrcJarRelativePath(outputJar.getRelativePath());
if (srcJarRelPath == null) {
private static ArtifactLocation guessClassJarLocation(ArtifactLocation outputJar) {
// copied and adapted from BlazeJavaWorkspaceImporter
var classJarRelPath = guessJarSiblingPathWithSuffix(outputJar.getRelativePath(), ".jar");
if (classJarRelPath == null) {
return null;
}
// we don't check whether the source jar actually exists, to avoid unnecessary file system
// we don't check whether the class jar actually exists, to avoid unnecessary file system
// operations
return ArtifactLocation.Builder.copy(outputJar).setRelativePath(srcJarRelPath).build();
return ArtifactLocation.Builder.copy(outputJar).setRelativePath(classJarRelPath).build();
}

private static String guessSrcJarRelativePath(String relPath) {
// copied from BlazeJavaWorkspaceImporter
private static String guessJarSiblingPathWithSuffix(String relPath, String suffix) {
// copied and adapted from BlazeJavaWorkspaceImporter
if (relPath.endsWith("-hjar.jar")) {
return relPath.substring(0, relPath.length() - "-hjar.jar".length()) + "-src.jar";
return relPath.substring(0, relPath.length() - "-hjar.jar".length()) + suffix;
}
if (relPath.endsWith("-ijar.jar")) {
return relPath.substring(0, relPath.length() - "-ijar.jar".length()) + "-src.jar";
return relPath.substring(0, relPath.length() - "-ijar.jar".length()) + suffix;
}
if (relPath.endsWith(".jar")) {
return relPath.substring(0, relPath.length() - ".jar".length()) + suffix;
}
return null;
}

/**
* Uses a filename heuristic to guess the location of a source jar corresponding to the given output jar.
*/
private static ArtifactLocation guessSrcJarLocation(ArtifactLocation outputJar) {
// copied and adapted from BlazeJavaWorkspaceImporter
var srcJarRelPath = guessJarSiblingPathWithSuffix(outputJar.getRelativePath(), "-src.jar");
if (srcJarRelPath == null) {
return null;
}
// we don't check whether the source jar actually exists, to avoid unnecessary file system
// operations
return ArtifactLocation.Builder.copy(outputJar).setRelativePath(srcJarRelPath).build();
}

static boolean isJavaProtoTarget(TargetIdeInfo target) {
return (target.getJavaIdeInfo() != null)
&& (JavaBlazeRules.getJavaProtoLibraryKinds().contains(target.getKind())
Expand Down Expand Up @@ -326,9 +343,21 @@ public CompileAndRuntimeClasspath compute() throws CoreException {
// It's in the target's jdeps, but our aspect never attached to the target building it.
// Perhaps it's an implicit dependency, or not referenced in an attribute we propagate
// along. Make a best-effort attempt to add it to the project anyway.
var jarPath = decodeArtifactLocation(artifact);
var targetLabel = readTargetLabel(jarPath.toPath());
if (targetLabel == null) {
LOG.error(
"Unable to resolve compile jar '{}' from jdeps info of '{}'. Not a proper Bazel jar without a target label!",
artifact,
bazelProject);
// still continue to have it on the classpath
}
var classJar = guessClassJarLocation(artifact);
var srcJar = guessSrcJarLocation(artifact);
var srcJars = srcJar != null ? List.of(srcJar) : List.<ArtifactLocation> of();
library = new BlazeJarLibrary(new LibraryArtifact(artifact, null, srcJars), /* targetKey= */ null);
var libraryArtifact = new LibraryArtifact(artifact, classJar, srcJars);
var targetKey = targetLabel != null ? TargetKey.forPlainTarget(targetLabel) : null;
library = new BlazeJarLibrary(libraryArtifact, targetKey);
}
var entry = resolveLibrary(library);
if (entry != null) {
Expand Down
11 changes: 0 additions & 11 deletions ...l.eclipse.core/src/com/salesforce/bazel/eclipse/core/model/discovery/JavaAspectsInfo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import static org.eclipse.core.runtime.IPath.fromPath;

import java.io.IOException;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
Expand Down Expand Up @@ -52,7 +51,6 @@
import com.google.idea.blaze.java.sync.importer.ExecutionPathHelper;
import com.google.idea.blaze.java.sync.model.BlazeJarLibrary;
import com.salesforce.bazel.eclipse.core.model.BazelWorkspace;
import com.salesforce.bazel.eclipse.core.util.jar.BazelJarFile;
import com.salesforce.bazel.eclipse.core.util.jar.SourceJarFinder;
import com.salesforce.bazel.sdk.aspects.intellij.IntellijAspects;
import com.salesforce.bazel.sdk.aspects.intellij.IntellijAspects.OutputGroup;
Expand Down Expand Up @@ -241,15 +239,6 @@ public List<BlazeJarLibrary> getRuntimeClasspath(TargetKey targetKey) {
return null;
}

private Label readTargetLabel(Path jarPath) {
try (var jarFile = new BazelJarFile(jarPath)) {
return jarFile.getTargetLabel();
} catch (IOException e) {
LOG.warn("Error inspecting manifest of jar '{}': {}", jarPath, e.getMessage(), e);
}
return null;
}

private ArtifactLocation toArtifactLocation(LocalFileArtifact localJar) {
// check for SourceArtifact and treat specal
if (localJar instanceof SourceArtifact sourceArtifact) {
Expand Down
53 changes: 46 additions & 7 deletions ...c/com/salesforce/bazel/eclipse/core/model/discovery/JavaClasspathJarLocationResolver.java
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
package com.salesforce.bazel.eclipse.core.model.discovery;

import static java.lang.String.format;
import static org.eclipse.core.runtime.IPath.forPosix;

import java.io.IOException;
import java.nio.file.Path;

import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IPath;
import org.eclipse.core.runtime.Path;
import org.eclipse.jdt.core.IClasspathEntry;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -14,6 +17,7 @@
import com.google.idea.blaze.base.command.info.BlazeInfo;
import com.google.idea.blaze.base.ideinfo.ArtifactLocation;
import com.google.idea.blaze.base.ideinfo.LibraryArtifact;
import com.google.idea.blaze.base.model.primitives.Label;
import com.google.idea.blaze.base.model.primitives.WorkspaceRoot;
import com.google.idea.blaze.base.sync.workspace.ArtifactLocationDecoder;
import com.google.idea.blaze.base.sync.workspace.ArtifactLocationDecoderImpl;
Expand All @@ -23,6 +27,7 @@
import com.salesforce.bazel.eclipse.core.model.BazelWorkspace;
import com.salesforce.bazel.eclipse.core.model.BazelWorkspaceBlazeInfo;
import com.salesforce.bazel.eclipse.core.model.discovery.classpath.ClasspathEntry;
import com.salesforce.bazel.eclipse.core.util.jar.BazelJarFile;

/**
* A utility for resolving jar information from Bazel for Eclipse classpath computation.
Expand All @@ -44,6 +49,31 @@ public JavaClasspathJarLocationResolver(BazelWorkspace bazelWorkspace) throws Co
locationDecoder = new ArtifactLocationDecoderImpl(blazeInfo, new WorkspacePathResolverImpl(workspaceRoot));
}

/**
* Convenience method to decode an {@link ArtifactLocation} using the {@link #getLocationDecoder()}.
*
* @param artifactLocation
* the {@link ArtifactLocation} (must not be <code>null</code>)
* @return the absolute location on the file system (never <code>null</code>)
* @throws IllegalStateException
* if the artifact location cannot be decoded
*/
protected IPath decodeArtifactLocation(ArtifactLocation artifactLocation) {
if (artifactLocation.isMainWorkspaceSourceArtifact()) {
return forPosix(locationDecoder.resolveSource(artifactLocation).toString());
}

var blazeArtifact = locationDecoder.resolveOutput(artifactLocation);
if (blazeArtifact instanceof LocalFileArtifact localArtifact) {
return forPosix(localArtifact.getPath().toString());
}

throw new IllegalStateException(
format(
"Unable to decode artifact location '%s'. It's is neither a source nor a local output artifact.",
artifactLocation));
}

public ArtifactLocation generatedJarLocation(BazelPackage bazelPackage, IPath jar) {
// the jar is expected to be generated inside the given package
var executionRootRelativePath = bazelPackage.getWorkspaceRelativePath().append(jar).toString();
Expand Down Expand Up @@ -75,6 +105,15 @@ public WorkspaceRoot getWorkspaceRoot() {
return workspaceRoot;
}

protected Label readTargetLabel(Path jarPath) {
try (var jarFile = new BazelJarFile(jarPath)) {
return jarFile.getTargetLabel();
} catch (IOException e) {
LOG.warn("Error inspecting manifest of jar '{}': {}", jarPath, e.getMessage(), e);
}
return null;
}

/**
* Resolves a {@link LibraryArtifact jar} into a {@link ClasspathEntry}.
* <p>
Expand All @@ -90,31 +129,31 @@ public ClasspathEntry resolveJar(LibraryArtifact jar) {
// prefer the class jar because this is much better in Eclipse when debugging/stepping through code/code navigation/etc.
var jarArtifactForIde = jar.getClassJar() != null ? jar.getClassJar() : jar.jarForIntellijLibrary();
if (jarArtifactForIde.isMainWorkspaceSourceArtifact()) {
IPath jarPath = new Path(locationDecoder.resolveSource(jarArtifactForIde).toString());
var jarPath = forPosix(locationDecoder.resolveSource(jarArtifactForIde).toString());
var sourceJar = jar.getSourceJars().stream().findFirst();
if (!sourceJar.isPresent()) {
if (LOG.isDebugEnabled()) {
LOG.debug(
"Found jar for '{}': {} without source",
new Path(jarArtifactForIde.getExecutionRootRelativePath()).lastSegment(),
forPosix(jarArtifactForIde.getExecutionRootRelativePath()).lastSegment(),
jarPath);
}
return ClasspathEntry.newLibraryEntry(jarPath, null, null, false /* test only */);
}

IPath srcJarPath = new Path(locationDecoder.resolveSource(sourceJar.get()).toString());
var srcJarPath = forPosix(locationDecoder.resolveSource(sourceJar.get()).toString());
if (LOG.isDebugEnabled()) {
LOG.debug(
"Found jar for '{}': {} (source {})",
new Path(jarArtifactForIde.getExecutionRootRelativePath()).lastSegment(),
forPosix(jarArtifactForIde.getExecutionRootRelativePath()).lastSegment(),
jarPath,
srcJarPath);
}
return ClasspathEntry.newLibraryEntry(jarPath, srcJarPath, null, false /* test only */);
}
var jarArtifact = locationDecoder.resolveOutput(jarArtifactForIde);
if (jarArtifact instanceof LocalFileArtifact localJar) {
IPath jarPath = new Path(localJar.getPath().toString());
var jarPath = forPosix(localJar.getPath().toString());
var sourceJar = jar.getSourceJars().stream().findFirst();
if (!sourceJar.isPresent()) {
if (LOG.isDebugEnabled()) {
Expand All @@ -124,7 +163,7 @@ public ClasspathEntry resolveJar(LibraryArtifact jar) {
}
var srcJarArtifact = locationDecoder.resolveOutput(sourceJar.get());
if (srcJarArtifact instanceof LocalFileArtifact localSrcJar) {
IPath srcJarPath = new Path(localSrcJar.getPath().toString());
var srcJarPath = forPosix(localSrcJar.getPath().toString());
if (LOG.isDebugEnabled()) {
LOG.debug(
"Found jar for '{}': {} (source {})",
Expand Down

0 comments on commit facf206

Please sign in to comment.