Skip to content

Commit a3f0bfa

Browse files
comiuscopybara-github
comius
authored and
copybara-github
committed
Starlarkify ProtoInfo
The Starlark version needs to follow fields as they were exposed on ProtoInfoApi. The change is slightly confusing, because native fields are sometimes named differently from Starlark ones. Here's the mapping: Type | Native field | Starlark field :------------------ | :--------------------- | :--- Artifact | directDescriptorSet | direct_descriptor_set List<ProtoSource> | directSources | _direct_proto_sources (private) List<Artifact> | directProtoSources | direct_sources = extractProtoSources(directSources) PathFragment | directProtoSourceRoot | proto_source_root = directProtoSourceRoot.getSafePathString(); NestedSet<ProtoSource> | transitiveSources | _transitive_proto_sources (private) NestedSet<Artifact> | transitiveProtoSources | transitive_imports, transitive_sources NestedSet<String> | transitiveProtoSourceRoots | transitive_proto_path NestedSet<Artifact> | strictImportableProto-SourcesForDependents | check_deps_sources NestedSet<Artifact> | transitiveDescriptorSets | transitive_descriptor_sets NestedSet<ProtoSource> | exportedSources | _exported_sources (private) The fields `transitive_imports` and `transitive_sources` are duplicated in Starlark. Native `directProtoSourceRoot` was interned using `ProtoCommon.memoryEfficientProtoSourceRoot`. There's no optimisation on `proto_source_root`, for now it doesn't seem necessary and I have ideas how to optimise `ProtoInfo` further without it. The change introduces a slight regression in retained heap on benchmarked builds <0.1%. It's caused because of extra containers. That is in Starlark each instance needs two references and an Object array, whereas natively there's no need for references or arrays: `StarlarkInfo { ProtoInfo.class, Object[]{ f1, f2 } } > ProtoInfo { f1,f2 }`. PiperOrigin-RevId: 511854620 Change-Id: I239aef427898e30e4f264f002e79b69eeaaa34db
1 parent 32e4f23 commit a3f0bfa

17 files changed

Lines changed: 210 additions & 457 deletions

File tree

