Skip to content

Commit

Permalink
bzlmod: Handle non-registry overrides in ModuleFileFunction
Browse files Browse the repository at this point in the history 
(#13316)

Now that we can properly fetch repos with non-registry overrides in RepositoryDelegatorFunction, we can also handle them correctly during discovery.

PiperOrigin-RevId: 384889161
  • Loading branch information
@Wyverald @copybara-github
Wyverald authored and copybara-github committed Jul 15, 2021
1 parent 44fd4a8 commit d382e91
Show file tree
Hide file tree
Showing 5 changed files with 223 additions and 7 deletions.
3 changes: 2 additions & 1 deletion src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,15 +68,16 @@ java_library(
":common",
":registry",
"//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
"//src/main/java/com/google/devtools/build/lib/cmdline:cmdline-primitives",
"//src/main/java/com/google/devtools/build/lib/concurrent",
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/packages",
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_directory_value",
"//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value",
"//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
"//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/repository",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//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",
Expand Down
29 changes: 25 additions & 4 deletions src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,15 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.actions.FileValue;
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.NoSuchThingException;
import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue;
import com.google.devtools.build.lib.skyframe.PrecomputedValue;
import com.google.devtools.build.lib.skyframe.PrecomputedValue.Precomputed;
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;
import com.google.devtools.build.lib.vfs.RootedPath;
import com.google.devtools.build.skyframe.SkyFunction;
Expand Down Expand Up @@ -110,7 +112,8 @@ public SkyValue compute(SkyKey skyKey, Environment env)
private SkyValue computeForRootModule(StarlarkSemantics starlarkSemantics, Environment env)
throws SkyFunctionException, InterruptedException {
RootedPath moduleFilePath =
RootedPath.toRootedPath(Root.fromPath(workspaceRoot), PathFragment.create("MODULE.bazel"));
RootedPath.toRootedPath(
Root.fromPath(workspaceRoot), LabelConstants.MODULE_DOT_BAZEL_FILE_NAME);
if (env.getValue(FileValue.key(moduleFilePath)) == null) {
return null;
}
Expand Down Expand Up @@ -166,8 +169,26 @@ private GetModuleFileResult getModuleFile(
// If there is a non-registry override for this module, we need to fetch the corresponding repo
// first and read the module file from there.
if (override instanceof NonRegistryOverride) {
// TODO(wyv): implement
throw errorf("Non-registry overrides are not implemented yet");
// The canonical repo name of a module with a non-registry override is always the name of the
// module.
String repoName = key.getName();
RepositoryDirectoryValue repoDir =
(RepositoryDirectoryValue)
env.getValue(
RepositoryDirectoryValue.key(
RepositoryName.createFromValidStrippedName(repoName)));
if (repoDir == null) {
return null;
}
RootedPath moduleFilePath =
RootedPath.toRootedPath(
Root.fromPath(repoDir.getPath()), LabelConstants.MODULE_DOT_BAZEL_FILE_NAME);
if (env.getValue(FileValue.key(moduleFilePath)) == null) {
return null;
}
GetModuleFileResult result = new GetModuleFileResult();
result.moduleFileContents = readFile(moduleFilePath.asPath());
return result;
}

// Otherwise, we should get the module file from a registry.
Expand Down
3 changes: 3 additions & 0 deletions src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,13 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/bazel/repository/starlark",
"//src/main/java/com/google/devtools/build/lib/packages",
"//src/main/java/com/google/devtools/build/lib/pkgcache",
"//src/main/java/com/google/devtools/build/lib/rules:repository/local_repository_rule",
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function",
"//src/main/java/com/google/devtools/build/lib/skyframe:bzl_compile",
"//src/main/java/com/google/devtools/build/lib/skyframe:containing_package_lookup_function",
"//src/main/java/com/google/devtools/build/lib/skyframe:file_function",
"//src/main/java/com/google/devtools/build/lib/skyframe:ignored_package_prefixes_function",
"//src/main/java/com/google/devtools/build/lib/skyframe:managed_directories_knowledge",
"//src/main/java/com/google/devtools/build/lib/skyframe:package_lookup_function",
"//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_function",
"//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value",
Expand Down
101 changes: 100 additions & 1 deletion src/test/java/com/google/devtools/build/lib/bazel/bzlmod/DiscoveryFunctionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +21,33 @@

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.FileStateValue;
import com.google.devtools.build.lib.actions.FileValue;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
import com.google.devtools.build.lib.analysis.ServerDirectories;
import com.google.devtools.build.lib.analysis.util.AnalysisMock;
import com.google.devtools.build.lib.bazel.repository.starlark.StarlarkRepositoryModule;
import com.google.devtools.build.lib.packages.PackageFactory;
import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
import com.google.devtools.build.lib.rules.repository.LocalRepositoryFunction;
import com.google.devtools.build.lib.rules.repository.LocalRepositoryRule;
import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction;
import com.google.devtools.build.lib.rules.repository.RepositoryFunction;
import com.google.devtools.build.lib.skyframe.BazelSkyframeExecutorConstants;
import com.google.devtools.build.lib.skyframe.BzlmodRepoRuleFunction;
import com.google.devtools.build.lib.skyframe.ExternalFilesHelper;
import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction;
import com.google.devtools.build.lib.skyframe.FileFunction;
import com.google.devtools.build.lib.skyframe.FileStateFunction;
import com.google.devtools.build.lib.skyframe.ManagedDirectoriesKnowledge;
import com.google.devtools.build.lib.skyframe.PrecomputedFunction;
import com.google.devtools.build.lib.skyframe.PrecomputedValue;
import com.google.devtools.build.lib.skyframe.SkyFunctions;
import com.google.devtools.build.lib.starlarkbuildapi.repository.RepositoryBootstrap;
import com.google.devtools.build.lib.testutil.FoundationTestCase;
import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.Root;
Expand All @@ -49,6 +61,8 @@
import com.google.devtools.build.skyframe.SequentialBuildDriver;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunctionName;
import java.util.Optional;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
import net.starlark.java.eval.StarlarkSemantics;
import org.junit.Before;
Expand Down Expand Up @@ -90,7 +104,20 @@ public void setup() throws Exception {
packageLocator,
ExternalFileAction.DEPEND_ON_EXTERNAL_PKG_FOR_EXTERNAL_REPO_PATHS,
directories);
ConfiguredRuleClassProvider.Builder builder = new ConfiguredRuleClassProvider.Builder();
TestRuleClassProvider.addStandardRules(builder);
builder
.clearWorkspaceFileSuffixForTesting()
.addStarlarkBootstrap(new RepositoryBootstrap(new StarlarkRepositoryModule()));
ConfiguredRuleClassProvider ruleClassProvider = builder.build();

PackageFactory packageFactory =
AnalysisMock.get()
.getPackageFactoryBuilderForTesting(directories)
.build(ruleClassProvider, fileSystem);

ImmutableMap<String, RepositoryFunction> repositoryHandlers =
ImmutableMap.of(LocalRepositoryRule.NAME, new LocalRepositoryFunction());
MemoizingEvaluator evaluator =
new InMemoryMemoizingEvaluator(
ImmutableMap.<SkyFunctionName, SkyFunction>builder()
Expand All @@ -106,11 +133,39 @@ public void setup() throws Exception {
SkyFunctions.MODULE_FILE,
new ModuleFileFunction(registryFactory, workspaceRoot))
.put(SkyFunctions.PRECOMPUTED, new PrecomputedFunction())
.put(
SkyFunctions.REPOSITORY_DIRECTORY,
new RepositoryDelegatorFunction(
repositoryHandlers,
null,
new AtomicBoolean(true),
ImmutableMap::of,
directories,
ManagedDirectoriesKnowledge.NO_MANAGED_DIRECTORIES,
BazelSkyframeExecutorConstants.EXTERNAL_PACKAGE_HELPER))
.put(
BzlmodRepoRuleValue.BZLMOD_REPO_RULE,
new BzlmodRepoRuleFunction(
packageFactory,
ruleClassProvider,
directories,
new BzlmodRepoRuleHelperImpl()))
.build(),
differencer);
driver = new SequentialBuildDriver(evaluator);

PrecomputedValue.STARLARK_SEMANTICS.set(differencer, StarlarkSemantics.DEFAULT);
RepositoryDelegatorFunction.REPOSITORY_OVERRIDES.set(differencer, ImmutableMap.of());
RepositoryDelegatorFunction.DEPENDENCY_FOR_UNCONDITIONAL_FETCHING.set(
differencer, RepositoryDelegatorFunction.DONT_FETCH_UNCONDITIONALLY);
PrecomputedValue.PATH_PACKAGE_LOCATOR.set(differencer, packageLocator.get());
RepositoryDelegatorFunction.RESOLVED_FILE_INSTEAD_OF_WORKSPACE.set(
differencer, Optional.empty());
PrecomputedValue.REPO_ENV.set(differencer, ImmutableMap.of());
RepositoryDelegatorFunction.OUTPUT_VERIFICATION_REPOSITORY_RULES.set(
differencer, ImmutableSet.of());
RepositoryDelegatorFunction.RESOLVED_FILE_FOR_VERIFICATION.set(differencer, Optional.empty());
RepositoryDelegatorFunction.ENABLE_BZLMOD.set(differencer, true);
}

@Test
Expand Down Expand Up @@ -357,5 +412,49 @@ public void testRegistryOverride() throws Exception {
.build());
}

// TODO(wyv): test local path override
@Test
public void testLocalPathOverride() throws Exception {
Path pathToC = scratch.dir("/pathToC");
scratch.file(
pathToC.getRelative("MODULE.bazel").getPathString(), "module(name='C',version='2.0')");
scratch.file(pathToC.getRelative("WORKSPACE").getPathString());
scratch.file(
workspaceRoot.getRelative("MODULE.bazel").getPathString(),
"module(name='A',version='0.1')",
"bazel_dep(name='B',version='0.1')",
"local_path_override(module_name='C',path='" + pathToC.getPathString() + "')");
FakeRegistry registry =
registryFactory
.newFakeRegistry()
.addModule(
createModuleKey("B", "0.1"),
"module(name='B', version='0.1');bazel_dep(name='C',version='1.0')")
.addModule(createModuleKey("C", "1.0"), "module(name='C', version='1.0');");
ModuleFileFunction.REGISTRIES.set(differencer, ImmutableList.of(registry.getUrl()));

EvaluationResult<DiscoveryValue> result =
driver.evaluate(ImmutableList.of(DiscoveryValue.KEY), evaluationContext);
if (result.hasError()) {
fail(result.getError().toString());
}
DiscoveryValue discoveryValue = result.get(DiscoveryValue.KEY);
assertThat(discoveryValue.getRootModuleName()).isEqualTo("A");
assertThat(discoveryValue.getDepGraph())
.containsExactly(
createModuleKey("A", ""),
Module.builder()
.setName("A")
.setVersion(Version.parse("0.1"))
.addDep("B", createModuleKey("B", "0.1"))
.build(),
createModuleKey("B", "0.1"),
Module.builder()
.setName("B")
.setVersion(Version.parse("0.1"))
.addDep("C", createModuleKey("C", ""))
.setRegistry(registry)
.build(),
createModuleKey("C", ""),
Module.builder().setName("C").setVersion(Version.parse("2.0")).build());
}
}
Loading

0 comments on commit d382e91

Please sign in to comment.