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 committed Jan 11, 2024
1 parent 449f230 commit 5627d7f
Show file tree
Hide file tree
Showing 27 changed files with 167 additions and 289 deletions.
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 @@ -337,8 +339,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
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 @@ -281,7 +282,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
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
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 @@ -739,6 +749,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 @@ -846,6 +857,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
Original file line number Diff line number Diff line change
Expand Up @@ -237,12 +237,16 @@ private Rule getRepositoryRule(
}
}

// fallback to look up the repository in the WORKSPACE file.
try {
return getRepoRuleFromWorkspace(repositoryName, env);
} catch (NoSuchRepositoryException e) {
return null;
if (starlarkSemantics.getBool(BuildLanguageOptions.ENABLE_WORKSPACE)) {
// fallback to look up the repository in the WORKSPACE file.
try {
return getRepoRuleFromWorkspace(repositoryName, env);
} catch (NoSuchRepositoryException e) {
return null;
}
}

return null;
}

@Nullable
Expand Down Expand Up @@ -428,47 +432,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()) {
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) {
// 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(
"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
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
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() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,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
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
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 @@ -157,22 +159,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
Original file line number Diff line number Diff line change
Expand Up @@ -59,16 +59,19 @@ public SkyValue compute(SkyKey skyKey, Environment env)
// danger (i.e. going through repo mapping is idempotent).
return WorkspaceNameValue.withName(ruleClassProvider.getRunfilesPrefix());
}
PackageValue externalPackageValue =
(PackageValue) env.getValue(LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER);
if (externalPackageValue == null) {
return null;
}
Package externalPackage = externalPackageValue.getPackage();
if (externalPackage.containsErrors()) {
throw new WorkspaceNameFunctionException();
if (starlarkSemantics.getBool(BuildLanguageOptions.ENABLE_WORKSPACE)) {
PackageValue externalPackageValue =
(PackageValue) env.getValue(LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER);
if (externalPackageValue == null) {
return null;
}
Package externalPackage = externalPackageValue.getPackage();
if (externalPackage.containsErrors()) {
throw new WorkspaceNameFunctionException();
}
return WorkspaceNameValue.withName(externalPackage.getWorkspaceName());
}
return WorkspaceNameValue.withName(externalPackage.getWorkspaceName());
throw new WorkspaceNameFunctionException();
}

private static class WorkspaceNameFunctionException extends SkyFunctionException {
Expand Down
Loading

0 comments on commit 5627d7f

Please sign in to comment.