Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.1.0] Add new flag --enable_workspace that allows us to disable WORKSPACE… #20855

Merged
merged 1 commit into from
Jan 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading