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

Write a repo mapping manifest in the runfiles directory #16321

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 20 additions & 0 deletions src/main/java/com/google/devtools/build/lib/analysis/BUILD
Expand Up @@ -336,6 +336,7 @@ java_library(
":package_specification_provider",
":platform_options",
":provider_collection",
":repo_mapping_manifest_action",
":required_config_fragments_provider",
":resolved_toolchain_context",
":rule_configured_object_value",
Expand Down Expand Up @@ -983,6 +984,25 @@ java_library(
],
)

java_library(
name = "repo_mapping_manifest_action",
srcs = ["RepoMappingManifestAction.java"],
deps = [
":actions/abstract_file_write_action",
":actions/deterministic_writer",
"//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:commandline_item",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/collect/nestedset",
"//src/main/java/com/google/devtools/build/lib/util",
"//src/main/java/net/starlark/java/eval",
"//third_party:auto_value",
"//third_party:guava",
"//third_party:jsr305",
],
)

java_library(
name = "required_config_fragments_provider",
srcs = ["RequiredConfigFragmentsProvider.java"],
Expand Down
@@ -0,0 +1,130 @@
// Copyright 2022 The Bazel Authors. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.devtools.build.lib.analysis;

import static java.nio.charset.StandardCharsets.ISO_8859_1;
import static java.util.Comparator.comparing;

import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.ActionOwner;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
import com.google.devtools.build.lib.actions.CommandLineExpansionException;
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.analysis.actions.AbstractFileWriteAction;
import com.google.devtools.build.lib.analysis.actions.DeterministicWriter;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.Order;
import com.google.devtools.build.lib.util.Fingerprint;
import java.io.BufferedWriter;
import java.io.OutputStreamWriter;
import java.io.Writer;
import java.util.List;
import java.util.UUID;
import javax.annotation.Nullable;
import net.starlark.java.eval.EvalException;

/** Creates a manifest file describing the repos and mappings relevant for a runfile tree. */
public class RepoMappingManifestAction extends AbstractFileWriteAction {
Wyverald marked this conversation as resolved.
Show resolved Hide resolved
private static final UUID MY_UUID = UUID.fromString("458e351c-4d30-433d-b927-da6cddd4737f");

private final ImmutableList<Entry> entries;
private final String workspaceName;

/** An entry in the repo mapping manifest file. */
@AutoValue
public abstract static class Entry {
public static Entry of(
RepositoryName sourceRepo, String targetRepoApparentName, RepositoryName targetRepo) {
Wyverald marked this conversation as resolved.
Show resolved Hide resolved
return new AutoValue_RepoMappingManifestAction_Entry(
sourceRepo, targetRepoApparentName, targetRepo);
}

public abstract RepositoryName sourceRepo();

public abstract String targetRepoApparentName();

public abstract RepositoryName targetRepo();
}

public RepoMappingManifestAction(
ActionOwner owner, Artifact output, List<Entry> entries, String workspaceName) {
super(owner, NestedSetBuilder.emptySet(Order.STABLE_ORDER), output, /*makeExecutable=*/ false);
this.entries =
ImmutableList.sortedCopyOf(
comparing((Entry e) -> e.sourceRepo().getName())
.thenComparing(Entry::targetRepoApparentName)
.thenComparing(e -> e.targetRepo().getName()),
entries);
this.workspaceName = workspaceName;
}

@Override
public String getMnemonic() {
return "RepoMappingManifest";
}

@Override
protected String getRawProgressMessage() {
return "writing repo mapping manifest for " + getOwner().getLabel();
}

@Override
protected void computeKey(
ActionKeyContext actionKeyContext,
@Nullable ArtifactExpander artifactExpander,
Fingerprint fp)
throws CommandLineExpansionException, EvalException, InterruptedException {
fp.addUUID(MY_UUID);
fp.addString(workspaceName);
for (Entry entry : entries) {
fp.addString(entry.sourceRepo().getName());
fp.addString(entry.targetRepoApparentName());
fp.addString(entry.targetRepo().getName());
}
}

@Override
public DeterministicWriter newDeterministicWriter(ActionExecutionContext ctx)
throws InterruptedException, ExecException {
return out -> {
Writer writer = new BufferedWriter(new OutputStreamWriter(out, ISO_8859_1));
for (Entry entry : entries) {
if (entry.targetRepoApparentName().isEmpty()) {
// The apparent repo name can only be empty for the main repo. We skip this line as
// Rlocation paths can't reference an empty apparent name anyway.
continue;
}
writer.write(entry.sourceRepo().getName());
writer.write(',');
writer.write(entry.targetRepoApparentName());
writer.write(',');
if (entry.targetRepo().isMain()) {
// The canonical name of the main repo is the empty string, which is not a valid name for
// a directory, so the "workspace name" is used the name of the directory under the
// runfiles tree for it.
writer.write(workspaceName);
} else {
writer.write(entry.targetRepo().getName());
}
writer.write('\n');
}
writer.flush();
};
}
}
Expand Up @@ -14,26 +14,33 @@

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

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

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.ActionEnvironment;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.CommandLine;
import com.google.devtools.build.lib.analysis.RepoMappingManifestAction.Entry;
import com.google.devtools.build.lib.analysis.SourceManifestAction.ManifestType;
import com.google.devtools.build.lib.analysis.actions.ActionConstructionContext;
import com.google.devtools.build.lib.analysis.actions.SymlinkTreeAction;
import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue;
import com.google.devtools.build.lib.analysis.config.RunUnder;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.TargetUtils;
import com.google.devtools.build.lib.packages.Type;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.Collection;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -76,11 +83,13 @@ public final class RunfilesSupport {
private static final String RUNFILES_DIR_EXT = ".runfiles";
private static final String INPUT_MANIFEST_EXT = ".runfiles_manifest";
private static final String OUTPUT_MANIFEST_BASENAME = "MANIFEST";
private static final String REPO_MAPPING_MANIFEST_EXT = ".repo_mapping";

private final Runfiles runfiles;

private final Artifact runfilesInputManifest;
private final Artifact runfilesManifest;
private final Artifact repoMappingManifest;
private final Artifact runfilesMiddleman;
private final Artifact owningExecutable;
private final boolean buildRunfileLinks;
Expand Down Expand Up @@ -132,15 +141,19 @@ private static RunfilesSupport create(
runfilesInputManifest = null;
runfilesManifest = null;
}
Artifact repoMappingManifest =
createRepoMappingManifestAction(ruleContext, runfiles, owningExecutable);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noticed that we aren't adding the repo mapping manifest to the runfiles manifest, which makes it more difficult to find it at runtime on Windows. If we move this call further up, we could pass the Artifact into SourceManifestAction.

Artifact runfilesMiddleman =
createRunfilesMiddleman(ruleContext, owningExecutable, runfiles, runfilesManifest);
createRunfilesMiddleman(
ruleContext, owningExecutable, runfiles, runfilesManifest, repoMappingManifest);

boolean runfilesEnabled = ruleContext.getConfiguration().runfilesEnabled();

return new RunfilesSupport(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How will repo mapping files work with sandboxes? I suppose they should appear there, but how would that work?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really don't know how to answer this question :( Does the existing runfiles manifest file work with sandboxes? If so, it doesn't look like the repo mapping manifest file should be any different.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, a test case would dispel all my fears :)

The reason why I'm asking is that because I don't know off the top of my head how the runfiles manifest is plumbed to sandboxes and remote execution workers and since they are very special, I can't say with any confidence that the repo mapping file will be there. Ideally, by the time this changes is merged, we both would understand how runfiles output manifests appear in sandboxes / on RBE and be convinced that the repo mapping file does, too (and there would be a test also, of course!)

runfiles,
runfilesInputManifest,
runfilesManifest,
repoMappingManifest,
runfilesMiddleman,
owningExecutable,
buildRunfileLinks,
Expand All @@ -153,6 +166,7 @@ private RunfilesSupport(
Runfiles runfiles,
Artifact runfilesInputManifest,
Artifact runfilesManifest,
Artifact repoMappingManifest,
Artifact runfilesMiddleman,
Artifact owningExecutable,
boolean buildRunfileLinks,
Expand All @@ -162,6 +176,7 @@ private RunfilesSupport(
this.runfiles = runfiles;
this.runfilesInputManifest = runfilesInputManifest;
this.runfilesManifest = runfilesManifest;
this.repoMappingManifest = repoMappingManifest;
this.runfilesMiddleman = runfilesMiddleman;
this.owningExecutable = owningExecutable;
this.buildRunfileLinks = buildRunfileLinks;
Expand Down Expand Up @@ -268,6 +283,17 @@ public Artifact getRunfilesManifest() {
return runfilesManifest;
}

/**
* Returns the foo.repo_mapping file if Bazel is run with transitive package tracking turned on
* (see {@code SkyframeExecutor#getForcedSingleSourceRootIfNoExecrootSymlinkCreation}) and any of
* the transitive packages come from a repository with strict deps (see {@code
* #collectRepoMappings}). Otherwise, returns null.
*/
@Nullable
public Artifact getRepoMappingManifest() {
return repoMappingManifest;
}

/** Returns the root directory of the runfiles symlink farm; otherwise, returns null. */
@Nullable
public Path getRunfilesDirectory() {
Expand Down Expand Up @@ -327,12 +353,16 @@ private static Artifact createRunfilesMiddleman(
ActionConstructionContext context,
Artifact owningExecutable,
Runfiles runfiles,
@Nullable Artifact runfilesManifest) {
@Nullable Artifact runfilesManifest,
Artifact repoMappingManifest) {
NestedSetBuilder<Artifact> deps = NestedSetBuilder.stableOrder();
deps.addTransitive(runfiles.getAllArtifacts());
if (runfilesManifest != null) {
deps.add(runfilesManifest);
}
if (repoMappingManifest != null) {
deps.add(repoMappingManifest);
}
return context
.getAnalysisEnvironment()
.getMiddlemanFactory()
Expand Down Expand Up @@ -495,4 +525,82 @@ public static Path inputManifestPath(Path runfilesDir) {
public static Path outputManifestPath(Path runfilesDir) {
return runfilesDir.getRelative(OUTPUT_MANIFEST_BASENAME);
}

@Nullable
private static Artifact createRepoMappingManifestAction(
RuleContext ruleContext, Runfiles runfiles, Artifact owningExecutable) {
NestedSet<Package> transitivePackages =
ruleContext.getTransitivePackagesForRunfileRepoMappingManifest();
if (transitivePackages == null) {
// For environments where transitive packages are not tracked, we don't have external repos,
// so don't build the repo mapping manifest in such cases.
return null;
}

ImmutableList<Entry> entries = collectRepoMappings(transitivePackages, runfiles);
if (entries == null) {
// If nobody needs to read the repo mapping manifest anyway, don't generate it.
return null;
}

PathFragment executablePath =
(owningExecutable != null)
? owningExecutable.getOutputDirRelativePath(
ruleContext.getConfiguration().isSiblingRepositoryLayout())
: ruleContext.getPackageDirectory().getRelative(ruleContext.getLabel().getName());
Artifact repoMappingManifest =
ruleContext.getDerivedArtifact(
executablePath.replaceName(executablePath.getBaseName() + REPO_MAPPING_MANIFEST_EXT),
ruleContext.getBinDirectory());
ruleContext
.getAnalysisEnvironment()
.registerAction(
new RepoMappingManifestAction(
ruleContext.getActionOwner(),
repoMappingManifest,
entries,
ruleContext.getWorkspaceName()));
return repoMappingManifest;
}

/**
* Returns the list of entries (unsorted) that should appear in the repo mapping manifest, or null
* if the repo mapping manifest should not be generated at all.
*/
@Nullable
private static ImmutableList<Entry> collectRepoMappings(
NestedSet<Package> transitivePackages, Runfiles runfiles) {
ImmutableSet<RepositoryName> reposContributingRunfiles =
runfiles.getAllArtifacts().toList().stream()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This being a path not taken in Blaze and not taken in Bazel by default makes me a bit worried it may regress analysis phase performance outside Google. Were you able to get benchmark data on that or have other reasons to believe it's safe?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't get benchmark data. This change was made out of necessity rather than convenience, since things break internally if a Package is used as to compute an action key in any way.

Also, this is actually still run in Blaze, so its effects will be visible. It's just not run in certain other modes of builds in Google (where the transitive package tracking doesn't happen).

.filter(a -> a.getOwner() != null)
.map(a -> a.getOwner().getRepository())
.collect(toImmutableSet());
Set<RepositoryName> seenRepos = new HashSet<>();
ImmutableList.Builder<Entry> entries = ImmutableList.builder();
// There exist repos that use strict deps iff Bzlmod is enabled. So if none of the repos we come
// upon use strict deps, either Bzlmod is disabled, or we're using a very weird subset of
// packages that *only* come from WORKSPACE-repos. In either case, it's unnecessary to output
// the repo mapping manifest.
boolean shouldGenerateRepoMappingManifest = false;
for (Package pkg : transitivePackages.toList()) {
if (!seenRepos.add(pkg.getPackageIdentifier().getRepository())) {
// Any package from the same repo would have the same repo mapping.
continue;
}
if (pkg.getRepositoryMapping().usesStrictDeps()) {
shouldGenerateRepoMappingManifest = true;
}
for (Map.Entry<String, RepositoryName> repoMappingEntry :
pkg.getRepositoryMapping().entries().entrySet()) {
if (reposContributingRunfiles.contains(repoMappingEntry.getValue())) {
entries.add(
Entry.of(
pkg.getPackageIdentifier().getRepository(),
repoMappingEntry.getKey(),
repoMappingEntry.getValue()));
}
}
}
return shouldGenerateRepoMappingManifest ? entries.build() : null;
}
}