From c2473399705a43b0b554416298a0bffcacdafe42 Mon Sep 17 00:00:00 2001 From: Alejandro R Mosteo Date: Wed, 6 May 2020 14:43:07 +0200 Subject: [PATCH] Log external detection as busy status (#385) * Log external detection as busy status This message is generally not wanted in the final output, since it's used only to signal alr is not dead/stuck, but busy. Also, it must be entirely removed from some commands like `alr setenv` or the output is no longer parseable. * Detect output redirection to adjust logging * Switch --no-tty to disable control chars in output * Fix tests for the changed output --- deps/simple_logging | 2 +- src/alire/alire-externals-lists.adb | 41 ++++++++++----- src/alire/alire_early_elaboration.adb | 22 ++++++++ src/alr/alr-commands-list.adb | 5 +- src/alr/alr-commands-search.adb | 3 +- src/alr/alr-commands-setenv.adb | 11 +++- src/alr/alr-commands.adb | 12 +++++ src/alr/alr-utils.adb | 52 ------------------- src/alr/alr-utils.ads | 21 -------- testsuite/tests/get/system-hint/test.py | 5 +- .../tests/index/external-available/test.py | 5 +- testsuite/tests/index/external-hint/test.py | 3 +- testsuite/tests/with/no-double-add/test.py | 3 +- 13 files changed, 85 insertions(+), 100 deletions(-) diff --git a/deps/simple_logging b/deps/simple_logging index a2515a78..73a5dd32 160000 --- a/deps/simple_logging +++ b/deps/simple_logging @@ -1 +1 @@ -Subproject commit a2515a787485764fb405ae8c0f5bed80c8a81125 +Subproject commit 73a5dd32f3f71d718e982a55969d0d915817c0d9 diff --git a/src/alire/alire-externals-lists.adb b/src/alire/alire-externals-lists.adb index ae543756..571fe2ff 100644 --- a/src/alire/alire-externals-lists.adb +++ b/src/alire/alire-externals-lists.adb @@ -1,6 +1,8 @@ with Alire.Properties.Labeled; with Alire.TOML_Keys; +with Simple_Logging; + package body Alire.Externals.Lists is ------------ @@ -13,19 +15,32 @@ package body Alire.Externals.Lists is return Containers.Release_Set is begin - Trace.Info ("Looking for external crate: " & (+Name)); - return Detected : Containers.Release_Set do - for External of This loop - if External.Available.Check (Env) then - Trace.Debug ("Attempting detection of available external: " - & (+Name)); - Detected.Union (External.Detect (Name)); - else - Trace.Debug ("Skipping detection of unavailable external: " - & (+Name)); - end if; - end loop; - end return; + + -- Avoid the log message if there's nothing to detect + + if This.Is_Empty then + return Containers.Release_Sets.Empty_Set; + end if; + + declare + Busy : Simple_Logging.Ongoing := Simple_Logging.Activity + ("Looking for external crate: " & (+Name)); + begin + return Detected : Containers.Release_Set do + for External of This loop + if External.Available.Check (Env) then + Trace.Debug ("Attempting detection of available external: " + & (+Name)); + Detected.Union (External.Detect (Name)); + else + Trace.Debug ("Skipping detection of unavailable external: " + & (+Name)); + end if; + + Busy.Step; + end loop; + end return; + end; end Detect; ----------- diff --git a/src/alire/alire_early_elaboration.adb b/src/alire/alire_early_elaboration.adb index 43099724..40289098 100644 --- a/src/alire/alire_early_elaboration.adb +++ b/src/alire/alire_early_elaboration.adb @@ -5,6 +5,8 @@ with Alire; with GNAT.Command_Line; with GNAT.OS_Lib; +with GNATCOLL.Terminal; + with Simple_Logging.Filtering; package body Alire_Early_Elaboration is @@ -134,6 +136,26 @@ package body Alire_Early_Elaboration is end if; end Early_Switch_Detection; + ------------------- + -- TTY_Detection -- + ------------------- + + procedure TTY_Detection is + use GNATCOLL.Terminal; + Info : Terminal_Info; + begin + Init_For_Stdout (Info); + + -- Internally, GNATCOLL uses _istty to ascertain color availability, so + -- this serves us too to check if output is being redirected, in which + -- case we don't want certain log output to be emitted. + + if not Has_Colors (Info) then + Simple_Logging.Is_TTY := False; + end if; + end TTY_Detection; + begin + TTY_Detection; Early_Switch_Detection; end Alire_Early_Elaboration; diff --git a/src/alr/alr-commands-list.adb b/src/alr/alr-commands-list.adb index 16b69355..048fe60e 100644 --- a/src/alr/alr-commands-list.adb +++ b/src/alr/alr-commands-list.adb @@ -4,6 +4,8 @@ with Alire.Index; with Alr.Utils; +with Simple_Logging; + package body Alr.Commands.List is ------------- @@ -28,7 +30,8 @@ package body Alr.Commands.List is Requires_Full_Index; declare - Busy : Utils.Busy_Prompt := Utils.Busy_Activity ("Searching..."); + Busy : Simple_Logging.Ongoing := + Simple_Logging.Activity ("Searching"); begin for Crate of Alire.Index.All_Crates.all loop if Num_Arguments = 0 or else diff --git a/src/alr/alr-commands-search.adb b/src/alr/alr-commands-search.adb index 2c5559d5..03974374 100644 --- a/src/alr/alr-commands-search.adb +++ b/src/alr/alr-commands-search.adb @@ -118,7 +118,8 @@ package body Alr.Commands.Search is Tab.Append ("NOTES"); declare - Busy : Utils.Busy_Prompt := Utils.Busy_Activity ("Searching..."); + Busy : Simple_Logging.Ongoing := + Simple_Logging.Activity ("Searching"); ------------------------ -- List_All_Or_Latest -- diff --git a/src/alr/alr-commands-setenv.adb b/src/alr/alr-commands-setenv.adb index c3fb0cfa..87dcbbbf 100644 --- a/src/alr/alr-commands-setenv.adb +++ b/src/alr/alr-commands-setenv.adb @@ -14,7 +14,16 @@ package body Alr.Commands.Setenv is Requires_Valid_Session; - Alr.Build_Env.Print (Alr.Root.Current); + -- Temporarily raise the log level to avoid spurious status output that + -- would prevent directly sourcing the output. TODO: remove this once + -- the lockfile is used, that will make this unnecessary. + declare + Old_Level : constant Simple_Logging.Levels := Alire.Log_Level; + begin + Alire.Log_Level := Simple_Logging.Always; + Alr.Build_Env.Print (Alr.Root.Current); + Alire.Log_Level := Old_Level; + end; end Execute; ------------- diff --git a/src/alr/alr-commands.adb b/src/alr/alr-commands.adb index bf893ae3..47bac8fa 100644 --- a/src/alr/alr-commands.adb +++ b/src/alr/alr-commands.adb @@ -84,6 +84,9 @@ package body Alr.Commands is Prefer_Oldest : aliased Boolean := False; -- Catches the --prefer-oldest policy switch + No_TTY : aliased Boolean := False; + -- Used to disable control characters in output + ----------- -- Image -- ----------- @@ -175,6 +178,11 @@ package body Alr.Commands is "-n", "--non-interactive", "Assume default answers for all user prompts"); + Define_Switch (Config, + No_TTY'Access, + Long_Switch => "--no-tty", + Help => "Disables control characters in output"); + Define_Switch (Config, Prefer_Oldest'Access, Long_Switch => "--prefer-oldest", @@ -593,6 +601,10 @@ package body Alr.Commands is Alire.Config.Set_Path (Command_Line_Config_Path.all); end if; + if No_TTY then + Simple_Logging.Is_TTY := False; + end if; + exception when Exit_From_Command_Line | Invalid_Switch | Invalid_Parameter => -- Getopt has already displayed some help diff --git a/src/alr/alr-utils.adb b/src/alr/alr-utils.adb index f9dbef95..6503ee4c 100644 --- a/src/alr/alr-utils.adb +++ b/src/alr/alr-utils.adb @@ -1,21 +1,5 @@ -with Ada.Text_IO; - package body Alr.Utils is - Indicator : constant String := ".oOo"; - - ------------------- - -- Busy_Activity -- - ------------------- - - function Busy_Activity (Activity : String) return Busy_Prompt is - begin - return Busy : Busy_Prompt (Activity'Length) do - Busy.Activity := Activity; - Busy.Step; - end return; - end Busy_Activity; - -------------- -- Contains -- -------------- @@ -31,40 +15,4 @@ package body Alr.Utils is return False; end Contains; - -------------- - -- Finalize -- - -------------- - - overriding procedure Finalize (This : in out Busy_Prompt) is - begin - if Trace.Level = Info then - Ada.Text_IO.Put - (ASCII.CR & (1 .. This.Activity'Length + 1 => ' ') & ASCII.CR); - Ada.Text_IO.Flush; - end if; - exception - when others => - null; - end Finalize; - - ---------- - -- Step -- - ---------- - - procedure Step (This : in out Busy_Prompt) is - use Ada.Calendar; - begin - if Trace.Level = Info and then Clock - This.Last >= 0.1 then - Ada.Text_IO.Put - (ASCII.CR & This.Activity & " " & Indicator (This.Pos)); - Ada.Text_IO.Flush; - - This.Last := Clock; - This.Pos := This.Pos + 1; - if This.Pos > Indicator'Last then - This.Pos := Indicator'First; - end if; - end if; - end Step; - end Alr.Utils; diff --git a/src/alr/alr-utils.ads b/src/alr/alr-utils.ads index 31fb3354..e7744996 100644 --- a/src/alr/alr-utils.ads +++ b/src/alr/alr-utils.ads @@ -1,6 +1,3 @@ -private with Ada.Calendar; -private with Ada.Finalization; - with Alire.Utils; package Alr.Utils is @@ -45,26 +42,8 @@ package Alr.Utils is function Contains (V : String_Vector; Subst : String) return Boolean; -- Any of the strings contains it - type Busy_Prompt (<>) is tagged limited private; - -- Busy prompt for a scope. Will only work in Info level - - function Busy_Activity (Activity : String) return Busy_Prompt; - - procedure Step (This : in out Busy_Prompt); - -- Say that progress was made - private function Quote (S : String) return String is ("""" & S & """"); - type Busy_Prompt (Len : Natural) - is new Ada.Finalization.Limited_Controlled - with record - Last : Ada.Calendar.Time := Ada.Calendar.Time_Of (1976, 9, 6); - Activity : String (1 .. Len); - Pos : Positive := 1; - end record; - - overriding procedure Finalize (This : in out Busy_Prompt); - end Alr.Utils; diff --git a/testsuite/tests/get/system-hint/test.py b/testsuite/tests/get/system-hint/test.py index 06950f5d..b56412e0 100644 --- a/testsuite/tests/get/system-hint/test.py +++ b/testsuite/tests/get/system-hint/test.py @@ -9,11 +9,10 @@ from drivers.asserts import assert_match import re -p = run_alr('get', 'libhello=0.9-test_unav_native', +p = run_alr('get', '--no-tty', 'libhello=0.9-test_unav_native', complain_on_error=True, quiet=False) -assert_match('Looking for external crate: make\n' - 'Warning: The following external dependencies are unavailable within Alire:\n' +assert_match('Warning: The following external dependencies are unavailable within Alire:\n' 'Warning: make\*\n' 'Warning: They should be made available in the environment by the user.\n', p.out, flags=re.S) diff --git a/testsuite/tests/index/external-available/test.py b/testsuite/tests/index/external-available/test.py index ca3a277e..d5f4e698 100644 --- a/testsuite/tests/index/external-available/test.py +++ b/testsuite/tests/index/external-available/test.py @@ -28,10 +28,9 @@ assert_match(".*Executable make --version .* False.*", # 3rd test: crate is not detected because it is unavailable. It would be # detectable otherwise (make is installed in all test images) -p = run_alr('show', 'crate', '--external-detect', quiet=False) +p = run_alr('show', '--no-tty', 'crate', '--external-detect', quiet=False) -assert_match("Looking for external crate: crate\n" - "Not found: crate with Newest version.*", +assert_match("Not found: crate with Newest version.*", p.out, flags=re.S) diff --git a/testsuite/tests/index/external-hint/test.py b/testsuite/tests/index/external-hint/test.py index 8cfc2628..f78fc05b 100644 --- a/testsuite/tests/index/external-hint/test.py +++ b/testsuite/tests/index/external-hint/test.py @@ -19,8 +19,7 @@ p = run_alr('get', 'crate', quiet=False, complain_on_error=False) if distro_is_known(): assert_match(".*Hint: This is a custom hint.*", p.out, flags=re.S) else: - assert_eq('Looking for external crate: crate\n' - 'ERROR: No source release indexed for the requested crate, and ' + assert_eq('ERROR: No source release indexed for the requested crate, and ' 'cannot use system packages in unknown distribution\n' 'ERROR: alr get unsuccessful\n', p.out) diff --git a/testsuite/tests/with/no-double-add/test.py b/testsuite/tests/with/no-double-add/test.py index 24229429..c0283574 100644 --- a/testsuite/tests/with/no-double-add/test.py +++ b/testsuite/tests/with/no-double-add/test.py @@ -14,8 +14,7 @@ os.chdir('xxx') p = run_alr('with', 'libhello') p = run_alr('with', 'libhello', quiet=False) -assert_eq('Not adding libhello because libhello* is already a dependency\n' - 'Looking for external crate: libhello\n', +assert_eq('Not adding libhello because libhello* is already a dependency\n', p.out) print('SUCCESS') -- 2.39.5