Skip to content

Commit

Permalink
[6.2.0]Add module_ctx.is_dev_dependency (bazelbuild#17934)
Browse files Browse the repository at this point in the history 
* Add `module_ctx.is_dev_dependency`

Allows module extensions to determine whether a given tag represents a dev dependency.

Fixes bazelbuild#17101
Work towards bazelbuild#17908

Closes bazelbuild#17909.

PiperOrigin-RevId: 520645663
Change-Id: I3e3136a09d01d25fc706bcd0dfd7e53b6e7d5285

* Revert section that was accidentally cherry-picked

---------

Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
Co-authored-by: keertk <keerthanakumar@google.com>
  • Loading branch information
@ShreeM01 @fmeum @keertk
3 people committed Mar 31, 2023
1 parent d24f7cb commit 3ea18cc
Show file tree
Hide file tree
Showing 8 changed files with 213 additions and 67 deletions.
18 changes: 18 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,20 @@ 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();
}
}
133 changes: 76 additions & 57 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 @@ -25,6 +25,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.docgen.annot.DocumentMethods;
import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileGlobals.ModuleExtensionUsageBuilder.ModuleExtensionProxy;
import com.google.devtools.build.lib.bazel.bzlmod.Version.ParseException;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import java.util.ArrayList;
Expand Down Expand Up @@ -63,7 +64,7 @@ public class ModuleFileGlobals {
private final boolean ignoreDevDeps;
private final Module.Builder module;
private final Map<String, ModuleKey> deps = new LinkedHashMap<>();
private final List<ModuleExtensionProxy> extensionProxies = new ArrayList<>();
private final List<ModuleExtensionUsageBuilder> extensionUsageBuilders = new ArrayList<>();
private final Map<String, ModuleOverride> overrides = new HashMap<>();
private final Map<String, RepoNameUsage> repoNameUsages = new HashMap<>();

Expand Down Expand Up @@ -373,38 +374,37 @@ public void registerToolchains(Sequence<?> toolchainLabels) throws EvalException
},
useStarlarkThread = true)
public ModuleExtensionProxy useExtension(
String extensionBzlFile, String extensionName, boolean devDependency, StarlarkThread thread)
throws EvalException {
ModuleExtensionProxy newProxy =
new ModuleExtensionProxy(extensionBzlFile, extensionName, thread.getCallerLocation());
String extensionBzlFile, String extensionName, boolean devDependency, StarlarkThread thread) {
ModuleExtensionUsageBuilder newUsageBuilder =
new ModuleExtensionUsageBuilder(
extensionBzlFile, extensionName, thread.getCallerLocation());

if (ignoreDevDeps && devDependency) {
// This is a no-op proxy.
return newProxy;
return newUsageBuilder.getProxy(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;
// Find an existing usage builder corresponding to this extension.
for (ModuleExtensionUsageBuilder usageBuilder : extensionUsageBuilders) {
if (usageBuilder.extensionBzlFile.equals(extensionBzlFile)
&& usageBuilder.extensionName.equals(extensionName)) {
return usageBuilder.getProxy(devDependency);
}
}

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

@StarlarkBuiltin(name = "module_extension_proxy", documented = false)
class ModuleExtensionProxy implements Structure {
class ModuleExtensionUsageBuilder {
private final String extensionBzlFile;
private final String extensionName;
private final Location location;
private final HashBiMap<String, String> imports;
private final ImmutableList.Builder<Tag> tags;

ModuleExtensionProxy(String extensionBzlFile, String extensionName, Location location) {
ModuleExtensionUsageBuilder(String extensionBzlFile, String extensionName, Location location) {
this.extensionBzlFile = extensionBzlFile;
this.extensionName = extensionName;
this.location = location;
Expand All @@ -422,50 +422,69 @@ ModuleExtensionUsage buildUsage() {
.build();
}

void addImport(String localRepoName, String exportedName, Location location)
throws EvalException {
RepositoryName.validateUserProvidedRepoName(localRepoName);
RepositoryName.validateUserProvidedRepoName(exportedName);
addRepoNameUsage(localRepoName, "by a use_repo() call", location);
if (imports.containsValue(exportedName)) {
String collisionRepoName = imports.inverse().get(exportedName);
throw Starlark.errorf(
"The repo exported as '%s' by module extension '%s' is already imported at %s",
exportedName, extensionName, repoNameUsages.get(collisionRepoName).getWhere());
}
imports.put(localRepoName, exportedName);
/**
* Creates a proxy with the specified dev_dependency bit that shares accumulated imports and
* tags with all other such proxies, thus preserving their order across dev/non-dev deps.
*/
ModuleExtensionProxy getProxy(boolean devDependency) {
return new ModuleExtensionProxy(devDependency);
}

@Nullable
@Override
public Object getValue(String tagName) throws EvalException {
return new StarlarkValue() {
@StarlarkMethod(
name = "call",
selfCall = true,
documented = false,
extraKeywords = @Param(name = "kwargs"),
useStarlarkThread = true)
public void call(Dict<String, Object> kwargs, StarlarkThread thread) {
tags.add(
Tag.builder()
.setTagName(tagName)
.setAttributeValues(kwargs)
.setLocation(thread.getCallerLocation())
.build());
@StarlarkBuiltin(name = "module_extension_proxy", documented = false)
class ModuleExtensionProxy implements Structure {

private final boolean devDependency;

private ModuleExtensionProxy(boolean devDependency) {
this.devDependency = devDependency;
}

void addImport(String localRepoName, String exportedName, Location location)
throws EvalException {
RepositoryName.validateUserProvidedRepoName(localRepoName);
RepositoryName.validateUserProvidedRepoName(exportedName);
addRepoNameUsage(localRepoName, "by a use_repo() call", location);
if (imports.containsValue(exportedName)) {
String collisionRepoName = imports.inverse().get(exportedName);
throw Starlark.errorf(
"The repo exported as '%s' by module extension '%s' is already imported at %s",
exportedName, extensionName, repoNameUsages.get(collisionRepoName).getWhere());
}
};
}
imports.put(localRepoName, exportedName);
}

@Override
public ImmutableCollection<String> getFieldNames() {
return ImmutableList.of();
}
@Nullable
@Override
public Object getValue(String tagName) throws EvalException {
return new StarlarkValue() {
@StarlarkMethod(
name = "call",
selfCall = true,
documented = false,
extraKeywords = @Param(name = "kwargs"),
useStarlarkThread = true)
public void call(Dict<String, Object> kwargs, StarlarkThread thread) {
tags.add(
Tag.builder()
.setTagName(tagName)
.setAttributeValues(kwargs)
.setDevDependency(devDependency)
.setLocation(thread.getCallerLocation())
.build());
}
};
}

@Nullable
@Override
public String getErrorMessageForUnknownField(String field) {
return null;
@Override
public ImmutableCollection<String> getFieldNames() {
return ImmutableList.of();
}

@Nullable
@Override
public String getErrorMessageForUnknownField(String field) {
return null;
}
}
}

Expand Down Expand Up @@ -821,8 +840,8 @@ public Module buildModule() {
.setDeps(ImmutableMap.copyOf(deps))
.setOriginalDeps(ImmutableMap.copyOf(deps))
.setExtensionUsages(
extensionProxies.stream()
.map(ModuleExtensionProxy::buildUsage)
extensionUsageBuilders.stream()
.map(ModuleExtensionUsageBuilder::buildUsage)
.collect(toImmutableList()))
.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
10 changes: 6 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,8 @@ 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 +498,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 +512,8 @@ 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
Loading

0 comments on commit 3ea18cc

Please sign in to comment.