From 61135001cce78bde22767969aaeb9aea49ae7aa3 Mon Sep 17 00:00:00 2001 From: Rudi Grinberg Date: Sat, 9 Oct 2021 00:53:52 -0600 Subject: [PATCH] fix: cram file path escaping Fix #4162 Signed-off-by: Rudi Grinberg --- CHANGES.md | 2 + src/dune_rules/cram_exec.ml | 64 ++++++++++++++++--------- test/blackbox-tests/test-cases/gh4162.t | 6 +-- 3 files changed, 46 insertions(+), 26 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 8b796c8ce518..789057a7d001 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,6 +1,8 @@ Unreleased ---------- +- Allow spaces in cram test paths (#.., fixes #4162, @rgrinberg) + - Fix `foreign_stubs` inside a `tests` stanza. Previously, dune would crash when this field was present (#4942, fix #4946, @rgrinberg) diff --git a/src/dune_rules/cram_exec.ml b/src/dune_rules/cram_exec.ml index cc4974c66aef..28f8e7885916 100644 --- a/src/dune_rules/cram_exec.ml +++ b/src/dune_rules/cram_exec.ml @@ -107,18 +107,6 @@ let translate_path_for_sh = | Some cygpath -> Process.run_capture_line Strict cygpath [ Path.to_absolute_filename fn ] -(* Quote a filename for sh, independently of whether we are on Windows or Unix. - On Windows, we still generate a [sh] script so we need to quote using Unix - conventions. *) -let quote_for_sh fn = - let buf = Buffer.create (String.length fn + 2) in - Buffer.add_char buf '\''; - String.iter fn ~f:(function - | '\'' -> Buffer.add_string buf "'\\''" - | c -> Buffer.add_char buf c); - Buffer.add_char buf '\''; - Buffer.contents buf - let cram_stanzas lexbuf = let rec loop acc = match Cram_lexer.block lexbuf with @@ -144,6 +132,24 @@ let run_expect_test file ~f = let fprln oc fmt = Printf.fprintf oc (fmt ^^ "\n") +type escaped = + { assign : string + ; use : string + } + +let ascii_escape ~var s = + let s = + let buf = Buffer.create (String.length s * 4) in + for i = 0 to String.length s - 1 do + Buffer.add_string buf "\\x"; + Buffer.add_string buf (sprintf "%x" (Char.code s.[i])) + done; + Buffer.contents buf + in + { assign = sprintf {|printf -v %s '%s'|} var s + ; use = sprintf {|"${%s}"|} var + } + (* Produce the following shell code: {v cat <<"EOF_" EOF_ v} @@ -164,7 +170,11 @@ let cat_eof ~dest oc lines = sentinel in let sentinel = loop 0 in - prln "cat >%s <<%S" (quote_for_sh (Path.to_absolute_filename dest)) sentinel; + let cram_fn = + ascii_escape ~var:"__cram_fn" (Path.to_absolute_filename dest) + in + prln "%s" cram_fn.assign; + prln "cat >%s <<%s" cram_fn.use sentinel; List.iter lines ~f:(prln "%s"); prln "%s" sentinel @@ -324,12 +334,15 @@ let create_sh_script cram_stanzas ~temp_dir : sh_script Fiber.t = let oc = Io.open_out ~binary:true script in let file name = Path.relative temp_dir name in let open Fiber.O in - let sh_path path = + let sh_path ~var path = let+ path = translate_path_for_sh path in - quote_for_sh path + ascii_escape ~var path in let metadata_file = file "cram.metadata" in - let* metadata_file_sh_path = sh_path metadata_file in + let* metadata_file_sh_path = + sh_path metadata_file ~var:"__cram_metadata_file" + in + fprln oc "%s" metadata_file_sh_path.assign; let i = ref 0 in let loop block = match (block : _ Cram_lexer.block) with @@ -344,17 +357,24 @@ let create_sh_script cram_stanzas ~temp_dir : sh_script Fiber.t = rest of the script. So instead, we dump each user written shell phrase into a file and then source it in the main script. *) let user_shell_code_file = file ~ext:".sh" in - let* user_shell_code_file_sh_path = sh_path user_shell_code_file in + let* user_shell_code_file_sh_path = + sh_path ~var:"__cram_user_shell_code_file_sh_path" user_shell_code_file + in cat_eof oc lines ~dest:user_shell_code_file; (* Where we store the output of shell code written by the user *) let user_shell_code_output_file = file ~ext:".output" in let+ user_shell_code_output_file_sh_path = - sh_path user_shell_code_output_file + sh_path ~var:"__cram_user_shell_code_output_file_sh_path" + user_shell_code_output_file in - fprln oc ". %s > %s 2>&1" user_shell_code_file_sh_path - user_shell_code_output_file_sh_path; - fprln oc {|printf "%%d\0%%s\0" $? $%s >> %s|} - Action_exec._BUILD_PATH_PREFIX_MAP metadata_file_sh_path; + + fprln oc "%s" user_shell_code_file_sh_path.assign; + fprln oc "%s" user_shell_code_output_file_sh_path.assign; + + fprln oc ". %s > %s 2>&1" user_shell_code_file_sh_path.use + user_shell_code_output_file_sh_path.use; + fprln oc {|printf "%%d\0%%s\0" $? "${%s}" >> %s|} + Action_exec._BUILD_PATH_PREFIX_MAP metadata_file_sh_path.use; Cram_lexer.Command { command = lines ; output_file = user_shell_code_output_file diff --git a/test/blackbox-tests/test-cases/gh4162.t b/test/blackbox-tests/test-cases/gh4162.t index 384db0a5130b..c88fa7b2527f 100644 --- a/test/blackbox-tests/test-cases/gh4162.t +++ b/test/blackbox-tests/test-cases/gh4162.t @@ -14,8 +14,6 @@ Cram doesn't like tests in paths $ echo foo $ dune build @foo - sh (internal) (exit 1) - (cd _build/.sandbox/54ffcedad804d81955c7b33d18d42e68/default && /run/current-system/sw/bin/sh /var/folders/nc/x9_nmmsj0rjbfyzxb_kjk6qr0000gn/T/build_95f84b_dune/dune_cram_01b015_.foo.t/main.sh) - /var/folders/nc/x9_nmmsj0rjbfyzxb_kjk6qr0000gn/T/build_95f84b_dune/dune_cram_01b015_.foo.t/main.sh: line 5: printf: bbb/_build/.sandbox/54ffcedad804d81955c7b33d18d42e68/default:$TESTCASE_ROOT=$TESTCASE_ROOT/aaa: invalid number - -> required by alias foo + File "foo.t", line 1, characters 0-0: + Error: Files _build/default/foo.t and _build/default/foo.t.corrected differ. [1]