Fixing driver tests on Windows #20209

gmittert · 2018-11-01T02:23:36Z

Path separator normalization
Appending .exe or .EXE to binaries

See also #20172

compnerd · 2018-11-02T17:01:26Z

The path separator changes are great. We are going to need a bunch of that in the debug info tests as we check the emitted paths. Why the change from swift to swiftc.EXE?

compnerd · 2018-11-02T17:01:34Z

CC: @jrose-apple

jrose-apple · 2018-11-02T18:25:46Z

Thanks for doing this!

The path separator changes are great.

Since you missed a few and things are still passing, I question whether we actually need this! At least for inputs to swiftc and other LLVMish tools.

Why the change from swift to swiftc.EXE?

I'm guessing this is because swiftc isn't just a symlink to swift on Windows, and so the frontend invocations still have the "c" (and of course the ".exe"). I do think it's fine to write the regexes combined: swift{{c?(.EXE)?}}, which is slightly easier for me to read.

gmittert · 2018-11-02T18:38:15Z

Yeah, not all the paths need changing. Swift for the most parts doesn't seem to have too many issues with backslashes. I think mostly just the ones that get written to .json files, since the unescaped backslashes make the json invalid.

On windows, %target-swiftc_driver gets resolved to "swiftc.EXE", e.g.

$ ":" "RUN: at line 1"
$ "not" "C:\\Users\\jmittertreiner\\LocalSwift\\build\\Ninja-ReleaseAssert\\swift-windows-amd64\\bin\\swiftc.EXE" "-target" "x86_64-unknown-windows-msvc" "-module-cache-path" "C:\\Users\\jmittertreiner\\LocalSwift\\build\\Ninja-ReleaseAssert\\swift-windows-amd64\\swift-test-results\\x86_64-unknown-windows-msvc\\clang-module-cache" "-Xfrontend" "-color-diagnostics" "-emit-executable" "-o" "C:\Users\jmittertreiner\LocalSwift\build\Ninja-ReleaseAssert\swift-windows-amd64\test-windows-x86_64\Driver\Output\color-diagnostics.swift.tmp" "C:\Users\jmittertreiner\LocalSwift\swift\test\Driver\color-diagnostics.swift"
$ "C:\\Python27\\python.exe" "C:\\Users\\jmittertreiner\\LocalSwift\\swift\\utils\\PathSanitizingFileCheck" "--sanitize" "BUILD_DIR=C:/Users/jmittertreiner/LocalSwift/build/Ninja-ReleaseAssert/swift-windows-amd64" "--sanitize" "SOURCE_DIR=C:/Users/jmittertreiner/LocalSwift/swift" "--use-filecheck" "C:\\Users\\jmittertreiner\\LocalSwift\\build\\Ninja-ReleaseAssert\\llvm-windows-amd64\\bin\\FileCheck.EXE" "C:\Users\jmittertreiner\LocalSwift\swift\test\Driver\color-diagnostics.swift"

which then gets used in the rest of the driver. Checking for c.EXE was just the easiest fix.

compnerd · 2018-12-04T22:35:21Z

@jrose-apple also note that things aren't really passing from the perspective of the unit tests. There are a bunch of failing tests still.

jrose-apple · 2018-12-04T22:42:38Z

The conversions from e.g. %t to %/t are starting to concern me. Aren't we going to need that for literally every test?

gmittert · 2018-12-04T22:50:33Z

@jrose-apple no, just for ones that print paths to .json files since the backslashes are invalid without escaping. It's possible I added a few where they weren't strictly needed though.

jrose-apple

Thanks for the explanation! Here are a few more specific questions I had, then.

jrose-apple · 2018-12-04T23:01:19Z

test/Driver/filelists.swift

@@ -1,3 +1,4 @@
+// UNSUPPORTED: win32


Why is this one unsupported on Windows?

This one uses driver-use-frontend-path, see also #21052

jrose-apple · 2018-12-04T23:01:41Z

test/Driver/linker.swift

@@ -1,3 +1,4 @@
+// UNSUPPORTED: win32


I don't think this can both be unsupported on Windows and have slash changes.

jrose-apple · 2018-12-04T23:02:46Z

test/Driver/loaded_module_trace_multifile.swift

+// CHECK: "{{[^"]*[/\\]}}Module2.swiftmodule"
+// CHECK: "{{[^"]*[/\\]}}Module.swiftmodule"
+// CHECK: "{{[^"]*[/\\]}}Swift.swiftmodule"
+// CHECK: "{{[^"]*[/\\]}}SwiftOnoneSupport.swiftmodule"


This looks incorrect; this file is supposed to be JSON, right?

Should be correct, it's looking for either a forwards or backslash, on windows the backslash gets escaped, so it'll be something like filepath\\Module2.swiftmodule which should match

Right, but the pattern you provided isn't checking for \\ because that's a character class.

Oh, I guess it happens to work anyway because of the preceding character class. Okay.

jrose-apple · 2018-12-04T23:03:47Z

test/Driver/parseable_output.swift

@@ -1,44 +1,42 @@
 // RUN: %swiftc_driver_plain -emit-executable %s -o %t.out -emit-module -emit-module-path %t.swiftmodule -emit-objc-header-path %t.h -serialize-diagnostics -emit-dependencies -parseable-output -driver-skip-execution 2>&1 | %FileCheck %s

