diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD index fe2c9c40f6bc83..d2417ebd714e8b 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD +++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD @@ -473,6 +473,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/buildeventstream", "//src/main/java/com/google/devtools/build/lib/buildeventstream/proto:build_event_stream_java_proto", "//src/main/java/com/google/devtools/build/lib/events", + "//src/main/java/com/google/devtools/build/lib/util:string_encoding", "//third_party:guava", ], ) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildInfoEvent.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildInfoEvent.java index 3d7680a4974def..21ddb3e6b2f984 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BuildInfoEvent.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildInfoEvent.java @@ -23,6 +23,7 @@ import com.google.devtools.build.lib.buildeventstream.BuildEventWithOrderConstraint; import com.google.devtools.build.lib.buildeventstream.GenericBuildEvent; import com.google.devtools.build.lib.events.ExtendedEventHandler; +import com.google.devtools.build.lib.util.StringEncoding; import java.util.Collection; import java.util.Map; @@ -67,8 +68,8 @@ public BuildEventStreamProtos.BuildEvent asStreamProto(BuildEventContext convert for (Map.Entry entry : getBuildInfoMap().entrySet()) { status.addItem( BuildEventStreamProtos.WorkspaceStatus.Item.newBuilder() - .setKey(entry.getKey()) - .setValue(entry.getValue()) + .setKey(StringEncoding.internalToUnicode(entry.getKey())) + .setValue(StringEncoding.internalToUnicode(entry.getValue())) .build()); } return GenericBuildEvent.protoChaining(this).setWorkspaceStatus(status.build()).build(); diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelWorkspaceStatusModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelWorkspaceStatusModule.java index 0506100ac5d7f3..e687414c83e422 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BazelWorkspaceStatusModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelWorkspaceStatusModule.java @@ -14,7 +14,7 @@ package com.google.devtools.build.lib.bazel; import static com.google.common.base.StandardSystemProperty.USER_NAME; -import static java.nio.charset.StandardCharsets.UTF_8; +import static java.nio.charset.StandardCharsets.ISO_8859_1; import static java.util.stream.Collectors.joining; import com.google.common.collect.ImmutableList; @@ -58,7 +58,6 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.OutputStream; -import java.nio.charset.StandardCharsets; import java.time.Instant; import java.time.ZoneOffset; import java.time.format.DateTimeFormatter; @@ -117,7 +116,9 @@ private String getAdditionalWorkspaceStatus( } catch (IOException e) { throw createExecutionException(e, Code.STDERR_IO_EXCEPTION); } - return stdoutStream.toString(UTF_8); + // Keep the output in Bazel's internal string encoding (see StringEncoding), like the + // other entries in the status maps. + return stdoutStream.toString(ISO_8859_1); } } catch (BadExitStatusException e) { throw createExecutionException(e, Code.NON_ZERO_EXIT); @@ -154,7 +155,9 @@ private static byte[] printStatusMap(Map map) { .map(entry -> entry.getKey() + " " + entry.getValue()) .collect(joining("\n")); s += "\n"; - return s.getBytes(StandardCharsets.UTF_8); + // The values are in Bazel's internal string encoding (see StringEncoding), write them out as + // is. + return s.getBytes(ISO_8859_1); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BUILD b/src/main/java/com/google/devtools/build/lib/runtime/BUILD index cc651e39206912..471435bd6a08fa 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BUILD +++ b/src/main/java/com/google/devtools/build/lib/runtime/BUILD @@ -106,6 +106,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/buildeventstream", "//src/main/java/com/google/devtools/build/lib/buildeventstream/proto:build_event_stream_java_proto", "//src/main/java/com/google/devtools/build/lib/util:pair", + "//src/main/java/com/google/devtools/build/lib/util:string_encoding", "//src/main/java/com/google/devtools/common/options", "//src/main/protobuf:command_line_java_proto", "//src/main/protobuf:option_filters_java_proto", @@ -326,6 +327,7 @@ java_library( deps = [ "//src/main/java/com/google/devtools/build/lib/buildeventstream", "//src/main/java/com/google/devtools/build/lib/buildeventstream/proto:build_event_stream_java_proto", + "//src/main/java/com/google/devtools/build/lib/util:string_encoding", "//third_party:guava", ], ) diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BuildMetadataEvent.java b/src/main/java/com/google/devtools/build/lib/runtime/BuildMetadataEvent.java index 099571f9acbc73..1e248d54a33fd5 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BuildMetadataEvent.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BuildMetadataEvent.java @@ -21,6 +21,7 @@ import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildEventId; import com.google.devtools.build.lib.buildeventstream.BuildEventWithOrderConstraint; import com.google.devtools.build.lib.buildeventstream.GenericBuildEvent; +import com.google.devtools.build.lib.util.StringEncoding; import java.util.Collection; import java.util.Map; @@ -56,7 +57,9 @@ public BuildEventStreamProtos.BuildEvent asStreamProto(BuildEventContext convert BuildEventStreamProtos.BuildMetadata.Builder metadataBuilder = BuildEventStreamProtos.BuildMetadata.newBuilder(); for (Map.Entry entry : buildMetadata.entrySet()) { - metadataBuilder.putMetadata(entry.getKey(), entry.getValue()); + metadataBuilder.putMetadata( + StringEncoding.internalToUnicode(entry.getKey()), + StringEncoding.internalToUnicode(entry.getValue())); } return GenericBuildEvent.protoChaining(this).setBuildMetadata(metadataBuilder.build()).build(); } diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommandLineEvent.java b/src/main/java/com/google/devtools/build/lib/runtime/CommandLineEvent.java index 92234a9cf44b5b..2be92bfb4a7604 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/CommandLineEvent.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/CommandLineEvent.java @@ -16,6 +16,7 @@ import com.google.common.base.Joiner; import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableList; +import com.google.common.collect.Iterables; import com.google.common.io.BaseEncoding; import com.google.devtools.build.lib.buildeventstream.BuildEventContext; import com.google.devtools.build.lib.buildeventstream.BuildEventIdUtil; @@ -31,6 +32,7 @@ import com.google.devtools.build.lib.runtime.proto.CommandLineOuterClass.Option; import com.google.devtools.build.lib.runtime.proto.CommandLineOuterClass.OptionList; import com.google.devtools.build.lib.util.Pair; +import com.google.devtools.build.lib.util.StringEncoding; import com.google.devtools.common.options.OptionDefinition; import com.google.devtools.common.options.OptionEffectTag; import com.google.devtools.common.options.OptionMetadataTag; @@ -165,15 +167,15 @@ private Option createOption( String combinedForm, @Nullable String value) { Option.Builder option = Option.newBuilder(); - option.setCombinedForm(combinedForm); + option.setCombinedForm(StringEncoding.internalToUnicode(combinedForm)); option.setOptionName(optionDefinition.getOptionName()); if (value != null) { - option.setOptionValue(value); + option.setOptionValue(StringEncoding.internalToUnicode(value)); } option.addAllEffectTags(getProtoEffectTags(optionDefinition.getOptionEffectTags())); option.addAllMetadataTags(getProtoMetadataTags(optionDefinition.getOptionMetadataTags())); if (source != null) { - option.setSource(source); + option.setSource(StringEncoding.internalToUnicode(source)); } return option.build(); } @@ -205,10 +207,10 @@ Option createSingleStarlarkOption(String starlarkFlag, @Nullable Object value) { } } Option.Builder option = Option.newBuilder(); - option.setCombinedForm(sb.toString()); + option.setCombinedForm(StringEncoding.internalToUnicode(sb.toString())); option.setOptionName(starlarkFlag); if (value != null) { - option.setOptionValue(String.valueOf(value)); + option.setOptionValue(StringEncoding.internalToUnicode(String.valueOf(value))); } return option.build(); } @@ -244,13 +246,16 @@ CommandLineSection getResidual() { CommandLineSection.newBuilder().setSectionLabel("residual"); if (commandName.equals("run") && !includeResidueInRunBepEvent && !residue.isEmpty()) { String target = residue.get(0); - ChunkList.Builder residual = ChunkList.newBuilder().addChunk(target); + ChunkList.Builder residual = + ChunkList.newBuilder().addChunk(StringEncoding.internalToUnicode(target)); if (residue.size() > 1) { residual.addChunk("REDACTED"); } builder.setChunkList(residual); } else { - builder.setChunkList(ChunkList.newBuilder().addAllChunk(residue)); + builder.setChunkList( + ChunkList.newBuilder() + .addAllChunk(Iterables.transform(residue, StringEncoding::internalToUnicode))); } return builder.build(); } diff --git a/src/test/java/com/google/devtools/build/lib/runtime/BUILD b/src/test/java/com/google/devtools/build/lib/runtime/BUILD index da89842814771b..fe1a9397429491 100644 --- a/src/test/java/com/google/devtools/build/lib/runtime/BUILD +++ b/src/test/java/com/google/devtools/build/lib/runtime/BUILD @@ -150,6 +150,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code", "//src/main/java/com/google/devtools/build/lib/util:exit_code", "//src/main/java/com/google/devtools/build/lib/util:os", + "//src/main/java/com/google/devtools/build/lib/util:string_encoding", "//src/main/java/com/google/devtools/build/lib/util/io:io-proto", "//src/main/java/com/google/devtools/build/lib/util/io:out-err", "//src/main/java/com/google/devtools/build/lib/vfs", diff --git a/src/test/java/com/google/devtools/build/lib/runtime/CommandLineEventTest.java b/src/test/java/com/google/devtools/build/lib/runtime/CommandLineEventTest.java index 93af3c657922a0..0cb2e5bb78265b 100644 --- a/src/test/java/com/google/devtools/build/lib/runtime/CommandLineEventTest.java +++ b/src/test/java/com/google/devtools/build/lib/runtime/CommandLineEventTest.java @@ -31,6 +31,7 @@ import com.google.devtools.build.lib.runtime.proto.CommandLineOuterClass.CommandLineSection.SectionTypeCase; import com.google.devtools.build.lib.runtime.proto.CommandLineOuterClass.OptionList; import com.google.devtools.build.lib.util.Pair; +import com.google.devtools.build.lib.util.StringEncoding; import com.google.devtools.common.options.OptionPriority.PriorityCategory; import com.google.devtools.common.options.OptionsParser; import com.google.devtools.common.options.OptionsParsingException; @@ -342,6 +343,65 @@ public void testOptionsAtVariousPriorities_canonicalCommandLine() throws Options assertThat(line.getSections(4).getChunkList().getChunkCount()).isEqualTo(0); } + @Test + public void testNonAsciiOptionValue_canonicalCommandLine() throws OptionsParsingException { + OptionsParser fakeStartupOptions = + OptionsParser.builder().optionsClasses(BlazeServerStartupOptions.class).build(); + OptionsParser fakeCommandOptions = + OptionsParser.builder().optionsClasses(TestOptions.class).build(); + // Option values reach the server in Bazel's internal string encoding (see StringEncoding). + fakeCommandOptions.parse( + PriorityCategory.COMMAND_LINE, + "command line", + ImmutableList.of("--test_string=" + StringEncoding.unicodeToInternal("¡Buenos días!"))); + + CommandLine line = + new CanonicalCommandLineEvent( + "testblaze", + fakeStartupOptions, + "someCommandName", + fakeCommandOptions.getResidue(), + false, + ImmutableSortedMap.of(), + ImmutableSortedMap.of(), + ImmutableSet.of(), + fakeCommandOptions.asListOfCanonicalOptions(), + /* replaceable= */ false) + .asStreamProto(null) + .getStructuredCommandLine(); + + // The proto fields are Unicode strings, so the value has to be reencoded. + assertThat(line.getSections(3).getOptionList().getOption(0).getOptionValue()) + .isEqualTo("¡Buenos días!"); + assertThat(line.getSections(3).getOptionList().getOption(0).getCombinedForm()) + .isEqualTo("--test_string=¡Buenos días!"); + } + + @Test + public void testNonAsciiResidue_canonicalCommandLine() throws OptionsParsingException { + OptionsParser fakeStartupOptions = + OptionsParser.builder().optionsClasses(BlazeServerStartupOptions.class).build(); + OptionsParser fakeCommandOptions = + OptionsParser.builder().optionsClasses(TestOptions.class).build(); + + CommandLine line = + new CanonicalCommandLineEvent( + "testblaze", + fakeStartupOptions, + "someCommandName", + ImmutableList.of(StringEncoding.unicodeToInternal("//foo:bär")), + false, + ImmutableSortedMap.of(), + ImmutableSortedMap.of(), + ImmutableSet.of(), + fakeCommandOptions.asListOfCanonicalOptions(), + /* replaceable= */ false) + .asStreamProto(null) + .getStructuredCommandLine(); + + assertThat(line.getSections(4).getChunkList().getChunk(0)).isEqualTo("//foo:bär"); + } + @Test public void testExpansionOption_originalCommandLine() throws OptionsParsingException { OptionsParser fakeStartupOptions = diff --git a/src/test/shell/integration/workspace_status_test.sh b/src/test/shell/integration/workspace_status_test.sh index c665a8cf6e459b..edf00f4b7aba51 100755 --- a/src/test/shell/integration/workspace_status_test.sh +++ b/src/test/shell/integration/workspace_status_test.sh @@ -123,4 +123,26 @@ function test_embed_label_must_be_single_line() { expect_log "Value must not contain multiple lines" } +function test_non_ascii_status_value() { + local script="$TEST_TMPDIR/non_ascii.sh" + cat > "$script" <<'EOF' +#!/usr/bin/env bash +echo "STABLE_GREETING ¡Buenos días!" +EOF + chmod +x "$script" + + bazel build --stamp --workspace_status_command="$script" \ + --build_event_json_file="$TEST_TMPDIR/bep.json" >& "$TEST_log" \ + || fail "Build failed" + + # The status file holds the raw bytes emitted by the script. + local status_file="$(bazel info output_path)/stable-status.txt" + grep -q "STABLE_GREETING ¡Buenos días!" "$status_file" \ + || fail "Expected UTF-8 bytes in stable-status.txt, got: $(cat "$status_file")" + + # The BEP is a proto with Unicode string fields, so the value must be reencoded. + grep -q '¡Buenos días!' "$TEST_TMPDIR/bep.json" \ + || fail "Expected correctly encoded value in BEP" +} + run_suite "${PRODUCT_NAME} workspace status command tests"