From 73bdc56d5073e98886095e9c3804d09fdf8c0497 Mon Sep 17 00:00:00 2001 From: YR Chen Date: Wed, 8 Dec 2021 07:01:27 +0800 Subject: [PATCH 1/8] Explicitly handle empty paths on Windows --- Sources/TSCBasic/Path.swift | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/Sources/TSCBasic/Path.swift b/Sources/TSCBasic/Path.swift index 6575cd7f..2559cd85 100644 --- a/Sources/TSCBasic/Path.swift +++ b/Sources/TSCBasic/Path.swift @@ -479,11 +479,12 @@ private struct UNIXPath: Path { defer { fsr.deallocate() } let path: String = String(cString: fsr) - return path.withCString(encodedAs: UTF16.self) { + let result: String = path.withCString(encodedAs: UTF16.self) { let data = UnsafeMutablePointer(mutating: $0) PathCchRemoveFileSpec(data, path.count) return String(decodingCString: data, as: UTF16.self) } + return result.isEmpty ? "." : result #else // FIXME: This method seems too complicated; it should be simplified, // if possible, and certainly optimized (using UTF8View). @@ -715,6 +716,12 @@ private struct UNIXPath: Path { init(validatingAbsolutePath path: String) throws { #if os(Windows) + // Explicitly handle the empty path, since retrieving + // `fileSystemRepresentation` of it is illegal. + guard !path.isEmpty else { + throw PathValidationError.invalidAbsolutePath(path) + } + let fsr: UnsafePointer = path.fileSystemRepresentation defer { fsr.deallocate() } @@ -737,6 +744,12 @@ private struct UNIXPath: Path { init(validatingRelativePath path: String) throws { #if os(Windows) + // Explicitly handle the empty path, since retrieving + // `fileSystemRepresentation` of it is illegal. + guard !path.isEmpty else { + throw PathValidationError.invalidRelativePath(path) + } + let fsr: UnsafePointer = path.fileSystemRepresentation defer { fsr.deallocate() } From ec177497c99246978db8afc6d3c4b4a8813c8b89 Mon Sep 17 00:00:00 2001 From: YR Chen Date: Wed, 8 Dec 2021 07:03:37 +0800 Subject: [PATCH 2/8] Implicitly search for `.exe` on Windows --- Sources/TSCBasic/misc.swift | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/Sources/TSCBasic/misc.swift b/Sources/TSCBasic/misc.swift index c999bec1..59198029 100644 --- a/Sources/TSCBasic/misc.swift +++ b/Sources/TSCBasic/misc.swift @@ -224,6 +224,11 @@ public func lookupExecutablePath( guard let value = value, !value.isEmpty else { return nil } +#if os(Windows) + let isPath = value.contains(":") || value.contains("\\") || value.contains("/") +#else + let isPath = value.contains("/") +#endif var paths: [AbsolutePath] = [] @@ -237,9 +242,14 @@ public func lookupExecutablePath( } // Ensure the value is not a path. - if !value.contains("/") { + if !isPath { // Try to locate in search paths. paths.append(contentsOf: searchPaths.map({ $0.appending(component: value) })) +#if os(Windows) + if !value.contains(".") { + paths.append(contentsOf: searchPaths.map({ $0.appending(component: value + executableFileSuffix) })) + } +#endif } return paths.first(where: { localFileSystem.isExecutableFile($0) }) From 0c931dd4469291f2f2492479ebf09537715acb55 Mon Sep 17 00:00:00 2001 From: YR Chen Date: Wed, 8 Dec 2021 07:04:12 +0800 Subject: [PATCH 3/8] Respect default search paths on Windows --- Sources/TSCBasic/Process.swift | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/Sources/TSCBasic/Process.swift b/Sources/TSCBasic/Process.swift index 2d717aca..a2beb19a 100644 --- a/Sources/TSCBasic/Process.swift +++ b/Sources/TSCBasic/Process.swift @@ -14,6 +14,7 @@ import var Foundation.NSLocalizedDescriptionKey #if os(Windows) import Foundation +import WinSDK #endif @_implementationOnly import TSCclibc @@ -471,10 +472,26 @@ public final class Process { pathString: ProcessEnv.path, currentWorkingDirectory: cwdOpt ) + var searchPaths: [AbsolutePath] = [] +#if os(Windows) + var buffer = Array(repeating: 0, count: Int(MAX_PATH + 1)) + // The 32-bit Windows system directory + if GetSystemDirectoryW(&buffer, .init(buffer.count)) > 0 { + searchPaths.append(AbsolutePath(String(decodingCString: buffer, as: UTF16.self))) + } + if GetWindowsDirectoryW(&buffer, .init(buffer.count)) > 0 { + let windowsDirectory = String(decodingCString: buffer, as: UTF16.self) + // The 16-bit Windows system directory + searchPaths.append(AbsolutePath("\(windowsDirectory)\\System")) + // The Windows directory + searchPaths.append(AbsolutePath(windowsDirectory)) + } +#endif + searchPaths.append(contentsOf: envSearchPaths) let value = lookupExecutablePath( filename: program, currentWorkingDirectory: cwdOpt, - searchPaths: envSearchPaths + searchPaths: searchPaths ) return value } From ec7e1bff3ef2dea2df2b0ba718a3fd6d79c55ab5 Mon Sep 17 00:00:00 2001 From: YR Chen Date: Wed, 8 Dec 2021 08:22:33 +0800 Subject: [PATCH 4/8] Update `testFindExecutable` for Windows --- Tests/TSCBasicTests/ProcessTests.swift | 40 +++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/Tests/TSCBasicTests/ProcessTests.swift b/Tests/TSCBasicTests/ProcessTests.swift index 1ff03416..08beed32 100644 --- a/Tests/TSCBasicTests/ProcessTests.swift +++ b/Tests/TSCBasicTests/ProcessTests.swift @@ -108,12 +108,13 @@ class ProcessTests: XCTestCase { } func testFindExecutable() throws { + #if !os(Windows) try testWithTemporaryDirectory { tmpdir in // This process should always work. - XCTAssertTrue(Process.findExecutable("ls") != nil) + XCTAssertNotNil(Process.findExecutable("ls")) - XCTAssertEqual(Process.findExecutable("nonExistantProgram"), nil) - XCTAssertEqual(Process.findExecutable(""), nil) + XCTAssertNil(Process.findExecutable("nonExistantProgram")) + XCTAssertNil(Process.findExecutable("")) // Create a local nonexecutable file to test. let tempExecutable = tmpdir.appending(component: "nonExecutableProgram") @@ -124,9 +125,40 @@ class ProcessTests: XCTestCase { """) try withCustomEnv(["PATH": tmpdir.pathString]) { - XCTAssertEqual(Process.findExecutable("nonExecutableProgram"), nil) + XCTAssertNil(Process.findExecutable("nonExecutableProgram")) } } + #else + try testWithTemporaryDirectory { tmpdir in + // Test System32 without .exe suffix. + XCTAssertNotNil(Process.findExecutable("cmd")) + + // Test Windows with .exe suffix. + XCTAssertNotNil(Process.findExecutable("explorer.exe")) + + // Test non-existant programs. + XCTAssertNil(Process.findExecutable("nonExistantProgram")) + XCTAssertNil(Process.findExecutable("")) + + // Copy an executable file to test. + let tempExecutable = tmpdir.appending(component: "executableProgram.exe") + try localFileSystem.copy(from: Process.findExecutable("cmd")!, to: tempExecutable) + + // Create a non-executable file to test. + let tempNonExecutable = tmpdir.appending(component: "program.bat") + try localFileSystem.writeFileContents(tempNonExecutable, bytes: """ + @echo off + exit + + """) + + try withCustomEnv(["Path": tmpdir.pathString]) { + XCTAssertNotNil(Process.findExecutable("executableProgram.exe")) + XCTAssertNotNil(Process.findExecutable("executableProgram")) + XCTAssertNil(Process.findExecutable("program.bat")) + } + } + #endif } func testNonExecutableLaunch() throws { From 947f56fbb054d2d2d5b5c91aee4a156c42f5b961 Mon Sep 17 00:00:00 2001 From: YR Chen Date: Fri, 22 Apr 2022 14:21:27 +0800 Subject: [PATCH 5/8] Fix `Process.findExecutable` --- Sources/TSCBasic/Process.swift | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Sources/TSCBasic/Process.swift b/Sources/TSCBasic/Process.swift index a2beb19a..444eb4e5 100644 --- a/Sources/TSCBasic/Process.swift +++ b/Sources/TSCBasic/Process.swift @@ -463,6 +463,13 @@ public final class Process { if localFileSystem.isExecutableFile(abs) { return abs } +#if os(Windows) + if abs.extension != "exe" && abs.extension != "", + case let abs = abs.parentDirectory.appending(component: abs.basename + executableFileSuffix), + localFileSystem.isExecutableFile(abs) { + return abs + } +#endif } return nil } From 283c833888dee728191b03b05a1f7521e7edab41 Mon Sep 17 00:00:00 2001 From: YR Chen Date: Fri, 22 Apr 2022 14:32:18 +0800 Subject: [PATCH 6/8] Use clearer variable name --- Sources/TSCBasic/misc.swift | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Sources/TSCBasic/misc.swift b/Sources/TSCBasic/misc.swift index 59198029..2cea465b 100644 --- a/Sources/TSCBasic/misc.swift +++ b/Sources/TSCBasic/misc.swift @@ -225,9 +225,9 @@ public func lookupExecutablePath( return nil } #if os(Windows) - let isPath = value.contains(":") || value.contains("\\") || value.contains("/") + let isFileName = !value.contains(":") && !value.contains("\\") && !value.contains("/") #else - let isPath = value.contains("/") + let isFileName = !value.contains("/") #endif var paths: [AbsolutePath] = [] @@ -241,8 +241,8 @@ public func lookupExecutablePath( paths.append(absPath) } - // Ensure the value is not a path. - if !isPath { + // Only search in PATH if the value is a single path component. + if isFileName { // Try to locate in search paths. paths.append(contentsOf: searchPaths.map({ $0.appending(component: value) })) #if os(Windows) From 05f4e57485c0c1ec803446683d0465712d940cd9 Mon Sep 17 00:00:00 2001 From: YR Chen Date: Fri, 22 Apr 2022 15:23:13 +0800 Subject: [PATCH 7/8] Add note for Windows path searching strategy --- Sources/TSCBasic/Process.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Sources/TSCBasic/Process.swift b/Sources/TSCBasic/Process.swift index 444eb4e5..e70c8c66 100644 --- a/Sources/TSCBasic/Process.swift +++ b/Sources/TSCBasic/Process.swift @@ -481,6 +481,8 @@ public final class Process { ) var searchPaths: [AbsolutePath] = [] #if os(Windows) + // NOTE: `CreateProcess` the Windows system API always searchs System and Windows directories first. + // See https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw#parameters var buffer = Array(repeating: 0, count: Int(MAX_PATH + 1)) // The 32-bit Windows system directory if GetSystemDirectoryW(&buffer, .init(buffer.count)) > 0 { From 145667751e99d0e6b2c554141084e27eae4ea526 Mon Sep 17 00:00:00 2001 From: YR Chen Date: Fri, 22 Apr 2022 16:03:37 +0800 Subject: [PATCH 8/8] Mark `withCustomEnv(_:body:)` as broken on Windows --- Sources/TSCTestSupport/misc.swift | 5 +++++ Tests/TSCBasicTests/ProcessTests.swift | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/Sources/TSCTestSupport/misc.swift b/Sources/TSCTestSupport/misc.swift index 6d58aa17..ece8b255 100644 --- a/Sources/TSCTestSupport/misc.swift +++ b/Sources/TSCTestSupport/misc.swift @@ -9,6 +9,7 @@ */ import func XCTest.XCTFail +import struct XCTest.XCTSkip import class Foundation.NSDate import class Foundation.Thread @@ -61,6 +62,10 @@ public func systemQuietly(_ args: String...) throws { /// from different threads, the environment will neither be setup nor restored /// correctly. public func withCustomEnv(_ env: [String: String], body: () throws -> Void) throws { +#if os(Windows) + // FIXME + throw XCTSkip("Test helper 'withCustomEnv' is known to be broken on Windows") +#endif let state = Array(env.keys).map({ ($0, ProcessEnv.vars[$0]) }) let restore = { for (key, value) in state { diff --git a/Tests/TSCBasicTests/ProcessTests.swift b/Tests/TSCBasicTests/ProcessTests.swift index 08beed32..ddf6ee78 100644 --- a/Tests/TSCBasicTests/ProcessTests.swift +++ b/Tests/TSCBasicTests/ProcessTests.swift @@ -149,7 +149,7 @@ class ProcessTests: XCTestCase { try localFileSystem.writeFileContents(tempNonExecutable, bytes: """ @echo off exit - + """) try withCustomEnv(["Path": tmpdir.pathString]) {