‎src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,6 @@
109109
import com.google.devtools.build.lib.rules.proto.BazelProtoCommon;
110110
import com.google.devtools.build.lib.rules.proto.BazelProtoLibraryRule;
111111
import com.google.devtools.build.lib.rules.proto.ProtoConfiguration;
112-
import com.google.devtools.build.lib.rules.proto.ProtoInfo;
113112
import com.google.devtools.build.lib.rules.proto.ProtoLangToolchainRule;
114113
import com.google.devtools.build.lib.rules.python.PyCcLinkParamsProvider;
115114
import com.google.devtools.build.lib.rules.python.PyInfo;
@@ -322,7 +321,6 @@ public void init(ConfiguredRuleClassProvider.Builder builder) {
322321

323322
ProtoBootstrap bootstrap =
324323
new ProtoBootstrap(
325-
ProtoInfo.PROVIDER,
326324
BazelProtoCommon.INSTANCE,
327325
new StarlarkAspectStub(),
328326
new ProviderStub());

‎src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -852,7 +852,11 @@ private static PathFragment getProtoOutputRoot(RuleContext ruleContext) {
852852
}
853853

854854
private static boolean isProtoRule(ConfiguredTarget base) {
855-
return base.get(ProtoInfo.PROVIDER) != null;
855+
try {
856+
return base.get(ProtoInfo.PROVIDER) != null;
857+
} catch (RuleErrorException e) {
858+
return false;
859+
}
856860
}
857861

858862
/** Returns a mutable List of objc output files. */

‎src/main/java/com/google/devtools/build/lib/rules/proto/BUILD‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ java_library(
2525
["*.java"],
2626
),
2727
deps = [
28-
"//src/main/java/com/google/devtools/build/docgen/annot",
2928
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
3029
"//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
3130
"//src/main/java/com/google/devtools/build/lib/analysis:config/build_options",

‎src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoCommon.java‎

Lines changed: 0 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -14,77 +14,11 @@
1414

1515
package com.google.devtools.build.lib.rules.proto;
1616

17-
import com.google.devtools.build.lib.actions.Artifact;
18-
import com.google.devtools.build.lib.collect.nestedset.Depset;
1917
import com.google.devtools.build.lib.starlarkbuildapi.proto.ProtoCommonApi;
20-
import com.google.devtools.build.lib.vfs.PathFragment;
21-
import net.starlark.java.annot.Param;
22-
import net.starlark.java.annot.StarlarkMethod;
23-
import net.starlark.java.eval.EvalException;
24-
import net.starlark.java.eval.StarlarkList;
25-
import net.starlark.java.eval.StarlarkThread;
2618

2719
/** Protocol buffers support for Starlark. */
2820
public class BazelProtoCommon implements ProtoCommonApi {
2921
public static final BazelProtoCommon INSTANCE = new BazelProtoCommon();
3022

3123
protected BazelProtoCommon() {}
32-
33-
@StarlarkMethod(
34-
name = "ProtoSource",
35-
documented = false,
36-
parameters = {
37-
@Param(name = "source_file", doc = "The proto file."),
38-
@Param(name = "original_source_file", doc = "Original proto file."),
39-
@Param(name = "proto_path", doc = "Path to proto file."),
40-
},
41-
useStarlarkThread = true)
42-
public ProtoSource protoSource(
43-
Artifact sourceFile, Artifact originalSourceFile, String sourceRoot, StarlarkThread thread)
44-
throws EvalException {
45-
ProtoCommon.checkPrivateStarlarkificationAllowlist(thread);
46-
return new ProtoSource(sourceFile, originalSourceFile, PathFragment.create(sourceRoot));
47-
}
48-
49-
@StarlarkMethod(
50-
name = "ProtoInfo",
51-
documented = false,
52-
parameters = {
53-
@Param(name = "direct_sources", doc = "Direct sources."),
54-
@Param(name = "proto_path", doc = "Proto path."),
55-
@Param(name = "transitive_sources", doc = "Transitive sources."),
56-
@Param(name = "transitive_proto_sources", doc = "Transitive proto sources."),
57-
@Param(name = "transitive_proto_path", doc = "Transitive proto path."),
58-
@Param(name = "check_deps_sources", doc = "Check deps sources."),
59-
@Param(name = "direct_descriptor_set", doc = "Direct descriptor set."),
60-
@Param(name = "transitive_descriptor_set", doc = "Transitive descriptor sets."),
61-
@Param(name = "exported_sources", doc = "Exported sources"),
62-
},
63-
useStarlarkThread = true)
64-
@SuppressWarnings("unchecked")
65-
public ProtoInfo protoInfo(
66-
StarlarkList<? extends ProtoSource> directSources,
67-
String directProtoSourceRoot,
68-
Depset transitiveProtoSources,
69-
Depset transitiveSources,
70-
Depset transitiveProtoSourceRoots,
71-
Depset strictImportableProtoSourcesForDependents,
72-
Artifact directDescriptorSet,
73-
Depset transitiveDescriptorSets,
74-
Depset exportedSources,
75-
StarlarkThread thread)
76-
throws EvalException {
77-
ProtoCommon.checkPrivateStarlarkificationAllowlist(thread);
78-
return new ProtoInfo(
79-
((StarlarkList<ProtoSource>) directSources).getImmutableList(),
80-
PathFragment.create(directProtoSourceRoot),
81-
Depset.cast(transitiveSources, ProtoSource.class, "transitive_sources"),
82-
Depset.cast(transitiveProtoSources, Artifact.class, "transitive_proto_sources"),
83-
Depset.cast(transitiveProtoSourceRoots, String.class, "transitive_proto_path"),
84-
Depset.cast(
85-
strictImportableProtoSourcesForDependents, Artifact.class, "check_deps_sources"),
86-
directDescriptorSet,
87-
Depset.cast(transitiveDescriptorSets, Artifact.class, "transitive_descriptor_set"),
88-
Depset.cast(exportedSources, ProtoSource.class, "exported_sources"));
89-
}
9024
}

‎src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCommon.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ public static ImmutableList<Artifact> declareGeneratedFiles(
8181
declareGeneratedFiles,
8282
ImmutableList.of(
8383
/* actions */ ruleContext.getStarlarkRuleContext().actions(),
84-
/* proto_info */ protoTarget.get(ProtoInfo.PROVIDER),
84+
/* proto_info */ protoTarget.get(ProtoInfo.PROVIDER.getKey()),
8585
/* extension */ extension),
8686
ImmutableMap.of());
8787
try {
@@ -107,7 +107,7 @@ public static void compile(
107107
compile,
108108
ImmutableList.of(
109109
/* actions */ ruleContext.getStarlarkRuleContext().actions(),
110-
/* proto_info */ protoTarget.get(ProtoInfo.PROVIDER),
110+
/* proto_info */ protoTarget.get(ProtoInfo.PROVIDER.getKey()),
111111
/* proto_lang_toolchain_info */ protoLangToolchainInfo,
112112
/* generated_files */ StarlarkList.immutableCopyOf(generatedFiles),
113113
/* plugin_output */ pluginOutput == null ? Starlark.NONE : pluginOutput),
@@ -131,7 +131,7 @@ public static Sequence<Artifact> filterSources(
131131
ruleContext.callStarlarkOrThrowRuleError(
132132
filterSources,
133133
ImmutableList.of(
134-
/* proto_info */ protoTarget.get(ProtoInfo.PROVIDER),
134+
/* proto_info */ protoTarget.get(ProtoInfo.PROVIDER.getKey()),
135135
/* proto_lang_toolchain_info */ protoLangToolchainInfo),
136136
ImmutableMap.of()))
137137
.get(0),

0 commit comments

Comments
 (0)