Skip to content

Commit

Permalink
Annotate BEP output files with "prefix" information sufficient to rec…
Browse files Browse the repository at this point in the history
…onstruct the full output path.

This should be backwards-compatible, as we are just adding a new field to the File proto.

RELNOTES: None
PiperOrigin-RevId: 250485470
  • Loading branch information
ericfelly authored and irengrig committed Jun 18, 2019
1 parent b5b5783 commit 0aeecf3
Show file tree
Hide file tree
Showing 7 changed files with 88 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@

package com.google.devtools.build.lib.actions;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Interner;
import com.google.common.collect.Interners;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
Expand All @@ -26,6 +28,7 @@
import com.google.devtools.build.lib.vfs.Root;
import java.io.Serializable;
import java.util.Objects;
import javax.annotation.Nullable;

/**
* A root for an artifact. The roots are the directories containing artifacts, and they are mapped
Expand All @@ -47,7 +50,6 @@
@Immutable
public final class ArtifactRoot implements Comparable<ArtifactRoot>, Serializable, FileRootApi {
private static final Interner<ArtifactRoot> INTERNER = Interners.newWeakInterner();

/**
* Do not use except in tests and in {@link
* com.google.devtools.build.lib.skyframe.SkyframeExecutor}.
Expand All @@ -59,17 +61,35 @@ public static ArtifactRoot asSourceRoot(Root root) {
}

/**
* Returns the given path as a derived root, relative to the given exec root. The root must be a
* proper sub-directory of the exec root (i.e. not equal). Neither may be {@code null}.
* Constructs an ArtifactRoot given the output prefixes. (eg, "bin"), and (eg, "testlogs")
* relative to the execRoot.
*
* <p>Be careful with this method - all derived roots must be registered with the artifact factory
* before the analysis phase.
*/
public static ArtifactRoot asDerivedRoot(Path execRoot, Path root) {
public static ArtifactRoot asDerivedRoot(Path execRoot, PathFragment... prefixes) {
Path root = execRoot;
for (PathFragment prefix : prefixes) {
root = root.getRelative(prefix);
}
Preconditions.checkArgument(root.startsWith(execRoot));
Preconditions.checkArgument(!root.equals(execRoot));
PathFragment execPath = root.relativeTo(execRoot);
return INTERNER.intern(new ArtifactRoot(Root.fromPath(root), execPath, RootType.Output));
return INTERNER.intern(
new ArtifactRoot(
Root.fromPath(root), execPath, RootType.Output, ImmutableList.copyOf(prefixes)));
}

/**
* Returns the given path as a derived root, relative to the given exec root. The root must be a
* proper sub-directory of the exec root (i.e. not equal). Neither may be {@code null}.
*
* <p>Be careful with this method - all derived roots must be registered with the artifact factory
* before the analysis phase.
*/
@VisibleForTesting
public static ArtifactRoot asDerivedRoot(Path execRoot, Path root) {
return asDerivedRoot(execRoot, root.relativeTo(execRoot));
}

public static ArtifactRoot middlemanRoot(Path execRoot, Path outputDir) {
Expand All @@ -82,8 +102,9 @@ public static ArtifactRoot middlemanRoot(Path execRoot, Path outputDir) {

@AutoCodec.VisibleForSerialization
@AutoCodec.Instantiator
static ArtifactRoot createForSerialization(Root root, PathFragment execPath, RootType rootType) {
return INTERNER.intern(new ArtifactRoot(root, execPath, rootType));
static ArtifactRoot createForSerialization(
Root root, PathFragment execPath, RootType rootType, ImmutableList<PathFragment> components) {
return INTERNER.intern(new ArtifactRoot(root, execPath, rootType, components));
}

@AutoCodec.VisibleForSerialization
Expand All @@ -96,11 +117,18 @@ enum RootType {
private final Root root;
private final PathFragment execPath;
private final RootType rootType;
@Nullable private final ImmutableList<PathFragment> components;

private ArtifactRoot(Root root, PathFragment execPath, RootType rootType) {
private ArtifactRoot(
Root root, PathFragment execPath, RootType rootType, ImmutableList<PathFragment> components) {
this.root = Preconditions.checkNotNull(root);
this.execPath = execPath;
this.rootType = rootType;
this.components = components;
}

private ArtifactRoot(Root root, PathFragment execPath, RootType rootType) {
this(root, execPath, rootType, /* components= */ null);
}

public Root getRoot() {
Expand All @@ -120,6 +148,9 @@ public String getExecPathString() {
return getExecPath().getPathString();
}

public ImmutableList<PathFragment> getComponents() {
return components;
}

public boolean isSourceRoot() {
return rootType == RootType.Source;
Expand All @@ -136,7 +167,7 @@ public int compareTo(ArtifactRoot o) {

@Override
public int hashCode() {
return Objects.hash(root, execPath, rootType);
return Objects.hash(root, execPath, rootType, components);
}

@Override
Expand All @@ -148,7 +179,10 @@ public boolean equals(Object o) {
return false;
}
ArtifactRoot r = (ArtifactRoot) o;
return root.equals(r.root) && execPath.equals(r.execPath) && rootType == r.rootType;
return root.equals(r.root)
&& execPath.equals(r.execPath)
&& rootType == r.rootType
&& Objects.equals(components, r.components);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.util.StringCanonicalizer;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.Objects;

/**
Expand Down Expand Up @@ -185,7 +186,8 @@ public Path getEmbeddedBinariesRoot() {
* {@link BlazeDirectories} of this server instance. Nothing else should be placed here.
*/
public ArtifactRoot getBuildDataDirectory(String workspaceName) {
return ArtifactRoot.asDerivedRoot(getExecRoot(workspaceName), getOutputPath(workspaceName));
return ArtifactRoot.asDerivedRoot(
getExecRoot(workspaceName), PathFragment.create(getRelativeOutputPath(productName)));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
import com.google.devtools.build.lib.syntax.Type;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.skyframe.SkyValue;
import java.util.Collection;
import java.util.function.Function;
Expand Down Expand Up @@ -117,6 +118,11 @@ public Artifact getExecutable() {
private final ExecutableTargetData executableTargetData;
private final boolean bepReportOnlyImportantArtifacts;

private static final String DISABLE_PREFIX_NAME =
"com.google.devtools.build.lib.analysis.TargetCompleteEvent.disable_path_prefix";
private static final boolean INCLUDE_PATH_PREFIX =
!"1".equals(System.getProperty(DISABLE_PREFIX_NAME));

private TargetCompleteEvent(
ConfiguredTargetAndData targetAndData,
NestedSet<Cause> rootCauses,
Expand Down Expand Up @@ -305,11 +311,26 @@ private static void addImportantOutputs(
String name = artifactNameFunction.apply(artifact);
String uri = converters.pathConverter().apply(pathResolver.toPath(artifact));
if (uri != null) {
builder.addImportantOutput(File.newBuilder().setName(name).setUri(uri).build());
builder.addImportantOutput(newFileFromArtifact(name, artifact).setUri(uri).build());
}
}
}

public static BuildEventStreamProtos.File.Builder newFileFromArtifact(
String name, Artifact artifact) {
File.Builder builder =
File.newBuilder().setName(name == null ? artifact.getRootRelativePathString() : name);
if (INCLUDE_PATH_PREFIX && artifact.getRoot().getComponents() != null) {
builder.addAllPathPrefix(
Iterables.transform(artifact.getRoot().getComponents(), PathFragment::getPathString));
}
return builder;
}

public static BuildEventStreamProtos.File.Builder newFileFromArtifact(Artifact artifact) {
return newFileFromArtifact(/* name= */ null, artifact);
}

@Override
public Collection<LocalFile> referencedLocalFiles() {
ImmutableList.Builder<LocalFile> builder = ImmutableList.builder();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,13 +104,17 @@ public ArtifactRoot getRoot(
// e.g., execroot/repo1
Path execRoot = directories.getExecRoot(mainRepositoryName.strippedName());
// e.g., execroot/repo1/bazel-out/config/bin
Path outputDir =
execRoot.getRelative(directories.getRelativeOutputPath()).getRelative(outputDirName);
if (middleman) {
Path outputDir =
execRoot.getRelative(directories.getRelativeOutputPath()).getRelative(outputDirName);
return ArtifactRoot.middlemanRoot(execRoot, outputDir);
}
// e.g., [[execroot/repo1]/bazel-out/config/bin]
return ArtifactRoot.asDerivedRoot(execRoot, outputDir.getRelative(nameFragment));
return ArtifactRoot.asDerivedRoot(
execRoot,
PathFragment.create(directories.getRelativeOutputPath()),
PathFragment.create(outputDirName),
nameFragment);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,13 @@ message TargetConfigured {
}

message File {
// A sequence of prefixes to apply to the file name to construct a full path.
// In most but not all cases, there will be 3 entries:
// 1. A root output directory, eg "bazel-out"
// 2. A configuration mnemonic, eg "k8-fastbuild"
// 3. An output category, eg "genfiles"
repeated string path_prefix = 4;

// identifier indicating the nature of the file (e.g., "stdout", "stderr")
string name = 1;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

package com.google.devtools.build.lib.runtime;

import static com.google.devtools.build.lib.analysis.TargetCompleteEvent.newFileFromArtifact;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.Artifact;
Expand Down Expand Up @@ -81,10 +83,9 @@ public BuildEventStreamProtos.BuildEvent asStreamProto(BuildEventContext convert
if (artifact.isMiddlemanArtifact()) {
continue;
}
String name = artifact.getRootRelativePathString();
String uri = pathConverter.apply(pathResolver.toPath(artifact));
if (uri != null) {
builder.addFiles(BuildEventStreamProtos.File.newBuilder().setName(name).setUri(uri));
builder.addFiles(newFileFromArtifact(artifact).setUri(uri));
}
}
for (NestedSetView<Artifact> child : view.transitives()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,9 @@ public void testBadAsDerivedRootSameForBoth() throws IOException {
}

@Test
public void testBadAsDerivedRootNullDir() throws IOException {
public void testBadAsDerivedRootIsExecRoot() throws IOException {
Path execRoot = scratch.dir("/exec");
assertThrows(NullPointerException.class, () -> ArtifactRoot.asDerivedRoot(execRoot, null));
assertThrows(IllegalArgumentException.class, () -> ArtifactRoot.asDerivedRoot(execRoot));
}

@Test
Expand Down

0 comments on commit 0aeecf3

Please sign in to comment.