Skip to content

Commit

Permalink
Diagnose missing executable when running an action (#1493)
Browse files Browse the repository at this point in the history
* Fix command not found in actions

* New test
  • Loading branch information
mosteo authored Nov 8, 2023
1 parent 1a1e869 commit fe97634
Show file tree
Hide file tree
Showing 12 changed files with 76 additions and 13 deletions.
17 changes: 17 additions & 0 deletions src/alire/alire-os_lib.adb
Original file line number Diff line number Diff line change
Expand Up @@ -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;
3 changes: 3 additions & 0 deletions src/alire/alire-os_lib.ads
Original file line number Diff line number Diff line change
Expand Up @@ -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;
16 changes: 16 additions & 0 deletions src/alire/alire-properties-actions-executor.adb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
with Alire.Directories;
with Alire.Errors;
with Alire.Flags;
with Alire.OS_Lib.Subprocess;
with Alire.Properties.Actions.Runners;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
5 changes: 2 additions & 3 deletions src/alire/alire-properties-actions-runners.ads
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion testsuite/drivers/alr.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions testsuite/drivers/asserts.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
10 changes: 5 additions & 5 deletions testsuite/tests/action/conditional/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion testsuite/tests/action/list/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 17 additions & 0 deletions testsuite/tests/action/missing-exec/test.py
Original file line number Diff line number Diff line change
@@ -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')
3 changes: 3 additions & 0 deletions testsuite/tests/action/missing-exec/test.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
driver: python-script
build_mode: both
indexes: {}
2 changes: 1 addition & 1 deletion testsuite/tests/build/incremental/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')
4 changes: 2 additions & 2 deletions testsuite/tests/index/custom-action/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit fe97634

Please sign in to comment.