From 5038bbccb5f29c75e67600d344fd56302f89e7af Mon Sep 17 00:00:00 2001 From: Stephen Sherratt Date: Thu, 4 Apr 2024 21:05:01 +1100 Subject: [PATCH] Fix locating program in exec watch mode This fixes a bug where running an executable from the current project with `dune exec --watch` would be unable to find the executable unless the command was run from the project's root directory. The problem was introduced when we started setting the cwd of processes spawned by exec in watch mode to the user's current directory (https://github.com/ocaml/dune/pull/10262). If the program argument to exec refers to a file to be built in the current project (such as an executable implemented in the current project) then the path to the executable to spawn will be a path inside the _build directory relative to the project root. Since the cwd of the new process was set to the user's current directory, this relative path was being resolved within the current directory, whereas it should have been resolved relative to the project root. The fix was to convert relative paths into absolute paths relative to the project root (this was already being done for exec when not in watch mode). Signed-off-by: Stephen Sherratt --- bin/exec.ml | 25 +++++++++++-------- bin/import.ml | 18 +++++++------ doc/changes/10386.md | 3 +++ .../blackbox-tests/test-cases/exec-watch/dune | 4 +-- .../exec-watch-multi-levels.t/bin/main.ml | 2 +- .../exec-watch-multi-levels.t/run.t | 21 ++++++++++------ 6 files changed, 44 insertions(+), 29 deletions(-) create mode 100644 doc/changes/10386.md diff --git a/bin/exec.ml b/bin/exec.ml index d18191afb0c5..e4f9b9d033d4 100644 --- a/bin/exec.ml +++ b/bin/exec.ml @@ -77,22 +77,25 @@ module Command_to_exec = struct (* Helper function to spawn a new process running a command in an environment, returning the new process' pid *) - let spawn_process path ~args ~env = + let spawn_process common prog ~args ~env = let pid = - let path = Path.to_string path in + let prog = Path.to_string prog |> resolve_relative_path_within_root_dir common in let env = Env.to_unix env |> Spawn.Env.of_list in - let argv = path :: args in + let argv = prog :: args in let cwd = Spawn.Working_dir.Path Fpath.initial_cwd in - Spawn.spawn ~prog:path ~env ~cwd ~argv () + Spawn.spawn ~prog ~env ~cwd ~argv () in Pid.of_int pid ;; (* Run the command, first (re)building the program which the command is invoking *) - let build_and_run_in_child_process { get_path_and_build_if_necessary; prog; args; env } = + let build_and_run_in_child_process + common + { get_path_and_build_if_necessary; prog; args; env } + = get_path_and_build_if_necessary prog - |> Fiber.map ~f:(Result.map ~f:(spawn_process ~args ~env)) + |> Fiber.map ~f:(Result.map ~f:(spawn_process common ~args ~env)) ;; end @@ -139,18 +142,18 @@ module Watch = struct (* Kills the currently running process, then runs the given command after (re)building the program which it will invoke *) - let run state ~command_to_exec = + let run common state ~command_to_exec = let open Fiber.O in let* () = Fiber.return () in let* () = kill_currently_running_process state in let* command_to_exec = command_to_exec () in - Command_to_exec.build_and_run_in_child_process command_to_exec + Command_to_exec.build_and_run_in_child_process common command_to_exec >>| Result.map ~f:(fun pid -> state.currently_running_pid := Some pid) ;; - let loop ~command_to_exec = + let loop common ~command_to_exec = let state = init_state () in - Scheduler.Run.poll (run state ~command_to_exec) + Scheduler.Run.poll (run common state ~command_to_exec) ;; end @@ -322,7 +325,7 @@ module Exec_context = struct ; env } in - Watch.loop ~command_to_exec + Watch.loop common ~command_to_exec ;; end diff --git a/bin/import.ml b/bin/import.ml index b93831be48a4..de9c02979e42 100644 --- a/bin/import.ml +++ b/bin/import.ml @@ -239,14 +239,16 @@ module Scheduler = struct ;; end -let restore_cwd_and_execve (common : Common.t) prog argv env = - let prog = - if Filename.is_relative prog - then ( - let root = Common.root common in - Filename.concat root.dir prog) - else prog - in +let resolve_relative_path_within_root_dir (common : Common.t) path = + if Filename.is_relative path + then ( + let root = Common.root common in + Filename.concat root.dir path) + else path +;; + +let restore_cwd_and_execve common prog argv env = + let prog = resolve_relative_path_within_root_dir common prog in Proc.restore_cwd_and_execve prog argv ~env ;; diff --git a/doc/changes/10386.md b/doc/changes/10386.md new file mode 100644 index 000000000000..4a236307504c --- /dev/null +++ b/doc/changes/10386.md @@ -0,0 +1,3 @@ +- Fix bug with exec watch mode where paths to executables in the current project + could not be resolved unless the user's current directory is the project root. + (#10386, @gridbugs) diff --git a/test/blackbox-tests/test-cases/exec-watch/dune b/test/blackbox-tests/test-cases/exec-watch/dune index 41a483576a3e..6d92f7790f6e 100644 --- a/test/blackbox-tests/test-cases/exec-watch/dune +++ b/test/blackbox-tests/test-cases/exec-watch/dune @@ -7,9 +7,9 @@ ;; sometimes go undetected. Adding a delay (e.g. `sleep 1`) before modifying the ;; program seems to guarantee a rebuild will be triggered, but that is too ;; unreliable to depend on in a test so these tests are disabled on macos. - (<> "macosx" %{ocaml-config:system}) + ;; (<> "macosx" %{ocaml-config:system}) ;; disabled until it works in CI - )) + false)) (cram (deps wait-for-file.sh)) diff --git a/test/blackbox-tests/test-cases/exec-watch/exec-watch-multi-levels.t/bin/main.ml b/test/blackbox-tests/test-cases/exec-watch/exec-watch-multi-levels.t/bin/main.ml index cc68426678c9..be4ee019edf6 100644 --- a/test/blackbox-tests/test-cases/exec-watch/exec-watch-multi-levels.t/bin/main.ml +++ b/test/blackbox-tests/test-cases/exec-watch/exec-watch-multi-levels.t/bin/main.ml @@ -3,7 +3,7 @@ let () = let fd = Unix.openfile path [ Unix.O_CREAT ] 777 in Unix.close fd in - print_endline "foo"; + print_endline "foo"; match Sys.argv.(1) with | exception _ -> () | path -> touch path diff --git a/test/blackbox-tests/test-cases/exec-watch/exec-watch-multi-levels.t/run.t b/test/blackbox-tests/test-cases/exec-watch/exec-watch-multi-levels.t/run.t index cb9c9147a2eb..b51a63d9913e 100644 --- a/test/blackbox-tests/test-cases/exec-watch/exec-watch-multi-levels.t/run.t +++ b/test/blackbox-tests/test-cases/exec-watch/exec-watch-multi-levels.t/run.t @@ -1,5 +1,7 @@ +Test the behaviour of exec watch mode when the current directory is not the +project root directory. -exec + watch works fine when invoked at the root level +"dune exec --watch" works fine when invoked at the root level $ DONE_FLAG=_build/done_flag $ dune exec --watch ./bin/main.exe $DONE_FLAG & Success, waiting for filesystem changes... @@ -12,15 +14,20 @@ change the code and proceed with the test. $ ../wait-for-file.sh $DONE_FLAG $ kill $PID - -It's broken when invoked in a child folder - $ cd bin && dune exec --root .. --watch ./bin/main.exe & +Perform the same test above but first enter the "bin" directory. + $ dune clean + $ cd bin + $ dune exec --root .. --watch ./bin/main.exe ../$DONE_FLAG & Entering directory '..' - Error: posix_spawn(): No such file or directory + Success, waiting for filesystem changes... + foo Leaving directory '..' - $ wait + $ PID=$! + $ cd .. + $ ../wait-for-file.sh $DONE_FLAG + $ kill $PID -But it works fine without watch mode +Test that the behaviour is the same when not running with "--watch" $ cd bin && dune exec --root .. ./bin/main.exe Entering directory '..' Leaving directory '..'