Skip to content

Commit

Permalink
Add module_ctx.is_dev_dependency
Browse files Browse the repository at this point in the history 
Allows module extensions to determine whether a given tag represents a
dev dependency.

Fixes #17101
Work towards #17908
  • Loading branch information
@fmeum
fmeum committed Mar 29, 2023
1 parent 8ab9c6e commit a18f6cc
Show file tree
Hide file tree
Showing 8 changed files with 133 additions and 14 deletions.
17 changes: 17 additions & 0 deletions src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
import com.google.devtools.build.skyframe.SkyFunction.Environment;
import java.util.Map;
import javax.annotation.Nullable;
import net.starlark.java.annot.Param;
import net.starlark.java.annot.ParamType;
import net.starlark.java.annot.StarlarkBuiltin;
import net.starlark.java.annot.StarlarkMethod;
import net.starlark.java.eval.EvalException;
Expand Down Expand Up @@ -99,4 +101,19 @@ protected ImmutableMap<String, String> getRemoteExecProperties() throws EvalExce
public StarlarkList<StarlarkBazelModule> getModules() {
return modules;
}

@StarlarkMethod(
name = "is_dev_dependency",
doc = "Returns whether the given tag was specified on the result of a <a "
+ "href=\"globals.html#use_extension\">use_extension</a> call with "
+ "<code>devDependency = True</code>.",
parameters = {
@Param(
name = "tag",
doc = "A tag obtained from <a href=\"bazel_module.html#tags\">bazel_module.tags</a>.",
allowedTypes = {@ParamType(type = TypeCheckedTag.class)})
})
public boolean isDevDependency(TypeCheckedTag tag) {
return tag.isDevDependency();
}
}
29 changes: 24 additions & 5 deletions src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileGlobals.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -375,27 +375,26 @@ public void registerToolchains(Sequence<?> toolchainLabels) throws EvalException
},
useStarlarkThread = true)
public ModuleExtensionProxy useExtension(
String extensionBzlFile, String extensionName, boolean devDependency, StarlarkThread thread)
throws EvalException {
String extensionBzlFile, String extensionName, boolean devDependency, StarlarkThread thread) {
ModuleExtensionProxy newProxy =
new ModuleExtensionProxy(extensionBzlFile, extensionName, thread.getCallerLocation());

if (ignoreDevDeps && devDependency) {
// This is a no-op proxy.
return newProxy;
return newProxy.withDevDependency(devDependency);
}

// Find an existing proxy object corresponding to this extension.
for (ModuleExtensionProxy proxy : extensionProxies) {
if (proxy.extensionBzlFile.equals(extensionBzlFile)
&& proxy.extensionName.equals(extensionName)) {
return proxy;
return proxy.withDevDependency(devDependency);
}
}

// If no such proxy exists, we can just use a new one.
extensionProxies.add(newProxy);
return newProxy;
return newProxy.withDevDependency(devDependency);
}

