Skip to content

Commit

Permalink
fix: calling programs with spaces on Windows (#4515)
Browse files Browse the repository at this point in the history
This entire API has been carefully optimized for maximum pain output

---------

Co-authored-by: Mac Malone <tydeu@hatpress.net>
  • Loading branch information
Kha and tydeu authored Jul 26, 2024
1 parent c02aa98 commit 9b342ef
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 26 deletions.
1 change: 1 addition & 0 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
cmake_minimum_required(VERSION 3.10)
cmake_policy(SET CMP0054 NEW)
cmake_policy(SET CMP0110 NEW)
if(NOT (${CMAKE_GENERATOR} MATCHES "Unix Makefiles"))
message(FATAL_ERROR "The only supported CMake generator at the moment is 'Unix Makefiles'")
endif()
Expand Down
9 changes: 8 additions & 1 deletion src/runtime/process.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,12 @@ static obj_res spawn(string_ref const & proc_name, array_ref<string_ref> const &
object * parent_stdout = box(0); setup_stdio(&saAttr, &child_stdout, &parent_stdout, false, stdout_mode);
object * parent_stderr = box(0); setup_stdio(&saAttr, &child_stderr, &parent_stderr, false, stderr_mode);

std::string command = proc_name.to_std_string();
std::string program = proc_name.to_std_string();

// Always escape program in cmdline, in case it contains spaces
std::string command = "\"";
command += program;
command += "\"";

// This needs some thought, on Windows we must pass a command string
// which is a valid command, that is a fully assembled command to be executed.
Expand Down Expand Up @@ -247,6 +252,8 @@ static obj_res spawn(string_ref const & proc_name, array_ref<string_ref> const &

// Create the child process.
bool bSuccess = CreateProcess(
// Passing `program` here should be more robust, but would require adding a `.exe` extension
// and searching through `PATH` where necessary
NULL,
const_cast<char *>(command.c_str()), // command line
NULL, // process security attributes
Expand Down
42 changes: 17 additions & 25 deletions src/shell/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -115,18 +115,14 @@ ENDFOREACH(T)

# LEAN BENCHMARK TESTS
# do not test all .lean files in bench/
if(${CMAKE_SYSTEM_NAME} MATCHES "Windows")
message(STATUS "Skipping compiler tests on Windows because of shared library limit on number of exported symbols")
else()
file(GLOB LEANBENCHTESTS "${LEAN_SOURCE_DIR}/../tests/bench/*.lean.expected.out")
FOREACH(T_OUT ${LEANBENCHTESTS})
string(REPLACE ".expected.out" "" T ${T_OUT})
GET_FILENAME_COMPONENT(T_NAME ${T} NAME)
add_test(NAME "leanbenchtest_${T_NAME}"
WORKING_DIRECTORY "${LEAN_SOURCE_DIR}/../tests/bench"
COMMAND bash -c "${TEST_VARS} ./test_single.sh ${T_NAME}")
ENDFOREACH(T_OUT)
endif()
file(GLOB LEANBENCHTESTS "${LEAN_SOURCE_DIR}/../tests/bench/*.lean.expected.out")
FOREACH(T_OUT ${LEANBENCHTESTS})
string(REPLACE ".expected.out" "" T ${T_OUT})
GET_FILENAME_COMPONENT(T_NAME ${T} NAME)
add_test(NAME "leanbenchtest_${T_NAME}"
WORKING_DIRECTORY "${LEAN_SOURCE_DIR}/../tests/bench"
COMMAND bash -c "${TEST_VARS} ./test_single.sh ${T_NAME}")
ENDFOREACH(T_OUT)

file(GLOB LEANINTERPTESTS "${LEAN_SOURCE_DIR}/../tests/plugin/*.lean")
FOREACH(T ${LEANINTERPTESTS})
Expand All @@ -146,19 +142,15 @@ FOREACH(T ${LEANT0TESTS})
ENDFOREACH(T)

# LEAN PACKAGE TESTS
if(${CMAKE_SYSTEM_NAME} MATCHES "Windows")
message(STATUS "Skipping compiler tests on Windows because of shared library limit on number of exported symbols")
else()
file(GLOB LEANPKGTESTS "${LEAN_SOURCE_DIR}/../tests/pkg/*")
FOREACH(T ${LEANPKGTESTS})
if(IS_DIRECTORY ${T})
GET_FILENAME_COMPONENT(T_NAME ${T} NAME)
add_test(NAME "leanpkgtest_${T_NAME}"
WORKING_DIRECTORY "${T}"
COMMAND bash -c "${TEST_VARS} ./test.sh")
endif()
ENDFOREACH(T)
endif()
file(GLOB LEANPKGTESTS "${LEAN_SOURCE_DIR}/../tests/pkg/*")
FOREACH(T ${LEANPKGTESTS})
if(IS_DIRECTORY ${T})
GET_FILENAME_COMPONENT(T_NAME ${T} NAME)
add_test(NAME "leanpkgtest_${T_NAME}"
WORKING_DIRECTORY "${T}"
COMMAND bash -c "${TEST_VARS} ./test.sh")
endif()
ENDFOREACH(T)

# LEAN SERVER TESTS
file(GLOB LEANTESTS "${LEAN_SOURCE_DIR}/../tests/lean/server/*.lean")
Expand Down
1 change: 1 addition & 0 deletions tests/pkg/path with spaces/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/.lake
2 changes: 2 additions & 0 deletions tests/pkg/path with spaces/Main.lean
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
def main : IO Unit :=
IO.println s!"Hello, world!"
6 changes: 6 additions & 0 deletions tests/pkg/path with spaces/lakefile.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
name = "path with spaces"
defaultTargets = ["«path with spaces»"]

[[lean_exe]]
name = "path with spaces"
root = "Main"
7 changes: 7 additions & 0 deletions tests/pkg/path with spaces/test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#!/usr/bin/env bash

rm -rf .lake/build
lake exe "path with spaces"
# presence of this file should not break process spawn
touch .lake/build/bin/path
lake exe "path with spaces"

0 comments on commit 9b342ef

Please sign in to comment.