Windows: remove incompatible_windows_style_arg_escaping
In this PR:
- Removed the
`--incompatible_windows_style_arg_escaping` flag
and all code for its "false" case. The flag is
flipped to true.
- In WindowsProcessesTest, the
testDoesNotQuoteArgWithDoubleQuote method is
replaced by testQuotesArgWithDoubleQuote, i.e.
the test logic is reversed. The new test shows
the correct behavior, the old test was wrong.
See https://github.com/bazelbuild/bazel/issues/7122
See https://github.com/bazelbuild/bazel/issues/7454
RELNOTES[INC]: The --incompatible_windows_style_arg_escaping flag is flipped to "true", and the "false" case unsupported. Bazel no longer accepts this flag.
Closes #8003.
PiperOrigin-RevId: 255929367
diff --git a/src/test/java/com/google/devtools/build/lib/windows/WindowsSubprocessTest.java b/src/test/java/com/google/devtools/build/lib/windows/WindowsSubprocessTest.java
index d392230..77ccd85 100644
--- a/src/test/java/com/google/devtools/build/lib/windows/WindowsSubprocessTest.java
+++ b/src/test/java/com/google/devtools/build/lib/windows/WindowsSubprocessTest.java
@@ -17,17 +17,14 @@
import static com.google.common.truth.Truth.assertThat;
import static java.nio.charset.StandardCharsets.UTF_8;
-import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.shell.ShellUtils;
import com.google.devtools.build.lib.shell.Subprocess;
import com.google.devtools.build.lib.shell.SubprocessBuilder;
import com.google.devtools.build.lib.testutil.TestSpec;
import com.google.devtools.build.lib.util.OS;
-import com.google.devtools.build.lib.windows.jni.WindowsProcesses;
import com.google.devtools.build.runfiles.Runfiles;
import java.io.File;
-import java.util.function.Function;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
@@ -65,9 +62,9 @@
}
}
- private void assertSystemRootIsSetByDefault(boolean windowsStyleArgEscaping) throws Exception {
- SubprocessBuilder subprocessBuilder =
- new SubprocessBuilder(new WindowsSubprocessFactory(windowsStyleArgEscaping));
+ @Test
+ public void testSystemRootIsSetByDefault() throws Exception {
+ SubprocessBuilder subprocessBuilder = new SubprocessBuilder(WindowsSubprocessFactory.INSTANCE);
subprocessBuilder.setWorkingDirectory(new File("."));
subprocessBuilder.setArgv(mockBinary, "-jar", mockSubprocess, "O$SYSTEMROOT");
process = subprocessBuilder.start();
@@ -80,18 +77,8 @@
}
@Test
- public void testSystemRootIsSetByDefaultNoWindowsStyleArgEscaping() throws Exception {
- assertSystemRootIsSetByDefault(false);
- }
-
- @Test
- public void testSystemRootIsSetByDefaultWithWindowsStyleArgEscaping() throws Exception {
- assertSystemRootIsSetByDefault(true);
- }
-
- private void assertSystemDriveIsSetByDefault(boolean windowsStyleArgEscaping) throws Exception {
- SubprocessBuilder subprocessBuilder =
- new SubprocessBuilder(new WindowsSubprocessFactory(windowsStyleArgEscaping));
+ public void testSystemDriveIsSetByDefault() throws Exception {
+ SubprocessBuilder subprocessBuilder = new SubprocessBuilder(WindowsSubprocessFactory.INSTANCE);
subprocessBuilder.setWorkingDirectory(new File("."));
subprocessBuilder.setArgv(mockBinary, "-jar", mockSubprocess, "O$SYSTEMDRIVE");
process = subprocessBuilder.start();
@@ -104,18 +91,8 @@
}
@Test
- public void testSystemDriveIsSetByDefaultNoWindowsStyleArgEscaping() throws Exception {
- assertSystemDriveIsSetByDefault(false);
- }
-
- @Test
- public void testSystemDriveIsSetByDefaultWithWindowsStyleArgEscaping() throws Exception {
- assertSystemDriveIsSetByDefault(true);
- }
-
- private void assertSystemRootIsSet(boolean windowsStyleArgEscaping) throws Exception {
- SubprocessBuilder subprocessBuilder =
- new SubprocessBuilder(new WindowsSubprocessFactory(windowsStyleArgEscaping));
+ public void testSystemRootIsSet() throws Exception {
+ SubprocessBuilder subprocessBuilder = new SubprocessBuilder(WindowsSubprocessFactory.INSTANCE);
subprocessBuilder.setWorkingDirectory(new File("."));
subprocessBuilder.setArgv(mockBinary, "-jar", mockSubprocess, "O$SYSTEMROOT");
// Case shouldn't matter on Windows
@@ -130,18 +107,8 @@
}
@Test
- public void testSystemRootIsSetNoWindowsStyleArgEscaping() throws Exception {
- assertSystemRootIsSet(false);
- }
-
- @Test
- public void testSystemRootIsSetWithWindowsStyleArgEscaping() throws Exception {
- assertSystemRootIsSet(true);
- }
-
- private void assertSystemDriveIsSet(boolean windowsStyleArgEscaping) throws Exception {
- SubprocessBuilder subprocessBuilder =
- new SubprocessBuilder(new WindowsSubprocessFactory(windowsStyleArgEscaping));
+ public void testSystemDriveIsSet() throws Exception {
+ SubprocessBuilder subprocessBuilder = new SubprocessBuilder(WindowsSubprocessFactory.INSTANCE);
subprocessBuilder.setWorkingDirectory(new File("."));
subprocessBuilder.setArgv(mockBinary, "-jar", mockSubprocess, "O$SYSTEMDRIVE");
// Case shouldn't matter on Windows
@@ -155,16 +122,6 @@
assertThat(new String(buf, UTF_8).trim()).isEqualTo("X:");
}
- @Test
- public void testSystemDriveIsSetNoWindowsStyleArgEscaping() throws Exception {
- assertSystemDriveIsSet(false);
- }
-
- @Test
- public void testSystemDriveIsSetWithWindowsStyleArgEscaping() throws Exception {
- assertSystemDriveIsSet(true);
- }
-
/**
* An argument and its command-line-escaped counterpart.
*
@@ -181,9 +138,7 @@
};
/** Asserts that a subprocess correctly receives command line arguments. */
- private void assertSubprocessReceivesArgsAsIntended(
- boolean windowsStyleArgEscaping, Function<String, String> escaper, ArgPair... args)
- throws Exception {
+ private void assertSubprocessReceivesArgsAsIntended(ArgPair... args) throws Exception {
// Look up the path of the printarg.exe utility.
String printArgExe =
runfiles.rlocation("io_bazel/src/test/java/com/google/devtools/build/lib/printarg.exe");
@@ -191,11 +146,11 @@
for (ArgPair arg : args) {
// Assert that the command-line encoding logic works as intended.
- assertThat(escaper.apply(arg.original)).isEqualTo(arg.escaped);
+ assertThat(ShellUtils.windowsEscapeArg(arg.original)).isEqualTo(arg.escaped);
// Create a separate subprocess just for this argument.
SubprocessBuilder subprocessBuilder =
- new SubprocessBuilder(new WindowsSubprocessFactory(windowsStyleArgEscaping));
+ new SubprocessBuilder(WindowsSubprocessFactory.INSTANCE);
subprocessBuilder.setWorkingDirectory(new File("."));
subprocessBuilder.setArgv(printArgExe, arg.original);
process = subprocessBuilder.start();
@@ -212,25 +167,8 @@
}
@Test
- public void testSubprocessReceivesArgsAsIntendedNoWindowsStyleArgEscaping() throws Exception {
+ public void testSubprocessReceivesArgsAsIntended() throws Exception {
assertSubprocessReceivesArgsAsIntended(
- false,
- x -> WindowsProcesses.quoteCommandLine(ImmutableList.of(x)),
- new ArgPair("", "\"\""),
- new ArgPair(" ", "\" \""),
- new ArgPair("foo", "foo"),
- new ArgPair("foo\\bar", "foo\\bar"),
- new ArgPair("foo bar", "\"foo bar\""));
- // TODO(laszlocsomor): the escaping logic in WindowsProcesses.quoteCommandLine is wrong, because
- // it fails to properly escape things like a single backslash followed by a quote, e.g. a\"b
- // Fix the escaping logic and add more test here.
- }
-
- @Test
- public void testSubprocessReceivesArgsAsIntendedWithWindowsStyleArgEscaping() throws Exception {
- assertSubprocessReceivesArgsAsIntended(
- true,
- x -> ShellUtils.windowsEscapeArg(x),
new ArgPair("", "\"\""),
new ArgPair(" ", "\" \""),
new ArgPair("\"", "\"\\\"\""),