@StarlarkBuiltin(name = "module_extension_proxy", documented = false)
Expand All @@ -405,13 +404,32 @@ class ModuleExtensionProxy implements Structure {
private final Location location;
private final HashBiMap<String, String> imports;
private final ImmutableList.Builder<Tag> tags;
private final boolean devDependency;

ModuleExtensionProxy(String extensionBzlFile, String extensionName, Location location) {
this.extensionBzlFile = extensionBzlFile;
this.extensionName = extensionName;
this.location = location;
this.imports = HashBiMap.create();
this.tags = ImmutableList.builder();
this.devDependency = false;
}

private ModuleExtensionProxy(ModuleExtensionProxy other, boolean devDependency) {
this.extensionBzlFile = other.extensionBzlFile;
this.extensionName = other.extensionName;
this.location = other.location;
this.imports = other.imports;
this.tags = other.tags;
this.devDependency = devDependency;
}

/**
* Creates a proxy with the specified devDependency bit that shares accumulated imports and tags
* with the current one, thus preserving tag specification order.
*/
ModuleExtensionProxy withDevDependency(boolean devDependency) {
return devDependency ? new ModuleExtensionProxy(this, true) : this;
}

ModuleExtensionUsage buildUsage() {
Expand Down Expand Up @@ -453,6 +471,7 @@ public void call(Dict<String, Object> kwargs, StarlarkThread thread) {
Tag.builder()
.setTagName(tagName)
.setAttributeValues(kwargs)
.setDevDependency(devDependency)
.setLocation(thread.getCallerLocation())
.build());
}
Expand Down
5 changes: 5 additions & 0 deletions src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Tag.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ public abstract class Tag {
/** All keyword arguments supplied to the tag instance. */
public abstract Dict<String, Object> getAttributeValues();

/** Whether this tag was created using a proxy created with dev_dependency = True. */
public abstract boolean isDevDependency();

/** The source location in the module file where this tag was created. */
public abstract Location getLocation();

Expand All @@ -48,6 +51,8 @@ public abstract static class Builder {

public abstract Builder setAttributeValues(Dict<String, Object> value);

public abstract Builder setDevDependency(boolean value);

public abstract Builder setLocation(Location value);

public abstract Tag build();
Expand Down
14 changes: 12 additions & 2 deletions src/main/java/com/google/devtools/build/lib/bazel/bzlmod/TypeCheckedTag.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,12 @@
public class TypeCheckedTag implements Structure {
private final TagClass tagClass;
private final Object[] attrValues;
private final boolean devDependency;

private TypeCheckedTag(TagClass tagClass, Object[] attrValues) {
private TypeCheckedTag(TagClass tagClass, Object[] attrValues, boolean devDependency) {
this.tagClass = tagClass;
this.attrValues = attrValues;
this.devDependency = devDependency;
}

/** Creates a {@link TypeCheckedTag}. */
Expand Down Expand Up @@ -95,7 +97,15 @@ public static TypeCheckedTag create(TagClass tagClass, Tag tag, LabelConverter l
attrValues[i] = Attribute.valueToStarlark(attr.getDefaultValueUnchecked());
}
}
return new TypeCheckedTag(tagClass, attrValues);
return new TypeCheckedTag(tagClass, attrValues, tag.isDevDependency());
}

/**
* Whether the tag was specified on an extension proxy created with
* <code>dev_dependency=True</code>.
*/
public boolean isDevDependency() {
return devDependency;
}

@Override
Expand Down
8 changes: 8 additions & 0 deletions src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodTestUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,7 @@ public static TagClass createTagClass(Attribute... attrs) {
public static class TestTagBuilder {
private final Dict.Builder<String, Object> attrValuesBuilder = Dict.builder();
private final String tagName;
private boolean devDependency = false;

private TestTagBuilder(String tagName) {
this.tagName = tagName;
Expand All @@ -294,11 +295,18 @@ public TestTagBuilder addAttr(String attrName, Object attrValue) {
return this;
}

@CanIgnoreReturnValue
public TestTagBuilder setDevDependency() {
devDependency = true;
return this;
}

public Tag build() {
return Tag.builder()
.setTagName(tagName)
.setLocation(Location.BUILTIN)
.setAttributeValues(attrValuesBuilder.buildImmutable())
.setDevDependency(devDependency)
.build();
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ public void multipleModules_devDependency() throws Exception {
" data_str = 'modules:'",
" for mod in ctx.modules:",
" for tag in mod.tags.tag:",
" data_str += ' ' + tag.data",
" data_str += ' ' + tag.data + ' ' + str(ctx.is_dev_dependency(tag))",
" data_repo(name='ext_repo',data=data_str)",
"tag=tag_class(attrs={'data':attr.string()})",
"ext=module_extension(implementation=_ext_impl,tag_classes={'tag':tag})");
Expand All @@ -457,7 +457,7 @@ public void multipleModules_devDependency() throws Exception {
if (result.hasError()) {
throw result.getError().getException();
}
assertThat(result.get(skyKey).getModule().getGlobal("data")).isEqualTo("modules: root bar@2.0");
assertThat(result.get(skyKey).getModule().getGlobal("data")).isEqualTo("modules: root True bar@2.0 False");
}

@Test
Expand Down Expand Up @@ -497,7 +497,7 @@ public void multipleModules_ignoreDevDependency() throws Exception {
" data_str = 'modules:'",
" for mod in ctx.modules:",
" for tag in mod.tags.tag:",
" data_str += ' ' + tag.data",
" data_str += ' ' + tag.data + ' ' + str(ctx.is_dev_dependency(tag))",
" data_repo(name='ext_repo',data=data_str)",
"tag=tag_class(attrs={'data':attr.string()})",
"ext=module_extension(implementation=_ext_impl,tag_classes={'tag':tag})");
Expand All @@ -511,7 +511,7 @@ public void multipleModules_ignoreDevDependency() throws Exception {
if (result.hasError()) {
throw result.getError().getException();
}
assertThat(result.get(skyKey).getModule().getGlobal("data")).isEqualTo("modules: bar@2.0");
assertThat(result.get(skyKey).getModule().getGlobal("data")).isEqualTo("modules: bar@2.0 False");
}

@Test
Expand Down
57 changes: 56 additions & 1 deletion src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,7 @@ public void testModuleExtensions_good() throws Exception {
Dict.<String, Object>builder()
.put("key", "val")
.buildImmutable())
.setDevDependency(false)
.setLocation(
Location.fromFileLineColumn("mymod@1.0/MODULE.bazel", 4, 11))
.build())
Expand All @@ -501,6 +502,7 @@ public void testModuleExtensions_good() throws Exception {
Dict.<String, Object>builder()
.put("key1", "val1")
.buildImmutable())
.setDevDependency(false)
.setLocation(
Location.fromFileLineColumn("mymod@1.0/MODULE.bazel", 7, 12))
.build())
Expand All @@ -511,6 +513,7 @@ public void testModuleExtensions_good() throws Exception {
Dict.<String, Object>builder()
.put("key2", "val2")
.buildImmutable())
.setDevDependency(false)
.setLocation(
Location.fromFileLineColumn("mymod@1.0/MODULE.bazel", 8, 12))
.build())
Expand All @@ -529,6 +532,7 @@ public void testModuleExtensions_good() throws Exception {
Dict.<String, Object>builder()
.put("coord", "junit")
.buildImmutable())
.setDevDependency(false)
.setLocation(
Location.fromFileLineColumn("mymod@1.0/MODULE.bazel", 12, 10))
.build())
Expand All @@ -539,6 +543,7 @@ public void testModuleExtensions_good() throws Exception {
Dict.<String, Object>builder()
.put("coord", "guava")
.buildImmutable())
.setDevDependency(false)
.setLocation(
Location.fromFileLineColumn("mymod@1.0/MODULE.bazel", 14, 10))
.build())
Expand All @@ -551,12 +556,16 @@ public void testModuleExtensions_duplicateProxy_asRoot() throws Exception {
scratch.file(
rootDirectory.getRelative("MODULE.bazel").getPathString(),
"myext1 = use_extension('//:defs.bzl','myext',dev_dependency=True)",
"myext1.tag(name = 'tag1')",
"use_repo(myext1, 'alpha')",
"myext2 = use_extension('//:defs.bzl','myext')",
"myext2.tag(name = 'tag2')",
"use_repo(myext2, 'beta')",
"myext3 = use_extension('//:defs.bzl','myext',dev_dependency=True)",
"myext3.tag(name = 'tag3')",
"use_repo(myext3, 'gamma')",
"myext4 = use_extension('//:defs.bzl','myext')",
"myext4.tag(name = 'tag4')",
"use_repo(myext4, 'delta')");
ModuleFileFunction.REGISTRIES.set(differencer, ImmutableList.of());

Expand All @@ -580,6 +589,34 @@ public void testModuleExtensions_duplicateProxy_asRoot() throws Exception {
ImmutableBiMap.of(
"alpha", "alpha", "beta", "beta", "gamma", "gamma", "delta",
"delta"))
.addTag(Tag.builder()
.setTagName("tag")
.setAttributeValues(
Dict.<String, Object>builder().put("name", "tag1").buildImmutable())
.setDevDependency(true)
.setLocation(Location.fromFileLineColumn("<root>/MODULE.bazel", 2, 11))
.build())
.addTag(Tag.builder()
.setTagName("tag")
.setAttributeValues(
Dict.<String, Object>builder().put("name", "tag2").buildImmutable())
.setDevDependency(false)
.setLocation(Location.fromFileLineColumn("<root>/MODULE.bazel", 5, 11))
.build())
.addTag(Tag.builder()
.setTagName("tag")
.setAttributeValues(
Dict.<String, Object>builder().put("name", "tag3").buildImmutable())
.setDevDependency(true)
.setLocation(Location.fromFileLineColumn("<root>/MODULE.bazel", 8, 11))
.build())
.addTag(Tag.builder()
.setTagName("tag")
.setAttributeValues(
Dict.<String, Object>builder().put("name", "tag4").buildImmutable())
.setDevDependency(false)
.setLocation(Location.fromFileLineColumn("<root>/MODULE.bazel", 11, 11))
.build())
.build())
.build());
}
Expand All @@ -593,12 +630,16 @@ public void testModuleExtensions_duplicateProxy_asDep() throws Exception {
createModuleKey("mymod", "1.0"),
"module(name='mymod',version='1.0')",
"myext1 = use_extension('//:defs.bzl','myext',dev_dependency=True)",
"myext1.tag(name = 'tag1')",
"use_repo(myext1, 'alpha')",
"myext2 = use_extension('//:defs.bzl','myext')",
"myext2.tag(name = 'tag2')",
"use_repo(myext2, 'beta')",
"myext3 = use_extension('//:defs.bzl','myext',dev_dependency=True)",
"myext3.tag(name = 'tag3')",
"use_repo(myext3, 'gamma')",
"myext4 = use_extension('//:defs.bzl','myext')",
"myext4.tag(name = 'tag4')",
"use_repo(myext4, 'delta')");
ModuleFileFunction.REGISTRIES.set(differencer, ImmutableList.of(registry.getUrl()));

