From 114e23f5017fff32cc637383af108e104d2cbc31 Mon Sep 17 00:00:00 2001 From: Alejandro R Mosteo Date: Wed, 5 Apr 2023 17:21:18 +0200 Subject: [PATCH] Config option to globally share dependencies (#1367) * Add config option to override dependencies location Note that pins are not affected, as those are likely to be conflicting across projects. * Ensure shared deps have a manifest, show in `alr version` * Test for new shared dependencies * Force regeneration of config when global dep sharing * Check that configs are regenerated every time --- doc/user-changes.md | 18 ++++++ src/alire/alire-config-edit.adb | 14 +++++ src/alire/alire-config-edit.ads | 22 ++++++- src/alire/alire-config.ads | 2 + src/alire/alire-crate_configuration.adb | 5 +- src/alire/alire-crate_configuration.ads | 4 +- src/alire/alire-directories.ads | 3 + src/alire/alire-releases.adb | 13 ++-- src/alire/alire-roots.adb | 15 ++++- src/alire/alire-roots.ads | 3 +- src/alr/alr-commands-version.adb | 14 +++++ testsuite/drivers/alr.py | 4 ++ testsuite/drivers/asserts.py | 4 +- .../tests/config/shared-deps-profiles/test.py | 44 ++++++++++++++ .../config/shared-deps-profiles/test.yaml | 3 + testsuite/tests/config/shared-deps/test.py | 59 +++++++++++++++++++ testsuite/tests/config/shared-deps/test.yaml | 3 + 17 files changed, 214 insertions(+), 16 deletions(-) create mode 100644 testsuite/tests/config/shared-deps-profiles/test.py create mode 100644 testsuite/tests/config/shared-deps-profiles/test.yaml create mode 100644 testsuite/tests/config/shared-deps/test.py create mode 100644 testsuite/tests/config/shared-deps/test.yaml diff --git a/doc/user-changes.md b/doc/user-changes.md index 2fd5ec36..c133ed28 100644 --- a/doc/user-changes.md +++ b/doc/user-changes.md @@ -6,6 +6,24 @@ stay on top of `alr` new features. ## Release `1.3-dev` +### Global sharing of dependencies via config setting + +PR [#1367](https://github.com/alire-project/alire/pull/1367) + +A new built-in configuration key can be used to define a directory where all +dependencies will be stored: + +`alr config --set --global dependencies.dir /abs/path/to/existing/dir` + +Without `--global`, as usual, the setting will only affect the working crate. + +The use of this feature entails a penalty in that crate configuration files will +be regenerated before every build to ensure consistent build profiles. + +Caveat emptor: dependencies built by several dependents with different +configuration options or scenario variables might cause race conditions or +other unexpected issues. Use this feature with caution. + ### Test of a working crate with `alr test` PR [#1356](https://github.com/alire-project/alire/pull/1356) diff --git a/src/alire/alire-config-edit.adb b/src/alire/alire-config-edit.adb index bedfb046..7967b37f 100644 --- a/src/alire/alire-config-edit.adb +++ b/src/alire/alire-config-edit.adb @@ -220,6 +220,19 @@ package body Alire.Config.Edit is when Cfg_Absolute_Path => Result := Value.Kind = TOML_String and then Check_Absolute_Path (Value.As_String); + when Cfg_Existing_Absolute_Path => + Result := Value.Kind = TOML_String + and then Check_Absolute_Path (Value.As_String); + if Result and then + not Directories.Is_Directory + (Directories.Full_Name (Value.As_String)) + then + Trace.Error + ("Given path '" & TTY.URL (Value.As_String) + & "' is not an existing directory, " + & "please create it beforehand or recheck it."); + return False; + end if; when Cfg_Email => Result := Value.Kind = TOML_String and then Alire.Utils.Could_Be_An_Email (Value.As_String, @@ -254,6 +267,7 @@ package body Alire.Config.Edit is when Cfg_Bool => "Boolean", when Cfg_String => "String", when Cfg_Absolute_Path => "Absolute path", + when Cfg_Existing_Absolute_Path => "Absolute path already existing", when Cfg_Email => "Email address", when Cfg_GitHub_Login => "GitHub login"); diff --git a/src/alire/alire-config-edit.ads b/src/alire/alire-config-edit.ads index 423ea828..f8768f25 100644 --- a/src/alire/alire-config-edit.ads +++ b/src/alire/alire-config-edit.ads @@ -1,9 +1,11 @@ -with TOML; +with AAA.Strings; + +with Alire.Directories; +with Alire.Paths; with CLIC.Config; -with AAA.Strings; -with Alire.Directories; +with TOML; package Alire.Config.Edit is @@ -75,6 +77,7 @@ private type Builtin_Kind is (Cfg_Int, Cfg_Float, Cfg_Bool, Cfg_String, Cfg_Absolute_Path, + Cfg_Existing_Absolute_Path, Cfg_Email, Cfg_GitHub_Login); type Builtin_Entry is record @@ -96,6 +99,19 @@ private Builtins : constant array (Natural range <>) of Builtin_Entry := ( + (+Keys.Dependencies_Dir, + Cfg_Existing_Absolute_Path, + +("Overrides the default storage directory of regular (non-binary) " + & " dependencies. When unset, releases are stored inside each " + & "workspace at '" & TTY.URL + (Paths.Working_Folder_Inside_Root + / Paths.Cache_Folder_Inside_Working_Folder + / Paths.Deps_Folder_Inside_Cache_Folder) & "'. " + & "Sharing dependencies across workspaces may save disk space, but " + & "it is generally not recommended as different dependents may need " + & "to configure dependencies differently. Use at your own risk." + )), + (+Keys.Index_Auto_Community, Cfg_Bool, +("When unset (default) or true, the community index will be added " & diff --git a/src/alire/alire-config.ads b/src/alire/alire-config.ads index 115c2e33..d23c6480 100644 --- a/src/alire/alire-config.ads +++ b/src/alire/alire-config.ads @@ -23,6 +23,8 @@ package Alire.Config with Preelaborate is -- A few predefined keys that are used in several places. This list is -- not exhaustive. + Dependencies_Dir : constant Config_Key := "dependencies.dir"; + Editor_Cmd : constant Config_Key := "editor.cmd"; Distribution_Disable_Detection : constant Config_Key := diff --git a/src/alire/alire-crate_configuration.adb b/src/alire/alire-crate_configuration.adb index aaee4f7c..5434f0e6 100644 --- a/src/alire/alire-crate_configuration.adb +++ b/src/alire/alire-crate_configuration.adb @@ -12,7 +12,6 @@ with Alire.Releases; with Alire.Roots; with Alire.Origins; with Alire.Warnings; -with Alire.Config; with Alire.Config.Edit; with Alire.Properties.Build_Profiles; @@ -552,7 +551,9 @@ package body Alire.Crate_Configuration is is use type Profile_Maps.Map; begin - return This.Profile_Map /= Last_Build_Profiles; + return + Config.DB.Get (Config.Keys.Dependencies_Dir, "") /= "" or else + This.Profile_Map /= Last_Build_Profiles; end Must_Regenerate; --------------------------- diff --git a/src/alire/alire-crate_configuration.ads b/src/alire/alire-crate_configuration.ads index abf904bc..cc08f6fe 100644 --- a/src/alire/alire-crate_configuration.ads +++ b/src/alire/alire-crate_configuration.ads @@ -65,7 +65,9 @@ package Alire.Crate_Configuration is -- more/fewer crates in a new run if dependencies have changed. function Must_Regenerate (This : Global_Config) return Boolean; - -- Say if some profile has changed so config files must be regenerated + -- Say if some profile has changed so config files must be regenerated. + -- This call will always return True if global sharing of dependencies is + -- in effect. type Profile_Wildcards is (To_None, -- No wildcard given To_Unset, -- '%' (not set otherwise) diff --git a/src/alire/alire-directories.ads b/src/alire/alire-directories.ads index 24d7b031..7fe05ba7 100644 --- a/src/alire/alire-directories.ads +++ b/src/alire/alire-directories.ads @@ -32,6 +32,9 @@ package Alire.Directories is function Parent (Path : Any_Path) return String renames Ada.Directories.Containing_Directory; + function Full_Name (Path : Any_Path) return String + renames Ada.Directories.Full_Name; + function Detect_Root_Path (Starting_At : Absolute_Path := Current) return String; -- Return either the valid enclosing root folder, or "" diff --git a/src/alire/alire-releases.adb b/src/alire/alire-releases.adb index 93bff683..19a83c89 100644 --- a/src/alire/alire-releases.adb +++ b/src/alire/alire-releases.adb @@ -320,11 +320,14 @@ package body Alire.Releases is Backup_Upstream_Manifest; - if Create_Manifest then - Create_Authoritative_Manifest (if Include_Origin - then Manifest.Index - else Manifest.Local); - end if; + end if; + + -- Create manifest if requested + + if Create_Manifest then + Create_Authoritative_Manifest (if Include_Origin + then Manifest.Index + else Manifest.Local); end if; -- Run post-fetch actions on first retrieval diff --git a/src/alire/alire-roots.adb b/src/alire/alire-roots.adb index 28f8bbc4..25cef86f 100644 --- a/src/alire/alire-roots.adb +++ b/src/alire/alire-roots.adb @@ -1,6 +1,7 @@ with Ada.Unchecked_Deallocation; with Alire.Conditional; +with Alire.Config; with Alire.Dependencies.Containers; with Alire.Directories; with Alire.Environment; @@ -618,6 +619,13 @@ package body Alire.Roots is procedure Deploy_Dependencies (This : in out Roots.Root) is + ----------------------------- + -- Dependencies_Are_Shared -- + ----------------------------- + + function Dependencies_Are_Shared return Boolean + is (Config.DB.Get (Config.Keys.Dependencies_Dir, "") /= ""); + -------------------- -- Deploy_Release -- -------------------- @@ -703,7 +711,7 @@ package body Alire.Roots is Perform_Actions => False, Was_There => Was_There, Create_Manifest => - Dep.Is_Shared, + Dep.Is_Shared or else Dependencies_Are_Shared, Include_Origin => Dep.Is_Shared); @@ -1264,8 +1272,11 @@ package body Alire.Roots is if This.Solution.State (Crate).Is_Solved then if This.Solution.State (Crate).Is_Shared then return Shared.Path; + elsif Config.DB.Get (Config.Keys.Dependencies_Dir, "") /= "" then + return Config.DB.Get (Config.Keys.Dependencies_Dir, ""); else - return This.Cache_Dir + return + This.Cache_Dir / Paths.Deps_Folder_Inside_Cache_Folder; end if; else diff --git a/src/alire/alire-roots.ads b/src/alire/alire-roots.ads index 0abc0e64..e14e863b 100644 --- a/src/alire/alire-roots.ads +++ b/src/alire/alire-roots.ads @@ -377,6 +377,7 @@ private Crate : Crate_Name) return Any_Path; -- The path at which dependencies have to be deployed, which for regular - -- releases is simply ./alire/cache/dependencies. + -- releases is simply ./alire/cache/dependencies, unless overriden by the + -- config option `dependencies.dir` end Alire.Roots; diff --git a/src/alr/alr-commands-version.adb b/src/alr/alr-commands-version.adb index 68e7e899..a7d69275 100644 --- a/src/alr/alr-commands-version.adb +++ b/src/alr/alr-commands-version.adb @@ -1,7 +1,9 @@ with Alire.Config.Edit; +with Alire.Directories; with Alire.Index; with Alire.Index_On_Disk.Loading; with Alire.Milestones; +with Alire.Paths; with Alire.Properties; with Alire.Roots.Optional; with Alire.Shared; @@ -27,6 +29,8 @@ package body Alr.Commands.Version is procedure Execute (Cmd : in out Command; Args : AAA.Strings.Vector) is + use Alire; + use Alire.Directories.Operators; use all type Alire.Roots.Optional.States; Table : Alire.Utils.Tables.Table; Index_Outcome : Alire.Outcome; @@ -35,6 +39,9 @@ package body Alr.Commands.Version is (Alire.Config.Edit.Indexes_Directory, Index_Outcome); Root : constant Alire.Roots.Optional.Root := Alire.Roots.Optional.Search_Root (Alire.Directories.Current); + + Deps_Dir : constant String := + Config.DB.Get (Alire.Config.Keys.Dependencies_Dir, ""); begin if Args.Count /= 0 then Reportaise_Wrong_Arguments (Cmd.Name & " doesn't take arguments"); @@ -53,6 +60,13 @@ package body Alr.Commands.Version is Table.Append ("CONFIGURATION").New_Row; Table.Append ("config folder:").Append (Alire.Config.Edit.Path).New_Row; Table.Append ("cache folder:").Append (Alire.Shared.Path).New_Row; + Table.Append ("dependencies folder:") + .Append (if Deps_Dir = "" + then "" + / Paths.Working_Folder_Inside_Root + / Paths.Cache_Folder_Inside_Working_Folder + / Paths.Deps_Folder_Inside_Cache_Folder + else Deps_Dir).New_Row; Table.Append ("force flag:").Append (Alire.Force'Image).New_Row; Table.Append ("non-interactive flag:") .Append (CLIC.User_Input.Not_Interactive'Image).New_Row; diff --git a/testsuite/drivers/alr.py b/testsuite/drivers/alr.py index 7f4f6c85..55276c11 100644 --- a/testsuite/drivers/alr.py +++ b/testsuite/drivers/alr.py @@ -238,6 +238,10 @@ def init_local_crate(name="xxx", binary=True, enter=True, update=True): os.chdir(name) +def alr_workspace_cache(): + return os.path.join("alire", "cache") + + def alr_lockfile(): return os.path.join("alire", "alire.lock") diff --git a/testsuite/drivers/asserts.py b/testsuite/drivers/asserts.py index 07f26fdd..5097fec4 100644 --- a/testsuite/drivers/asserts.py +++ b/testsuite/drivers/asserts.py @@ -67,11 +67,11 @@ def assert_profile(profile: str, crate: str, root: str = "."): Verify that a crate was built with a certain profile root: path to where the crate root is """ - line = f' Build_Profile : Build_Profile_Kind := "{profile}";\n' + line = f' Build_Profile : Build_Profile_Kind := "{profile.lower()}";\n' file = os.path.join(root, "config", f"{crate}_config.gpr") assert line in lines_of(file), \ f"Unexpected contents: missing line '{line}' in {file}:\n" + \ - f"{content_of(file)}" + f"{lines_of(file)}" def match_solution(regex, escape=False, whole=False): diff --git a/testsuite/tests/config/shared-deps-profiles/test.py b/testsuite/tests/config/shared-deps-profiles/test.py new file mode 100644 index 00000000..fde5a9aa --- /dev/null +++ b/testsuite/tests/config/shared-deps-profiles/test.py @@ -0,0 +1,44 @@ +""" +Check that config is regenerated when dependencies are shared +""" + +import os + +from drivers.alr import alr_with, init_local_crate, run_alr +from drivers.asserts import assert_profile + +deps_dir = "deps" + +# Enable sharing of dependencies +os.mkdir(deps_dir) +run_alr("config", "--global", "--set", "dependencies.dir", + os.path.abspath(deps_dir)) + +# Create a crate with a dependency +init_local_crate() +alr_with("libhello") + +# Build normally and check build profile of dependency +run_alr("build") +os.chdir("..") +assert_profile(crate="libhello", profile="release", + root=os.path.join(deps_dir, "libhello_1.0.0_filesystem")) + +# Create second crate with the same dependency and build with debug profile +init_local_crate("yyy") +alr_with("libhello") +run_alr("build", "--profiles=libhello=development") + +# And verify that the new profile has been applied +os.chdir("..") +assert_profile(crate="libhello", profile="development", + root=os.path.join(deps_dir, "libhello_1.0.0_filesystem")) + +# Go back to the first crate and recheck, as now we have a previous profile +# stored that normally wouldn't be regenerated +os.chdir("xxx") +run_alr("build") +assert_profile(crate="libhello", profile="release", + root=os.path.join("..", deps_dir, "libhello_1.0.0_filesystem")) + +print('SUCCESS') diff --git a/testsuite/tests/config/shared-deps-profiles/test.yaml b/testsuite/tests/config/shared-deps-profiles/test.yaml new file mode 100644 index 00000000..872fc127 --- /dev/null +++ b/testsuite/tests/config/shared-deps-profiles/test.yaml @@ -0,0 +1,3 @@ +driver: python-script +indexes: + basic_index: {} diff --git a/testsuite/tests/config/shared-deps/test.py b/testsuite/tests/config/shared-deps/test.py new file mode 100644 index 00000000..1e19ebee --- /dev/null +++ b/testsuite/tests/config/shared-deps/test.py @@ -0,0 +1,59 @@ +""" +Check that globally sharing dependencies works as expected +""" + +import os +from drivers.alr import alr_with, alr_workspace_cache, init_local_crate, run_alr +from drivers.asserts import assert_contents, assert_file_exists +from drivers.helpers import lines_of + +deps_dir = "deps" + +# Check that using a relative path fails, even if it exists +os.mkdir("fake") +run_alr("config", "--global", "--set", "dependencies.dir", "fake", + complain_on_error=False) + +# Check that using an absolute non-existing path fails +run_alr("config", "--global", "--set", "dependencies.dir", + os.path.abspath(deps_dir), + complain_on_error=False) + +# Succeed after creating the destination +os.mkdir(deps_dir) +run_alr("config", "--global", "--set", "dependencies.dir", + os.path.abspath(deps_dir)) + +# Create a crate with a dependency +init_local_crate() +alr_with("hello") + +# Ensure the dependencies are where expected +assert_file_exists(os.path.join("..", deps_dir, "hello_1.0.1_filesystem")) +assert_file_exists(os.path.join("..", deps_dir, "libhello_1.0.0_filesystem")) + +# Check contents of one of the dependencies to make even surer +assert_contents(os.path.join("..", deps_dir, "hello_1.0.1_filesystem"), + ['../deps/hello_1.0.1_filesystem/alire', + '../deps/hello_1.0.1_filesystem/alire.toml', + '../deps/hello_1.0.1_filesystem/config', + '../deps/hello_1.0.1_filesystem/config/hello_config.ads', + '../deps/hello_1.0.1_filesystem/config/hello_config.gpr', + '../deps/hello_1.0.1_filesystem/config/hello_config.h', + '../deps/hello_1.0.1_filesystem/hello.gpr', + '../deps/hello_1.0.1_filesystem/src', + '../deps/hello_1.0.1_filesystem/src/hello.adb']) + +# And that the crate usual cache dir doesn't exist +assert not os.path.exists(alr_workspace_cache()) + +# Import the dependency in our code to ensure build works with the new cache +# location +new_code = ["with Hello;\n"] + lines_of(os.path.join("src", "xxx.adb")) +with open(os.path.join("src", "xxx.adb"), "w") as f: + f.writelines(new_code) + +run_alr("build") + + +print('SUCCESS') diff --git a/testsuite/tests/config/shared-deps/test.yaml b/testsuite/tests/config/shared-deps/test.yaml new file mode 100644 index 00000000..872fc127 --- /dev/null +++ b/testsuite/tests/config/shared-deps/test.yaml @@ -0,0 +1,3 @@ +driver: python-script +indexes: + basic_index: {} -- 2.39.5