Default --incompatible_disallow_unsound_directory_outputs to true.
The downstream pipeline didn't detect any breakage.
A few tests need to explicitly set the flag to false, or be rewritten to not rely on directory-producing genrules.
Progress on #18646.
PiperOrigin-RevId: 568852835
Change-Id: Ib7508be7f9c6c9448435145a11149c70fbbfa854
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java b/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java
index 3e9d941..5e5754b 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java
@@ -174,7 +174,7 @@
@Option(
name = "incompatible_disallow_unsound_directory_outputs",
- defaultValue = "false",
+ defaultValue = "true",
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
metadataTags = OptionMetadataTag.INCOMPATIBLE_CHANGE,
effectTags = {OptionEffectTag.BAZEL_INTERNAL_CONFIGURATION},
diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/DirectoryArtifactWarningTest.java b/src/test/java/com/google/devtools/build/lib/buildtool/DirectoryArtifactWarningTest.java
index 0ffb744..3e1d5c0 100644
--- a/src/test/java/com/google/devtools/build/lib/buildtool/DirectoryArtifactWarningTest.java
+++ b/src/test/java/com/google/devtools/build/lib/buildtool/DirectoryArtifactWarningTest.java
@@ -40,6 +40,7 @@
public void testOutputArtifactDirectoryWarning_forGenrule() throws Exception {
setupGenruleWithOutputArtifactDirectory();
+ addOptions("--noincompatible_disallow_unsound_directory_outputs");
buildTarget("//x");
events.assertContainsWarning(
@@ -81,6 +82,7 @@
public void testOutputArtifactDirectoryWarning_forStarlarkRule() throws Exception {
setupStarlarkRuleWithOutputArtifactDirectory();
+ addOptions("--noincompatible_disallow_unsound_directory_outputs");
buildTarget("//x");
events.assertContainsWarning(
diff --git a/src/test/java/com/google/devtools/build/lib/metrics/MetricsCollectorTest.java b/src/test/java/com/google/devtools/build/lib/metrics/MetricsCollectorTest.java
index 14ca0f2..106c926 100644
--- a/src/test/java/com/google/devtools/build/lib/metrics/MetricsCollectorTest.java
+++ b/src/test/java/com/google/devtools/build/lib/metrics/MetricsCollectorTest.java
@@ -75,9 +75,8 @@
"foo/BUILD",
"genrule(",
" name = 'foo',",
- " outs = ['dir'],",
- " cmd = '/bin/mkdir $(location dir)',",
- " srcs = [],",
+ " outs = ['out'],",
+ " cmd = 'touch $@',",
")");
}
@@ -577,9 +576,9 @@
"foo/BUILD",
"genrule(",
" name = 'foo',",
- " outs = ['dir'],",
+ " outs = ['out'],",
" srcs = ['//noexist:noexist'],",
- " cmd = '/bin/mkdir $(location dir)',",
+ " cmd = 'touch $@',",
")");
addOptions("--analyze");
@@ -591,7 +590,7 @@
"foo/BUILD",
"genrule(",
" name = 'foo',",
- " outs = ['dir'],",
+ " outs = ['out'],",
" cmd = '/bin/false',",
")");
@@ -628,15 +627,13 @@
"foo/BUILD",
"genrule(",
" name = 'foo',",
- " outs = ['dir'],",
- " cmd = '/bin/mkdir $(location dir)',",
- " srcs = [],",
+ " outs = ['out'],",
+ " cmd = 'touch $@',",
")",
"genrule(",
" name = 'bar',",
- " outs = ['dir2'],",
- " cmd = '/bin/mkdir $(location dir2)',",
- " srcs = [],",
+ " outs = ['out2'],",
+ " cmd = 'touch $@',",
")");
addOptions("--experimental_merged_skyframe_analysis_execution");
BuildGraphMetrics expected =
diff --git a/src/test/py/bazel/bazel_windows_test.py b/src/test/py/bazel/bazel_windows_test.py
index 91b3da9..f7a5eb5 100644
--- a/src/test/py/bazel/bazel_windows_test.py
+++ b/src/test/py/bazel/bazel_windows_test.py
@@ -262,25 +262,57 @@
env_add={'BAZEL_VC': 'C:/not/exists/VC'},
)
- def testDeleteReadOnlyFileAndDirectory(self):
+ def testDeleteReadOnlyFile(self):
self.CreateWorkspaceWithDefaultRepos('WORKSPACE')
- self.ScratchFile('BUILD', [
- 'genrule(',
- ' name = "gen_read_only_dir",',
- ' cmd_bat = "mkdir $@ && attrib +r $@",',
- ' outs = ["dir_foo"],',
- ')',
- '',
- 'genrule(',
- ' name = "gen_read_only_file",',
- ' cmd_bat = "echo hello > $@ && attrib +r $@",',
- ' outs = ["file_foo"],',
- ')',
- ])
+ self.ScratchFile(
+ 'BUILD',
+ [
+ 'genrule(',
+ ' name = "gen_read_only_file",',
+ ' cmd_bat = "echo hello > $@ && attrib +r $@",',
+ ' outs = ["file_foo"],',
+ ')',
+ ],
+ )
self.RunBazel(['build', '//...'])
self.RunBazel(['clean'])
+ def testDeleteReadOnlyDirectory(self):
+ self.CreateWorkspaceWithDefaultRepos('WORKSPACE')
+ self.ScratchFile(
+ 'defs.bzl',
+ [
+ 'def _impl(ctx):',
+ ' dir = ctx.actions.declare_directory(ctx.label.name)',
+ ' bat = ctx.actions.declare_file(ctx.label.name + ".bat")',
+ ' ctx.actions.write(',
+ ' output = bat,',
+ ' content = "attrib +r " + dir.path,',
+ ' is_executable = True,',
+ ' )',
+ ' ctx.actions.run(',
+ ' outputs = [dir],',
+ ' executable = bat,',
+ ' use_default_shell_env = True,',
+ ' )',
+ ' return DefaultInfo(files = depset([dir]))',
+ 'read_only_dir = rule(_impl)',
+ ],
+ )
+ self.ScratchFile(
+ 'BUILD',
+ [
+ 'load(":defs.bzl", "read_only_dir")',
+ 'read_only_dir(',
+ ' name = "gen_read_only_dir",',
+ ')',
+ ],
+ )
+
+ self.RunBazel(['build', '--subcommands', '--verbose_failures', '//...'])
+ self.RunBazel(['clean'])
+
def testBuildJavaTargetWithClasspathJar(self):
self.CreateWorkspaceWithDefaultRepos('WORKSPACE')
self.ScratchFile('BUILD', [
diff --git a/src/test/shell/bazel/bazel_hermetic_sandboxing_test.sh b/src/test/shell/bazel/bazel_hermetic_sandboxing_test.sh
index 9515c3a..215a7a8 100755
--- a/src/test/shell/bazel/bazel_hermetic_sandboxing_test.sh
+++ b/src/test/shell/bazel/bazel_hermetic_sandboxing_test.sh
@@ -369,7 +369,8 @@
# Test that the sandbox is able to handle various types of artifacts.
# Regression test for Issue #15340
function test_other_artifacts() {
- bazel build examples/hermetic:other_artifacts &> $TEST_log
+ bazel build --noincompatible_disallow_unsound_directory_outputs \
+ examples/hermetic:other_artifacts &> $TEST_log
assert_contains ".regular_file_artifact" "bazel-bin/examples/hermetic/other_artifacts.result"
assert_contains ".unresolved_symlink_artifact" "bazel-bin/examples/hermetic/other_artifacts.result"
assert_contains ".directory_artifact" "bazel-bin/examples/hermetic/other_artifacts.result"
diff --git a/src/test/shell/bazel/remote/remote_execution_test.sh b/src/test/shell/bazel/remote/remote_execution_test.sh
index 3b903f2..2818dcc 100755
--- a/src/test/shell/bazel/remote/remote_execution_test.sh
+++ b/src/test/shell/bazel/remote/remote_execution_test.sh
@@ -936,6 +936,7 @@
bazel build \
--incompatible_remote_symlinks \
--noincompatible_remote_disallow_symlink_in_tree_artifact \
+ --noincompatible_disallow_unsound_directory_outputs \
--remote_executor=grpc://localhost:${worker_port} \
--remote_download_all \
--spawn_strategy=remote \
@@ -961,6 +962,7 @@
bazel build \
--incompatible_remote_symlinks \
--noincompatible_remote_disallow_symlink_in_tree_artifact \
+ --noincompatible_disallow_unsound_directory_outputs \
--remote_cache=grpc://localhost:${worker_port} \
--remote_download_all \
--spawn_strategy=local \
@@ -971,6 +973,7 @@
bazel build \
--incompatible_remote_symlinks \
--noincompatible_remote_disallow_symlink_in_tree_artifact \
+ --noincompatible_disallow_unsound_directory_outputs \
--remote_cache=grpc://localhost:${worker_port} \
--remote_download_all \
--spawn_strategy=local \
diff --git a/src/test/shell/integration/force_delete_output_test.sh b/src/test/shell/integration/force_delete_output_test.sh
index fd2834f..5beeac8 100755
--- a/src/test/shell/integration/force_delete_output_test.sh
+++ b/src/test/shell/integration/force_delete_output_test.sh
@@ -60,13 +60,28 @@
function test_delete_tree_in_unwritable_dir() {
mkdir -p x || fail "Can't create x"
+ cat > x/defs.bzl << 'EOF'
+def _impl(ctx):
+ dir = ctx.actions.declare_directory("unwritable/somedir")
+ ctx.actions.run_shell(
+ outputs = [dir],
+ command = "mkdir -p $1 && echo $2 $1/somefile && chmod a-w $(dirname $1)",
+ arguments = [dir.path, ctx.attr.content],
+ execution_requirements = {"local": "1"},
+ )
+ return DefaultInfo(files = depset([dir]))
+
+tree_in_unwritable_dir = rule(
+ implementation = _impl,
+ attrs = {"content": attr.string()},
+)
+EOF
+
cat > x/BUILD << 'EOF'
-genrule(
- name = "unwritable",
- srcs = [],
- outs = ["unwritable/somedir"],
- local = 1,
- cmd = "mkdir -p $@; echo 'some output' > $@/somefile.out; chmod a-w $$(dirname $@)"
+load(":defs.bzl", "tree_in_unwritable_dir")
+tree_in_unwritable_dir(
+ name = "unwritable",
+ content = "foo",
)
EOF
@@ -75,12 +90,10 @@
# Now modify the build file to force a rebuild while creating the same output
# within the write-protected directory.
cat > x/BUILD << 'EOF'
-genrule(
+load(":defs.bzl", "tree_in_unwritable_dir")
+tree_in_unwritable_dir(
name = "unwritable",
- srcs = [],
- outs = ["unwritable/somedir"],
- local = 1,
- cmd = "mkdir -p $@; echo 'some other output' > $@/somefile.out; chmod a-w $$(dirname $@)"
+ content = "bar",
)
EOF
