Remove charset from ParameterFileWriteAction. The sole use of UTF-8 was a bug.
Generally when blaze has a String on its heap, the contents are encoded as
latin1, regardless of their interpretation outside blaze. Such a String will
losslessly round-trip with byte[], which is typically all that we require.
RELNOTES: None.
PiperOrigin-RevId: 312343886
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/ParameterFileWriteAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/ParameterFileWriteAction.java
index 39ac0462..5ca54b0 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/ParameterFileWriteAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/ParameterFileWriteAction.java
@@ -14,6 +14,8 @@
package com.google.devtools.build.lib.analysis.actions;
+import static java.nio.charset.StandardCharsets.ISO_8859_1;
+
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.collect.Iterables;
@@ -39,10 +41,9 @@
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.OutputStream;
-import java.nio.charset.Charset;
/** Action to write a parameter file for a {@link CommandLine}. */
-@Immutable // if commandLine and charset are immutable
+@Immutable // if commandLine is immutable
@AutoCodec
public final class ParameterFileWriteAction extends AbstractFileWriteAction {
@@ -50,7 +51,6 @@
private final CommandLine commandLine;
private final ParameterFileType type;
- private final Charset charset;
private final boolean hasInputArtifactToExpand;
/**
@@ -60,11 +60,10 @@
* @param output the Artifact that will be created by executing this Action
* @param commandLine the contents to be written to the file
* @param type the type of the file
- * @param charset the charset of the file
*/
- public ParameterFileWriteAction(ActionOwner owner, Artifact output, CommandLine commandLine,
- ParameterFileType type, Charset charset) {
- this(owner, NestedSetBuilder.emptySet(Order.STABLE_ORDER), output, commandLine, type, charset);
+ public ParameterFileWriteAction(
+ ActionOwner owner, Artifact output, CommandLine commandLine, ParameterFileType type) {
+ this(owner, NestedSetBuilder.emptySet(Order.STABLE_ORDER), output, commandLine, type);
}
/**
@@ -76,7 +75,6 @@
* @param output the Artifact that will be created by executing this Action
* @param commandLine the contents to be written to the file
* @param type the type of the file
- * @param charset the charset of the file
*/
@AutoCodec.Instantiator
public ParameterFileWriteAction(
@@ -84,12 +82,10 @@
NestedSet<Artifact> inputs,
Artifact output,
CommandLine commandLine,
- ParameterFileType type,
- Charset charset) {
+ ParameterFileType type) {
super(owner, inputs, output, false);
this.commandLine = commandLine;
this.type = type;
- this.charset = charset;
this.hasInputArtifactToExpand = !inputs.isEmpty();
}
@@ -117,8 +113,8 @@
@VisibleForTesting
public String getStringContents() throws CommandLineExpansionException, IOException {
ByteArrayOutputStream out = new ByteArrayOutputStream();
- ParameterFile.writeParameterFile(out, getArguments(), type, charset);
- return new String(out.toByteArray(), charset);
+ ParameterFile.writeParameterFile(out, getArguments(), type, ISO_8859_1);
+ return new String(out.toByteArray(), ISO_8859_1);
}
@Override
@@ -144,7 +140,7 @@
} catch (CommandLineExpansionException e) {
throw new UserExecException(e);
}
- return new ParamFileWriter(arguments, type, charset);
+ return new ParamFileWriter(arguments, type);
}
@VisibleForSerialization
@@ -155,17 +151,15 @@
private static class ParamFileWriter implements DeterministicWriter {
private final Iterable<String> arguments;
private final ParameterFileType type;
- private final Charset charset;
- ParamFileWriter(Iterable<String> arguments, ParameterFileType type, Charset charset) {
+ ParamFileWriter(Iterable<String> arguments, ParameterFileType type) {
this.arguments = arguments;
this.type = type;
- this.charset = charset;
}
@Override
public void writeOutputFile(OutputStream out) throws IOException {
- ParameterFile.writeParameterFile(out, arguments, type, charset);
+ ParameterFile.writeParameterFile(out, arguments, type, ISO_8859_1);
}
}
@@ -175,7 +169,6 @@
fp.addString(GUID);
fp.addString(String.valueOf(makeExecutable));
fp.addString(type.toString());
- fp.addString(charset.toString());
commandLine.addToFingerprint(actionKeyContext, fp);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/StarlarkActionFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/StarlarkActionFactory.java
index ae1bd8c..8b62916 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/StarlarkActionFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/StarlarkActionFactory.java
@@ -307,8 +307,7 @@
NestedSetBuilder.wrap(Order.STABLE_ORDER, args.getDirectoryArtifacts()),
(Artifact) output,
args.build(),
- args.getParameterFileType(),
- StandardCharsets.UTF_8);
+ args.getParameterFileType());
} else {
throw new AssertionError("Unexpected type: " + content.getClass().getSimpleName());
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java
index ff1e3db..555464a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java
@@ -18,7 +18,6 @@
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.devtools.build.lib.packages.Type.STRING;
-import static java.nio.charset.StandardCharsets.ISO_8859_1;
import com.google.auto.value.AutoValue;
import com.google.common.base.Function;
@@ -1464,8 +1463,7 @@
NestedSetBuilder.create(Order.STABLE_ORDER, inputTree),
paramFile,
args,
- ParameterFile.ParameterFileType.SHELL_QUOTED,
- ISO_8859_1));
+ ParameterFile.ParameterFileType.SHELL_QUOTED));
ruleContext.registerAction(
singleJarSpawnActionBuilder(ruleContext)
.setMnemonic("MergeDexZips")
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
index 25a95f8..96dbc10 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
@@ -14,7 +14,6 @@
package com.google.devtools.build.lib.rules.cpp;
-import static java.nio.charset.StandardCharsets.ISO_8859_1;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
@@ -1006,8 +1005,7 @@
paramFileActionInputs,
paramFile,
linkCommandLine.paramCmdLine(),
- ParameterFile.ParameterFileType.UNQUOTED,
- ISO_8859_1);
+ ParameterFile.ParameterFileType.UNQUOTED);
actionConstructionContext.registerAction(parameterFileWriteAction);
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java
index c3ca52b..8b47165 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java
@@ -1283,8 +1283,7 @@
treeObjFiles.build(),
objList,
objFilesToLinkParam.build(),
- ParameterFile.ParameterFileType.UNQUOTED,
- ISO_8859_1));
+ ParameterFile.ParameterFileType.UNQUOTED));
return this;
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java
index cddda1c..cc7f069 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java
@@ -43,8 +43,6 @@
import com.google.devtools.build.lib.util.io.FileOutErr;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
-import java.nio.charset.Charset;
-import java.nio.charset.StandardCharsets;
import java.util.Collection;
import java.util.List;
import org.junit.Before;
@@ -145,8 +143,7 @@
inputTreeArtifacts,
outputArtifact,
commandLine,
- ParameterFileType.UNQUOTED,
- StandardCharsets.ISO_8859_1);
+ ParameterFileType.UNQUOTED);
}
private CommandLine createNormalCommandLine() {
@@ -211,7 +208,6 @@
private enum KeyAttributes {
COMMANDLINE,
FILE_TYPE,
- CHARSET,
}
@Test
@@ -226,16 +222,8 @@
attributesToFlip.contains(KeyAttributes.FILE_TYPE)
? ParameterFileType.SHELL_QUOTED
: ParameterFileType.UNQUOTED;
- Charset charset =
- attributesToFlip.contains(KeyAttributes.CHARSET)
- ? StandardCharsets.UTF_8
- : StandardCharsets.US_ASCII;
return new ParameterFileWriteAction(
- ActionsTestUtil.NULL_ACTION_OWNER,
- outputArtifact,
- commandLine,
- parameterFileType,
- charset);
+ ActionsTestUtil.NULL_ACTION_OWNER, outputArtifact, commandLine, parameterFileType);
},
actionKeyContext);
}
