Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Diagnose missing executable when running an action #1493

Merged
merged 2 commits into from
Nov 8, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/alire/alire-os_lib.adb
Original file line number Diff line number Diff line change
@@ -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
@@ -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;
@@ -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)
5 changes: 2 additions & 3 deletions src/alire/alire-properties-actions-runners.ads
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion testsuite/drivers/alr.py
Original file line number Diff line number Diff line change
@@ -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
8 changes: 8 additions & 0 deletions testsuite/drivers/asserts.py
Original file line number Diff line number Diff line change
@@ -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
@@ -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)

2 changes: 1 addition & 1 deletion testsuite/tests/action/list/test.py
Original file line number Diff line number Diff line change
@@ -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
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
@@ -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
@@ -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