Expand All @@ -617,8 +658,22 @@ public void testModuleExtensions_duplicateProxy_asDep() throws Exception {
ModuleExtensionUsage.builder()
.setExtensionBzlFile("//:defs.bzl")
.setExtensionName("myext")
.setLocation(Location.fromFileLineColumn("mymod@1.0/MODULE.bazel", 4, 23))
.setLocation(Location.fromFileLineColumn("mymod@1.0/MODULE.bazel", 5, 23))
.setImports(ImmutableBiMap.of("beta", "beta", "delta", "delta"))
.addTag(Tag.builder()
.setTagName("tag")
.setAttributeValues(
Dict.<String, Object>builder().put("name", "tag2").buildImmutable())
.setDevDependency(false)
.setLocation(Location.fromFileLineColumn("mymod@1.0/MODULE.bazel", 6, 11))
.build())
.addTag(Tag.builder()
.setTagName("tag")
.setAttributeValues(
Dict.<String, Object>builder().put("name", "tag4").buildImmutable())
.setDevDependency(false)
.setLocation(Location.fromFileLineColumn("mymod@1.0/MODULE.bazel", 12, 11))
.build())
.build())
.build());
}
Expand Down
9 changes: 7 additions & 2 deletions src/test/java/com/google/devtools/build/lib/bazel/bzlmod/TypeCheckedTagTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,11 @@ public void basic() throws Exception {
TypeCheckedTag typeCheckedTag =
TypeCheckedTag.create(
createTagClass(attr("foo", Type.INTEGER).build()),
buildTag("tag_name").addAttr("foo", StarlarkInt.of(3)).build(),
buildTag("tag_name").addAttr("foo", StarlarkInt.of(3)).setDevDependency().build(),
/*labelConverter=*/ null);
assertThat(typeCheckedTag.getFieldNames()).containsExactly("foo");
assertThat(getattr(typeCheckedTag, "foo")).isEqualTo(StarlarkInt.of(3));
assertThat(typeCheckedTag.isDevDependency()).isTrue();
}

