Refactor --config as --settings (#1627)

* Refactor `ALR_CONFIG` to `ALIRE_SETTINGS`

* Replace --config with --settings

* Code review

* Post-merge fix

src/alire/alire-features.ads

@@ -10,9 +10,10 @@ package Alire.Features is
 
    use type Min_Version;
 
-   Env_Alr_Config_Deprecated : constant On_Version := +"3.0";
+   Config_Deprecated : constant On_Version := +"1.0";
    --  We migrate ALR_CONFIG to ALIRE_SETTINGS_DIR, but allow the use of the
    --  former with a warning during our next major release to ease transition.
+   --  Likewise for the -c/--config switch
 
    package Index is
 

src/alire/alire-settings-edit-early_load.adb

@@ -6,7 +6,7 @@ package body Alire.Settings.Edit.Early_Load is
 
    procedure Load_Settings is
    begin
-      Alire.Settings.Edit.Load_Config;
+      Alire.Settings.Edit.Load_Settings;
    end Load_Settings;
 
 begin

src/alire/alire-settings-edit.adb

@@ -39,7 +39,7 @@ package body Alire.Settings.Edit is
       end if;
 
       --  Reload after change
-      Load_Config;
+      Load_Settings;
    end Set_Locally;
 
    ------------------
@@ -56,7 +56,7 @@ package body Alire.Settings.Edit is
       end if;
 
       --  Reload after change
-      Load_Config;
+      Load_Settings;
    end Set_Globally;
 
    ---------
@@ -86,7 +86,7 @@ package body Alire.Settings.Edit is
       if CLIC.Config.Edit.Unset (Filepath (Level), Key, Quiet => True) then
          Trace.Debug ("Config key " & Key & " unset from " & Level'Image
                       & "configuration at " & Filepath (Level));
-         Load_Config;
+         Load_Settings;
       else
          Trace.Debug ("Config key " & Key & " requested to be unset at level "
                       & Level'Image & " but it was already unset at "
@@ -109,7 +109,7 @@ package body Alire.Settings.Edit is
       Assert (Set_Boolean_Impl (Filepath (Level), Key, Value, Check),
               "Cannot set config key '" & Key & "' at level " & Level'Image);
       --  Reload after change
-      Load_Config;
+      Load_Settings;
    end Set_Boolean;
 
    --------------
@@ -180,11 +180,11 @@ package body Alire.Settings.Edit is
       return Location (Current);
    end Filepath;
 
-   -----------------
-   -- Load_Config --
-   -----------------
+   -------------------
+   -- Load_Settings --
+   -------------------
 
-   procedure Load_Config is
+   procedure Load_Settings is
    begin
       DB_Instance.Clear;
 
@@ -206,7 +206,7 @@ package body Alire.Settings.Edit is
       if Platforms.Current.Disable_Distribution_Detection then
          Trace.Debug ("Distribution detection disabled by configuration");
       end if;
-   end Load_Config;
+   end Load_Settings;
 
    Default_Config_Path : constant Absolute_Path := Platforms.Folders.Config;
 
@@ -224,7 +224,7 @@ package body Alire.Settings.Edit is
    begin
       --  Warn or fail depending on version
       if OS_Lib.Getenv (Environment.Config, Unset) /= Unset then
-         if Version.Semver.Current < Features.Env_Alr_Config_Deprecated then
+         if Version.Semver.Current < Features.Config_Deprecated then
             Warnings.Warn_Once (Msg, Level => Warning);
          else
             Raise_Checked_Error (Msg);

src/alire/alire-settings-edit.ads

@@ -82,7 +82,7 @@ package Alire.Settings.Edit is
 
 private
 
-   procedure Load_Config;
+   procedure Load_Settings;
    --  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. This is done during startup from

src/alire/alire_early_elaboration.adb

@@ -2,7 +2,9 @@ with AAA.Strings;
 
 with Ada.Directories;
 
+with Alire.Features;
 with Alire.Settings.Edit.Early_Load;
+with Alire.Version.Semver;
 
 with GNAT.Command_Line;
 with GNAT.OS_Lib;
@@ -71,26 +73,26 @@ package body Alire_Early_Elaboration is
 
       procedure Check_Switches is
 
-         -------------------------
-         -- Config_Switch_Error --
-         -------------------------
+         ---------------------------
+         -- Settings_Switch_Error --
+         ---------------------------
 
-         procedure Config_Switch_Error (Switch : String) is
+         procedure Settings_Switch_Error (Switch : String) is
          begin
             GNAT.IO.Put_Line
                ("ERROR: Switch " & Switch & " requires argument (global).");
             Early_Error ("try ""alr --help"" for more information.");
-         end Config_Switch_Error;
+         end Settings_Switch_Error;
 
          ---------------------
          -- Set_Config_Path --
          ---------------------
 
-         procedure Set_Config_Path (Path : String) is
+         procedure Set_Config_Path (Switch, Path : String) is
             package Adirs renames Ada.Directories;
          begin
             if Path = "" then
-               Config_Switch_Error ("--config");
+               Settings_Switch_Error (Switch);
             elsif not Adirs.Exists (Path) then
                Early_Error
                  ("Invalid non-existing configuration path: " & Path);
@@ -102,6 +104,42 @@ package body Alire_Early_Elaboration is
             end if;
          end Set_Config_Path;
 
+         -----------------------------
+         -- Check_Config_Deprecated --
+         -----------------------------
+
+         use type Alire.Version.Semver.Version;
+         Config_Deprecated : constant Boolean
+           := Alire.Version.Semver.Current >= Alire.Features.Config_Deprecated;
+
+         procedure Check_Config_Deprecated is
+         begin
+            if Config_Deprecated then
+               Early_Error
+                 ("--config is deprecated, use --settings instead");
+            end if;
+         end Check_Config_Deprecated;
+
+         ----------------
+         -- Check_Seen --
+         ----------------
+
+         Settings_Seen : Boolean := False;
+
+         procedure Check_Settings_Seen is
+         begin
+            if Settings_Seen then
+               Early_Error
+                 ("Only one of -s, --settings"
+                  & (if not Config_Deprecated
+                    then ", -c, --config"
+                    else "")
+                  & " allowed");
+            else
+               Settings_Seen := True;
+            end if;
+         end Check_Settings_Seen;
+
          -----------------------
          -- Check_Long_Switch --
          -----------------------
@@ -116,26 +154,36 @@ package body Alire_Early_Elaboration is
                Switch_D := True;
                Add_Scopes (Tail (Switch, "="));
             elsif Has_Prefix (Switch, "--config") then
-               Set_Config_Path (Tail (Switch, "="));
+               Check_Config_Deprecated;
+               Check_Settings_Seen;
+               Set_Config_Path (Switch, Tail (Switch, "="));
+            elsif Has_Prefix (Switch, "--settings") then
+               Check_Settings_Seen;
+               Set_Config_Path (Switch, Tail (Switch, "="));
             end if;
          end Check_Long_Switch;
 
          Option : Character;
+
       begin
          loop
             --  We use the simpler Getopt form to avoid built-in help and other
             --  shenanigans.
-            Option := Getopt ("* d? --debug? q v c= --config=");
+            Option := Getopt ("* d? --debug? q v c= --config= s= --settings=");
             case Option is
                when ASCII.NUL =>
                   exit;
                when '*' =>
                   if not Subcommand_Seen then
                      Check_Long_Switch (Full_Switch);
                   end if;
-               when 'c' =>
+               when 'c' | 's' =>
                   if not Subcommand_Seen then
-                     Set_Config_Path (Parameter);
+                     Check_Settings_Seen;
+                     if Option = 'c' then
+                        Check_Config_Deprecated;
+                     end if;
+                     Set_Config_Path ("-" & Option, Parameter);
                   end if;
                when 'd' =>
                   if not Subcommand_Seen then
@@ -164,8 +212,8 @@ package body Alire_Early_Elaboration is
          end loop;
       exception
          when GNAT.Command_Line.Invalid_Parameter =>
-            if Option = 'c' then
-               Config_Switch_Error ("-c");
+            if Option in 'c' | 's' then
+               Settings_Switch_Error ("-" & Option);
             end if;
          when Exit_From_Command_Line =>
             --  Something unexpected happened but it will be properly dealt

src/alr/alr-commands-version.adb

@@ -72,7 +72,8 @@ package body Alr.Commands.Version is
 
       Table.Append ("").New_Row;
       Table.Append ("CONFIGURATION").New_Row;
-      Table.Append ("config folder:").Append (Settings.Edit.Path).New_Row;
+      Table.Append ("settings folder:")
+        .Append (Alire.Settings.Edit.Path).New_Row;
       Table.Append ("cache folder:")
         .Append (Alire.Settings.Edit.Cache_Path).New_Row;
       Table.Append ("vault folder:").Append (Paths.Vault.Path).New_Row;

src/alr/alr-commands.adb

@@ -11,6 +11,7 @@ with Alire_Early_Elaboration;
 with Alire.Settings.Builtins;
 with Alire.Settings.Edit;
 with Alire.Errors;
+with Alire.Features;
 with Alire.Index_On_Disk.Loading;
 with Alire.Index_On_Disk.Updates;
 with Alire.Lockfiles;
@@ -19,6 +20,7 @@ with Alire.Platforms.Current;
 with Alire.Root;
 with Alire.Solutions;
 with Alire.Toolchains;
+with Alire.Version.Semver;
 
 with Alr.Commands.Action;
 with Alr.Commands.Build;
@@ -137,12 +139,22 @@ package body Alr.Commands is
    procedure Set_Global_Switches
      (Config : in out CLIC.Subcommand.Switches_Configuration)
    is
+      use Alire;
       use CLIC.Subcommand;
+      use type Alire.Version.Semver.Version;
    begin
+      if Alire.Version.Semver.Current < Features.Config_Deprecated then
+         Define_Switch (Config,
+                        Command_Line_Config_Path'Access,
+                        "-c=", "--config=",
+                        TTY.Error ("Deprecated")
+                        & ". See -s/--settings switch");
+      end if;
+
       Define_Switch (Config,
                      Command_Line_Config_Path'Access,
-                     "-c=", "--config=",
-                     "Override configuration folder location");
+                     "-s=", "--settings=",
+                     "Override settings folder location");
 
       Define_Switch (Config,
                      Command_Line_Chdir_Target_Path'Access,

testsuite/drivers/alr.py

@@ -44,47 +44,47 @@ def prepare_env(settings_dir, env):
     settings_dir = os.path.abspath(settings_dir)
     mkdir(settings_dir)
     env['ALIRE_SETTINGS_DIR'] = settings_dir
-    #  We pass config location explicitly in the following calls since env is
+    #  We pass settings location explicitly in the following calls since env is
     #  not yet applied (it's just a dict to be passed later to subprocess)
 
     if platform.system() == "Windows":
         # Disable msys inadvertent installation
-        run_alr("-c", settings_dir, "settings", "--global",
+        run_alr("-s", settings_dir, "settings", "--global",
                 "--set", "msys2.do_not_install", "true")
 
         # And configure the one set up in the environment so it is used by
         # tests that need it.
-        run_alr("-c", settings_dir, "settings", "--global",
+        run_alr("-s", settings_dir, "settings", "--global",
                 "--set", "msys2.install_dir",
                 os.path.join(
                     os.environ.get("LocalAppData"), "alire", "cache", "msys64"))
 
     # Disable autoconfig of the community index, to prevent unintended use of
     # it in tests, besides the overload of fetching it
-    run_alr("-c", settings_dir, "settings", "--global",
+    run_alr(f"-s", settings_dir, "settings", "--global",
             "--set", "index.auto_community", "false")
 
     # Disable selection of toolchain to preserve older behavior. Tests that
     # require a configured compiler will have to set it up explicitly.
-    run_alr("-c", settings_dir, "toolchain", "--disable-assistant")
+    run_alr("-s", settings_dir, "toolchain", "--disable-assistant")
 
     # Disable warning on old index, to avoid having to update index versions
     # when they're still compatible.
-    run_alr("-c", settings_dir, "settings", "--global",
+    run_alr("-s", settings_dir, "settings", "--global",
             "--set", "warning.old_index", "false")
 
     # Disable shared dependencies (keep old pre-2.0 behavior) not to break lots
     # of tests. The post-2.0 behavior will have its own tests.
-    run_alr("-c", settings_dir, "settings", "--global",
+    run_alr("-s", settings_dir, "settings", "--global",
             "--set", "dependencies.shared", "false")
 
     # Disable index auto-updates, which is not expected by most tests
-    run_alr("-c", settings_dir, "settings", "--global",
+    run_alr("-s", settings_dir, "settings", "--global",
             "--set", "index.auto_update", "0")
 
     # If distro detection is disabled via environment, configure so in alr
     if "ALIRE_TESTSUITE_DISABLE_DISTRO" in env:
-        run_alr("-c", settings_dir, "settings", "--global",
+        run_alr("-s", settings_dir, "settings", "--global",
                 "--set", "distribution.disable_detection", "true")
 
 
@@ -616,4 +616,4 @@ def unselect_compiler():
     assistant.
     """
     run_alr("settings", "--global", "--unset", "toolchain.use.gnat")
-    run_alr("settings", "--global", "--unset", "toolchain.external.gnat")
+    run_alr("settings", "--global", "--unset", "toolchain.external.gnat")

testsuite/drivers/driver/python_script.py

@@ -168,7 +168,7 @@ def run(self):
             self.result.log.log += "Build mode: SHARED\n"
             # Activate shared builds. Using "-c" is needed as the environment
             # still isn't activated at the driver script level.
-            run_alr("-c", pristine_env["ALIRE_SETTINGS_DIR"],
+            run_alr(f"--settings={pristine_env['ALIRE_SETTINGS_DIR']}",
                     "settings", "--global", "--set",
                     "dependencies.shared", "true")
             p = self.run_script(copy.deepcopy(pristine_env))

testsuite/tests/config/missing-config-path/test.py

@@ -1,18 +1,27 @@
 """
-Verify that errors are properly handled when no config path is given
+Verify that errors are properly handled when no settings path is given
 """
 
+import os
 from drivers.alr import run_alr
 from drivers.asserts import assert_match
 
 import re
 
-p = run_alr("--config", complain_on_error=False)
-assert p.status != 0, "command should fail"
-assert_match("ERROR: Switch --config requires argument.*", p.out, flags=re.S)
+switch = "--settings"
+short = "-s"
 
-p = run_alr("-c", complain_on_error=False)
-assert p.status != 0, "command should fail"
-assert_match("ERROR: Switch -c requires argument.*", p.out, flags=re.S)
+p = run_alr(switch, complain_on_error=False)
+assert_match(f"ERROR: Switch {switch} requires argument.*", p.out, flags=re.S)
+
+p = run_alr(short, complain_on_error=False)
+assert_match(f"ERROR: Switch {short} requires argument.*", p.out, flags=re.S)
+
+# Check also failure in case of duplication of switch
+path = os.getcwd()
+p = run_alr(f"{short}", path, f"{switch}={path}", "version",
+            complain_on_error=False)
+assert_match(".*Only one of .* allowed",
+             p.out)
 
 print('SUCCESS')