Skip to content

Commit

Permalink
Allow //external package in non-main repos
Browse files Browse the repository at this point in the history
  • Loading branch information
fmeum committed Oct 29, 2024
1 parent 419fdac commit 0ca0334
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 30 deletions.
18 changes: 12 additions & 6 deletions src/main/java/com/google/devtools/build/lib/cmdline/Label.java
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,7 @@ public final class Label implements Comparable<Label>, StarlarkValue, SkyKey, Co
// Used for select's `//conditions:default` label (not a target)
"conditions",
// Used for the public and private visibility labels (not targets)
"visibility",
// There is only one //external package
LabelConstants.EXTERNAL_PACKAGE_NAME.getPathString());
"visibility");

// Intern "__pkg__" and "__subpackages__" pseudo-targets, which appears in labels used for
// visibility specifications. This saves a couple tenths of a percent of RAM off the loading
Expand Down Expand Up @@ -181,9 +179,17 @@ private static RepositoryName computeRepoNameWithRepoContext(
if (parts.repo() == null) {
// Certain package names when used without a "@" part are always absolutely in the main repo,
// disregarding the current repo and repo mappings.
return ABSOLUTE_PACKAGE_NAMES.contains(parts.pkg())
? RepositoryName.MAIN
: repoContext.currentRepo();
if (ABSOLUTE_PACKAGE_NAMES.contains(parts.pkg())) {
return RepositoryName.MAIN;
}
// The //external package can only be referenced by the main repo or by external repositories
// that don't use strict visibility. For the main module repoContext.currentRepo() is equal to
// RepositoryName.MAIN.
if (LabelConstants.EXTERNAL_PACKAGE_NAME.getPathString().equals(parts.pkg())
&& repoContext.repoMapping().ownerRepo() == null) {
return RepositoryName.MAIN;
}
return repoContext.currentRepo();
}
if (parts.repoIsCanonical()) {
// This label uses the canonical label literal syntax starting with two @'s ("@@foo//bar").
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,20 +253,6 @@ public String getOwnerRepoDisplayString() {
}
}

// Must only be called if isVisible() returns true.
public String getOwnerModuleDisplayString() {
Preconditions.checkNotNull(ownerRepoIfNotVisible);
if (ownerRepoIfNotVisible.isMain()) {
return "root module";
} else {
return String.format(
"module '%s'",
ownerRepoIfNotVisible
.getName()
.substring(0, ownerRepoIfNotVisible.getName().indexOf('+')));
}
}

/** Returns if this is the main repository. */
public boolean isMain() {
return equals(MAIN);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -282,9 +282,11 @@ private Iterable<SkyKey> getSubdirDeps(
}
String basename = dirent.getName();
PathFragment subdirectory = rootRelativePath.getRelative(basename);
if (!siblingRepositoryLayout && subdirectory.equals(LabelConstants.EXTERNAL_PACKAGE_NAME)) {
// Subpackages under //external can be processed only when
// --experimental_sibling_repository_layout is set.
if (!siblingRepositoryLayout
&& subdirectory.equals(LabelConstants.EXTERNAL_PACKAGE_NAME)
&& !repositoryName.isMain()) {
// Subpackages under //external in the main repo can be processed only
// when --experimental_sibling_repository_layout is set.
continue;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@
import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileFunction;
import com.google.devtools.build.lib.clock.BlazeClock;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelConstants;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
Expand Down Expand Up @@ -1278,12 +1277,7 @@ protected Rule scratchRule(String packageName, String ruleName, String... lines)
throws Exception {
// Allow to create the BUILD file also in the top package.
String buildFilePathString = packageName.isEmpty() ? "BUILD" : packageName + "/BUILD";
if (packageName.equals(LabelConstants.EXTERNAL_PACKAGE_NAME.getPathString())) {
buildFilePathString = "WORKSPACE";
scratch.overwriteFile(buildFilePathString, lines);
} else {
scratch.file(buildFilePathString, lines);
}
scratch.file(buildFilePathString, lines);
skyframeExecutor.invalidateFilesUnderPathForTesting(
reporter,
new ModifiedFileSet.Builder().modify(PathFragment.create(buildFilePathString)).build(),
Expand Down
56 changes: 56 additions & 0 deletions src/test/shell/bazel/execroot_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -222,4 +222,60 @@ EOF
|| fail "build failed"
}

function test_external_package_in_other_repo() {
cat > $(setup_module_dot_bazel) <<EOF
local_repository = use_repo_rule("@bazel_tools//tools/build_defs/repo:local.bzl", "local_repository")
local_repository(
name="other_repo",
path="other_repo",
)
EOF

mkdir -p other_repo/external/java/a
touch other_repo/REPO.bazel

cat > other_repo/external/java/a/BUILD <<EOF
java_library(name='a', srcs=['A.java'])
EOF

cat > other_repo/external/java/a/A.java << EOF
package a;
public class A {
public static void main(String[] args) {
System.out.println("hello world");
}
}
EOF

bazel build @other_repo//external/java/a:a || fail "build failed"
}

function test_external_dir_in_other_repo() {
cat > $(setup_module_dot_bazel) <<EOF
local_repository = use_repo_rule("@bazel_tools//tools/build_defs/repo:local.bzl", "local_repository")
local_repository(
name="other_repo",
path="other_repo",
)
EOF

mkdir -p other_repo/external/java/a
touch other_repo/REPO.bazel

cat > other_repo/BUILD <<EOF
java_library(name='a', srcs=['external/java/a/A.java'])
EOF

cat > other_repo/external/java/a/A.java << EOF
package a;
public class A {
public static void main(String[] args) {
System.out.println("hello world");
}
}
EOF

bazel build @other_repo//:a || fail "build failed"
}

run_suite "execution root tests"

0 comments on commit 0ca0334

Please sign in to comment.