Skip to content

Commit

Permalink
Add new flag --enable_workspace that allows us to disable WORKSPACE…
Browse files Browse the repository at this point in the history 
… handling

* This flag defaults to true
* When turned off (`--noenable_workspace`), no WORKSPACE file reading is ever done (including `WORKSPACE.bzlmod`, `WORKSPACE.resolved`, prefixes, suffixes, etc). No `//external` package is created or used.
* Bzlmod and WORKSPACE can't both be turned off.
* We no longer mandate that a WORKSPACE file has to be at the root of a repo; that is now replaced by `REPO.bazel`. But it's still okay to have a `WORKSPACE` file and no `REPO.bazel` file at the root of a `local_repository`, for example.
* All Bzlmod integration tests now have `--noenable_workspace` set.

Fixes #20185.

RELNOTES: Added a flag `--enable_workspace` (defaults to True) that allows the user to completely disable WORKSPACE logic when turned off.
PiperOrigin-RevId: 589978780
Change-Id: I07a1b8674aaad2758ded57c3a48dba2a19c91e04
  • Loading branch information
@Wyverald @copybara-github
Wyverald authored and copybara-github committed Dec 11, 2023
1 parent 65190f6 commit 47e48a1
Show file tree
Hide file tree
Showing 27 changed files with 172 additions and 289 deletions.
10 changes: 8 additions & 2 deletions ...a/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryFunction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager;
import com.google.devtools.build.lib.bazel.repository.starlark.RepoFetchingSkyKeyComputeState.Signal;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelConstants;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.packages.BazelStarlarkContext;
Expand All @@ -44,6 +45,7 @@
import com.google.devtools.build.lib.skyframe.IgnoredPackagePrefixesValue;
import com.google.devtools.build.lib.skyframe.PrecomputedValue;
import com.google.devtools.build.lib.util.CPU;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.SyscallCache;
Expand Down Expand Up @@ -350,8 +352,12 @@ private RepositoryDirectoryValue.Builder fetchInternal(
new IOException(rule + " must create a directory"), Transience.TRANSIENT);
}

