Skip to content

Commit

Permalink
Almost passing tests
Browse files Browse the repository at this point in the history
  • Loading branch information
fmeum committed Aug 16, 2024
1 parent c742dd4 commit 5b1a5d5
Show file tree
Hide file tree
Showing 9 changed files with 116 additions and 49 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,8 @@ public Map<PathFragment, Artifact> getAllSymlinksForLogging(
@Nullable Artifact repoMappingManifest) {
// The log consumer can reconstruct the legacy runfiles if needed.
ManifestBuilder builder = new ManifestBuilder(suffix, /* legacyExternalRunfiles= */ false);
// Avoid creating the empty <workspace>/.runfiles file.
builder.sawWorkspaceName = true;
ConflictChecker checker = ConflictChecker.IGNORE_CHECKER;
builder.addUnderWorkspace(getSymlinksAsMap(checker), checker);
builder.add(getRootSymlinksAsMap(checker), checker);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@ public final class RunfilesSupport {
private static final String OUTPUT_MANIFEST_BASENAME = "MANIFEST";
private static final String REPO_MAPPING_MANIFEST_EXT = ".repo_mapping";

private static class RunfilesTreeImpl implements RunfilesTree {
@VisibleForTesting
public static class RunfilesTreeImpl implements RunfilesTree {

private static final WeakReference<Map<PathFragment, Artifact>> NOT_YET_COMPUTED =
new WeakReference<>(null);
Expand Down Expand Up @@ -128,12 +129,11 @@ private RunfilesTreeImpl(
}

@VisibleForTesting
public RunfilesTreeImpl(
PathFragment execPath, Runfiles runfiles, @Nullable Artifact repoMappingManifest) {
public RunfilesTreeImpl(PathFragment execPath, Runfiles runfiles) {
this(
execPath,
runfiles,
repoMappingManifest,
/* repoMappingManifest= */ null,
/* buildRunfileLinks= */ false,
/* cacheMapping= */ false,
RunfileSymlinksMode.EXTERNAL);
Expand Down
23 changes: 20 additions & 3 deletions src/main/java/com/google/devtools/build/lib/exec/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ java_library(
"SpawnLogContext.java",
],
deps = [
":spawn_log_context_utils",
":stable_sort",
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
"//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
Expand All @@ -298,23 +298,40 @@ java_library(
)

java_library(
name = "spawn_log_context_utils",
name = "spawn_log_reconstructor",
srcs = [
"SpawnLogReconstructor.java",
"StableSort.java",
],
deps = [
":stable_sort",
"//src/main/java/com/google/devtools/build/lib/bazel/rules/python",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/profiler",
"//src/main/java/com/google/devtools/build/lib/util:pair",
"//src/main/java/com/google/devtools/build/lib/util/io:io-proto",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//src/main/protobuf:spawn_java_proto",
"//third_party:guava",
"//third_party:jsr305",
"@zstd-jni",
],
)

java_library(
name = "stable_sort",
srcs = ["StableSort.java"],
deps = [
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/profiler",
"//src/main/java/com/google/devtools/build/lib/util:pair",
"//src/main/java/com/google/devtools/build/lib/util/io:io-proto",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//src/main/protobuf:spawn_java_proto",
"//third_party:guava",
"//third_party:jsr305",
],
)

java_library(
name = "spawn_runner",
srcs = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ private static void addMapping(
}

@VisibleForTesting
void addSingleRunfilesTreeToInputs(
public void addSingleRunfilesTreeToInputs(
RunfilesTree runfilesTree,
Map<PathFragment, ActionInput> inputMap,
ArtifactExpander artifactExpander,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,19 @@
package com.google.devtools.build.lib.exec;

import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableSet.toImmutableSet;

import com.github.luben.zstd.ZstdInputStream;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.bazel.rules.python.BazelPyBuiltins;
import com.google.devtools.build.lib.cmdline.LabelConstants;
import com.google.devtools.build.lib.exec.Protos.ExecLogEntry;
import com.google.devtools.build.lib.exec.Protos.File;
import com.google.devtools.build.lib.exec.Protos.SpawnExec;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.util.io.MessageInputStream;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.io.IOException;
import java.io.InputStream;
import java.util.ArrayDeque;
Expand All @@ -32,6 +35,7 @@
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Set;
import java.util.SortedMap;
import java.util.SortedSet;
import java.util.TreeMap;
Expand Down Expand Up @@ -232,7 +236,8 @@ private Pair<String, Collection<File>> reconstructRunfilesDir(
builder.put(newPath, file);
}
}
reconstructInputs(runfilesTree.getArtifactsId()).values().stream()
Collection<File> inputs = reconstructInputs(runfilesTree.getArtifactsId()).values();
inputs.stream()
.flatMap(
file ->
getRunfilesPaths(file.getPath())
Expand All @@ -242,6 +247,23 @@ private Pair<String, Collection<File>> reconstructRunfilesDir(
.setPath(runfilesTree.getPath() + "/" + relativePath)
.build()))
.forEach(file -> builder.put(file.getPath(), file));
if (runfilesTree.getCreateInitPy()) {
Set<PathFragment> manifest =
Stream.concat(
runfilesTree.getSymlinksMap().keySet().stream(),
inputs.stream().flatMap(file -> getRunfilesPaths(file.getPath())))
.map(PathFragment::create)
.collect(toImmutableSet());
for (PathFragment emptyFilePath : BazelPyBuiltins.GET_INIT_PY_FILES.getExtraPaths(manifest)) {
String path = runfilesTree.getPath() + "/" + emptyFilePath.getPathString();
builder.put(path, File.newBuilder().setPath(path).build());
}
}
String workspaceDir = runfilesTree.getPath() + "/" + workspaceRunfilesDirectory + "/";
if (builder.keySet().stream().noneMatch(p -> p.startsWith(workspaceDir))) {
String emptyRunfile = workspaceDir + ".runfile";
builder.put(emptyRunfile, File.newBuilder().setPath(emptyRunfile).build());
}
return Pair.of(runfilesTree.getPath(), ImmutableList.copyOf(builder.values()));
}

Expand Down
17 changes: 9 additions & 8 deletions src/main/protobuf/spawn.proto
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,14 @@ message ExecLogEntry {
}

message RunfilesTree {
message SymlinkTarget {
oneof target {
int32 file_id = 1;
int32 directory_id = 2;
int32 unresolved_symlink_id = 3;
}
}

// The runfiles tree path.
string path = 1;
// The entry ID of the set of artifacts in the runfiles tree that are
Expand All @@ -279,14 +287,7 @@ message ExecLogEntry {
// The artifacts in the runfiles tree that are symlinked at custom
// locations.
map<string, SymlinkTarget> symlinks = 3;

message SymlinkTarget {
oneof target {
int32 file_id = 1;
int32 directory_id = 2;
int32 unresolved_symlink_id = 3;
}
}
bool create_init_py = 4;
}

// A set of spawn inputs.
Expand Down
4 changes: 3 additions & 1 deletion src/test/java/com/google/devtools/build/lib/exec/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
"//src/main/java/com/google/devtools/build/lib/analysis:config/build_configuration",
"//src/main/java/com/google/devtools/build/lib/analysis:configured_target",
"//src/main/java/com/google/devtools/build/lib/bazel/rules/python",
"//src/main/java/com/google/devtools/build/lib/buildeventstream/proto:build_event_stream_java_proto",
"//src/main/java/com/google/devtools/build/lib/clock",
"//src/main/java/com/google/devtools/build/lib/collect/nestedset",
Expand All @@ -51,10 +52,11 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/exec:spawn_exec_exception",
"//src/main/java/com/google/devtools/build/lib/exec:spawn_input_expander",
"//src/main/java/com/google/devtools/build/lib/exec:spawn_log_context",
"//src/main/java/com/google/devtools/build/lib/exec:spawn_log_context_utils",
"//src/main/java/com/google/devtools/build/lib/exec:spawn_log_reconstructor",
"//src/main/java/com/google/devtools/build/lib/exec:spawn_runner",
"//src/main/java/com/google/devtools/build/lib/exec:spawn_strategy_policy",
"//src/main/java/com/google/devtools/build/lib/exec:spawn_strategy_registry",
"//src/main/java/com/google/devtools/build/lib/exec:stable_sort",
"//src/main/java/com/google/devtools/build/lib/exec:standalone_test_result",
"//src/main/java/com/google/devtools/build/lib/exec:standalone_test_strategy",
"//src/main/java/com/google/devtools/build/lib/exec:streamed_test_output",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,12 @@
import static com.google.common.base.Preconditions.checkState;
import static com.google.devtools.build.lib.exec.SpawnLogContext.millisToProto;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import com.google.common.base.Utf8;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSortedMap;
import com.google.common.collect.ImmutableSortedSet;
import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
Expand All @@ -34,6 +33,7 @@
import com.google.devtools.build.lib.actions.FileArtifactValue;
import com.google.devtools.build.lib.actions.InputMetadataProvider;
import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType;
import com.google.devtools.build.lib.actions.PathMapper;
import com.google.devtools.build.lib.actions.RunfilesTree;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnMetrics;
Expand All @@ -42,8 +42,9 @@
import com.google.devtools.build.lib.actions.StaticInputMetadataProvider;
import com.google.devtools.build.lib.actions.cache.VirtualActionInput;
import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.Order;
import com.google.devtools.build.lib.analysis.Runfiles;
import com.google.devtools.build.lib.analysis.RunfilesSupport;
import com.google.devtools.build.lib.bazel.rules.python.BazelPyBuiltins;
import com.google.devtools.build.lib.exec.Protos.Digest;
import com.google.devtools.build.lib.exec.Protos.EnvironmentVariable;
import com.google.devtools.build.lib.exec.Protos.File;
Expand All @@ -69,11 +70,11 @@
import java.io.IOException;
import java.time.Duration;
import java.time.Instant;
import java.util.Arrays;
import java.util.Date;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.SortedMap;
import java.util.TreeMap;
import org.junit.Test;

/** Base class for {@link SpawnLogContext} tests. */
Expand Down Expand Up @@ -342,7 +343,7 @@ public void testRunfilesFileInput() throws Exception {
writeFile(runfilesInput, "abc");

PathFragment runfilesRoot = outputDir.getExecPath().getRelative("foo.runfiles");
RunfilesTree runfilesTree = createRunfilesTree(runfilesRoot, ImmutableMap.of(), runfilesInput);
RunfilesTree runfilesTree = createRunfilesTree(runfilesRoot, runfilesInput);

Spawn spawn = defaultSpawnBuilder().withInput(runfilesMiddleman).build();

Expand Down Expand Up @@ -381,7 +382,7 @@ public void testRunfilesDirectoryInput(@TestParameter DirContents dirContents) t
}

PathFragment runfilesRoot = outputDir.getExecPath().getRelative("foo.runfiles");
RunfilesTree runfilesTree = createRunfilesTree(runfilesRoot, ImmutableMap.of(), runfilesInput);
RunfilesTree runfilesTree = createRunfilesTree(runfilesRoot, runfilesInput);

Spawn spawn = defaultSpawnBuilder().withInput(runfilesMiddleman).build();

Expand Down Expand Up @@ -416,10 +417,17 @@ public void testRunfilesDirectoryInput(@TestParameter DirContents dirContents) t
@Test
public void testRunfilesEmptyInput() throws Exception {
Artifact runfilesMiddleman = ActionsTestUtil.createArtifact(middlemanDir, "runfiles");
Artifact runfilesInput = ActionsTestUtil.createArtifact(rootDir, "sub/dir/script.py");
PathFragment runfilesRoot = outputDir.getExecPath().getRelative("foo.runfiles");
HashMap<String, Artifact> mapping = new HashMap<>();
mapping.put("__init__.py", null);
RunfilesTree runfilesTree = createRunfilesTree(runfilesRoot, mapping);
RunfilesTree runfilesTree =
createRunfilesTree(
runfilesRoot,
ImmutableMap.of(),
ImmutableMap.of(),
/* createEmptyFiles= */ true,
runfilesInput);

writeFile(runfilesInput, "abc");

Spawn spawn = defaultSpawnBuilder().withInput(runfilesMiddleman).build();

Expand All @@ -439,7 +447,12 @@ public void testRunfilesEmptyInput() throws Exception {
closeAndAssertLog(
context,
defaultSpawnExecBuilder()
.addInputs(File.newBuilder().setPath("out/foo.runfiles/__init__.py"))
.addInputs(File.newBuilder().setPath("out/foo.runfiles/_main/sub/__init__.py"))
.addInputs(File.newBuilder().setPath("out/foo.runfiles/_main/sub/dir/__init__.py"))
.addInputs(
File.newBuilder()
.setPath("out/foo.runfiles/_main/sub/dir/script.py")
.setDigest(getDigest("abc")))
.build());
}

Expand Down Expand Up @@ -990,25 +1003,29 @@ protected static SpawnExec.Builder defaultSpawnExecBuilder() {
.setMetrics(Protos.SpawnMetrics.getDefaultInstance());
}

protected static RunfilesTree createRunfilesTree(PathFragment root, Artifact... artifacts) {
return createRunfilesTree(root, ImmutableMap.of(), ImmutableMap.of(), false, artifacts);
}

protected static RunfilesTree createRunfilesTree(
PathFragment root, Map<String, Artifact> symlinks, Artifact... artifacts) {
LinkedHashMap<PathFragment, Artifact> symlinksByPath = new LinkedHashMap<>();
PathFragment root,
Map<String, Artifact> symlinks,
Map<String, Artifact> rootSymlinks,
boolean createEmptyFiles,
Artifact... artifacts) {
Runfiles.Builder runfiles =
new Runfiles.Builder(TestConstants.WORKSPACE_NAME, /* legacyExternalRunfiles= */ true);
runfiles.addArtifacts(Arrays.asList(artifacts));
for (Map.Entry<String, Artifact> entry : symlinks.entrySet()) {
symlinksByPath.put(PathFragment.create(entry.getKey()), entry.getValue());
runfiles.addSymlink(PathFragment.create(entry.getKey()), entry.getValue());
}
LinkedHashMap<PathFragment, Artifact> mapping = new LinkedHashMap<>(symlinksByPath);
PathFragment workspaceRoot = PathFragment.create(TestConstants.WORKSPACE_NAME);
for (Artifact artifact : artifacts) {
mapping.put(workspaceRoot.getRelative(artifact.getRunfilesPath()), artifact);
for (Map.Entry<String, Artifact> entry : rootSymlinks.entrySet()) {
runfiles.addRootSymlink(PathFragment.create(entry.getKey()), entry.getValue());
}
if (createEmptyFiles) {
runfiles.setEmptyFilesSupplier(BazelPyBuiltins.GET_INIT_PY_FILES);
}
RunfilesTree runfilesTree = mock(RunfilesTree.class);
when(runfilesTree.getWorkspaceName()).thenReturn(TestConstants.WORKSPACE_NAME);
when(runfilesTree.getExecPath()).thenReturn(root);
when(runfilesTree.getMapping()).thenReturn(mapping);
when(runfilesTree.getAllSymlinksForLogging()).thenReturn(symlinksByPath);
when(runfilesTree.getArtifactsAtCanonicalLocationsForLogging())
.thenReturn(NestedSetBuilder.wrap(Order.STABLE_ORDER, ImmutableList.copyOf(artifacts)));
return runfilesTree;
return new RunfilesSupport.RunfilesTreeImpl(root, runfiles.build());
}

protected static InputMetadataProvider createInputMetadataProvider(Artifact... artifacts)
Expand Down Expand Up @@ -1039,10 +1056,16 @@ protected static SortedMap<PathFragment, ActionInput> createInputMap(ActionInput

protected static SortedMap<PathFragment, ActionInput> createInputMap(
RunfilesTree runfilesTree, ActionInput... actionInputs) throws Exception {
ImmutableSortedMap.Builder<PathFragment, ActionInput> builder =
ImmutableSortedMap.naturalOrder();
TreeMap<PathFragment, ActionInput> builder = new TreeMap<>();

if (runfilesTree != null) {
new SpawnInputExpander(/* execRoot= */ null)
.addSingleRunfilesTreeToInputs(
runfilesTree,
builder,
treeArtifact -> ImmutableSortedSet.of(),
PathMapper.NOOP,
PathFragment.EMPTY_FRAGMENT);
// Emulate SpawnInputExpander: expand runfiles, replacing nulls with empty inputs.
PathFragment root = runfilesTree.getExecPath();
for (Map.Entry<PathFragment, Artifact> entry : runfilesTree.getMapping().entrySet()) {
Expand All @@ -1067,7 +1090,7 @@ protected static SortedMap<PathFragment, ActionInput> createInputMap(
builder.put(actionInput.getExecPath(), actionInput);
}
}
return builder.buildOrThrow();
return ImmutableSortedMap.copyOf(builder);
}

protected static TreeArtifactValue createTreeArtifactValue(Artifact tree) throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ java_library(
"ExecLogConverter.java",
],
deps = [
"//src/main/java/com/google/devtools/build/lib/exec:spawn_log_context_utils",
"//src/main/java/com/google/devtools/build/lib/exec:spawn_log_reconstructor",
"//src/main/java/com/google/devtools/build/lib/util/io:io-proto",
"//src/main/java/com/google/devtools/common/options",
"//src/main/protobuf:spawn_java_proto",
Expand Down

0 comments on commit 5b1a5d5

Please sign in to comment.