@Test
Expand All @@ -87,6 +88,7 @@ public void label() throws Exception {
Label.parseCanonicalUnchecked("@myrepo//mypkg:thing1"),
Label.parseCanonicalUnchecked("@myrepo//pkg:thing2"),
Label.parseCanonicalUnchecked("@other_repo//pkg:thing3")));
assertThat(typeCheckedTag.isDevDependency()).isFalse();
}

@Test
Expand All @@ -95,12 +97,13 @@ public void label_withoutDefaultValue() throws Exception {
TypeCheckedTag.create(
createTagClass(
attr("foo", BuildType.LABEL).allowedFileTypes(FileTypeSet.ANY_FILE).build()),
buildTag("tag_name").build(),
buildTag("tag_name").setDevDependency().build(),
new LabelConverter(
PackageIdentifier.parse("@myrepo//mypkg"),
createRepositoryMapping(createModuleKey("test", "1.0"), "repo", "other_repo")));
assertThat(typeCheckedTag.getFieldNames()).containsExactly("foo");
assertThat(getattr(typeCheckedTag, "foo")).isEqualTo(Starlark.NONE);
assertThat(typeCheckedTag.isDevDependency()).isTrue();
}

@Test
Expand All @@ -119,6 +122,7 @@ public void stringListDict_default() throws Exception {
Dict.builder()
.put("key", StarlarkList.immutableOf("value1", "value2"))
.buildImmutable());
assertThat(typeCheckedTag.isDevDependency()).isFalse();
}

@Test
Expand All @@ -139,6 +143,7 @@ public void multipleAttributesAndDefaults() throws Exception {
assertThat(getattr(typeCheckedTag, "bar")).isEqualTo(StarlarkInt.of(3));
assertThat(getattr(typeCheckedTag, "quux"))
.isEqualTo(StarlarkList.immutableOf("quuxValue1", "quuxValue2"));
assertThat(typeCheckedTag.isDevDependency()).isFalse();
}

@Test
Expand Down

0 comments on commit a18f6cc

Please sign in to comment.