From fe9763404ddafdf057f0f6d39dff9c24d4218aea Mon Sep 17 00:00:00 2001 From: Alejandro R Mosteo Date: Wed, 8 Nov 2023 12:38:29 +0100 Subject: [PATCH] Diagnose missing executable when running an action (#1493) * Fix command not found in actions * New test --- src/alire/alire-os_lib.adb | 17 +++++++++++++++++ src/alire/alire-os_lib.ads | 3 +++ src/alire/alire-properties-actions-executor.adb | 16 ++++++++++++++++ src/alire/alire-properties-actions-runners.ads | 5 ++--- testsuite/drivers/alr.py | 2 +- testsuite/drivers/asserts.py | 8 ++++++++ testsuite/tests/action/conditional/test.py | 10 +++++----- testsuite/tests/action/list/test.py | 2 +- testsuite/tests/action/missing-exec/test.py | 17 +++++++++++++++++ testsuite/tests/action/missing-exec/test.yaml | 3 +++ testsuite/tests/build/incremental/test.py | 2 +- testsuite/tests/index/custom-action/test.py | 4 ++-- 12 files changed, 76 insertions(+), 13 deletions(-) create mode 100644 testsuite/tests/action/missing-exec/test.py create mode 100644 testsuite/tests/action/missing-exec/test.yaml diff --git a/src/alire/alire-os_lib.adb b/src/alire/alire-os_lib.adb index faed2cc3c..a5e3022b9 100644 --- a/src/alire/alire-os_lib.adb +++ b/src/alire/alire-os_lib.adb @@ -61,4 +61,21 @@ package body Alire.OS_Lib is GNAT.OS_Lib.Setenv (Name, Value); end Setenv; + ------------------------- + -- Locate_Exec_On_Path -- + ------------------------- + + function Locate_Exec_On_Path (Exec_Name : String) return String is + Located : GNAT.OS_Lib.String_Access := + GNAT.OS_Lib.Locate_Exec_On_Path (Exec_Name); + begin + if Located not in null then + return Result : constant String := Located.all do + GNAT.OS_Lib.Free (Located); + end return; + else + return ""; + end if; + end Locate_Exec_On_Path; + end Alire.OS_Lib; diff --git a/src/alire/alire-os_lib.ads b/src/alire/alire-os_lib.ads index f386d3b99..81f99d538 100644 --- a/src/alire/alire-os_lib.ads +++ b/src/alire/alire-os_lib.ads @@ -16,4 +16,7 @@ package Alire.OS_Lib with Preelaborate is procedure Setenv (Name : String; Value : String); + function Locate_Exec_On_Path (Exec_Name : String) return String; + -- Return the location of an executable if found on PATH, or "" otherwise + end Alire.OS_Lib; diff --git a/src/alire/alire-properties-actions-executor.adb b/src/alire/alire-properties-actions-executor.adb index 6fc40d9e8..61dbc86d7 100644 --- a/src/alire/alire-properties-actions-executor.adb +++ b/src/alire/alire-properties-actions-executor.adb @@ -1,4 +1,5 @@ with Alire.Directories; +with Alire.Errors; with Alire.Flags; with Alire.OS_Lib.Subprocess; with Alire.Properties.Actions.Runners; @@ -41,6 +42,8 @@ package body Alire.Properties.Actions.Executor is Flags.Post_Fetch (CWD).Mark_Done; end if; exception + when Checked_Error => + raise; when E : others => Log_Exception (E); Trace.Warning ("A " & TOML_Adapters.Tomify (Moment'Image) @@ -73,7 +76,18 @@ package body Alire.Properties.Actions.Executor is Cmd : constant AAA.Strings.Vector := Prefix.Append (This.Command_Line); + Exec : String renames Cmd.First_Element; begin + if Alire.OS_Lib.Locate_Exec_On_Path (Exec) = "" and then + not GNAT.OS_Lib.Is_Executable_File (Exec) + then + Raise_Checked_Error + (Errors.New_Wrapper ("Cannot run action:") + .Wrap ("Command not found [" & TTY.Terminal (Exec) & "]") + .Wrap ("Working directory [" & TTY.URL (Current) & "]") + .Wrap ("Action description [" & This.Image & "]").Get); + end if; + if Capture then Code := Subprocess.Unchecked_Spawn_And_Capture (Command => Cmd.First_Element, @@ -109,6 +123,8 @@ package body Alire.Properties.Actions.Executor is Code => Unused_Code, Output => Unused_Output); exception + when Checked_Error => + raise; when E : others => Log_Exception (E); Trace.Warning ("A " & TOML_Adapters.Tomify (Moment'Image) diff --git a/src/alire/alire-properties-actions-runners.ads b/src/alire/alire-properties-actions-runners.ads index 81bdc99a5..5dd745e42 100644 --- a/src/alire/alire-properties-actions-runners.ads +++ b/src/alire/alire-properties-actions-runners.ads @@ -45,9 +45,8 @@ private function Image (This : Run) return String is (AAA.Strings.To_Mixed_Case (This.Moment'Img) & (if This.Name /= "" then " (" & This.Name & ")" else "") - & " run: ${CRATE_DIR}" & - (if This.Working_Folder /= "" then "/" else "") & - This.Working_Folder & "/" & This.Relative_Command_Line.Flatten); + & " run: " & This.Relative_Command_Line.Flatten + & " (from ${CRATE_ROOT}/" & This.Working_Folder & ")"); overriding function To_YAML (This : Run) return String diff --git a/testsuite/drivers/alr.py b/testsuite/drivers/alr.py index 8ae2fe38e..856a328b2 100644 --- a/testsuite/drivers/alr.py +++ b/testsuite/drivers/alr.py @@ -493,7 +493,7 @@ def alr_with(dep="", path="", url="", commit="", branch="", return run_alr(*args, force=force) -def add_action(type, command, name="", directory=""): +def add_action(type: str, command: [str], name="", directory=""): """ Add an action to the manifest in the current directory. :param str type: "pre-build", etc diff --git a/testsuite/drivers/asserts.py b/testsuite/drivers/asserts.py index a62f1d5b3..2cc332349 100644 --- a/testsuite/drivers/asserts.py +++ b/testsuite/drivers/asserts.py @@ -134,3 +134,11 @@ def match_deploy_dir(crate : str, path_fragment : str): assert_match(f".*[: ]{crate.upper()}_ALIRE_PREFIX=[^\\n]*" f"{re.escape(path_fragment)}[^\\n]*{crate}_.*", p.out) + + +def assert_substring(target: str, text: str): + """ + Check that a string is contained in another string + """ + assert target in text, \ + f"Missing expected string '{target}' in text:\n{text}" \ No newline at end of file diff --git a/testsuite/tests/action/conditional/test.py b/testsuite/tests/action/conditional/test.py index b8e746700..db55b0e5a 100644 --- a/testsuite/tests/action/conditional/test.py +++ b/testsuite/tests/action/conditional/test.py @@ -35,14 +35,14 @@ # Verify actions are there assert_match(".*" + - re.escape(""" Pre_Build run: ${CRATE_DIR}/./echo 1 + re.escape(""" Pre_Build run: echo 1 (from ${CRATE_ROOT}/.) case OS is - when others => Pre_Build run: ${CRATE_DIR}/./echo 2 + when others => Pre_Build run: echo 2 (from ${CRATE_ROOT}/.) case OS is - when others => Pre_Build run: ${CRATE_DIR}/./echo 3 - Pre_Build run: ${CRATE_DIR}/./echo 4 + when others => Pre_Build run: echo 3 (from ${CRATE_ROOT}/.) + Pre_Build run: echo 4 (from ${CRATE_ROOT}/.) case OS is - when others => Pre_Build run: ${CRATE_DIR}/./echo 5 + when others => Pre_Build run: echo 5 (from ${CRATE_ROOT}/.) """) + ".*", run_alr("show").out) diff --git a/testsuite/tests/action/list/test.py b/testsuite/tests/action/list/test.py index ebf229ae6..20a0995ab 100644 --- a/testsuite/tests/action/list/test.py +++ b/testsuite/tests/action/list/test.py @@ -9,7 +9,7 @@ from glob import glob expected_output = """main=1.0.0: - Post_Fetch run: ${CRATE_DIR}/./echo POST-FETCH MAIN + Post_Fetch run: echo POST-FETCH MAIN (from ${CRATE_ROOT}/.) """ # First test, list action in a root crate diff --git a/testsuite/tests/action/missing-exec/test.py b/testsuite/tests/action/missing-exec/test.py new file mode 100644 index 000000000..d467743e9 --- /dev/null +++ b/testsuite/tests/action/missing-exec/test.py @@ -0,0 +1,17 @@ +""" +Check that an action trying to run a missing executable reports such an error. +""" + +from drivers.alr import run_alr, add_action, init_local_crate +from drivers.asserts import assert_substring + +init_local_crate() + +add_action('pre-build', ['fake-missing-exec']) + +p = run_alr("build", complain_on_error=False) + +assert_substring("Command not found [fake-missing-exec]", + p.out) + +print('SUCCESS') diff --git a/testsuite/tests/action/missing-exec/test.yaml b/testsuite/tests/action/missing-exec/test.yaml new file mode 100644 index 000000000..00fb56e10 --- /dev/null +++ b/testsuite/tests/action/missing-exec/test.yaml @@ -0,0 +1,3 @@ +driver: python-script +build_mode: both +indexes: {} \ No newline at end of file diff --git a/testsuite/tests/build/incremental/test.py b/testsuite/tests/build/incremental/test.py index 4ffe21047..980ea2743 100644 --- a/testsuite/tests/build/incremental/test.py +++ b/testsuite/tests/build/incremental/test.py @@ -49,7 +49,7 @@ p = run_alr("build", complain_on_error=False) assert p.status != 0, "Command should have failed" -assert 'Command ["fake_alr_test_exec"] exited with code' in p.out, \ +assert 'Command not found [fake_alr_test_exec]' in p.out, \ "Output does not contain expected error, but: " + p.out print('SUCCESS') diff --git a/testsuite/tests/index/custom-action/test.py b/testsuite/tests/index/custom-action/test.py index 3628073a9..a4b862d96 100644 --- a/testsuite/tests/index/custom-action/test.py +++ b/testsuite/tests/index/custom-action/test.py @@ -26,13 +26,13 @@ def bad_action_check(type, command, name, error_re): # Add a proper custom action and verify its loading add_action(type="on-demand", name="my-action", command=["ls"]) p = run_alr("show") -assert_match(".*" + re.escape("On_Demand (my-action) run: ${CRATE_DIR}/./ls"), +assert_match(".*" + re.escape("On_Demand (my-action) run: ls (from ${CRATE_ROOT}/.)"), p.out) # Verify that regular action can also have a name add_action(type="post-fetch", name="action-2", command=["ls"]) p = run_alr("show") -assert_match(".*" + re.escape("Post_Fetch (action-2) run: ${CRATE_DIR}/./ls"), +assert_match(".*" + re.escape("Post_Fetch (action-2) run: ls (from ${CRATE_ROOT}/.)"), p.out) # Add an on-demand action without name and see it fails