diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java index 03829521e5be05..31606cbcb0131a 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java @@ -16,6 +16,7 @@ import static java.util.stream.Collectors.toList; import com.google.common.base.Joiner; +import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.actions.Action; @@ -220,29 +221,89 @@ public void doNothing(String mnemonic, Object inputs) throws EvalException { static final GeneratedMessage.GeneratedExtension SPAWN_INFO = SpawnInfo.spawnInfo; + private String progressMessageForSymlink( + Object /* String or None */ progressMessage, Artifact output) { + if (progressMessage != Starlark.NONE) { + // Should have been verified by Starlark before this function is called. + return (String)progressMessage; + } + + return "Creating symlink " + output.getRootRelativePathString(); + } + @Override - public void symlink(FileApi output, String path) throws EvalException { + public void symlink( + FileApi output, + Object /* Artifact or None */ targetFile, + Object /* String or None */ targetPath, + Boolean isExecutable, + Object /* String or None */ progressMessage) + throws EvalException { context.checkMutable("actions.symlink"); + if ((targetFile != Starlark.NONE && targetPath != Starlark.NONE) || + (targetFile == Starlark.NONE && targetPath == Starlark.NONE)) { + throw Starlark.errorf("\"target_file\" and \"target_path\" cannot be set at the same time"); + } + + // Should have been verified by Starlark before this function is called. + Artifact outputArtifact = (Artifact)output; + + if (targetFile != Starlark.NONE) { + Preconditions.checkState(targetPath == Starlark.NONE); + + if (outputArtifact.isSymlink()) { + // TODO(yannic): Do we allow symlinks from files created by declare_symlink() to artifacts? + throw Starlark.errorf( + "output of symlink action with \"target_file\" must be created by declare_file()"); + } + + // Should have been verified by Starlark before this function is called. + Artifact target = (Artifact)targetFile; + if (isExecutable) { + SymlinkAction action = + SymlinkAction.toExecutable( + ruleContext.getActionOwner(), + target, + outputArtifact, + progressMessageForSymlink(progressMessage, outputArtifact)); + registerAction(action); + return; + } + SymlinkAction action = + SymlinkAction.toArtifact( + ruleContext.getActionOwner(), + target, + outputArtifact, + progressMessageForSymlink(progressMessage, outputArtifact)); + registerAction(action); + return; + } + Preconditions.checkState(targetPath != Starlark.NONE); + if (!ruleContext.getConfiguration().allowUnresolvedSymlinks()) { - throw new EvalException( - null, - "actions.symlink() is not allowed; " + throw Starlark.errorf( + "actions.symlink() to unresolved symlink is not allowed; " + "use the --experimental_allow_unresolved_symlinks command line option"); } - PathFragment targetPath = PathFragment.create(path); - Artifact outputArtifact = (Artifact) output; if (!outputArtifact.isSymlink()) { throw Starlark.errorf("output of symlink action must be created by declare_symlink()"); } - Action action = + if (isExecutable) { + // TODO(yannic): Do we allow dangling symlinks to be executable? + throw Starlark.errorf("files created by declare_symlink() cannot be executable"); + } + + // Should have been verified by Starlark before this function is called. + String target = (String)targetPath; + SymlinkAction action = SymlinkAction.createUnresolved( ruleContext.getActionOwner(), outputArtifact, - targetPath, - "creating symlink " + ((Artifact) output).getRootRelativePathString()); + PathFragment.create(target), + progressMessageForSymlink(progressMessage, outputArtifact)); registerAction(action); } diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkActionFactoryApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkActionFactoryApi.java index ec0bf1fc9f1cea..d3409d02a3ea6b 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkActionFactoryApi.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkActionFactoryApi.java @@ -159,17 +159,70 @@ public interface SkylarkActionFactoryApi extends StarlarkValue { @SkylarkCallable( name = "symlink", doc = - "

Experimental. This parameter is experimental and may change at any " - + "time. Please do not depend on it. It may be enabled on an experimental basis by " - + "setting --experimental_allow_unresolved_symlinks

" - + "Creates a symlink in the file system. If the output file is a regular file, the " - + "symlink must point to a file. If the output is an unresolved symlink, a dangling " - + "symlink is allowed.", + "Creates a symlink in the file system." + + "

If the output file is a regular file (i.e. created by " + + "declare_file()), the symlink must " + + "point to a file. If the output is an unresolved symlink (i.e. created by " + + "declare_symlink()), a dangling " + + "symlink is allowed.

", parameters = { - @Param(name = "output", type = FileApi.class, doc = "The output path.", named = true), - @Param(name = "target", type = String.class, doc = "The target.", named = true), + @Param( + name = "output", + type = FileApi.class, + named = true, + positional = false, + doc = "The output of this action."), + @Param( + name = "target_file", + type = FileApi.class, + named = true, + positional = false, + noneable = true, + defaultValue = "None", + doc = + "The target of the symlink." + + "

If this parameter is set, target_path " + + "must be None.

"), + @Param( + name = "target_path", + type = String.class, + named = true, + positional = false, + noneable = true, + defaultValue = "None", + doc = + "The target path of the symlink." + + "

If this parameter is set, target_file " + + "must be None.

" + + "

Experimental This parameter is experimental and may change at " + + "any time. Please do not depend on it. It may be enabled on an experimental " + + "basis by setting --experimental_allow_unresolved_symlinks

"), + @Param( + name = "is_executable", + type = Boolean.class, + named = true, + positional = false, + defaultValue = "False", + doc = + "Whether the output file should be executable. " + + "

If this is True, target_path must also be " + + "executable.

"), + @Param( + name = "progress_message", + type = String.class, + named = true, + positional = false, + noneable = true, + defaultValue = "None", + doc = "Progress message to show to the user during the build.") }) - void symlink(FileApi output, String targetPath) throws EvalException; + void symlink( + FileApi output, + Object targetFile, + Object targetPath, + Boolean isExecutable, + Object progressMessage) + throws EvalException; @SkylarkCallable( name = "write", diff --git a/src/test/shell/bazel/BUILD b/src/test/shell/bazel/BUILD index d47e4d4af52810..ba12bf03692058 100644 --- a/src/test/shell/bazel/BUILD +++ b/src/test/shell/bazel/BUILD @@ -70,7 +70,6 @@ sh_test( ":test-deps", "@bazel_tools//tools/bash/runfiles", ], - tags = ["no_windows"], ) sh_test( diff --git a/src/test/shell/bazel/bazel_unresolved_symlink_test.sh b/src/test/shell/bazel/bazel_unresolved_symlink_test.sh index 4f7d69420b1b4a..1293c6353055c4 100755 --- a/src/test/shell/bazel/bazel_unresolved_symlink_test.sh +++ b/src/test/shell/bazel/bazel_unresolved_symlink_test.sh @@ -70,9 +70,25 @@ function set_up() { mkdir -p symlink touch symlink/BUILD cat > symlink/symlink.bzl < a/BUILD < a/BUILD <<'EOF' -load("//symlink:symlink.bzl", "symlink") -symlink(name="a", link_target="/nonexistent") +load("//symlink:symlink.bzl", "dangling_symlink") +dangling_symlink(name="a", link_target="/nonexistent") genrule(name="g", srcs=[":a"], outs=["go"], cmd="echo running genrule; echo GO > $@") EOF bazel build --experimental_allow_unresolved_symlinks //a:g >& $TEST_log || fail "build failed" @@ -112,10 +138,15 @@ EOF } function test_on_disk_cache_symlinks() { + if "$is_windows"; then + warn "Skipping test on Windows: Unresolved symlinks are not supported yet" + return 0 + fi + mkdir -p a cat > a/BUILD <<'EOF' -load("//symlink:symlink.bzl", "symlink") -symlink(name="a", link_target="/nonexistent") +load("//symlink:symlink.bzl", "dangling_symlink") +dangling_symlink(name="a", link_target="/nonexistent") genrule(name="g", srcs=[":a"], outs=["go"], cmd="echo running genrule; echo GO > $@") EOF bazel build --experimental_allow_unresolved_symlinks //a:g >& $TEST_log || fail "build failed" @@ -126,10 +157,15 @@ EOF } function test_no_inmemory_cache_symlinks() { + if "$is_windows"; then + warn "Skipping test on Windows: Unresolved symlinks are not supported yet" + return 0 + fi + mkdir -p a cat > a/BUILD <<'EOF' -load("//symlink:symlink.bzl", "symlink") -symlink(name="a", link_target="/nonexistent") +load("//symlink:symlink.bzl", "dangling_symlink") +dangling_symlink(name="a", link_target="/nonexistent") genrule(name="g", srcs=[":a"], outs=["go"], cmd="echo running genrule; echo GO > $@") EOF @@ -137,8 +173,8 @@ EOF expect_log "running genrule" cat > a/BUILD <<'EOF' -load("//symlink:symlink.bzl", "symlink") -symlink(name="a", link_target="/nonexistent2") +load("//symlink:symlink.bzl", "dangling_symlink") +dangling_symlink(name="a", link_target="/nonexistent2") genrule(name="g", srcs=[":a"], outs=["go"], cmd="echo running genrule; echo GO > $@") EOF @@ -147,10 +183,15 @@ EOF } function test_no_on_disk_cache_symlinks() { + if "$is_windows"; then + warn "Skipping test on Windows: Unresolved symlinks are not supported yet" + return 0 + fi + mkdir -p a cat > a/BUILD <<'EOF' -load("//symlink:symlink.bzl", "symlink") -symlink(name="a", link_target="/nonexistent") +load("//symlink:symlink.bzl", "dangling_symlink") +dangling_symlink(name="a", link_target="/nonexistent") genrule(name="g", srcs=[":a"], outs=["go"], cmd="echo running genrule; echo GO > $@") EOF @@ -158,8 +199,8 @@ EOF expect_log "running genrule" cat > a/BUILD <<'EOF' -load("//symlink:symlink.bzl", "symlink") -symlink(name="a", link_target="/nonexistent2") +load("//symlink:symlink.bzl", "dangling_symlink") +dangling_symlink(name="a", link_target="/nonexistent2") genrule(name="g", srcs=[":a"], outs=["go"], cmd="echo running genrule; echo GO > $@") EOF @@ -169,10 +210,15 @@ EOF } function test_replace_symlink_with_file() { + if "$is_windows"; then + warn "Skipping test on Windows: Unresolved symlinks are not supported yet" + return 0 + fi + mkdir -p a cat > a/BUILD <<'EOF' -load("//symlink:symlink.bzl", "symlink") -symlink(name="a", link_target="/nonexistent") +load("//symlink:symlink.bzl", "dangling_symlink") +dangling_symlink(name="a", link_target="/nonexistent") genrule(name="g", srcs=[":a"], outs=["go"], cmd="echo running genrule; echo GO > $@") EOF @@ -190,6 +236,11 @@ EOF } function test_replace_file_with_symlink() { + if "$is_windows"; then + warn "Skipping test on Windows: Unresolved symlinks are not supported yet" + return 0 + fi + mkdir -p a cat > a/BUILD <<'EOF' load("//symlink:symlink.bzl", "write") @@ -201,8 +252,8 @@ EOF expect_log "running genrule" cat > a/BUILD <<'EOF' -load("//symlink:symlink.bzl", "symlink") -symlink(name="a", link_target="/nonexistent") +load("//symlink:symlink.bzl", "dangling_symlink") +dangling_symlink(name="a", link_target="/nonexistent") genrule(name="g", srcs=[":a"], outs=["go"], cmd="echo running genrule; echo GO > $@") EOF @@ -211,18 +262,31 @@ EOF } function test_file_instead_of_symlink() { + if "$is_windows"; then + warn "Skipping test on Windows: Unresolved symlinks are not supported yet" + return 0 + fi + mkdir -p a cat > a/a.bzl <<'EOF' def _bad_symlink_impl(ctx): symlink = ctx.actions.declare_symlink(ctx.label.name) - ctx.actions.write(symlink, ctx.attr.link_target) # Oops, should be "symlink" + # Oops, should be "dangling_symlink" + ctx.actions.write( + output = symlink, + content = ctx.attr.link_target, + ) return DefaultInfo(files = depset([symlink])) bad_symlink = rule(implementation = _bad_symlink_impl, attrs = {"link_target": attr.string()}) def _bad_write_impl(ctx): output = ctx.actions.declare_file(ctx.label.name) - ctx.actions.symlink(output, ctx.attr.contents) # Oops, should be "write" + # Oops, should be "write" + ctx.actions.symlink( + output = output, + target_path = ctx.attr.contents, + ) return DefaultInfo(files = depset([output])) bad_write = rule(implementation = _bad_write_impl, attrs = {"contents": attr.string()}) @@ -250,7 +314,42 @@ EOF [[ "$?" == 1 ]] || fail "unexpected exit code" } +function test_symlink_instead_of_file() { + mkdir -p a + cat > a/a.bzl <<'EOF' +def _bad_symlink_impl(ctx): + target = ctx.actions.declare_file(ctx.label.name + ".target") + ctx.actions.write( + output = target, + content = "Hello, World!", + ) + + symlink = ctx.actions.declare_symlink(ctx.label.name + ".link") + ctx.actions.symlink( + output = symlink, + target_file = target, + ) + return DefaultInfo(files = depset([symlink])) + +bad_symlink = rule(implementation = _bad_symlink_impl) +EOF + + cat > a/BUILD <<'EOF' +load(":a.bzl", "bad_symlink") + +bad_symlink(name="bs") +EOF + + bazel build //a:bs && fail "build succeeded" + [[ "$?" == 1 ]] || fail "unexpected exit code" +} + function test_symlink_created_from_spawn() { + if "$is_windows"; then + warn "Skipping test on Windows: Unresolved symlinks are not supported yet" + return 0 + fi + mkdir -p a cat > a/a.bzl <<'EOF' def _a_impl(ctx): @@ -281,13 +380,21 @@ EOF assert_contains "input link is /somewhere/over/the/rainbow" bazel-bin/a/a.file } -function test_symlink_created_from_symlink_action() { +function test_dangling_symlink_created_from_symlink_action() { + if "$is_windows"; then + warn "Skipping test on Windows: Unresolved symlinks are not supported yet" + return 0 + fi + mkdir -p a cat > a/a.bzl <<'EOF' def _a_impl(ctx): symlink = ctx.actions.declare_symlink(ctx.label.name + ".link") output = ctx.actions.declare_file(ctx.label.name + ".file") - ctx.actions.symlink(symlink, ctx.attr.link_target) + ctx.actions.symlink( + output = symlink, + target_path = ctx.attr.link_target, + ) ctx.actions.run_shell( outputs = [output], inputs = depset([symlink]), @@ -308,4 +415,129 @@ EOF assert_contains "input link is /somewhere/in/my/heart" bazel-bin/a/a.file } +function test_symlink_created_from_symlink_action() { + mkdir -p a + cat > a/a.bzl <<'EOF' +def _a_impl(ctx): + target = ctx.actions.declare_file(ctx.label.name + ".target") + ctx.actions.write( + output = target, + content = "Hello, World!", + ) + + symlink = ctx.actions.declare_file(ctx.label.name + ".link") + ctx.actions.symlink( + output = symlink, + target_file = target, + ) + + output = ctx.actions.declare_file(ctx.label.name + ".file") + ctx.actions.run_shell( + outputs = [output], + inputs = depset([symlink]), + command = "echo input link is $(readlink " + symlink.path + ") > " + output.path, + ) + return DefaultInfo(files = depset([output])) + +a = rule(implementation = _a_impl) +EOF + + cat > a/BUILD <<'EOF' +load(":a.bzl", "a") + +a(name="a") +EOF + + bazel build //a:a || fail "build failed" + assert_contains "input link is $(bazel info bazel-bin)/a/a.link" bazel-bin/a/a.file +} + +function test_executable_dangling_symlink() { + mkdir -p a + cat > a/a.bzl <<'EOF' +def _a_impl(ctx): + symlink = ctx.actions.declare_symlink(ctx.label.name + ".link") + ctx.actions.symlink( + output = symlink, + target_path = "/foo/bar", + is_executable = True, + ) + return DefaultInfo(files = depset([symlink])) + +a = rule(implementation = _a_impl) +EOF + + cat > a/BUILD <<'EOF' +load(":a.bzl", "a") + +a(name="a") +EOF + + bazel build --experimental_allow_unresolved_symlinks //a:a && fail "build succeeded" + [[ "$?" == 1 ]] || fail "unexpected exit code" +} + +function test_executable_symlink() { + mkdir -p a + cat > a/a.bzl <<'EOF' +def _a_impl(ctx): + target = ctx.actions.declare_file(ctx.label.name + ".target") + ctx.actions.write( + output = target, + content = "Hello, World!", + is_executable = True, + ) + + symlink = ctx.actions.declare_file(ctx.label.name + ".link") + ctx.actions.symlink( + output = symlink, + target_file = target, + is_executable = True, + ) + return DefaultInfo(files = depset([symlink])) + +a = rule(implementation = _a_impl) +EOF + + cat > a/BUILD <<'EOF' +load(":a.bzl", "a") + +a(name="a") +EOF + + bazel build //a:a || fail "build failed" + assert_contains "Hello, World!" bazel-bin/a/a.link +} + +function test_executable_symlink_to_nonexecutable_file() { + mkdir -p a + cat > a/a.bzl <<'EOF' +def _a_impl(ctx): + target = ctx.actions.declare_file(ctx.label.name + ".target") + ctx.actions.write( + output = target, + content = "Hello, World!", + ) + + symlink = ctx.actions.declare_file(ctx.label.name + ".link") + ctx.actions.symlink( + output = symlink, + target_file = target, + is_executable = True, + ) + return DefaultInfo(files = depset([symlink])) + +a = rule(implementation = _a_impl) +EOF + + cat > a/BUILD <<'EOF' +load(":a.bzl", "a") + +a(name="a") +EOF + + # bazel build //a:a && fail "build succeeded" + # [[ "$?" == 1 ]] || fail "unexpected exit code" +} + run_suite "Tests for unresolved symlink artifacts"