Skip to content

Commit

Permalink
Use C.UTF-8 instead of en_US.UTF-8 locale for Java compilation
Browse files Browse the repository at this point in the history
Roll forward of d2f1971

#19249

PiperOrigin-RevId: 684536122
Change-Id: I70f399a9e90375e20bcec7011238af27c6c71c0b
  • Loading branch information
cushon authored and copybara-github committed Oct 10, 2024
1 parent b042810 commit ca5f6cf
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,24 @@

package com.google.devtools.build.lib.bazel.rules.java;

import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.rules.java.JavaSemantics;
import com.google.devtools.build.lib.rules.java.JavaUtil;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationConstant;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.List;

/**
* Semantics for Bazel Java rules
*/
/** Semantics for Bazel Java rules */
public class BazelJavaSemantics implements JavaSemantics {

/**
* {@code C.UTF-8} is now the universally accepted standard UTF-8 locale, to the point where some
* minimal distributions no longer ship with {@code en_US.UTF-8}.
*/
private static final ImmutableMap<String, String> UTF8_ENVIRONMENT =
ImmutableMap.of("LC_CTYPE", "C.UTF-8");

@SerializationConstant public static final BazelJavaSemantics INSTANCE = new BazelJavaSemantics();

private BazelJavaSemantics() {}
Expand Down Expand Up @@ -58,5 +64,8 @@ public PathFragment getDefaultJavaResourcePath(PathFragment path) {
return javaPath == null ? path : javaPath;
}

@Override
public ImmutableMap<String, String> utf8Environment() {
return UTF8_ENVIRONMENT;
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,7 @@ && getJavaConfiguration().experimentalEnableJspecify()
builder.setSourceFiles(sourceFiles);
builder.setSourceJars(sourceJars);
builder.setJavacOpts(javacopts);
builder.setUtf8Environment(semantics.utf8Environment());
builder.setJavacExecutionInfo(executionInfoInterner.intern(getExecutionInfo()));
builder.setCompressJar(true);
builder.setExtraData(JavaCommon.computePerPackageData(ruleContext, javaToolchain));
Expand Down Expand Up @@ -461,6 +462,7 @@ private JavaHeaderCompileAction.Builder getJavaHeaderCompileActionBuilder()
builder.setAdditionalInputs(additionalInputsForDatabinding);
builder.setToolsJars(javaToolchain.getTools());
builder.setExecGroup(execGroup);
builder.setUtf8Environment(semantics.utf8Environment());
return builder;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ enum CompilationType {

private final NestedSet<Artifact> tools;
private final CompilationType compilationType;
private final ActionEnvironment env;
private final ImmutableMap<String, String> executionInfo;
private final CommandLine executableLine;
private final CommandLine flagLine;
Expand All @@ -153,6 +154,7 @@ public JavaCompileAction(
NestedSet<Artifact> transitiveInputs,
NestedSet<Artifact> directJars,
ImmutableSet<Artifact> outputs,
ActionEnvironment env,
ImmutableMap<String, String> executionInfo,
ExtraActionInfoSupplier extraActionInfoSupplier,
CommandLine executableLine,
Expand All @@ -174,6 +176,7 @@ public JavaCompileAction(
}
this.tools = tools;
this.compilationType = compilationType;
this.env = env;
this.executionInfo =
configuration.modifiedExecutionInfo(executionInfo, compilationType.mnemonic);
this.executableLine = executableLine;
Expand Down Expand Up @@ -212,9 +215,7 @@ public NestedSet<Artifact> getTools() {

@Override
public ActionEnvironment getEnvironment() {
return configuration
.getActionEnvironment()
.withAdditionalFixedVariables(JavaCompileActionBuilder.UTF8_ENVIRONMENT);
return env;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
import com.google.devtools.build.lib.actions.ActionEnvironment;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.extra.ExtraActionInfo;
import com.google.devtools.build.lib.actions.extra.JavaCompileInfo;
Expand Down Expand Up @@ -51,10 +52,6 @@ public final class JavaCompileActionBuilder {

private static final String JACOCO_INSTRUMENTATION_PROCESSOR = "jacoco";

/** Environment variable that sets the UTF-8 charset. */
static final ImmutableMap<String, String> UTF8_ENVIRONMENT =
ImmutableMap.of("LC_CTYPE", "en_US.UTF-8");

static final String MNEMONIC = "Javac";

/** Returns true if this is a Java compile action. */
Expand Down Expand Up @@ -147,6 +144,7 @@ public void extend(ExtraActionInfo.Builder builder, ImmutableList<String> argume
private NestedSet<Artifact> compileTimeDependencyArtifacts =
NestedSetBuilder.emptySet(Order.STABLE_ORDER);
private ImmutableList<String> javacOpts = ImmutableList.of();
private ImmutableMap<String, String> utf8Environment = null;
private ImmutableMap<String, String> executionInfo = ImmutableMap.of();
private boolean compressJar;
private NestedSet<Artifact> classpathEntries = NestedSetBuilder.emptySet(Order.NAIVE_LINK_ORDER);
Expand All @@ -172,6 +170,8 @@ public JavaCompileActionBuilder(
}

public JavaCompileAction build() throws RuleErrorException, InterruptedException {
checkNotNull(utf8Environment, "utf8Environment must not be null");

// TODO(bazel-team): all the params should be calculated before getting here, and the various
// aggregation code below should go away.

Expand Down Expand Up @@ -235,6 +235,12 @@ public JavaCompileAction build() throws RuleErrorException, InterruptedException

CustomCommandLine executableLine = javaBuilder.getCommandLine(toolchain);

ActionEnvironment actionEnvironment =
ruleContext
.getConfiguration()
.getActionEnvironment()
.withAdditionalFixedVariables(utf8Environment);

return new JavaCompileAction(
/* compilationType= */ JavaCompileAction.CompilationType.JAVAC,
/* owner= */ ruleContext.getActionOwner(execGroup),
Expand All @@ -249,6 +255,7 @@ public JavaCompileAction build() throws RuleErrorException, InterruptedException
/* transitiveInputs= */ classpathEntries,
/* directJars= */ directJars,
/* outputs= */ allOutputs(),
/* env= */ actionEnvironment,
/* executionInfo= */ executionInfo,
/* extraActionInfoSupplier= */ extraActionInfoSupplier,
/* executableLine= */ executableLine,
Expand Down Expand Up @@ -382,6 +389,12 @@ public JavaCompileActionBuilder setJavacOpts(ImmutableList<String> copts) {
return this;
}

@CanIgnoreReturnValue
public JavaCompileActionBuilder setUtf8Environment(ImmutableMap<String, String> utf8Environment) {
this.utf8Environment = Preconditions.checkNotNull(utf8Environment);
return this;
}

@CanIgnoreReturnValue
public JavaCompileActionBuilder setJavacExecutionInfo(
ImmutableMap<String, String> executionInfo) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import static com.google.devtools.build.lib.actions.ActionAnalysisMetadata.mergeMaps;
import static com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType.UNQUOTED;
import static com.google.devtools.build.lib.packages.ExecGroup.DEFAULT_EXEC_GROUP_NAME;
import static com.google.devtools.build.lib.rules.java.JavaCompileActionBuilder.UTF8_ENVIRONMENT;
import static java.nio.charset.StandardCharsets.ISO_8859_1;

import com.google.common.collect.ImmutableList;
Expand Down Expand Up @@ -210,6 +209,8 @@ public static final class Builder {

private String execGroup = DEFAULT_EXEC_GROUP_NAME;

private ImmutableMap<String, String> utf8Environment = null;

private Builder(RuleContext ruleContext) {
this.ruleContext = ruleContext;
}
Expand Down Expand Up @@ -386,6 +387,13 @@ public Builder enableDirectClasspath(boolean enableDirectClasspath) {
return this;
}

@CanIgnoreReturnValue
public Builder setUtf8Environment(ImmutableMap<String, String> utf8Environment) {
checkNotNull(utf8Environment, "utf8Environment must not be null");
this.utf8Environment = utf8Environment;
return this;
}

/** Builds and registers the action for a header compilation. */
public void build(JavaToolchainProvider javaToolchain)
throws RuleErrorException, InterruptedException {
Expand All @@ -398,6 +406,7 @@ public void build(JavaToolchainProvider javaToolchain)
checkNotNull(directJars, "directJars must not be null");
checkNotNull(
compileTimeDependencyArtifacts, "compileTimeDependencyArtifacts must not be null");
checkNotNull(utf8Environment, "utf8Environment must not be null");

// Invariant: if strictJavaDeps is OFF, then directJars and
// dependencyArtifacts are ignored
Expand Down Expand Up @@ -433,7 +442,7 @@ public void build(JavaToolchainProvider javaToolchain)
ruleContext
.getConfiguration()
.getActionEnvironment()
.withAdditionalFixedVariables(UTF8_ENVIRONMENT);
.withAdditionalFixedVariables(utf8Environment);

OnDemandString progressMessage =
new ProgressMessage(
Expand Down Expand Up @@ -609,6 +618,7 @@ public void build(JavaToolchainProvider javaToolchain)
/* transitiveInputs= */ classpathEntries,
/* directJars= */ directJars,
/* outputs= */ outputs.build(),
/* env= */ actionEnvironment,
/* executionInfo= */ executionInfo.buildKeepingLast(),
/* extraActionInfoSupplier= */ null,
/* executableLine= */ executableLine,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@


import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.analysis.OutputGroupInfo;
import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
import com.google.devtools.build.lib.cmdline.Label;
Expand Down Expand Up @@ -74,4 +75,6 @@ static Label javaToolchainAttribute(RuleDefinitionEnvironment environment) {
*/
PathFragment getDefaultJavaResourcePath(PathFragment path);

/** Environment variable that sets the UTF-8 charset. */
ImmutableMap<String, String> utf8Environment();
}
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ public void testLocale() throws Exception {
JavaCompileAction action =
(JavaCompileAction) getGeneratingActionForLabel("//java/com/google/test:liba.jar");
assertThat(action.getIncompleteEnvironmentForTesting())
.containsEntry("LC_CTYPE", "en_US.UTF-8");
.containsEntry("LC_CTYPE", analysisMock.isThisBazel() ? "C.UTF-8" : "en_US.UTF-8");
}

@Test
Expand Down

0 comments on commit ca5f6cf

Please sign in to comment.