diff --git a/scripts/bootstrap/compile.sh b/scripts/bootstrap/compile.sh index 0475d4472d8604..eb39a4ebff08b9 100755 --- a/scripts/bootstrap/compile.sh +++ b/scripts/bootstrap/compile.sh @@ -335,8 +335,9 @@ if [ "${PLATFORM}" = "windows" ]; then # We don't rely on runfiles trees on Windows cat <<'EOF' >${ARCHIVE_DIR}/build-runfiles${EXE_EXT} #!/bin/sh -mkdir -p $2 -cp $1 $2/MANIFEST +# Skip over --allow_relative. +mkdir -p $3 +cp $2 $3/MANIFEST EOF else cat <<'EOF' >${ARCHIVE_DIR}/build-runfiles${EXE_EXT} @@ -350,8 +351,9 @@ else # bootstrap version of Bazel, but we'd still need a shell wrapper around it, so # it's not clear whether that would be a win over a few lines of Lovecraftian # code) -MANIFEST="$1" -TREE="$2" +# Skip over --allow_relative. +MANIFEST="$2" +TREE="$3" rm -fr "$TREE" mkdir -p "$TREE" diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD index 8308928001094b..95c4a26419f74a 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD +++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD @@ -249,6 +249,7 @@ java_library( "starlark/StarlarkRuleContext.java", "starlark/StarlarkRuleTransitionProvider.java", "starlark/StarlarkTransition.java", + "starlark/UnresolvedSymlinkAction.java", "test/AnalysisTestActionBuilder.java", "test/BaselineCoverageAction.java", "test/CoverageCommon.java", diff --git a/src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java b/src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java index dee6ab037f308a..9c89ea270198e8 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java @@ -13,16 +13,19 @@ // limitations under the License. package com.google.devtools.build.lib.analysis; +import static com.google.common.collect.ImmutableList.toImmutableList; import static java.nio.charset.StandardCharsets.ISO_8859_1; import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.annotations.VisibleForTesting; +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.analysis.actions.AbstractFileWriteAction; import com.google.devtools.build.lib.analysis.actions.DeterministicWriter; +import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; @@ -99,6 +102,8 @@ void writeEntry( private final boolean remotableSourceManifestActions; + private NestedSet symlinkArtifacts = null; + /** * Creates a new AbstractSourceManifestAction instance using latin1 encoding to write the manifest * file and with a specified root path for manifest entries. @@ -135,6 +140,17 @@ public SourceManifestAction( this.remotableSourceManifestActions = remotableSourceManifestActions; } + @Override + public synchronized NestedSet getInputs() { + if (symlinkArtifacts == null) { + ImmutableList symlinks = runfiles.getArtifacts().toList().stream() + .filter(Artifact::isSymlink) + .collect(toImmutableList()); + symlinkArtifacts = NestedSetBuilder.wrap(Order.STABLE_ORDER, symlinks); + } + return symlinkArtifacts; + } + @VisibleForTesting public void writeOutputFile(OutputStream out, EventHandler eventHandler) throws IOException { writeFile(out, runfiles.getRunfilesInputs(eventHandler, getOwner().getLocation())); @@ -227,7 +243,11 @@ public void writeEntry(Writer manifestWriter, PathFragment rootRelativePath, Art // This trailing whitespace is REQUIRED to process the single entry line correctly. manifestWriter.append(' '); if (symlink != null) { - manifestWriter.append(symlink.getPath().getPathString()); + if (symlink.isSymlink()) { + manifestWriter.append(symlink.getPath().readSymbolicLink().getPathString()); + } else { + manifestWriter.append(symlink.getPath().getPathString()); + } } manifestWriter.append('\n'); } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkAction.java index ec0cb0b4644cce..41715e4fff4827 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SymlinkAction.java @@ -36,7 +36,10 @@ import java.io.IOException; import javax.annotation.Nullable; -/** Action to create a symbolic link. */ +/** + * Action to create a symlink to a known-to-exist target with alias semantics similar to a true copy + * of the input (if any). + */ public final class SymlinkAction extends AbstractAction { private static final String GUID = "7f4fab4d-d0a7-4f0f-8649-1d0337a21fee"; @@ -152,13 +155,6 @@ public static SymlinkAction toFileset( owner, execPath, primaryInput, primaryOutput, progressMessage, TargetType.FILESET); } - public static SymlinkAction createUnresolved( - ActionOwner owner, Artifact primaryOutput, PathFragment targetPath, String progressMessage) { - Preconditions.checkArgument(primaryOutput.isSymlink()); - return new SymlinkAction( - owner, targetPath, null, primaryOutput, progressMessage, TargetType.OTHER); - } - /** * Creates a new SymlinkAction instance, where an input artifact is not present. This is useful * when dealing with special cases where input paths that are outside the exec root directory diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkActionFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkActionFactory.java index 83140f287f84f9..a0e5708d8c1cf5 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkActionFactory.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkActionFactory.java @@ -273,7 +273,7 @@ public void symlink( ? (String) progressMessageUnchecked : "Creating symlink " + outputArtifact.getExecPathString(); - SymlinkAction action; + Action action; if (targetFile != Starlark.NONE) { Artifact inputArtifact = (Artifact) targetFile; if (outputArtifact.isSymlink()) { @@ -324,10 +324,10 @@ public void symlink( } action = - SymlinkAction.createUnresolved( + new UnresolvedSymlinkAction( ruleContext.getActionOwner(), outputArtifact, - PathFragment.create((String) targetPath), + (String) targetPath, progressMessage); } registerAction(action); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/UnresolvedSymlinkAction.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/UnresolvedSymlinkAction.java new file mode 100644 index 00000000000000..b4e283ffbec37c --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/UnresolvedSymlinkAction.java @@ -0,0 +1,112 @@ +// 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.starlark; + +import com.google.common.collect.ImmutableSet; +import com.google.devtools.build.lib.actions.AbstractAction; +import com.google.devtools.build.lib.actions.ActionExecutionContext; +import com.google.devtools.build.lib.actions.ActionExecutionException; +import com.google.devtools.build.lib.actions.ActionKeyContext; +import com.google.devtools.build.lib.actions.ActionOwner; +import com.google.devtools.build.lib.actions.ActionResult; +import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander; +import com.google.devtools.build.lib.analysis.actions.SymlinkAction; +import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; +import com.google.devtools.build.lib.collect.nestedset.Order; +import com.google.devtools.build.lib.server.FailureDetails; +import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; +import com.google.devtools.build.lib.server.FailureDetails.SymlinkAction.Code; +import com.google.devtools.build.lib.util.DetailedExitCode; +import com.google.devtools.build.lib.util.Fingerprint; +import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.PathFragment; +import java.io.IOException; +import javax.annotation.Nullable; + +/** + * Action to create a possibly unresolved symbolic link to a raw path. + * + * To create a symlink to a known-to-exist target with alias semantics similar to a true copy of the + * input, use {@link SymlinkAction} instead. + */ +public final class UnresolvedSymlinkAction extends AbstractAction { + private static final String GUID = "0f302651-602c-404b-881c-58913193cfe7"; + + private final String target; + private final String progressMessage; + + public UnresolvedSymlinkAction( + ActionOwner owner, + Artifact primaryOutput, + String target, + String progressMessage) { + super( + owner, + NestedSetBuilder.emptySet(Order.STABLE_ORDER), + ImmutableSet.of(primaryOutput)); + this.target = target; + this.progressMessage = progressMessage; + } + + @Override + public ActionResult execute(ActionExecutionContext actionExecutionContext) + throws ActionExecutionException { + + Path outputPath = actionExecutionContext.getInputPath(getPrimaryOutput()); + try { + // TODO: PathFragment#create normalizes the symlink target, which may change how it resolves + // when combined with directory symlinks. Ideally, Bazel's file system abstraction would + // offer a way to create symlinks without any preprocessing of the target. + outputPath.createSymbolicLink(PathFragment.create(target)); + } catch (IOException e) { + String message = + String.format( + "failed to create symbolic link '%s' to '%s' due to I/O error: %s", + getPrimaryOutput().getExecPathString(), target, e.getMessage()); + DetailedExitCode code = createDetailedExitCode(message, Code.LINK_CREATION_IO_EXCEPTION); + throw new ActionExecutionException(message, e, this, false, code); + } + + return ActionResult.EMPTY; + } + + @Override + protected void computeKey( + ActionKeyContext actionKeyContext, + @Nullable ArtifactExpander artifactExpander, + Fingerprint fp) { + fp.addString(GUID); + fp.addString(target); + } + + @Override + public String getMnemonic() { + return "UnresolvedSymlink"; + } + + @Override + protected String getRawProgressMessage() { + return progressMessage; + } + + private static DetailedExitCode createDetailedExitCode(String message, Code detailedCode) { + return DetailedExitCode.of( + FailureDetail.newBuilder() + .setMessage(message) + .setSymlinkAction(FailureDetails.SymlinkAction.newBuilder().setCode(detailedCode)) + .build()); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java index 57c74e2c57cd40..a0a04a3eea1272 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java @@ -180,8 +180,8 @@ Command createCommand(Path execRoot, BinTools binTools, Map shel Preconditions.checkNotNull(shellEnvironment); List args = Lists.newArrayList(); args.add(binTools.getEmbeddedPath(BUILD_RUNFILES).asFragment().getPathString()); + args.add("--allow_relative"); if (filesetTree) { - args.add("--allow_relative"); args.add("--use_metadata"); } args.add(inputManifest.relativeTo(execRoot).getPathString()); diff --git a/src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeHelperTest.java b/src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeHelperTest.java index a15756cc520f44..64534b912ac81f 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeHelperTest.java +++ b/src/test/java/com/google/devtools/build/lib/exec/SymlinkTreeHelperTest.java @@ -48,10 +48,11 @@ public void checkCreatedSpawn() { assertThat(command.getEnvironment()).isEmpty(); assertThat(command.getWorkingDirectory()).isEqualTo(execRoot.getPathFile()); ImmutableList commandLine = command.getArguments(); - assertThat(commandLine).hasSize(3); + assertThat(commandLine).hasSize(4); assertThat(commandLine.get(0)).endsWith(SymlinkTreeHelper.BUILD_RUNFILES); - assertThat(commandLine.get(1)).isEqualTo("input_manifest"); - assertThat(commandLine.get(2)).isEqualTo("output/MANIFEST"); + assertThat(commandLine.get(1)).isEqualTo("--allow_relative"); + assertThat(commandLine.get(2)).isEqualTo("input_manifest"); + assertThat(commandLine.get(3)).isEqualTo("output/MANIFEST"); } @Test diff --git a/src/test/shell/bazel/bazel_symlink_test.sh b/src/test/shell/bazel/bazel_symlink_test.sh index df0059fcac1a94..09e4b67fd630aa 100755 --- a/src/test/shell/bazel/bazel_symlink_test.sh +++ b/src/test/shell/bazel/bazel_symlink_test.sh @@ -411,11 +411,11 @@ EOF cat > a/BUILD <<'EOF' load(":a.bzl", "a") -a(name="a", link_target="/somewhere/in/my/heart") +a(name="a", link_target="../somewhere/in/my/heart") EOF bazel build --experimental_allow_unresolved_symlinks //a:a || fail "build failed" - assert_contains "input link is /somewhere/in/my/heart" bazel-bin/a/a.file + assert_contains "input link is ../somewhere/in/my/heart" bazel-bin/a/a.file } function test_symlink_file_to_file_created_from_symlink_action() { @@ -731,4 +731,139 @@ EOF bazel --windows_enable_symlinks build --experimental_allow_unresolved_symlinks //a:exec || fail "build failed" } +function test_unresolved_symlink_hermetic_in_sandbox() { + if "$is_windows"; then + # TODO(#10298): Support unresolved symlinks on Windows. + return 0 + fi + + mkdir -p pkg + cat > pkg/def.bzl <<'EOF' +def _r(ctx): + symlink = ctx.actions.declare_symlink(ctx.label.name + "_s") + ctx.actions.symlink(output=symlink, target_path=ctx.file.file.basename) + + output = ctx.actions.declare_file(ctx.label.name) + ctx.actions.run_shell( + command = "{ cat %s || true; } > %s" % (symlink.path, output.path), + inputs = [symlink] + ([ctx.file.file] if ctx.attr.stage_target else []), + outputs = [output], + ) + return DefaultInfo(files=depset([output])) + +r = rule( + implementation=_r, + attrs = { + "file": attr.label(allow_single_file=True), + "stage_target": attr.bool(), + } +) +EOF + cat > pkg/BUILD <<'EOF' +load(":def.bzl", "r") + +genrule(name = "a", outs = ["file"], cmd = "echo hello >$@") +r(name="b", file="file") +r(name="c", file="file", stage_target=True) +EOF + + bazel build //pkg:b --experimental_allow_unresolved_symlinks --spawn_strategy=local + [ -s bazel-bin/pkg/b ] && fail "symlink should not resolve" + + bazel clean + bazel build //pkg:a --spawn_strategy=sandboxed + bazel build //pkg:b --experimental_allow_unresolved_symlinks --spawn_strategy=local + [ -s bazel-bin/pkg/b ] || fail "symlink should resolve" + + bazel clean + bazel build //pkg:c --experimental_allow_unresolved_symlinks --spawn_strategy=local + [ -s bazel-bin/pkg/c ] || fail "symlink should resolve" + + bazel clean + bazel build //pkg:b --experimental_allow_unresolved_symlinks --spawn_strategy=sandboxed + [ -s bazel-bin/pkg/b ] && fail "sandboxed build is not hermetic" + + bazel clean + bazel build //pkg:a --spawn_strategy=sandboxed + bazel build //pkg:b --experimental_allow_unresolved_symlinks --spawn_strategy=sandboxed + [ -s bazel-bin/pkg/b ] && fail "sandboxed build is not hermetic" + + bazel clean + bazel build //pkg:c --experimental_allow_unresolved_symlinks --spawn_strategy=sandboxed + [ -s bazel-bin/pkg/c ] || fail "symlink should resolve" +} + +function test_unresolved_symlink_in_runfiles() { + if "$is_windows"; then + # TODO(#10298): Support unresolved symlinks on Windows. + return 0 + fi + + mkdir -p pkg + cat > pkg/def.bzl <<'EOF' +def _r(ctx): + symlink = ctx.actions.declare_symlink(ctx.label.name + "_s") + target = ctx.file.file.basename + ctx.actions.symlink(output=symlink, target_path=target) + + script = ctx.actions.declare_file(ctx.label.name) + content = """ +#!/usr/bin/env bash +cd $0.runfiles/{workspace_name} +[ -L {symlink} ] || {{ echo "runfile is not a symlink"; exit 1; }} +[ $(readlink {symlink}) == "{target}" ] || {{ echo "runfile symlink does not have the expected target, got: $(readlink {symlink})"; exit 1; }} +[ -s {symlink} ] || {{ echo "runfile not resolved"; exit 1; }} +""".format( + symlink = symlink.short_path, + target = target, + workspace_name = ctx.workspace_name, + ) + ctx.actions.write(script, content, is_executable=True) + + runfiles = ctx.runfiles( + files = [symlink] + ([ctx.file.file] if ctx.attr.stage_file else []), + ) + return DefaultInfo(executable=script, runfiles=runfiles) + +r = rule( + implementation = _r, + attrs = { + "file": attr.label(allow_single_file = True), + "stage_file": attr.bool(), + }, + executable = True, +) +EOF + cat > pkg/BUILD <<'EOF' +load(":def.bzl", "r") + +genrule(name="a", outs=["file"], cmd="echo hello >$@") +r(name="tool", file="file", stage_file=True) +r(name="non_hermetic_tool", file="file", stage_file=False) +genrule( + name = "use_tool", + outs = ["out"], + cmd = "$(location :tool) && touch $@", + tools = [":tool"], +) +genrule( + name = "use_tool_non_hermetically", + outs = ["out_non_hermetic"], + cmd = "$(location :non_hermetic_tool) && touch $@", + # Stage file outside the runfiles tree. + tools = [":non_hermetic_tool", "file"], +) +EOF + + bazel build --experimental_allow_unresolved_symlinks --spawn_strategy=local //pkg:use_tool_non_hermetically && fail "symlink in runfiles resolved outside the runfiles tree" + + bazel clean + bazel build --experimental_allow_unresolved_symlinks --spawn_strategy=local //pkg:use_tool || fail "local build failed" + + bazel clean + bazel build --experimental_allow_unresolved_symlinks --spawn_strategy=sandboxed //pkg:use_tool || fail "sandboxed build failed" + # Keep the implicitly built //pkg:a around to make the symlink resolve outside the sandbox. + bazel build --experimental_allow_unresolved_symlinks --spawn_strategy=sandboxed //pkg:use_tool_non_hermetically && fail "sandboxed symlink in runfiles resolved outside the runfiles tree" || true +} + run_suite "Tests for symlink artifacts"