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

Refactor --config as --settings #1627

Merged
merged 6 commits into from
Mar 11, 2024
Merged
Show file tree
Hide file tree
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
3 changes: 2 additions & 1 deletion src/alire/alire-features.ads
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion src/alire/alire-settings-edit-early_load.adb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 10 additions & 10 deletions src/alire/alire-settings-edit.adb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ package body Alire.Settings.Edit is
end if;

-- Reload after change
Load_Config;
Load_Settings;
end Set_Locally;

------------------
Expand All @@ -53,7 +53,7 @@ package body Alire.Settings.Edit is
end if;

-- Reload after change
Load_Config;
Load_Settings;
end Set_Globally;

---------
Expand Down Expand Up @@ -83,7 +83,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 "
Expand All @@ -106,7 +106,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;

--------------
Expand Down Expand Up @@ -135,11 +135,11 @@ package body Alire.Settings.Edit is
end case;
end Filepath;

-----------------
-- Load_Config --
-----------------
-------------------
-- Load_Settings --
-------------------

procedure Load_Config is
procedure Load_Settings is
begin
DB_Instance.Clear;

Expand All @@ -161,7 +161,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;

Expand All @@ -179,7 +179,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);
Expand Down
2 changes: 1 addition & 1 deletion src/alire/alire-settings-edit.ads
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
74 changes: 61 additions & 13 deletions src/alire/alire_early_elaboration.adb
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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 --
-----------------------
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion src/alr/alr-commands-version.adb
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
16 changes: 14 additions & 2 deletions src/alr/alr-commands.adb
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
20 changes: 10 additions & 10 deletions testsuite/drivers/alr.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")


Expand Down Expand Up @@ -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")
2 changes: 1 addition & 1 deletion testsuite/drivers/driver/python_script.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
23 changes: 16 additions & 7 deletions testsuite/tests/config/missing-config-path/test.py
Original file line number Diff line number Diff line change
@@ -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')
Loading
Loading