-// XFAIL: freebsd, linux


Does this work now?

Nope :( That shouldn't have been removed. I've added it back.

jrose-apple · 2018-12-04T23:04:03Z

test/Driver/response-file.swift

@@ -1,5 +1,5 @@
 // RUN: echo "-DTEST0" > %t.0.resp
-// RUN: %target-build-swift -typecheck @%t.0.resp %s 2>&1 | %FileCheck %s -check-prefix=SHORT
+// RUN: %target-build-swift -typecheck %s @%/t.0.resp 2>&1 | %FileCheck %s -check-prefix=SHORT


I don't think this one would matter?

Yup, it doesn't. I'll remove it. It's broken because response files don't work on windows right now, not because of the path.

jrose-apple · 2018-12-05T23:38:58Z

@swift-ci Please test

swift-ci · 2018-12-05T23:45:13Z

Build failed
Swift Test Linux Platform
Git Sha - 4bbcf0dcdc9f3bf4b4eea93d80c282a94ce095c3

swift-ci · 2018-12-06T00:48:01Z

Build failed
Swift Test OS X Platform
Git Sha - 4bbcf0dcdc9f3bf4b4eea93d80c282a94ce095c3

gmittert · 2018-12-06T02:15:40Z

I should mention that this doesn't fix all the driver tests on windows, just the "easy" ones

color diagnostics work on windows, but don't get printed during testing, not sure why yet
response files don't work yet (I've got to fix that properly)
unicode file names don't work on windows yet (might be related to the response files not working)
the driver-frontend tests don't work (That should be fixable by adding additional arguments)
multi threading isn't implemented on windows yet

gmittert force-pushed the MoreWinTestFixes branch 2 times, most recently from 2a6e035 to 358162e Compare November 2, 2018 19:04

gmittert force-pushed the MoreWinTestFixes branch 2 times, most recently from d740106 to 4bbcf0d Compare December 4, 2018 22:35

gmittert force-pushed the MoreWinTestFixes branch from 4bbcf0d to bd1dead Compare December 4, 2018 22:37

jrose-apple reviewed Dec 4, 2018

View reviewed changes

gmittert force-pushed the MoreWinTestFixes branch from 9e0fbc1 to 3492f49 Compare December 5, 2018 23:21

Fixing Driver tests on Windows

2467b66

gmittert force-pushed the MoreWinTestFixes branch from 3492f49 to 2467b66 Compare December 5, 2018 23:32

jrose-apple self-assigned this Dec 6, 2018

jrose-apple merged commit 4e0b092 into apple:master Dec 11, 2018

compnerd mentioned this pull request Nov 2, 2018

[SR-9161] fix up test suite for Windows #51656

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixing driver tests on Windows #20209

Fixing driver tests on Windows #20209

gmittert commented Nov 1, 2018

compnerd commented Nov 2, 2018

compnerd commented Nov 2, 2018

jrose-apple commented Nov 2, 2018

gmittert commented Nov 2, 2018 •

edited

compnerd commented Dec 4, 2018

jrose-apple commented Dec 4, 2018

gmittert commented Dec 4, 2018

jrose-apple left a comment

jrose-apple Dec 4, 2018

gmittert Dec 5, 2018

jrose-apple Dec 4, 2018

jrose-apple Dec 4, 2018

gmittert Dec 5, 2018

jrose-apple Dec 5, 2018

jrose-apple Dec 5, 2018

jrose-apple Dec 4, 2018

gmittert Dec 5, 2018 •

edited

jrose-apple Dec 4, 2018

gmittert Dec 5, 2018 •

edited

jrose-apple commented Dec 5, 2018

swift-ci commented Dec 5, 2018

swift-ci commented Dec 6, 2018

gmittert commented Dec 6, 2018 •

edited

		@@ -1,44 +1,42 @@
		// RUN: %swiftc_driver_plain -emit-executable %s -o %t.out -emit-module -emit-module-path %t.swiftmodule -emit-objc-header-path %t.h -serialize-diagnostics -emit-dependencies -parseable-output -driver-skip-execution 2>&1 \| %FileCheck %s

		// XFAIL: freebsd, linux

Fixing driver tests on Windows #20209

Fixing driver tests on Windows #20209

Conversation

gmittert commented Nov 1, 2018

compnerd commented Nov 2, 2018

compnerd commented Nov 2, 2018

jrose-apple commented Nov 2, 2018

gmittert commented Nov 2, 2018 • edited

compnerd commented Dec 4, 2018

jrose-apple commented Dec 4, 2018

gmittert commented Dec 4, 2018

jrose-apple left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gmittert Dec 5, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gmittert Dec 5, 2018 • edited

Choose a reason for hiding this comment

jrose-apple commented Dec 5, 2018

swift-ci commented Dec 5, 2018

swift-ci commented Dec 6, 2018

gmittert commented Dec 6, 2018 • edited

gmittert commented Nov 2, 2018 •

edited

gmittert Dec 5, 2018 •

edited

gmittert Dec 5, 2018 •

edited

gmittert commented Dec 6, 2018 •

edited