diff --git a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java index 211963d86225d1..e768f7238caa92 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java +++ b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java @@ -707,6 +707,19 @@ public abstract class BuildLanguageOptions extends OptionsBase { + " attributes of symbolic macros or attribute default values.") public abstract boolean getIncompatibleSimplifyUnconditionalSelectsInRuleAttrs(); + @Option( + name = "incompatible_check_external_repo_source_dir_package_boundary", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS, + effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS}, + metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE}, + help = + "If true, a source directory in an external repository fails the build if it crosses a" + + " package boundary into a sub-package, matching the behavior that already applies" + + " to source directories in the main repository. If false, such package boundary" + + " crossings inside external repositories are not detected.") + public abstract boolean getIncompatibleCheckExternalRepoSourceDirPackageBoundary(); + @Option( name = "experimental_enable_starlark_set", defaultValue = "true", @@ -859,6 +872,9 @@ EXPERIMENTAL_SIBLING_REPOSITORY_LAYOUT, getExperimentalSiblingRepositoryLayout() .setBool( INCOMPATIBLE_ALWAYS_CHECK_DEPSET_ELEMENTS, getIncompatibleAlwaysCheckDepsetElements()) + .setBool( + INCOMPATIBLE_CHECK_EXTERNAL_REPO_SOURCE_DIR_PACKAGE_BOUNDARY, + getIncompatibleCheckExternalRepoSourceDirPackageBoundary()) .setBool(INCOMPATIBLE_DISALLOW_EMPTY_GLOB, getIncompatibleDisallowEmptyGlob()) .setBool( INCOMPATIBLE_PACKAGE_GROUP_HAS_PUBLIC_SYNTAX, @@ -1050,6 +1066,8 @@ public FlagConsumer setBool(String key, boolean ignored) { "-experimental_sibling_repository_layout"; public static final String INCOMPATIBLE_ALWAYS_CHECK_DEPSET_ELEMENTS = "+incompatible_always_check_depset_elements"; + public static final String INCOMPATIBLE_CHECK_EXTERNAL_REPO_SOURCE_DIR_PACKAGE_BOUNDARY = + "-incompatible_check_external_repo_source_dir_package_boundary"; // Note that INCOMPATIBLE_DISALLOW_EMPTY_GLOB differs in Google and in OSS Bazel. public static final String INCOMPATIBLE_DISALLOW_EMPTY_GLOB = "+incompatible_disallow_empty_glob"; diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD index eda02b5252ad9e..aa7dd8e4bf9149 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD @@ -2292,6 +2292,7 @@ java_library( ":dirents", ":filesystem_keys", ":package_lookup_value", + ":precomputed_value", ":sky_functions", ":tree_artifact_value", "//src/main/java/com/google/devtools/build/lib/actions", @@ -2304,6 +2305,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/io:file_symlink_infinite_expansion_uniqueness_function", "//src/main/java/com/google/devtools/build/lib/io:inconsistent_filesystem_exception", "//src/main/java/com/google/devtools/build/lib/packages", + "//src/main/java/com/google/devtools/build/lib/packages/semantics", "//src/main/java/com/google/devtools/build/lib/profiler", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization:visible-for-serialization", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec", @@ -2313,6 +2315,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//src/main/java/com/google/devtools/build/skyframe", "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects", + "//src/main/java/net/starlark/java/eval", "//src/main/protobuf:failure_details_java_proto", "//third_party:auto_value", "//third_party:error_prone_annotations", diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java index 857dbdfced6c3d..f5dbe14d9a63f8 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java @@ -39,6 +39,7 @@ import com.google.devtools.build.lib.io.FileSymlinkInfiniteExpansionUniquenessFunction; import com.google.devtools.build.lib.io.InconsistentFilesystemException; import com.google.devtools.build.lib.packages.BuildFileNotFoundException; +import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.profiler.ProfilerTask; import com.google.devtools.build.lib.profiler.SilentCloseable; @@ -72,6 +73,7 @@ import java.util.List; import java.util.Map; import javax.annotation.Nullable; +import net.starlark.java.eval.StarlarkSemantics; /** A {@link SkyFunction} to build {@link RecursiveFilesystemTraversalValue}s. */ public final class RecursiveFilesystemTraversalFunction implements SkyFunction { @@ -557,6 +559,14 @@ private PkgLookupResult checkIfPackage( // those are symlinks to other locations (e.g. in the case of a local_repository), so all other // roots are part of the main repository. if (rootPath.startsWith(externalDirectory)) { + StarlarkSemantics semantics = PrecomputedValue.STARLARK_SEMANTICS.get(env); + if (semantics == null) { + return null; + } + if (!semantics.getBool( + BuildLanguageOptions.INCOMPATIBLE_CHECK_EXTERNAL_REPO_SOURCE_DIR_PACKAGE_BOUNDARY)) { + return PkgLookupResult.directory(traversal, rootInfo); + } checkState( rootPath.relativeTo(externalDirectory).isSingleSegment(), "%s is expected to be directly under %s", diff --git a/src/test/java/com/google/devtools/build/lib/packages/semantics/ConsistencyTest.java b/src/test/java/com/google/devtools/build/lib/packages/semantics/ConsistencyTest.java index 4e32040d4133aa..ba5057e0daa9f4 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/semantics/ConsistencyTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/semantics/ConsistencyTest.java @@ -140,6 +140,7 @@ private static BuildLanguageOptions buildRandomOptions(Random rand) throws Excep "--experimental_dormant_deps=" + rand.nextBoolean(), "--force_starlark_stack_trace=" + rand.nextBoolean(), "--incompatible_always_check_depset_elements=" + rand.nextBoolean(), + "--incompatible_check_external_repo_source_dir_package_boundary=" + rand.nextBoolean(), "--incompatible_disallow_empty_glob=" + rand.nextBoolean(), "--incompatible_do_not_split_linking_cmdline=" + rand.nextBoolean(), "--incompatible_enable_deprecated_label_apis=" + rand.nextBoolean(), @@ -185,6 +186,9 @@ private static StarlarkSemantics buildRandomSemantics(Random rand) { .setBool(BuildLanguageOptions.EXPERIMENTAL_DORMANT_DEPS, rand.nextBoolean()) .setBool(StarlarkSemantics.FORCE_STARLARK_STACK_TRACE, rand.nextBoolean()) .setBool(BuildLanguageOptions.INCOMPATIBLE_ALWAYS_CHECK_DEPSET_ELEMENTS, rand.nextBoolean()) + .setBool( + BuildLanguageOptions.INCOMPATIBLE_CHECK_EXTERNAL_REPO_SOURCE_DIR_PACKAGE_BOUNDARY, + rand.nextBoolean()) .setBool(BuildLanguageOptions.INCOMPATIBLE_DISALLOW_EMPTY_GLOB, rand.nextBoolean()) .setBool(BuildLanguageOptions.INCOMPATIBLE_DO_NOT_SPLIT_LINKING_CMDLINE, rand.nextBoolean()) .setBool(BuildLanguageOptions.INCOMPATIBLE_ENABLE_DEPRECATED_LABEL_APIS, rand.nextBoolean()) diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/SourceDirectoryIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/SourceDirectoryIntegrationTest.java index b15b4d63e49d6f..5d40515d4a5a8b 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/SourceDirectoryIntegrationTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/SourceDirectoryIntegrationTest.java @@ -299,6 +299,74 @@ public void crossingRepoPackageCollision_doesNotFail() throws Exception { buildTarget("//:consume"); } + /** + * With --incompatible_check_external_repo_source_dir_package_boundary, a source directory in an + * external repository that crosses a package boundary into a sub-package of that same repository + * fails the build, just like in the main repository. + */ + @Test + public void crossingExternalRepoPackageBoundary_withFlag_fails() throws Exception { + if (!AnalysisMock.get().isThisBazel()) { + return; + } + setUpCrossingExternalRepoPackageBoundary(); + addOptions("--incompatible_check_external_repo_source_dir_package_boundary"); + + assertThrows(BuildFailedException.class, () -> buildTarget("//:consume")); + assertContainsEvent("crosses package boundary into package rooted at data/sub"); + } + + /** + * Without --incompatible_check_external_repo_source_dir_package_boundary (the default), the same + * package boundary crossing inside an external repository is not detected and the build succeeds. + */ + @Test + public void crossingExternalRepoPackageBoundary_withoutFlag_doesNotFail() throws Exception { + if (!AnalysisMock.get().isThisBazel()) { + return; + } + setUpCrossingExternalRepoPackageBoundary(); + + buildTarget("//:consume"); + } + + private void setUpCrossingExternalRepoPackageBoundary() throws Exception { + write( + "MODULE.bazel", + """ + bazel_dep(name = "ext") + local_path_override( + module_name = "ext", + path = "ext", + ) + """); + write("ext/MODULE.bazel", "module(name = 'ext')"); + write( + "ext/BUILD", + """ + filegroup( + name = "dir", + srcs = ["data"], + visibility = ["//visibility:public"], + ) + """); + // The presence of this BUILD file makes ext's `data/sub` a package, so the `data` source directory + // crosses a package boundary. + write("ext/data/sub/BUILD"); + writeIsoLatin1(getWorkspace().getRelative("ext/data/top"), "content"); + + write( + "BUILD", + """ + genrule( + name = "consume", + srcs = ["@ext//:dir"], + outs = ["consume.out"], + cmd = "touch $@", + ) + """); + } + private static final String GENRULE_EVENT = "Executing genrule //foo:foo"; private void assertInvalidatedByBuild() throws Exception {