Skip to content

Commit

Permalink
bzlmod: Remove overrides from DiscoveryValue and SelectionValue
Browse files Browse the repository at this point in the history
(#13316)

The overrides cannot be nicely encapsulated in SelectionValue because we need the overrides when fetching modules with non-registry overrides, which happens during discovery. Any consumers of overrides should get them from the ModuleFileValue of the root module directly instead.

PiperOrigin-RevId: 384236547
  • Loading branch information
Wyverald authored and copybara-github committed Jul 12, 2021
1 parent ec4be00 commit 6501f30
Show file tree
Hide file tree
Showing 6 changed files with 7 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
if (env.valuesMissing()) {
return null;
}
return DiscoveryValue.create(
root.getModule().getName(), ImmutableMap.copyOf(depGraph), overrides);
return DiscoveryValue.create(root.getModule().getName(), ImmutableMap.copyOf(depGraph));
}

private static Module rewriteDepKeys(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,11 @@ public abstract class DiscoveryValue implements SkyValue {
@AutoCodec public static final SkyKey KEY = () -> SkyFunctions.DISCOVERY;

public static DiscoveryValue create(
String rootModuleName,
ImmutableMap<ModuleKey, Module> depGraph,
ImmutableMap<String, ModuleOverride> overrides) {
return new AutoValue_DiscoveryValue(rootModuleName, depGraph, overrides);
String rootModuleName, ImmutableMap<ModuleKey, Module> depGraph) {
return new AutoValue_DiscoveryValue(rootModuleName, depGraph);
}

public abstract String getRootModuleName();

public abstract ImmutableMap<ModuleKey, Module> getDepGraph();

public abstract ImmutableMap<String, ModuleOverride> getOverrides();
}
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
throw new SelectionFunctionException(e);
}

return SelectionValue.create(
discovery.getRootModuleName(), walker.getNewDepGraph(), discovery.getOverrides());
return SelectionValue.create(discovery.getRootModuleName(), walker.getNewDepGraph());
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,11 @@ public abstract class SelectionValue implements SkyValue {
@AutoCodec public static final SkyKey KEY = () -> SkyFunctions.SELECTION;

public static SelectionValue create(
String rootModuleName,
ImmutableMap<ModuleKey, Module> depGraph,
ImmutableMap<String, ModuleOverride> overrides) {
return new AutoValue_SelectionValue(rootModuleName, depGraph, overrides);
String rootModuleName, ImmutableMap<ModuleKey, Module> depGraph) {
return new AutoValue_SelectionValue(rootModuleName, depGraph);
}

public abstract String getRootModuleName();

public abstract ImmutableMap<ModuleKey, Module> getDepGraph();

public abstract ImmutableMap<String, ModuleOverride> getOverrides();
}
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,6 @@ public void testSimpleDiamond() throws Exception {
.build(),
ModuleKey.create("D", "3.0"),
Module.builder().setName("D").setVersion("3.0").setRegistry(registry).build());
assertThat(discoveryValue.getOverrides()).isEmpty();
}

@Test
Expand Down Expand Up @@ -212,7 +211,6 @@ public void testCircularDependency() throws Exception {
.addDep("B", ModuleKey.create("B", "1.0"))
.setRegistry(registry)
.build());
assertThat(discoveryValue.getOverrides()).isEmpty();
}

@Test
Expand Down Expand Up @@ -252,7 +250,6 @@ public void testCircularDependencyOnRootModule() throws Exception {
.addDep("A", ModuleKey.create("A", ""))
.setRegistry(registry)
.build());
assertThat(discoveryValue.getOverrides()).isEmpty();
}

@Test
Expand Down Expand Up @@ -296,8 +293,6 @@ public void testSingleVersionOverride() throws Exception {
.build(),
ModuleKey.create("C", "2.0"),
Module.builder().setName("C").setVersion("2.0").setRegistry(registry).build());
assertThat(discoveryValue.getOverrides())
.containsExactly("C", SingleVersionOverride.create("2.0", "", ImmutableList.of(), 0));
}

@Test
Expand Down Expand Up @@ -351,9 +346,6 @@ public void testRegistryOverride() throws Exception {
.addDep("B", ModuleKey.create("B", "0.1"))
.setRegistry(registry2)
.build());
assertThat(discoveryValue.getOverrides())
.containsExactly(
"C", SingleVersionOverride.create("", registry2.getUrl(), ImmutableList.of(), 0));
}

// TODO(wyv): test local path override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ private void setUpDiscoveryResult(String rootModuleName, ImmutableMap<ModuleKey,
new SkyFunction() {
@Override
public SkyValue compute(SkyKey skyKey, Environment env) {
return DiscoveryValue.create(rootModuleName, depGraph, ImmutableMap.of());
return DiscoveryValue.create(rootModuleName, depGraph);
}

@Override
Expand Down

0 comments on commit 6501f30

Please sign in to comment.