if (!WorkspaceFileHelper.doesWorkspaceFileExistUnder(outputDirectory)) {
createWorkspaceFile(outputDirectory, rule.getTargetKind(), rule.getName());
if (!WorkspaceFileHelper.isValidRepoRoot(outputDirectory)) {
try {
FileSystemUtils.createEmptyFile(outputDirectory.getRelative(LabelConstants.REPO_FILE_NAME));
} catch (IOException e) {
throw new RepositoryFunctionException(e, Transience.TRANSIENT);
}
}

return RepositoryDirectoryValue.builder().setPath(outputDirectory);
Expand Down
8 changes: 7 additions & 1 deletion .../java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryFunction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import com.google.devtools.build.lib.bazel.rules.android.ndkcrosstools.StlImpls;
import com.google.devtools.build.lib.bazel.rules.android.ndkcrosstools.StlImpls.GnuLibStdCppStlImpl;
import com.google.devtools.build.lib.bazel.rules.android.ndkcrosstools.StlImpls.LibCppStlImpl;
import com.google.devtools.build.lib.cmdline.LabelConstants;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.Type;
Expand Down Expand Up @@ -270,7 +271,12 @@ public RepositoryDirectoryValue.Builder fetch(
if (environ == null) {
return null;
}
prepareLocalRepositorySymlinkTree(rule, outputDirectory);
try {
outputDirectory.createDirectoryAndParents();
FileSystemUtils.createEmptyFile(outputDirectory.getRelative(LabelConstants.REPO_FILE_NAME));
} catch (IOException e) {
throw new RepositoryFunctionException(e, Transience.TRANSIENT);
}
WorkspaceAttributeMapper attributes = WorkspaceAttributeMapper.of(rule);
PathFragment pathFragment;
String userDefinedPath = null;
Expand Down
9 changes: 8 additions & 1 deletion .../java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryFunction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.RuleDefinition;
import com.google.devtools.build.lib.bugreport.BugReport;
import com.google.devtools.build.lib.cmdline.LabelConstants;
import com.google.devtools.build.lib.io.InconsistentFilesystemException;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.Type;
Expand All @@ -37,6 +38,7 @@
import com.google.devtools.build.lib.util.ResourceFileLoader;
import com.google.devtools.build.lib.vfs.Dirent;
import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Root;
Expand Down Expand Up @@ -199,7 +201,12 @@ public RepositoryDirectoryValue.Builder fetch(
if (environ == null) {
return null;
}
prepareLocalRepositorySymlinkTree(rule, outputDirectory);
try {
outputDirectory.createDirectoryAndParents();
FileSystemUtils.createEmptyFile(outputDirectory.getRelative(LabelConstants.REPO_FILE_NAME));
} catch (IOException e) {
throw new RepositoryFunctionException(e, Transience.TRANSIENT);
}
WorkspaceAttributeMapper attributes = WorkspaceAttributeMapper.of(rule);
FileSystem fs = directories.getOutputBase().getFileSystem();
Path androidSdkPath;
Expand Down
12 changes: 12 additions & 0 deletions src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,16 @@ public final class BuildLanguageOptions extends OptionsBase {
+ " WORKSPACE. See https://bazel.build/docs/bzlmod for more information.")
public boolean enableBzlmod;

@Option(
name = "enable_workspace",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
effectTags = OptionEffectTag.LOADING_AND_ANALYSIS,
help =
"If true, enables the legacy WORKSPACE system for external dependencies. See"
+ " https://bazel.build/external/overview for more information.")
public boolean enableWorkspace;

@Option(
name = "experimental_isolated_extension_usages",
defaultValue = "false",
Expand Down Expand Up @@ -745,6 +755,7 @@ public StarlarkSemantics toStarlarkSemantics() {
EXPERIMENTAL_ENABLE_ANDROID_MIGRATION_APIS, experimentalEnableAndroidMigrationApis)
.setBool(EXPERIMENTAL_ENABLE_SCL_DIALECT, experimentalEnableSclDialect)
.setBool(ENABLE_BZLMOD, enableBzlmod)
.setBool(ENABLE_WORKSPACE, enableWorkspace)
.setBool(EXPERIMENTAL_ISOLATED_EXTENSION_USAGES, experimentalIsolatedExtensionUsages)
.setBool(
INCOMPATIBLE_EXISTING_RULES_IMMUTABLE_VIEW, incompatibleExistingRulesImmutableView)
Expand Down Expand Up @@ -852,6 +863,7 @@ public StarlarkSemantics toStarlarkSemantics() {
"-experimental_enable_android_migration_apis";
public static final String EXPERIMENTAL_ENABLE_SCL_DIALECT = "-experimental_enable_scl_dialect";
public static final String ENABLE_BZLMOD = "+enable_bzlmod";
public static final String ENABLE_WORKSPACE = "+enable_workspace";
public static final String EXPERIMENTAL_ISOLATED_EXTENSION_USAGES =
"-experimental_isolated_extension_usages";
public static final String INCOMPATIBLE_EXISTING_RULES_IMMUTABLE_VIEW =
Expand Down
53 changes: 12 additions & 41 deletions ...main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,18 +149,21 @@ public SkyValue compute(SkyKey skyKey, Environment env)
}
}

if (rule == null) {
if (rule == null && starlarkSemantics.getBool(BuildLanguageOptions.ENABLE_WORKSPACE)) {
// fallback to look up the repository in the WORKSPACE file.
try {
rule = getRepoRuleFromWorkspace(repositoryName, env);
if (env.valuesMissing()) {
return null;
}
} catch (NoSuchRepositoryException e) {
return new NoRepositoryDirectoryValue(
String.format("Repository '%s' is not defined", repositoryName.getCanonicalForm()));
rule = null;
}
}
if (rule == null) {
return new NoRepositoryDirectoryValue(
String.format("Repository '%s' is not defined", repositoryName.getCanonicalForm()));
}

RepositoryFunction handler = getHandler(rule);
if (handler == null) {
Expand Down Expand Up @@ -411,47 +414,15 @@ public static RepositoryDirectoryValue.Builder symlinkRepoRoot(
Transience.PERSISTENT);
}

// Check that the repository contains a WORKSPACE file.
// Note that we need to do this here since we're not creating a WORKSPACE file ourselves, but
// entrusting the entire contents of the repo root to this target directory.
FileValue workspaceFileValue = getWorkspaceFile(targetDirRootedPath, env);
if (workspaceFileValue == null) {
return null;
}

if (!workspaceFileValue.exists()) {
// Check that the directory contains a repo boundary file.
// Note that we need to do this here since we're not creating a repo boundary file ourselves,
// but entrusting the entire contents of the repo root to this target directory.
if (!WorkspaceFileHelper.isValidRepoRoot(destination)) {
throw new RepositoryFunctionException(
new IOException("No WORKSPACE file found in " + source), Transience.PERSISTENT);
}
return RepositoryDirectoryValue.builder().setPath(source);
}

@Nullable
private static FileValue getWorkspaceFile(RootedPath directory, Environment env)
throws RepositoryFunctionException, InterruptedException {
RootedPath workspaceRootedFile;
try {
workspaceRootedFile = WorkspaceFileHelper.getWorkspaceRootedFile(directory, env);
if (workspaceRootedFile == null) {
return null;
}
} catch (IOException e) {
throw new RepositoryFunctionException(
new IOException(
"Could not determine workspace file (\"WORKSPACE.bazel\" or \"WORKSPACE\"): "
+ e.getMessage()),
new IOException("No MODULE.bazel, REPO.bazel, or WORKSPACE file found in " + destination),
Transience.PERSISTENT);
}
SkyKey workspaceFileKey = FileValue.key(workspaceRootedFile);
FileValue value;
try {
value = (FileValue) env.getValueOrThrow(workspaceFileKey, IOException.class);
} catch (IOException e) {
throw new RepositoryFunctionException(
new IOException("Could not access " + workspaceRootedFile + ": " + e.getMessage()),
Transience.PERSISTENT);
}
return value;
return RepositoryDirectoryValue.builder().setPath(source);
}

// Escape a value for the marker file
Expand Down
30 changes: 0 additions & 30 deletions src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package com.google.devtools.build.lib.rules.repository;

import static com.google.common.base.Preconditions.checkState;
import static java.nio.charset.StandardCharsets.UTF_8;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
Expand Down Expand Up @@ -402,35 +401,6 @@ protected boolean isConfigure(Rule rule) {
return false;
}

protected Path prepareLocalRepositorySymlinkTree(Rule rule, Path repositoryDirectory)
throws RepositoryFunctionException {
try {
repositoryDirectory.createDirectoryAndParents();
} catch (IOException e) {
throw new RepositoryFunctionException(e, Transience.TRANSIENT);
}

// Add x/WORKSPACE.
createWorkspaceFile(repositoryDirectory, rule.getTargetKind(), rule.getName());
return repositoryDirectory;
}

public static void createWorkspaceFile(Path repositoryDirectory, String ruleKind, String ruleName)
throws RepositoryFunctionException {
try {
Path workspaceFile = repositoryDirectory.getRelative(LabelConstants.WORKSPACE_FILE_NAME);
FileSystemUtils.writeContent(
workspaceFile,
UTF_8,
String.format(
"# DO NOT EDIT: automatically generated WORKSPACE file for %s\n"
+ "workspace(name = \"%s\")\n",
ruleKind, ruleName));
} catch (IOException e) {
throw new RepositoryFunctionException(e, Transience.TRANSIENT);
}
}

protected static RepositoryDirectoryValue.Builder writeFile(
Path repositoryDirectory, String filename, String contents)
throws RepositoryFunctionException {
Expand Down
23 changes: 5 additions & 18 deletions src/main/java/com/google/devtools/build/lib/rules/repository/WorkspaceFileHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import com.google.devtools.build.lib.cmdline.LabelConstants;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Root;
import com.google.devtools.build.lib.vfs.RootedPath;
import com.google.devtools.build.skyframe.SkyFunction.Environment;
import com.google.devtools.build.skyframe.SkyKey;
Expand All @@ -27,12 +26,6 @@
/** A class to help dealing with WORKSPACE.bazel and WORKSAPCE file */
public class WorkspaceFileHelper {

public static RootedPath getWorkspaceRootedFile(Root directory, Environment env)
throws IOException, InterruptedException {
return getWorkspaceRootedFile(
RootedPath.toRootedPath(directory, PathFragment.EMPTY_FRAGMENT), env);
}

/**
* Get a RootedPath of the WORKSPACE file we should use for a given directory. This function
* returns a RootedPath to <directory>/WORKSPACE.bazel file if it exists and it's a regular file
Expand Down Expand Up @@ -70,24 +63,18 @@ public static RootedPath getWorkspaceRootedFile(RootedPath directory, Environmen
directory.getRootRelativePath().getRelative(LabelConstants.WORKSPACE_FILE_NAME));
}

public static boolean doesWorkspaceFileExistUnder(Path directory) {
public static boolean isValidRepoRoot(Path directory) {
// Keep in sync with //src/main/cpp/workspace_layout.h
return directory.getRelative(LabelConstants.WORKSPACE_DOT_BAZEL_FILE_NAME).exists()
|| directory.getRelative(LabelConstants.WORKSPACE_FILE_NAME).exists();
}

public static boolean matchWorkspaceFileName(String name) {
return matchWorkspaceFileName(PathFragment.create(name));
|| directory.getRelative(LabelConstants.WORKSPACE_FILE_NAME).exists()
|| directory.getRelative(LabelConstants.MODULE_DOT_BAZEL_FILE_NAME).exists()
|| directory.getRelative(LabelConstants.REPO_FILE_NAME).exists();
}

public static boolean matchWorkspaceFileName(PathFragment name) {
return name.equals(LabelConstants.WORKSPACE_DOT_BAZEL_FILE_NAME)
|| name.equals(LabelConstants.WORKSPACE_FILE_NAME);
}

public static boolean endsWithWorkspaceFileName(PathFragment pathFragment) {
return pathFragment.endsWith(LabelConstants.WORKSPACE_DOT_BAZEL_FILE_NAME)
|| pathFragment.endsWith(LabelConstants.WORKSPACE_FILE_NAME);
}

private WorkspaceFileHelper() {}
}
9 changes: 9 additions & 0 deletions src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,15 @@ private SkyValue getExternalPackage(Environment env)
if (env.valuesMissing()) {
return null;
}
if (!starlarkSemantics.getBool(BuildLanguageOptions.ENABLE_WORKSPACE)) {
throw PackageFunctionException.builder()
.setType(PackageFunctionException.Type.NO_SUCH_PACKAGE)
.setTransience(Transience.PERSISTENT)
.setPackageIdentifier(LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER)
.setMessage("the WORKSPACE file is disabled via --noenable_workspace")
.setPackageLoadingCode(PackageLoading.Code.WORKSPACE_FILE_ERROR)
.build();
}

SkyKey workspaceKey = ExternalPackageFunction.key();
PackageValue workspace = null;
Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)

if (packageKey.equals(LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER)) {
return semantics.getBool(BuildLanguageOptions.EXPERIMENTAL_DISABLE_EXTERNAL_PACKAGE)
|| !semantics.getBool(BuildLanguageOptions.ENABLE_WORKSPACE)
? PackageLookupValue.NO_BUILD_FILE_VALUE
: computeWorkspacePackageLookupValue(env);
}
Expand Down
36 changes: 21 additions & 15 deletions src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
}
RepositoryName repositoryName = ((RepositoryMappingValue.Key) skyKey).repoName();
boolean enableBzlmod = starlarkSemantics.getBool(BuildLanguageOptions.ENABLE_BZLMOD);
boolean enableWorkspace = starlarkSemantics.getBool(BuildLanguageOptions.ENABLE_WORKSPACE);

if (enableBzlmod) {
if (StarlarkBuiltinsValue.isBuiltinsRepo(repositoryName)) {
Expand Down Expand Up @@ -102,7 +103,8 @@ public SkyValue compute(SkyKey skyKey, Environment env)
}

if (repositoryName.isMain()
&& ((RepositoryMappingValue.Key) skyKey).rootModuleShouldSeeWorkspaceRepos()) {
&& ((RepositoryMappingValue.Key) skyKey).rootModuleShouldSeeWorkspaceRepos()
&& enableWorkspace) {
// The root module should be able to see repos defined in WORKSPACE. Therefore, we find all
// workspace repos and add them as extra visible repos in root module's repo mappings.
PackageValue externalPackageValue =
Expand Down Expand Up @@ -154,22 +156,26 @@ public SkyValue compute(SkyKey skyKey, Environment env)
}
}

PackageValue externalPackageValue =
(PackageValue) env.getValue(LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER);
RepositoryMappingValue rootModuleRepoMappingValue =
enableBzlmod
? (RepositoryMappingValue)
env.getValue(RepositoryMappingValue.KEY_FOR_ROOT_MODULE_WITHOUT_WORKSPACE_REPOS)
: null;
if (env.valuesMissing()) {
return null;
if (enableWorkspace) {
PackageValue externalPackageValue =
(PackageValue) env.getValue(LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER);
RepositoryMappingValue rootModuleRepoMappingValue =
enableBzlmod
? (RepositoryMappingValue)
env.getValue(RepositoryMappingValue.KEY_FOR_ROOT_MODULE_WITHOUT_WORKSPACE_REPOS)
: null;
if (env.valuesMissing()) {
return null;
}

RepositoryMapping rootModuleRepoMapping =
rootModuleRepoMappingValue == null
? null
: rootModuleRepoMappingValue.getRepositoryMapping();
return computeFromWorkspace(repositoryName, externalPackageValue, rootModuleRepoMapping);
}

RepositoryMapping rootModuleRepoMapping =
rootModuleRepoMappingValue == null
? null
: rootModuleRepoMappingValue.getRepositoryMapping();
return computeFromWorkspace(repositoryName, externalPackageValue, rootModuleRepoMapping);
throw new RepositoryMappingFunctionException();
}

/**
Expand Down
Loading

0 comments on commit 47e48a1

Please sign in to comment.