From bfb9f6c95997f8b698ddf63b4ab5ff5cf5c5756c Mon Sep 17 00:00:00 2001 From: Alejandro R Mosteo Date: Thu, 16 Nov 2023 07:23:22 +0100 Subject: [PATCH] Load configuration earlier from proper location (#1501) * Move configuration loading earlier Also make sure it's impossible to use configuration before the proper path is set. * New test * Better encapsulation * Windows fix --- src/alire/alire-config-edit-early_load.adb | 12 ++++ src/alire/alire-config-edit-early_load.ads | 7 ++ src/alire/alire-config-edit.adb | 10 ++- src/alire/alire-config-edit.ads | 3 +- src/alire/alire-config.adb | 14 ++++ src/alire/alire-config.ads | 8 ++- src/alire/alire-index.ads | 1 + src/alire/alire-toolchains.adb | 2 +- src/alire/alire_early_elaboration.adb | 72 ++++++++++++++----- .../alire-platforms-current__windows.adb | 3 + src/alr/alr-commands-config.adb | 4 +- src/alr/alr-commands.adb | 17 +++-- testsuite/tests/config/early-loading/test.py | 29 ++++++++ .../tests/config/early-loading/test.yaml | 7 ++ .../tests/config/relative_config_path/test.py | 2 +- 15 files changed, 152 insertions(+), 39 deletions(-) create mode 100644 src/alire/alire-config-edit-early_load.adb create mode 100644 src/alire/alire-config-edit-early_load.ads create mode 100644 testsuite/tests/config/early-loading/test.py create mode 100644 testsuite/tests/config/early-loading/test.yaml diff --git a/src/alire/alire-config-edit-early_load.adb b/src/alire/alire-config-edit-early_load.adb new file mode 100644 index 00000000..039df26f --- /dev/null +++ b/src/alire/alire-config-edit-early_load.adb @@ -0,0 +1,12 @@ +package body Alire.Config.Edit.Early_Load is + + ----------------- + -- Load_Config -- + ----------------- + + procedure Load_Config is + begin + Alire.Config.Edit.Load_Config; + end Load_Config; + +end Alire.Config.Edit.Early_Load; diff --git a/src/alire/alire-config-edit-early_load.ads b/src/alire/alire-config-edit-early_load.ads new file mode 100644 index 00000000..fbb8af50 --- /dev/null +++ b/src/alire/alire-config-edit-early_load.ads @@ -0,0 +1,7 @@ +package Alire.Config.Edit.Early_Load is + + procedure Load_Config; + -- For internal use of Alire_Early_Elaboration, DO NOT CALL otherwise. + -- This should be hidden but that would require a non-trivial refactoring. + +end Alire.Config.Edit.Early_Load; diff --git a/src/alire/alire-config-edit.adb b/src/alire/alire-config-edit.adb index 6b99a28a..a180184f 100644 --- a/src/alire/alire-config-edit.adb +++ b/src/alire/alire-config-edit.adb @@ -138,17 +138,19 @@ package body Alire.Config.Edit is procedure Load_Config is begin - DB.Clear; + DB_Instance.Clear; for Lvl in Level loop if Lvl /= Local or else Directories.Detect_Root_Path /= "" then - CLIC.Config.Load.From_TOML (C => DB, + CLIC.Config.Load.From_TOML (C => DB_Instance, Origin => Lvl'Img, Path => Filepath (Lvl), Check => Valid_Builtin'Access); end if; end loop; + Config_Loaded := True; + -- Set variables elsewhere Platforms.Current.Disable_Distribution_Detection := @@ -156,7 +158,6 @@ package body Alire.Config.Edit is if Platforms.Current.Disable_Distribution_Detection then Trace.Debug ("Distribution detection disabled by configuration"); end if; - end Load_Config; Default_Config_Path : constant Absolute_Path := Platforms.Folders.Config; @@ -314,7 +315,4 @@ package body Alire.Config.Edit is end loop; end Print_Builtins_Doc; -begin - Load_Config; - end Alire.Config.Edit; diff --git a/src/alire/alire-config-edit.ads b/src/alire/alire-config-edit.ads index 5157573f..8fb7d022 100644 --- a/src/alire/alire-config-edit.ads +++ b/src/alire/alire-config-edit.ads @@ -84,6 +84,7 @@ private procedure Load_Config; -- Clear and reload all configuration. Also set some values elsewhere -- used to break circularities. Bottom line, this procedure must leave - -- the program-wide configuration ready. + -- the program-wide configuration ready. This is done during startup from + -- Alire_Early_Elaboration so config is available ASAP. end Alire.Config.Edit; diff --git a/src/alire/alire-config.adb b/src/alire/alire-config.adb index a24f78b3..5b5de143 100644 --- a/src/alire/alire-config.adb +++ b/src/alire/alire-config.adb @@ -2,6 +2,20 @@ with Alire.Config.Edit; package body Alire.Config is + -------- + -- DB -- + -------- + + function DB return access constant CLIC.Config.Instance + is + begin + if Config_Loaded then + return DB_Instance'Access; + else + raise Program_Error with "Attempt to use config database too early"; + end if; + end DB; + --------- -- Get -- --------- diff --git a/src/alire/alire-config.ads b/src/alire/alire-config.ads index 5ce5ac1e..496c229c 100644 --- a/src/alire/alire-config.ads +++ b/src/alire/alire-config.ads @@ -8,8 +8,7 @@ with CLIC.Config; package Alire.Config is - DB : CLIC.Config.Instance; - -- The Alire user configuration database + function DB return access constant CLIC.Config.Instance; type Level is (Global, Local); -- Ordering is important, as Globals are loaded first and overridden by any @@ -83,6 +82,11 @@ package Alire.Config is private + Config_Loaded : Boolean := False; + + DB_Instance : aliased CLIC.Config.Instance; + -- The Alire user configuration database + type Builtin_Option is tagged record Key : Ada.Strings.Unbounded.Unbounded_String; Kind : Builtin_Kind; diff --git a/src/alire/alire-index.ads b/src/alire/alire-index.ads index 6ec987d0..f51ab115 100644 --- a/src/alire/alire-index.ads +++ b/src/alire/alire-index.ads @@ -1,4 +1,5 @@ private with Alire_Early_Elaboration; +pragma Elaborate_All (Alire_Early_Elaboration); pragma Unreferenced (Alire_Early_Elaboration); with Alire.Config.Builtins; diff --git a/src/alire/alire-toolchains.adb b/src/alire/alire-toolchains.adb index 5ba56ec9..afdb74a8 100644 --- a/src/alire/alire-toolchains.adb +++ b/src/alire/alire-toolchains.adb @@ -459,7 +459,7 @@ package body Alire.Toolchains is Level : Config.Level; Fail_If_Unset : Boolean := True) is begin - if CLIC.Config.Defined (Config.DB, Tool_Key (Crate)) and then + if CLIC.Config.Defined (Config.DB.all, Tool_Key (Crate)) and then not CLIC.Config.Edit.Unset (Config.Edit.Filepath (Level), Tool_Key (Crate)) diff --git a/src/alire/alire_early_elaboration.adb b/src/alire/alire_early_elaboration.adb index 7755c0f3..b09b7858 100644 --- a/src/alire/alire_early_elaboration.adb +++ b/src/alire/alire_early_elaboration.adb @@ -1,9 +1,12 @@ -with Ada.Text_IO; +with AAA.Strings; -with Alire; +with Ada.Directories; + +with Alire.Config.Edit.Early_Load; with GNAT.Command_Line; with GNAT.OS_Lib; +with GNAT.IO; with Interfaces.C_Streams; @@ -11,6 +14,16 @@ with Simple_Logging.Filtering; package body Alire_Early_Elaboration is + ----------------- + -- Early_Error -- + ----------------- + + procedure Early_Error (Text : String) is + begin + GNAT.IO.Put_Line ("ERROR: " & Text); + GNAT.OS_Lib.OS_Exit (1); + end Early_Error; + ---------------- -- Add_Scopes -- ---------------- @@ -39,8 +52,7 @@ package body Alire_Early_Elaboration is then -- Bypass debug channel, which was not entirely set up. -- Otherwise we get unwanted location/entity info already. - Ada.Text_IO.Put_Line ("ERROR: Invalid logging filters."); - GNAT.OS_Lib.OS_Exit (1); + Early_Error ("Invalid logging filters."); end if; end Add_Scopes; @@ -59,37 +71,56 @@ package body Alire_Early_Elaboration is procedure Check_Switches is - ---------------------- - -- Check_Long_Debug -- - ---------------------- + --------------------- + -- Set_Config_Path -- + --------------------- + + procedure Set_Config_Path (Path : String) is + package Adirs renames Ada.Directories; + begin + if not Adirs.Exists (Path) then + Early_Error + ("Invalid non-existing configuration path: " & Path); + elsif Adirs.Kind (Path) not in Adirs.Directory then + Early_Error + ("Given configuration path is not a directory: " & Path); + else + Alire.Config.Edit.Set_Path (Adirs.Full_Name (Path)); + end if; + end Set_Config_Path; + + ----------------------- + -- Check_Long_Switch -- + ----------------------- -- Take care manually of the -debug[ARG] optional ARG, since the -- simple Getopt below doesn't for us: - procedure Check_Long_Debug (Switch : String) is - Target : constant String := "--debug"; + procedure Check_Long_Switch (Switch : String) is + use AAA.Strings; begin if Switch (Switch'First) /= '-' then Subcommand_Seen := True; - - elsif Switch'Length >= Target'Length and then - Switch (Switch'First .. - Switch'First + Target'Length - 1) = Target - then + elsif Has_Prefix (Switch, "--debug") then Switch_D := True; - Add_Scopes - (Switch (Switch'First + Target'Length .. Switch'Last)); + Add_Scopes (Tail (Switch, "=")); + elsif Has_Prefix (Switch, "--config") then + Set_Config_Path (Tail (Switch, "=")); end if; - end Check_Long_Debug; + end Check_Long_Switch; begin loop -- We use the simpler Getopt form to avoid built-in help and other -- shenanigans. - case Getopt ("* d? --debug? q v c=") is + case Getopt ("* d? --debug? q v c= --config=") is when ASCII.NUL => exit; when '*' => if not Subcommand_Seen then - Check_Long_Debug (Full_Switch); + Check_Long_Switch (Full_Switch); + end if; + when 'c' => + if not Subcommand_Seen then + Set_Config_Path (Parameter); end if; when 'd' => if not Subcommand_Seen then @@ -147,6 +178,9 @@ package body Alire_Early_Elaboration is if Switch_D then Alire.Log_Debug := True; end if; + + -- Load config ASAP + Alire.Config.Edit.Early_Load.Load_Config; end Early_Switch_Detection; ------------------- diff --git a/src/alire/os_windows/alire-platforms-current__windows.adb b/src/alire/os_windows/alire-platforms-current__windows.adb index 0151159b..971f7e2d 100644 --- a/src/alire/os_windows/alire-platforms-current__windows.adb +++ b/src/alire/os_windows/alire-platforms-current__windows.adb @@ -3,6 +3,9 @@ with GNAT.OS_Lib; with AAA.Strings; +with Alire_Early_Elaboration; +pragma Unreferenced (Alire_Early_Elaboration); + with Alire.Environment; with Alire.OS_Lib; use Alire.OS_Lib; with Alire.Config.Builtins.Windows; diff --git a/src/alr/alr-commands-config.adb b/src/alr/alr-commands-config.adb index 00087dd3..ec395ab1 100644 --- a/src/alr/alr-commands-config.adb +++ b/src/alr/alr-commands-config.adb @@ -59,13 +59,13 @@ package body Alr.Commands.Config is when 0 => Trace.Always (CLIC.Config.Info.List - (Alire.Config.DB, + (Alire.Config.DB.all, Filter => ".*", Show_Origin => Cmd.Show_Origin).Flatten (ASCII.LF)); when 1 => Trace.Always (CLIC.Config.Info.List - (Alire.Config.DB, + (Alire.Config.DB.all, Filter => Args.First_Element, Show_Origin => Cmd.Show_Origin).Flatten (ASCII.LF)); when others => diff --git a/src/alr/alr-commands.adb b/src/alr/alr-commands.adb index ca76a58d..c8954f81 100644 --- a/src/alr/alr-commands.adb +++ b/src/alr/alr-commands.adb @@ -492,12 +492,15 @@ package body Alr.Commands is if Command_Line_Config_Path /= null and then Command_Line_Config_Path.all /= "" then - declare - Config_Path : constant Alire.Absolute_Path - := Ada.Directories.Full_Name (Command_Line_Config_Path.all); - begin - Alire.Config.Edit.Set_Path (Config_Path); - end; + -- Just verify that early processing catched it + pragma Assert + (Alire.Config.Edit.Path = + Ada.Directories.Full_Name (Command_Line_Config_Path.all), + "Unexpected mismatch of config paths:" + & Alire.New_Line + & "Early: " & Alire.Config.Edit.Path + & Alire.New_Line + & "Late : " & Command_Line_Config_Path.all); end if; -- chdir(2) if necessary. @@ -514,7 +517,7 @@ package body Alr.Commands is Set_Builtin_Aliases; - Sub_Cmd.Load_Aliases (Alire.Config.DB); + Sub_Cmd.Load_Aliases (Alire.Config.DB.all); Sub_Cmd.Execute; Log ("alr " & Sub_Cmd.What_Command & " done", Detail); diff --git a/testsuite/tests/config/early-loading/test.py b/testsuite/tests/config/early-loading/test.py new file mode 100644 index 00000000..40f45b3b --- /dev/null +++ b/testsuite/tests/config/early-loading/test.py @@ -0,0 +1,29 @@ +""" +Test for bug #1496, in which a configuration at a non-default location was +loaded too late. +""" + +import os +from drivers.alr import run_alr +from drivers.asserts import assert_eq, assert_match + +# Create a custom configuration dir + file +custom_config = "custom_config" +os.mkdir(custom_config) +with open(os.path.join(custom_config, "config.toml"), "w") as f: + f.write("test_value = 42\n") + +expected = "test_value=42\n" + +# Verify proper loading with both short and long config options +assert_eq(expected, + run_alr("-c", custom_config, "config", "--global").out) +assert_eq(expected, + run_alr(f"--config={custom_config}", "config", "--global").out) + +# Verify also when using environment variable +os.environ["ALR_CONFIG"] = os.path.abspath(custom_config) +assert_eq(expected, + run_alr("config", "--global").out) + +print('SUCCESS') diff --git a/testsuite/tests/config/early-loading/test.yaml b/testsuite/tests/config/early-loading/test.yaml new file mode 100644 index 00000000..45e528c2 --- /dev/null +++ b/testsuite/tests/config/early-loading/test.yaml @@ -0,0 +1,7 @@ +driver: python-script +build_mode: both # one of shared, sandboxed, both (default) +indexes: + compiler_only_index: {} + # Note that shared builds require a detected compiler to be able to compute + # build hashes, which is needed for many subcommands: build, get, printenv, + # update... \ No newline at end of file diff --git a/testsuite/tests/config/relative_config_path/test.py b/testsuite/tests/config/relative_config_path/test.py index 41af1784..9e79fe2e 100644 --- a/testsuite/tests/config/relative_config_path/test.py +++ b/testsuite/tests/config/relative_config_path/test.py @@ -10,7 +10,7 @@ from drivers.alr import run_alr from drivers.asserts import assert_eq from drivers.helpers import lines_of -run_alr("--config", ".", "config", "--global", +run_alr("--config=.", "config", "--global", "--set", "some_config_key", "true") -- 2.39.5