Skip to content

Commit

Permalink
[8.1.0] Convert proto output from internal string encoding to Unicode (…
Browse files Browse the repository at this point in the history 
…bazelbuild#25097)

Work towards #374

Closes bazelbuild#24935.

PiperOrigin-RevId: 718549143
Change-Id: Ibe6c685a2f8dd75430cae7f770d392de35bdeb68

Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
  • Loading branch information
@tjgq @fmeum
tjgq and fmeum authored Jan 27, 2025
1 parent 76b0f75 commit acae5b0
Show file tree
Hide file tree
Showing 11 changed files with 253 additions and 95 deletions.
39 changes: 20 additions & 19 deletions src/main/java/com/google/devtools/build/lib/packages/AttributeFormatter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import static com.google.devtools.build.lib.packages.Types.STRING_DICT;
import static com.google.devtools.build.lib.packages.Types.STRING_LIST;
import static com.google.devtools.build.lib.packages.Types.STRING_LIST_DICT;
import static com.google.devtools.build.lib.util.StringEncoding.internalToUnicode;

import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableSet;
Expand Down Expand Up @@ -131,7 +132,7 @@ private static Build.Attribute getAttributeProto(
boolean includeAttributeSourceAspects,
LabelPrinter labelPrinter) {
Build.Attribute.Builder attrPb = Build.Attribute.newBuilder();
attrPb.setName(name);
attrPb.setName(internalToUnicode(name));
attrPb.setExplicitlySpecified(explicitlySpecified);
maybeSetNoDep(type, attrPb);

Expand All @@ -149,7 +150,7 @@ private static Build.Attribute getAttributeProto(

if (includeAttributeSourceAspects) {
attrPb.setSourceAspectName(
sourceAspect != null ? sourceAspect.getAspectClass().getName() : "");
sourceAspect != null ? internalToUnicode(sourceAspect.getAspectClass().getName()) : "");
}

return attrPb.build();
Expand All @@ -174,7 +175,7 @@ private static void writeSelectorListToBuilder(
for (Selector<?> selector : selectorList.getSelectors()) {
Build.Attribute.Selector.Builder selectorBuilder =
Build.Attribute.Selector.newBuilder()
.setNoMatchError(selector.getNoMatchError())
.setNoMatchError(internalToUnicode(selector.getNoMatchError()))
.setHasDefaultValue(selector.hasDefault());

// Note that the order of entries returned by selector.getEntries is stable. The map's
Expand All @@ -184,7 +185,7 @@ private static void writeSelectorListToBuilder(
(condition, conditionValue) -> {
SelectorEntry.Builder selectorEntryBuilder =
SelectorEntry.newBuilder()
.setLabel(labelPrinter.toString(condition))
.setLabel(internalToUnicode(labelPrinter.toString(condition)))
.setIsDefaultValue(!selector.isValueSet(condition));

if (conditionValue != null) {
Expand All @@ -211,24 +212,24 @@ private static void writeAttributeValueToBuilder(
if (type == INTEGER) {
builder.setIntValue(((StarlarkInt) value).toIntUnchecked());
} else if (type == STRING || type == STRING_NO_INTERN) {
builder.setStringValue(value.toString());
builder.setStringValue(internalToUnicode(value.toString()));
} else if (type == LABEL
|| type == NODEP_LABEL
|| type == OUTPUT
|| type == GENQUERY_SCOPE_TYPE
|| type == DORMANT_LABEL) {
builder.setStringValue(labelPrinter.toString((Label) value));
builder.setStringValue(internalToUnicode(labelPrinter.toString((Label) value)));
} else if (type == STRING_LIST || type == DISTRIBUTIONS) {
for (Object entry : (Collection<?>) value) {
builder.addStringListValue(entry.toString());
builder.addStringListValue(internalToUnicode(entry.toString()));
}
} else if (type == LABEL_LIST
|| type == NODEP_LABEL_LIST
|| type == OUTPUT_LIST
|| type == GENQUERY_SCOPE_TYPE_LIST
|| type == DORMANT_LABEL_LIST) {
for (Label entry : (Collection<Label>) value) {
builder.addStringListValue(labelPrinter.toString(entry));
builder.addStringListValue(internalToUnicode(labelPrinter.toString(entry)));
}
} else if (type == INTEGER_LIST) {
for (Object elem : (Collection<?>) value) {
Expand All @@ -242,28 +243,28 @@ private static void writeAttributeValueToBuilder(
License license = (License) value;
Build.License.Builder licensePb = Build.License.newBuilder();
for (License.LicenseType licenseType : license.getLicenseTypes()) {
licensePb.addLicenseType(licenseType.toString());
licensePb.addLicenseType(internalToUnicode(licenseType.toString()));
}
for (Label exception : license.getExceptions()) {
licensePb.addException(exception.toString());
licensePb.addException(internalToUnicode(exception.toString()));
}
builder.setLicense(licensePb);
} else if (type == STRING_DICT) {
Map<String, String> dict = (Map<String, String>) value;
for (Map.Entry<String, String> keyValueList : dict.entrySet()) {
StringDictEntry.Builder entry =
StringDictEntry.newBuilder()
.setKey(keyValueList.getKey())
.setValue(keyValueList.getValue());
.setKey(internalToUnicode(keyValueList.getKey()))
.setValue(internalToUnicode(keyValueList.getValue()));
builder.addStringDictValue(entry);
}
} else if (type == STRING_LIST_DICT) {
Map<String, List<String>> dict = (Map<String, List<String>>) value;
for (Map.Entry<String, List<String>> dictEntry : dict.entrySet()) {
StringListDictEntry.Builder entry =
StringListDictEntry.newBuilder().setKey(dictEntry.getKey());
for (Object dictEntryValue : dictEntry.getValue()) {
entry.addValue(dictEntryValue.toString());
StringListDictEntry.newBuilder().setKey(internalToUnicode(dictEntry.getKey()));
for (String dictEntryValue : dictEntry.getValue()) {
entry.addValue(internalToUnicode(dictEntryValue));
}
builder.addStringListDictValue(entry);
}
Expand All @@ -272,17 +273,17 @@ private static void writeAttributeValueToBuilder(
for (Map.Entry<String, Label> dictEntry : dict.entrySet()) {
LabelDictUnaryEntry.Builder entry =
LabelDictUnaryEntry.newBuilder()
.setKey(dictEntry.getKey())
.setValue(labelPrinter.toString(dictEntry.getValue()));
.setKey(internalToUnicode(dictEntry.getKey()))
.setValue(internalToUnicode(labelPrinter.toString(dictEntry.getValue())));
builder.addLabelDictUnaryValue(entry);
}
} else if (type == LABEL_KEYED_STRING_DICT) {
Map<Label, String> dict = (Map<Label, String>) value;
for (Map.Entry<Label, String> dictEntry : dict.entrySet()) {
LabelKeyedStringDictEntry.Builder entry =
LabelKeyedStringDictEntry.newBuilder()
.setKey(labelPrinter.toString(dictEntry.getKey()))
.setValue(dictEntry.getValue());
.setKey(internalToUnicode(labelPrinter.toString(dictEntry.getKey())))
.setValue(internalToUnicode(dictEntry.getValue()));
builder.addLabelKeyedStringDictValue(entry);
}
} else {
Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/packages/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/util:filetype",
"//src/main/java/com/google/devtools/build/lib/util:hash_codes",
"//src/main/java/com/google/devtools/build/lib/util:string",
"//src/main/java/com/google/devtools/build/lib/util:string_encoding",
"//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:skyframe-objects",
Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/query2/query/output/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/query2/query/aspectresolvers",
"//src/main/java/com/google/devtools/build/lib/starlarkdocextract:labelrenderer",
"//src/main/java/com/google/devtools/build/lib/starlarkdocextract:ruleinfoextractor",
"//src/main/java/com/google/devtools/build/lib/util:string_encoding",
"//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/common/options",
Expand Down
72 changes: 45 additions & 27 deletions src/main/java/com/google/devtools/build/lib/query2/query/output/ProtoOutputFormatter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import static com.google.devtools.build.lib.query2.proto.proto2api.Build.Target.Discriminator.PACKAGE_GROUP;
import static com.google.devtools.build.lib.query2.proto.proto2api.Build.Target.Discriminator.RULE;
import static com.google.devtools.build.lib.query2.proto.proto2api.Build.Target.Discriminator.SOURCE_FILE;
import static com.google.devtools.build.lib.util.StringEncoding.internalToUnicode;

import com.google.common.base.Preconditions;
import com.google.common.base.Predicate;
Expand Down Expand Up @@ -192,10 +193,11 @@ public Build.Target toTargetProtoBuffer(
if (target instanceof Rule rule) {
Build.Rule.Builder rulePb =
Build.Rule.newBuilder()
.setName(labelPrinter.toString(rule.getLabel()))
.setRuleClass(rule.getRuleClass());
.setName(internalToUnicode(labelPrinter.toString(rule.getLabel())))
.setRuleClass(internalToUnicode(rule.getRuleClass()));
if (includeLocations) {
rulePb.setLocation(FormatUtils.getLocation(target, relativeLocations));
rulePb.setLocation(
internalToUnicode(FormatUtils.getLocation(target, relativeLocations)));
}
addAttributes(rulePb, rule, extraDataForAttrHash, labelPrinter);
byte[] transitiveDigest = rule.getRuleClassObject().getRuleDefinitionEnvironmentDigest();
Expand Down Expand Up @@ -248,27 +250,33 @@ public Build.Target toTargetProtoBuffer(
aspectsDependencies.values().stream()
.flatMap(m -> m.values().stream())
.distinct()
.forEach(dep -> rulePb.addRuleInput(labelPrinter.toString(dep)));
.forEach(dep -> rulePb.addRuleInput(internalToUnicode(labelPrinter.toString(dep))));
}
// Include explicit elements for all direct inputs and outputs of a rule; this goes beyond
// what is available from the attributes above, since it may also (depending on options)
// include implicit outputs, exec-configuration outputs, and default values.
rule.getSortedLabels(dependencyFilter)
.forEach(input -> rulePb.addRuleInput(labelPrinter.toString(input)));
.forEach(input -> rulePb.addRuleInput(internalToUnicode(labelPrinter.toString(input))));
rule.getOutputFiles().stream()
.distinct()
.forEach(output -> rulePb.addRuleOutput(labelPrinter.toString(output.getLabel())));
.forEach(
output ->
rulePb.addRuleOutput(
internalToUnicode(labelPrinter.toString(output.getLabel()))));
}
for (String feature : rule.getPackage().getPackageArgs().features().toStringList()) {
rulePb.addDefaultSetting(feature);
rulePb.addDefaultSetting(internalToUnicode(feature));
}

if (includeInstantiationStack) {
for (StarlarkThread.CallStackEntry fr : rule.reconstructCallStack()) {
// Always report relative locations.
// (New fields needn't honor relativeLocations.)
rulePb.addInstantiationStack(
FormatUtils.getRootRelativeLocation(fr.location, rule.getPackage()) + ": " + fr.name);
internalToUnicode(
FormatUtils.getRootRelativeLocation(fr.location, rule.getPackage())
+ ": "
+ fr.name));
}
}

Expand All @@ -277,7 +285,10 @@ public Build.Target toTargetProtoBuffer(
// Always report relative locations.
// (New fields needn't honor relativeLocations.)
rulePb.addDefinitionStack(
FormatUtils.getRootRelativeLocation(fr.location, rule.getPackage()) + ": " + fr.name);
internalToUnicode(
FormatUtils.getRootRelativeLocation(fr.location, rule.getPackage())
+ ": "
+ fr.name));
}
}

Expand All @@ -292,22 +303,25 @@ public Build.Target toTargetProtoBuffer(
Rule generatingRule = outputFile.getGeneratingRule();
GeneratedFile.Builder output =
GeneratedFile.newBuilder()
.setGeneratingRule(labelPrinter.toString(generatingRule.getLabel()))
.setName(labelPrinter.toString(label));
.setGeneratingRule(
internalToUnicode(labelPrinter.toString(generatingRule.getLabel())))
.setName(internalToUnicode(labelPrinter.toString(label)));

if (includeLocations) {
output.setLocation(FormatUtils.getLocation(target, relativeLocations));
output.setLocation(
internalToUnicode(FormatUtils.getLocation(target, relativeLocations)));
}
targetPb.setType(GENERATED_FILE);
targetPb.setGeneratedFile(output.build());
} else if (target instanceof InputFile inputFile) {
Label label = inputFile.getLabel();

Build.SourceFile.Builder input =
Build.SourceFile.newBuilder().setName(labelPrinter.toString(label));
Build.SourceFile.newBuilder().setName(internalToUnicode(labelPrinter.toString(label)));

if (includeLocations) {
input.setLocation(FormatUtils.getLocation(target, relativeLocations));
input.setLocation(
internalToUnicode(FormatUtils.getLocation(target, relativeLocations)));
}

if (inputFile.getName().equals("BUILD")) {
Expand All @@ -317,11 +331,11 @@ public Build.Target toTargetProtoBuffer(
: aspectResolver.computeBuildFileDependencies(inputFile.getPackage());

for (Label starlarkLoadLabel : starlarkLoadLabels) {
input.addSubinclude(labelPrinter.toString(starlarkLoadLabel));
input.addSubinclude(internalToUnicode(labelPrinter.toString(starlarkLoadLabel)));
}

for (String feature : inputFile.getPackage().getPackageArgs().features().toStringList()) {
input.addFeature(feature);
input.addFeature(internalToUnicode(feature));
}

input.setPackageContainsErrors(inputFile.getPackage().containsErrors());
Expand All @@ -331,45 +345,49 @@ public Build.Target toTargetProtoBuffer(
// default_visibility in the target. For files we do, but for rules we don't.

for (Label visibilityDependency : target.getVisibilityDependencyLabels()) {
input.addPackageGroup(labelPrinter.toString(visibilityDependency));
input.addPackageGroup(internalToUnicode(labelPrinter.toString(visibilityDependency)));
}

for (Label visibilityDeclaration : target.getVisibilityDeclaredLabels()) {
input.addVisibilityLabel(labelPrinter.toString(visibilityDeclaration));
input.addVisibilityLabel(internalToUnicode(labelPrinter.toString(visibilityDeclaration)));
}

targetPb.setType(SOURCE_FILE);
targetPb.setSourceFile(input);
} else if (target instanceof FakeLoadTarget) {
Label label = target.getLabel();
SourceFile.Builder input = SourceFile.newBuilder().setName(labelPrinter.toString(label));
SourceFile.Builder input =
SourceFile.newBuilder().setName(internalToUnicode(labelPrinter.toString(label)));

if (includeLocations) {
input.setLocation(FormatUtils.getLocation(target, relativeLocations));
input.setLocation(
internalToUnicode(FormatUtils.getLocation(target, relativeLocations)));
}
targetPb.setType(SOURCE_FILE);
targetPb.setSourceFile(input.build());
} else if (target instanceof PackageGroup packageGroup) {
Build.PackageGroup.Builder packageGroupPb =
Build.PackageGroup.newBuilder().setName(labelPrinter.toString(packageGroup.getLabel()));
Build.PackageGroup.newBuilder()
.setName(internalToUnicode(labelPrinter.toString(packageGroup.getLabel())));
for (String containedPackage :
packageGroup.getContainedPackages(packageGroupIncludesDoubleSlash)) {
packageGroupPb.addContainedPackage(containedPackage);
packageGroupPb.addContainedPackage(internalToUnicode(containedPackage));
}
for (Label include : packageGroup.getIncludes()) {
packageGroupPb.addIncludedPackageGroup(labelPrinter.toString(include));
packageGroupPb.addIncludedPackageGroup(internalToUnicode(labelPrinter.toString(include)));
}

targetPb.setType(PACKAGE_GROUP);
targetPb.setPackageGroup(packageGroupPb);
} else if (target instanceof EnvironmentGroup envGroup) {
Build.EnvironmentGroup.Builder envGroupPb =
Build.EnvironmentGroup.newBuilder().setName(labelPrinter.toString(envGroup.getLabel()));
Build.EnvironmentGroup.newBuilder()
.setName(internalToUnicode(labelPrinter.toString(envGroup.getLabel())));
for (Label env : envGroup.getEnvironments()) {
envGroupPb.addEnvironment(labelPrinter.toString(env));
envGroupPb.addEnvironment(internalToUnicode(labelPrinter.toString(env)));
}
for (Label defaultEnv : envGroup.getDefaults()) {
envGroupPb.addDefault(labelPrinter.toString(defaultEnv));
envGroupPb.addDefault(internalToUnicode(labelPrinter.toString(defaultEnv)));
}
targetPb.setType(ENVIRONMENT_GROUP);
targetPb.setEnvironmentGroup(envGroupPb);
Expand Down Expand Up @@ -570,7 +588,7 @@ private static class RuleClassInfoFormatter {
*/
public void addRuleClassKeyAndInfoIfNeeded(Build.Rule.Builder rulePb, Rule rule) {
String ruleClassKey = rule.getRuleClassObject().getKey();
rulePb.setRuleClassKey(ruleClassKey);
rulePb.setRuleClassKey(internalToUnicode(ruleClassKey));
if (ruleClassKeys.add(ruleClassKey)) {
// TODO(b/368091415): instead of rule.getRuleClass(), we should be using the rule's public
// name. But to find the public name, we would need access to the globals dictionary of
Expand Down
13 changes: 10 additions & 3 deletions src/main/java/com/google/devtools/build/lib/starlarkdocextract/AttributeInfoExtractor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

package com.google.devtools.build.lib.starlarkdocextract;

import static com.google.devtools.build.lib.util.StringEncoding.internalToUnicode;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.packages.Attribute;
Expand All @@ -23,6 +25,7 @@
import com.google.devtools.build.lib.packages.Types;
import com.google.devtools.build.lib.starlarkdocextract.StardocOutputProtos.AttributeInfo;
import com.google.devtools.build.lib.starlarkdocextract.StardocOutputProtos.AttributeType;
import com.google.devtools.build.lib.util.StringEncoding;
import java.util.Map;
import java.util.Optional;
import java.util.function.Consumer;
Expand All @@ -36,10 +39,12 @@ public final class AttributeInfoExtractor {
static AttributeInfo buildAttributeInfo(ExtractorContext context, Attribute attribute) {
AttributeInfo.Builder builder =
AttributeInfo.newBuilder()
.setName(attribute.getPublicName())
.setName(internalToUnicode(attribute.getPublicName()))
.setType(getAttributeType(context, attribute.getType(), attribute.getPublicName()))
.setMandatory(attribute.isMandatory());
Optional.ofNullable(attribute.getDoc()).ifPresent(builder::setDocString);
Optional.ofNullable(attribute.getDoc())
.map(StringEncoding::internalToUnicode)
.ifPresent(builder::setDocString);
if (!attribute.isConfigurable()) {
builder.setNonconfigurable(true);
}
Expand All @@ -57,7 +62,9 @@ static AttributeInfo buildAttributeInfo(ExtractorContext context, Attribute attr
if (!attribute.isMandatory()) {
try {
Object defaultValue = Attribute.valueToStarlark(attribute.getDefaultValueUnchecked());
builder.setDefaultValue(context.labelRenderer().reprWithoutLabelConstructor(defaultValue));
builder.setDefaultValue(
StringEncoding.internalToUnicode(
context.labelRenderer().reprWithoutLabelConstructor(defaultValue)));
} catch (InvalidStarlarkValueException e) {
builder.setDefaultValue(UNREPRESENTABLE_VALUE);
}
Expand Down
Loading

0 comments on commit acae5b0

Please sign in to comment.