Skip to content
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.

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

WIP: Rework OutputArtifactInfo to primarily provide the Path include blaze-out rather than excluding it. #5422

Open
wants to merge 1 commit into
base: google
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,9 @@ private LocalFileOutputArtifactWithoutDigest newLocalOutputArtifact(String path)
String execRoot = workspaceRoot.directory().getAbsolutePath();
String mnemonic = "k8-opt";
return new LocalFileOutputArtifactWithoutDigest(
new File(execRoot + "/blaze-out" + mnemonic + "/" + path), mnemonic + "/" + path, mnemonic);
new File(execRoot + "/blaze-out" + mnemonic + "/" + path),
Path.of("blaze-out" + mnemonic + "/" + path),
mnemonic);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
import java.io.StringWriter;
import java.io.Writer;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.zip.ZipEntry;
import java.util.zip.ZipOutputStream;
import org.jetbrains.annotations.Nullable;
Expand Down Expand Up @@ -185,8 +186,8 @@ public String getConfigurationMnemonic() {
}

@Override
public String getRelativePath() {
return file.getPath();
public Path getPath() {
return file.toPath();
}

@Nullable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package com.google.idea.blaze.base.command.buildresult;

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

/**
* A variant of {@link LocalFileOutputArtifactWithoutDigest} implementing {@link OutputArtifact}.
Expand All @@ -25,8 +26,8 @@ public class LocalFileOutputArtifact extends LocalFileOutputArtifactWithoutDiges
private final String digest;

public LocalFileOutputArtifact(
File file, String blazeOutRelativePath, String configurationMnemonic, String digest) {
super(file, blazeOutRelativePath, configurationMnemonic);
File file, Path blazeOutPath, String configurationMnemonic, String digest) {
super(file, blazeOutPath, configurationMnemonic);
this.digest = digest;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,22 @@
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.nio.file.Path;
import javax.annotation.Nullable;

/** A blaze output artifact which exists on the local file system. */
public class LocalFileOutputArtifactWithoutDigest
implements OutputArtifactWithoutDigest, LocalFileArtifact {

private final File file;
private final String blazeOutRelativePath;
// The path to the artifact starting with e.g. `blaze-out`
private final Path blazeOutPath;
private final String configurationMnemonic;

public LocalFileOutputArtifactWithoutDigest(
File file, String blazeOutRelativePath, String configurationMnemonic) {
File file, Path blazeOutPath, String configurationMnemonic) {
this.file = file;
this.blazeOutRelativePath = blazeOutRelativePath;
this.blazeOutPath = blazeOutPath;
this.configurationMnemonic = configurationMnemonic;
}

Expand All @@ -58,8 +60,8 @@ public String getConfigurationMnemonic() {
}

@Override
public String getRelativePath() {
return blazeOutRelativePath;
public Path getPath() {
return blazeOutPath;
}

@Override
Expand Down Expand Up @@ -91,6 +93,6 @@ public int hashCode() {

@Override
public String toString() {
return blazeOutRelativePath;
return blazeOutPath.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,26 @@
*/
package com.google.idea.blaze.base.command.buildresult;

import com.google.common.base.Joiner;
import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos;
import java.nio.file.Path;

/** A descriptor of an output artifact that contains data needed to identify the artifact. */
public interface OutputArtifactInfo {
/** The blaze-out-relative path. */
String getRelativePath();
default String getRelativePath() {
Path full = getPath();
if (full.getName(0).toString().endsWith("-out")) {
return full.subpath(1, full.getNameCount()).toString();
} else {
return full.toString();
}
}

/** The path to this artifact as extracted from the build event stream. */
Path getPath();

static Path pathFromBesFile(BuildEventStreamProtos.File file) {
return Path.of(Joiner.on('/').join(file.getPathPrefixList())).resolve(file.getName());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,13 @@
*/
package com.google.idea.blaze.base.command.buildresult;

import com.google.common.base.Joiner;
import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos;
import com.intellij.openapi.extensions.ExtensionPointName;
import com.intellij.util.io.URLUtil;
import java.io.File;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.Arrays;
import java.util.List;
import java.util.Objects;
import javax.annotation.Nullable;

Expand Down Expand Up @@ -60,27 +58,10 @@ public OutputArtifact parse(
try {
File f = new File(new URI(uri));
return new LocalFileOutputArtifact(
f,
getBlazeOutRelativePath(file, configurationMnemonic),
configurationMnemonic,
file.getDigest());
f, OutputArtifactInfo.pathFromBesFile(file), configurationMnemonic, file.getDigest());
} catch (URISyntaxException | IllegalArgumentException e) {
return null;
}
}
}

default String getBlazeOutRelativePath(
BuildEventStreamProtos.File file, String configurationMnemonic) {
List<String> pathPrefixList = file.getPathPrefixList();
if (pathPrefixList.size() <= 1) {
// fall back to using the configuration mnemonic
// TODO(brendandouglas): remove this backwards compatibility code after September 2019
return configurationMnemonic + "/" + file.getName();
}

// remove the initial 'bazel-out' path component
String prefix = Joiner.on('/').join(pathPrefixList.subList(1, pathPrefixList.size()));
return prefix + "/" + file.getName();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ private static OutputArtifact translateOutputArtifact(OutputArtifact it) {
}
File srcfsArtifact = new File(hashId.replaceFirst("/google_src", "/google/src"));
return new LocalFileOutputArtifact(
srcfsArtifact, it.getRelativePath(), it.getConfigurationMnemonic(), it.getDigest());
srcfsArtifact, it.getPath(), it.getConfigurationMnemonic(), it.getDigest());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ private static OutputArtifact translateOutputArtifact(OutputArtifact it) {
}
File srcfsArtifact = new File(hashId.replaceFirst("/google_src", "/google/src"));
return new LocalFileOutputArtifact(
srcfsArtifact, it.getRelativePath(), it.getConfigurationMnemonic(), it.getDigest());
srcfsArtifact, it.getPath(), it.getConfigurationMnemonic(), it.getDigest());
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,7 @@ public FileCache(CacheLayout cacheLayout) {
*/
public Optional<Path> getCacheFile(Path artifactPath) {
Path path =
cacheLayout
.getOutputArtifactDestinationAndLayout(artifactPath::toString)
.getCopyDestination();
cacheLayout.getOutputArtifactDestinationAndLayout(() -> artifactPath).getCopyDestination();
return Optional.ofNullable(Files.exists(path) ? path : null);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import com.google.idea.blaze.base.model.RemoteOutputArtifacts;
import com.intellij.openapi.util.io.FileUtil;
import java.io.File;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Objects;

Expand Down Expand Up @@ -107,8 +108,8 @@ private BlazeArtifact outputArtifactFromExecRoot(ArtifactLocation location) {
if (ix2 == -1) {
return new SourceArtifact(decode(location));
}
String blazeOutPath = execRootPath.substring(ix1 + 1);
String configMnemonic = execRootPath.substring(ix1 + 1, ix2);
return new LocalFileOutputArtifactWithoutDigest(decode(location), blazeOutPath, configMnemonic);
return new LocalFileOutputArtifactWithoutDigest(
decode(location), Path.of(execRootPath), configMnemonic);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -226,11 +226,11 @@ public void library_sources() throws Throwable {
.putAll(
OutputGroup.JARS,
TestOutputArtifact.builder()
.setRelativePath("out/test.jar")
.setPath("bazel-out/out/test.jar")
.setDigest("jar_digest")
.build(),
TestOutputArtifact.builder()
.setRelativePath("out/anothertest.jar")
.setPath("bazel-out/out/anothertest.jar")
.setDigest("anotherjar_digest")
.build())
.build())
Expand Down Expand Up @@ -281,7 +281,7 @@ public void library_sources_unbknown_lib() throws Throwable {
.put(
OutputGroup.JARS,
TestOutputArtifact.builder()
.setRelativePath("out/test.jar")
.setPath("bazel-out/out/test.jar")
.setDigest("jar_digest")
.build())
.build())
Expand Down Expand Up @@ -358,7 +358,7 @@ abstract static class TestOutputArtifact implements OutputArtifact {
new AutoValue_ArtifactTrackerImplTest_TestOutputArtifact.Builder()
.setLength(0)
.setDigest("digest")
.setRelativePath("path/file")
.setPath("bazel-out/path/file")
.setConfigurationMnemonic("mnemonic")
.build();

Expand All @@ -378,7 +378,7 @@ public BufferedInputStream getInputStream() throws IOException {
public abstract String getDigest();

@Override
public abstract String getRelativePath();
public abstract Path getPath();

@Override
public abstract String getConfigurationMnemonic();
Expand All @@ -398,7 +398,11 @@ abstract static class Builder {

public abstract Builder setDigest(String value);

public abstract Builder setRelativePath(String value);
public abstract Builder setPath(Path value);

public Builder setPath(String path) {
return setPath(Path.of(path));
}

public abstract Builder setConfigurationMnemonic(String value);

Expand All @@ -408,7 +412,7 @@ abstract static class Builder {

private static OutputArtifact artifactWithNameAndDigest(String fileName, String digest) {
return TestOutputArtifact.builder()
.setRelativePath("somewhere/" + fileName)
.setPath("bazel-out/somewhere/" + fileName)
.setDigest(digest)
.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.google.idea.blaze.base.filecache.ArtifactState;
import java.io.BufferedInputStream;
import java.io.IOException;
import java.nio.file.Path;
import javax.annotation.Nullable;
import org.junit.Rule;
import org.junit.Test;
Expand Down Expand Up @@ -111,8 +112,8 @@ public String getConfigurationMnemonic() {
}

@Override
public String getRelativePath() {
return "somewhere/" + fileName;
public Path getPath() {
return Path.of("somewhere/" + fileName);
}

@Nullable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,6 @@ private CacheDirectories createCacheDirectoryManager() {
}

private static OutputArtifactInfo testOutputArtifact(String fileName) {
return new OutputArtifactInfo() {
@Override
public String getRelativePath() {
return "somewhere/" + fileName;
}
};
return () -> Path.of("bazel-out/somewhere/" + fileName);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.nio.file.Path;
import org.jetbrains.annotations.Nullable;

/** Use local file to fake {@link RemoteOutputArtifact} that used by tests. */
Expand All @@ -48,8 +49,8 @@ public String getConfigurationMnemonic() {
}

@Override
public String getRelativePath() {
return file.getPath();
public Path getPath() {
return file.toPath();
}

@Nullable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.google.idea.blaze.base.ideinfo.ArtifactLocation;
import com.google.idea.blaze.base.sync.FakeRemoteOutputArtifact;
import java.io.File;
import java.nio.file.Path;
import org.jetbrains.annotations.Nullable;

/** Resolves all artifacts to local source files. */
Expand Down Expand Up @@ -58,6 +59,8 @@ public BlazeArtifact resolveOutput(ArtifactLocation artifact) {
return new FakeRemoteOutputArtifact(file);
}
return new LocalFileOutputArtifactWithoutDigest(
decode(artifact), artifact.getRelativePath(), artifact.getExecutionRootRelativePath());
decode(artifact),
Path.of(artifact.getRelativePath()),
artifact.getExecutionRootRelativePath());
}
}