From 6d9dc5ee6d03a5d9f188a8f367d10fbb5c02a1c7 Mon Sep 17 00:00:00 2001 From: Alejandro R Mosteo Date: Mon, 4 Sep 2023 17:49:53 +0200 Subject: [PATCH] Reuse hashing mechanism to regen config in sandboxed mode (#1438) * Regenerate configs on demand (sandboxed deps) Leverage the new build hashing to make sure when a configuration needs regenerating. This is pessimistic but safer than what we had, and more future-proof. * fix wrong switches generation * test fixes * New test, checking no unneeded rebuilds happen * Fixes for CI-detected issues --- alire.toml | 2 +- deps/aaa | 2 +- src/alire/alire-builds-hashes.adb | 137 ++++++++++++------ src/alire/alire-builds-hashes.ads | 27 +++- src/alire/alire-builds.adb | 5 +- src/alire/alire-crate_configuration.adb | 72 ++++----- src/alire/alire-crate_configuration.ads | 27 ++-- src/alire/alire-roots-editable.adb | 3 + src/alire/alire-roots.adb | 117 ++++++++++----- src/alire/alire-roots.ads | 19 ++- src/alire/alire-solutions.ads | 4 +- src/alire/alire-utils-text_files.adb | 14 ++ src/alire/alire-utils-text_files.ads | 2 + src/alr/alr-commands-build.adb | 8 +- testsuite/drivers/helpers.py | 9 ++ .../build/hashes/compiler-missing/test.py | 1 + .../build_profile/alr_build_switches/test.py | 12 ++ .../tests/crate_config/gen_control/test.py | 2 +- .../tests/crate_config/no-rebuilds/test.py | 25 ++++ .../tests/crate_config/no-rebuilds/test.yaml | 3 + .../get/external-tool-dependency/test.py | 1 + testsuite/tests/get/git-local/test.py | 1 + testsuite/tests/get/unpack-in-place/test.py | 1 + 23 files changed, 354 insertions(+), 140 deletions(-) create mode 100644 testsuite/tests/crate_config/no-rebuilds/test.py create mode 100644 testsuite/tests/crate_config/no-rebuilds/test.yaml diff --git a/alire.toml b/alire.toml index aa67ac66..1110ebb7 100644 --- a/alire.toml +++ b/alire.toml @@ -45,7 +45,7 @@ windows = { ALIRE_OS = "windows" } # Some dependencies require precise versions during the development cycle: [[pins]] -aaa = { url = "https://github.com/mosteo/aaa", commit = "c3b5a19adac66f42be45e22694c9463997b4f756" } +aaa = { url = "https://github.com/mosteo/aaa", commit = "ecc38772bd4a6b469b54c62363766ea1c0e9f912" } ada_toml = { url = "https://github.com/mosteo/ada-toml", commit = "da4e59c382ceb0de6733d571ecbab7ea4919b33d" } clic = { url = "https://github.com/alire-project/clic", commit = "6879b90876a1c918b4e112f59c6db0e25b713f52" } gnatcoll = { url = "https://github.com/alire-project/gnatcoll-core.git", commit = "4e663b87a028252e7e074f054f8f453661397166" } diff --git a/deps/aaa b/deps/aaa index c3b5a19a..ecc38772 160000 --- a/deps/aaa +++ b/deps/aaa @@ -1 +1 @@ -Subproject commit c3b5a19adac66f42be45e22694c9463997b4f756 +Subproject commit ecc38772bd4a6b469b54c62363766ea1c0e9f912 diff --git a/src/alire/alire-builds-hashes.adb b/src/alire/alire-builds-hashes.adb index f272b260..c682904f 100644 --- a/src/alire/alire-builds-hashes.adb +++ b/src/alire/alire-builds-hashes.adb @@ -13,9 +13,6 @@ package body Alire.Builds.Hashes is package SHA renames Alire.Hashes.SHA256_Impl; - subtype Variables is AAA.Strings.Set; - -- We'll store all variables that affect a Release in a deterministic order - ----------- -- Clear -- ----------- @@ -23,6 +20,7 @@ package body Alire.Builds.Hashes is procedure Clear (This : in out Hasher) is begin This.Hashes.Clear; + This.Inputs.Clear; end Clear; -------------- @@ -78,39 +76,9 @@ package body Alire.Builds.Hashes is end loop; This.Hashes.Insert (Rel.Name, SHA.Get_Digest (C)); + This.Inputs.Insert (Rel.Name, Vars); end Compute_Hash; - ------------------ - -- Write_Inputs -- - ------------------ - - procedure Write_Inputs is - File : constant Absolute_Path := - Builds.Path - / Rel.Base_Folder & "_" & This.Hashes (Rel.Name) - / Paths.Working_Folder_Inside_Root - / "build_hash_inputs"; - use Directories; - use Utils.Text_Files; - - Lines : AAA.Strings.Vector; - begin - -- First ensure we have a pristine file to work with - Delete_Tree (File); - Create_Tree (Parent (File)); - Touch (File); - - -- Now add the hashed contents for the record - - for Var of Vars loop - Lines.Append (Var); - end loop; - - Append_Lines (File, - Lines, - Backup => False); - end Write_Inputs; - ----------------- -- Add_Profile -- ----------------- @@ -180,7 +148,7 @@ package body Alire.Builds.Hashes is procedure Add_Configuration is begin Crate_Configuration.Hashes.Add_From - (Config => Root.Configuration, + (Config => Root.Configuration.all, Rel => Rel, Add => Add'Access); end Add_Configuration; @@ -190,11 +158,17 @@ package body Alire.Builds.Hashes is -- Add individual contributors to the hash input Add_Profile; -- Build profile - Add_Externals; -- GPR externals - Add_Environment; -- Environment variables - Add_Compiler; -- Compiler version Add_Configuration; -- Crate configuration variables + -- These are only relevant for shared dependencies, as they don't + -- appear in the contents of generated files. Not including them + -- allows things to work as they were for sandboxed dependencies. + if not Builds.Sandboxed_Dependencies then + Add_Externals; -- GPR externals + Add_Environment; -- Environment variables + Add_Compiler; -- Compiler version + end if; + -- Dependencies recursive hash? Since a crate can use a dependency -- config spec, it is possible in the worst case for a crate to -- require unique builds that include their dependencies hash @@ -209,9 +183,6 @@ package body Alire.Builds.Hashes is -- Final computation Compute_Hash; - -- Write the hash input for the record - Write_Inputs; - Trace.Debug (" build hashing release complete"); end Compute; @@ -226,13 +197,61 @@ package body Alire.Builds.Hashes is Root.Configuration.Ensure_Complete; - for Rel of Root.Solution.Releases loop - if Root.Requires_Build_Sync (Rel) then + for Rel of Root.Nonabstract_Releases loop -- includes the root release + if Rel.Origin.Requires_Build then Compute (Rel); end if; end loop; end Compute; + ---------------------- + -- Inputs_File_Name -- + ---------------------- + + function Inputs_File_Name (Root : in out Roots.Root; + Rel : Releases.Release) + return Absolute_Path + is (Root.Release_Base (Rel.Name, Roots.For_Build) + / Paths.Working_Folder_Inside_Root + / "build_hash_inputs"); + + ------------------ + -- Write_Inputs -- + ------------------ + + procedure Write_Inputs (This : Hasher; + Root : in out Roots.Root) + is + + ------------------ + -- Write_Inputs -- + ------------------ + + procedure Write_Inputs (Rel : Releases.Release) is + File : constant Absolute_Path := Inputs_File_Name (Root, Rel); + use Directories; + use Utils.Text_Files; + begin + -- First ensure we have a pristine file to work with + Delete_Tree (File); + Create_Tree (Parent (File)); + Touch (File); + + -- Now add the hashed contents for the record + + Append_Lines (File, + This.Inputs (Rel.Name).To_Vector, + Backup => False); + end Write_Inputs; + + begin + for Rel of Root.Nonabstract_Releases loop + if Rel.Origin.Requires_Build then + Write_Inputs (Rel); + end if; + end loop; + end Write_Inputs; + ---------- -- Hash -- ---------- @@ -242,4 +261,36 @@ package body Alire.Builds.Hashes is return String is (This.Hashes (Name)); + ------------ + -- Inputs -- + ------------ + + function Inputs (This : Hasher; + Name : Crate_Name) + return Variables + is (This.Inputs (Name)); + + ------------------- + -- Stored_Inputs -- + ------------------- + + function Stored_Inputs (Root : in out Roots.Root; + Rel : Releases.Release) + return Variables + is + Name : constant Absolute_Path := Inputs_File_Name (Root, Rel); + begin + if Directories.Is_File (Name) then + declare + File : Utils.Text_Files.File := + Utils.Text_Files.Load (Name); + Lines : constant AAA.Strings.Vector := File.Lines.all; + begin + return Lines.To_Set; + end; + else + return AAA.Strings.Empty_Set; + end if; + end Stored_Inputs; + end Alire.Builds.Hashes; diff --git a/src/alire/alire-builds-hashes.ads b/src/alire/alire-builds-hashes.ads index 272c1b7c..95622386 100644 --- a/src/alire/alire-builds-hashes.ads +++ b/src/alire/alire-builds-hashes.ads @@ -15,7 +15,11 @@ package Alire.Builds.Hashes is procedure Compute (This : in out Hasher; Root : in out Roots.Root); - -- Compute all hashes needed for a release + -- Compute all hashes needed for all releases in a Root solution + + procedure Write_Inputs (This : Hasher; + Root : in out Roots.Root); + -- Write the hashing inputs to /alire/build_hash_inputs function Hash (This : in out Hasher; Name : Crate_Name) @@ -23,13 +27,34 @@ package Alire.Builds.Hashes is with Pre => not This.Is_Empty; -- Retrieve the hash of a crate in Root's solution + subtype Variables is AAA.Strings.Set; + + function Inputs (This : Hasher; + Name : Crate_Name) + return Variables; + -- Returns the inputs that were used for the hash + + function Stored_Inputs (Root : in out Roots.Root; + Rel : Releases.Release) + return Variables; + -- Return the stored inputs (see Write_Inputs) for a release; may return an + -- empty set if the inputs are not yet written to disk. + private + use type Variables; + -- We'll store all variables that affect a Release in a deterministic + -- order. These look like `kind:name=value` in a single string each. + package Crate_Hash_Maps is new Ada.Containers.Indefinite_Ordered_Maps (Crate_Name, String); + package Crate_Input_Maps is new Ada.Containers.Indefinite_Ordered_Maps + (Crate_Name, Variables); + type Hasher is tagged record Hashes : Crate_Hash_Maps.Map; + Inputs : Crate_Input_Maps.Map; end record; end Alire.Builds.Hashes; diff --git a/src/alire/alire-builds.adb b/src/alire/alire-builds.adb index 2863e1f2..3926a6b3 100644 --- a/src/alire/alire-builds.adb +++ b/src/alire/alire-builds.adb @@ -43,6 +43,9 @@ package body Alire.Builds is else Directories.Delete_Tree (Dst); -- Ensure nothing interferes at destination with the new copy + + Directories.Create_Tree (Directories.Parent (Dst)); + -- And that the destination parent exists end if; declare @@ -63,7 +66,7 @@ package body Alire.Builds is end; -- At this point we can generate the final crate configuration - Root.Configuration.Generate_Config_Files (Root, Release); + Root.Configuration.Generate_Config_Files (Root, Release, Full => Force); declare use Directories; diff --git a/src/alire/alire-crate_configuration.adb b/src/alire/alire-crate_configuration.adb index 4e5a4847..ebcf24bf 100644 --- a/src/alire/alire-crate_configuration.adb +++ b/src/alire/alire-crate_configuration.adb @@ -102,7 +102,8 @@ package body Alire.Crate_Configuration is procedure Set_Build_Profile (This : in out Global_Config; Crate : Crate_Name; - Profile : Profile_Kind) + Profile : Profile_Kind; + Set_By : String := "library client") is Key : constant String := Build_Profile_Key (Crate); Val : Config_Setting := This.Var_Map (+Key); @@ -111,7 +112,7 @@ package body Alire.Crate_Configuration is begin -- Update config value that holds the profile value Val.Value := TOML.Create_String (To_Lower_Case (Profile'Image)); - Val.Set_By := +"library client"; + Val.Set_By := +Set_By; This.Var_Map (+Key) := Val; -- Update profile itself @@ -257,8 +258,8 @@ package body Alire.Crate_Configuration is Config : Alire.Utils.Switches.Switches_Configuration := (case Profile is - when Release => Default_Release_Switches, - when Validation => Default_Validation_Switches, + when Release => Default_Release_Switches, + when Validation => Default_Validation_Switches, when Development => Default_Development_Switches); Modif : Properties.Build_Switches.Profile_Modifier; @@ -288,7 +289,7 @@ package body Alire.Crate_Configuration is Properties.Build_Switches.Apply (Config, Modif.Development); end case; - This.Switches_Map.Insert (Rel.Name, Get_List (Config)); + This.Switches_Map.Include (Rel.Name, Get_List (Config)); end; end loop; end Make_Switches_Map; @@ -316,8 +317,6 @@ package body Alire.Crate_Configuration is Make_Build_Profile_Map (This, Root, Rel_Vect); - Make_Switches_Map (This, Root, Rel_Vect); - for Create of Rel_Vect loop This.Load_Settings (Root, Create); end loop; @@ -339,7 +338,7 @@ package body Alire.Crate_Configuration is ------------------------ -- Is_Config_Complete -- ------------------------ - -- Say if all variables in configuration are set, for all or one crate + function Is_Config_Complete (This : Global_Config; Crate : String := "") return Boolean @@ -375,8 +374,9 @@ package body Alire.Crate_Configuration is -- Generate_Config_Files -- --------------------------- - procedure Generate_Config_Files (This : Global_Config; - Root : in out Alire.Roots.Root) + procedure Generate_Config_Files (This : in out Global_Config; + Root : in out Alire.Roots.Root; + Full : Boolean) is Solution : constant Solutions.Solution := Root.Solution; begin @@ -393,7 +393,7 @@ package body Alire.Crate_Configuration is declare Rel : constant Releases.Release := Root.Release (Crate); begin - This.Generate_Config_Files (Root, Rel); + This.Generate_Config_Files (Root, Rel, Full); end; end loop; end Generate_Config_Files; @@ -402,9 +402,10 @@ package body Alire.Crate_Configuration is -- Generate_Config_Files -- --------------------------- - procedure Generate_Config_Files (This : Global_Config; + procedure Generate_Config_Files (This : in out Global_Config; Root : in out Alire.Roots.Root; - Rel : Releases.Release) + Rel : Releases.Release; + Full : Boolean) is use Alire.Directories; @@ -429,6 +430,13 @@ package body Alire.Crate_Configuration is return Ret; end; end Get_Config_Entry; + + Ent : constant Config_Entry := Get_Config_Entry (Rel); + + Conf_Dir : constant Absolute_Path := + Root.Release_Base (Rel.Name, Roots.For_Build) + / Ent.Output_Dir; + begin -- We don't create config files for external releases, since they -- are not sources built by Alire. @@ -440,14 +448,25 @@ package body Alire.Crate_Configuration is Warnings.Warn_Once ("Skipping generation of incomplete configuration files " & "for crate " & Utils.TTY.Name (Rel.Name_Str)); + + elsif not Full + and then Directories.Is_Directory (Conf_Dir) + and then This.Is_Config_Complete -- avoid hashing in Config_Outdated + and then not Root.Config_Outdated (Rel.Name) + then + Trace.Debug ("Skipping generation of up-to-date config for " + & Rel.Milestone.TTY_Image); + else - declare - Ent : constant Config_Entry := Get_Config_Entry (Rel); + Trace.Debug ("Generating config files for release: " + & Rel.Milestone.TTY_Image); - Conf_Dir : constant Absolute_Path := - Root.Release_Base (Rel.Name, Roots.For_Build) - / Ent.Output_Dir; + -- Update switches to match profile + This.Make_Switches_Map + (Root, + Containers.Crate_Name_Sets.To_Set (Rel.Name)); + declare Version_Str : constant String := Rel.Version.Image; begin if not Ent.Disabled then @@ -561,17 +580,6 @@ package body Alire.Crate_Configuration is is (not This.Profile_Map.Is_Empty); -- Because at a minimum it must contain the root crate profile - --------------------- - -- Must_Regenerate -- - --------------------- - - function Must_Regenerate (This : Global_Config) return Boolean - is - use type Profile_Maps.Map; - begin - return This.Profile_Map /= Last_Build_Profiles; - end Must_Regenerate; - --------------------------- -- Pretty_Print_Switches -- --------------------------- @@ -822,9 +830,7 @@ package body Alire.Crate_Configuration is Val : Assignment; Set_By : String) is - Val_Name_Lower : constant String := To_Lower_Case (+Val.Name); - Crate_Str : constant String := +Crate; - Name : constant Unbounded_String := (+Crate_Str) & "." & Val_Name_Lower; + Name : constant Unbounded_String := +Key (Crate, +Val.Name); begin -- TODO check if setting configuration of a dependency @@ -840,7 +846,7 @@ package body Alire.Crate_Configuration is if not Valid (Ref.Type_Def.Element, Val.Value) then Raise_Checked_Error - ("Invalid value from '" & Crate_Str & + ("Invalid value from '" & Crate.As_String & "'" & " for type " & Image (Ref.Type_Def.Element)); end if; diff --git a/src/alire/alire-crate_configuration.ads b/src/alire/alire-crate_configuration.ads index 452141f9..2bc65461 100644 --- a/src/alire/alire-crate_configuration.ads +++ b/src/alire/alire-crate_configuration.ads @@ -33,6 +33,11 @@ package Alire.Crate_Configuration is function Is_Valid (This : Global_Config) return Boolean; -- False until Load is called + function Is_Config_Complete (This : Global_Config; + Crate : String := "") + return Boolean; + -- Say if all variables in configuration are set, for all or one crate + procedure Ensure_Complete (This : Global_Config); -- Verify all variables have a value, or report and raise @@ -48,19 +53,23 @@ package Alire.Crate_Configuration is procedure Set_Build_Profile (This : in out Global_Config; Crate : Crate_Name; - Profile : Profile_Kind) + Profile : Profile_Kind; + Set_By : String := "library client") with Pre => This.Is_Valid; procedure Load (This : in out Global_Config; Root : in out Alire.Roots.Root); - procedure Generate_Config_Files (This : Global_Config; - Root : in out Alire.Roots.Root); + procedure Generate_Config_Files (This : in out Global_Config; + Root : in out Alire.Roots.Root; + Full : Boolean); + -- When Full, overwrite - procedure Generate_Config_Files (This : Global_Config; + procedure Generate_Config_Files (This : in out Global_Config; Root : in out Alire.Roots.Root; - Rel : Releases.Release); - -- Generate config files only for the given release + Rel : Releases.Release; + Full : Boolean); + -- Generate config files only for the given release. When Full, overwrite. procedure Save_Last_Build_Profiles (This : Global_Config); -- Record in local user configuration the last profiles used in crate @@ -70,11 +79,6 @@ package Alire.Crate_Configuration is -- Get the last profile used from user configuration. Note that we can have -- 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. - -- 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) To_All); -- '*' (total override) @@ -131,6 +135,7 @@ private type Global_Config is tagged record Var_Map : Config_Maps.Map; -- Mapping "crate.var" --> setting + -- Includes the Build_Profile var added by Alire Profile_Map : Profile_Maps.Map; -- Mapping crate -> profile, exists for all crates in solution diff --git a/src/alire/alire-roots-editable.adb b/src/alire/alire-roots-editable.adb index 55c9565a..81c11b4d 100644 --- a/src/alire/alire-roots-editable.adb +++ b/src/alire/alire-roots-editable.adb @@ -65,6 +65,9 @@ package body Alire.Roots.Editable is then Edited.Commit; Edited.Deploy_Dependencies; + if Builds.Sandboxed_Dependencies then + Edited.Generate_Configuration (Full => True); + end if; else Trace.Info ("No changes applied."); end if; diff --git a/src/alire/alire-roots.adb b/src/alire/alire-roots.adb index 0869985e..46976a8a 100644 --- a/src/alire/alire-roots.adb +++ b/src/alire/alire-roots.adb @@ -192,28 +192,30 @@ package body Alire.Roots is if Saved_Profiles then This.Set_Build_Profiles (Crate_Configuration.Last_Build_Profiles); - This.Build_Hasher.Clear; end if; This.Load_Configuration; This.Configuration.Ensure_Complete; -- For proceeding to build, the configuration must be complete - -- Check if crate configuration should be re-generated. This is the old - -- behavior; for shared builds, config needs to be generated only once. + -- Ensure sources and configurations are up to date + + if Builds.Sandboxed_Dependencies then + This.Generate_Configuration (Full => Force); + -- Will regenerate on demand only those changed - if Builds.Sandboxed_Dependencies - and then This.Configuration.Must_Regenerate - then - This.Generate_Configuration; elsif not Builds.Sandboxed_Dependencies then - This.Deploy_Dependencies; This.Sync_Builds; -- Changes in configuration may require new build dirs - This.Configuration.Generate_Config_Files (This, Release (This)); - -- Generate the config for the root crate. - -- TODO: generate only when changed (same optimization as for - -- sandboxed dependencies). + + This.Configuration.Generate_Config_Files (This, + Release (This), + Full => Force); + -- Generate the config for the root crate only, the previous sync + -- takes care of the rest. + + This.Build_Hasher.Write_Inputs (This); + -- Now, after the corresponding config files are in place end if; if Export_Build_Env or else not Builds.Sandboxed_Dependencies then @@ -497,14 +499,33 @@ package body Alire.Roots is ------------------- function Configuration (This : in out Root) - return Crate_Configuration.Global_Config + return access Crate_Configuration.Global_Config is begin This.Load_Configuration; - return This.Configuration.all; + return This.Configuration; end Configuration; + --------------------- + -- Config_Outdated -- + --------------------- + + function Config_Outdated (This : in out Root; + Name : Crate_Name) + return Boolean + is + Unused : constant String := This.Build_Hash (Name); + -- Ensure hashes are computed + Current : constant Builds.Hashes.Variables := + This.Build_Hasher.Inputs (Name); + Stored : constant Builds.Hashes.Variables := + Builds.Hashes.Stored_Inputs (This, Release (This, Name)); + use type Builds.Hashes.Variables; + begin + return Current /= Stored; + end Config_Outdated; + ------------------------ -- Load_Configuration -- ------------------------ @@ -527,6 +548,7 @@ package body Alire.Roots is begin This.Load_Configuration; This.Configuration.Set_Build_Profile (Crate, Profile); + This.Build_Hasher.Clear; end Set_Build_Profile; ------------------------ @@ -568,13 +590,17 @@ package body Alire.Roots is & Key (I).As_String); end if; end loop; + + This.Build_Hasher.Clear; end Set_Build_Profiles; ---------------------------- -- Generate_Configuration -- ---------------------------- - procedure Generate_Configuration (This : in out Root) is + procedure Generate_Configuration (This : in out Root; + Full : Boolean) + is Guard : Directories.Guard (Directories.Enter (Path (This))) with Unreferenced; -- At some point inside the configuration generation process the config @@ -582,7 +608,20 @@ package body Alire.Roots is -- which can't be directly used because of circularities. begin This.Load_Configuration; - This.Configuration.Generate_Config_Files (This); + This.Configuration.Generate_Config_Files (This, Full); + + if This.Configuration.Is_Config_Complete then + -- For incomplete configs this is secundary, as builds cannot be + -- performed anyway. + + if This.Build_Hasher.Is_Empty then + This.Build_Hasher.Compute (This); + end if; + + This.Build_Hasher.Write_Inputs (This); + -- We commit hashes to disk after generating the configuration, as we + -- rely on these hash inputs to know when config must be regenerated. + end if; end Generate_Configuration; ------------------ @@ -801,11 +840,7 @@ package body Alire.Roots is -- Update/Create configuration files if Builds.Sandboxed_Dependencies then - This.Load_Configuration; - This.Generate_Configuration; - -- TODO: this should be made more granular to only generate - -- configurations of newly deployed build sources, since with the - -- new shared builds system configs do not change once created. + This.Generate_Configuration (Full => Force); end if; -- Check that the solution does not contain suspicious dependencies, @@ -1343,23 +1378,31 @@ package body Alire.Roots is return Containers.Crate_Name_Sets.Set is Result : Containers.Crate_Name_Sets.Set; + begin + for Rel of This.Nonabstract_Releases loop + Result.Include (Rel.Name); + end loop; - procedure Filter (This : in out Alire.Roots.Root; - Solution : Solutions.Solution; - State : Solutions.Dependency_State) - is - pragma Unreferenced (This, Solution); - begin - if State.Has_Release and then not State.Is_Provided then - Result.Include (State.Crate); - end if; - end Filter; + return Result; + end Nonabstract_Crates; + -------------------------- + -- Nonabstract_Releases -- + -------------------------- + + function Nonabstract_Releases (This : in out Root) + return Releases.Containers.Release_Set + is + Result : Releases.Containers.Release_Set; begin - This.Traverse (Filter'Access); - Result.Include (This.Name); + for Rel of This.Solution.Releases loop + Result.Include (Rel); + end loop; + + Result.Include (Release (This)); -- The root release + return Result; - end Nonabstract_Crates; + end Nonabstract_Releases; ---------- -- Path -- @@ -1652,8 +1695,8 @@ package body Alire.Roots is if (for some Rel of This.Solution.Releases => This.Solution.State (Rel.Name).Is_Solved and then - not GNAT.OS_Lib.Is_Directory (This.Release_Base (Rel.Name, - For_Deploy))) + not Directories.New_Completion + (This.Release_Base (Rel.Name, For_Deploy)).Is_Complete) then Trace.Detail ("Detected missing dependency sources, updating workspace..."); @@ -1830,7 +1873,7 @@ package body Alire.Roots is -- Update/Create configuration files if Builds.Sandboxed_Dependencies then - This.Generate_Configuration; + This.Generate_Configuration (Full => True); end if; Trace.Detail ("Update completed"); diff --git a/src/alire/alire-roots.ads b/src/alire/alire-roots.ads index 614f7ddd..3db74ae9 100644 --- a/src/alire/alire-roots.ads +++ b/src/alire/alire-roots.ads @@ -151,6 +151,10 @@ package Alire.Roots is -- including root, excluding those that are provided by another crate. -- I.e., only actual regular releases. + function Nonabstract_Releases (This : in out Root) + return Releases.Containers.Release_Set; + -- Same as Nonabstract_Crates, but the releases themselves + function Solution (This : in out Root) return Solutions.Solution with Pre => This.Has_Lockfile; -- Returns the solution stored in the lockfile @@ -268,6 +272,12 @@ package Alire.Roots is return String; -- Returns the build hash of a crate if the solution; computes on demand. + function Config_Outdated (This : in out Root; + Name : Crate_Name) + return Boolean; + -- Say if the config on disk must be regenerated, comparing the hash on + -- disk with the newly computed hash. + procedure Install (This : in out Root; Prefix : Absolute_Path; @@ -277,7 +287,7 @@ package Alire.Roots is -- Call gprinstall on the releases in solution using --prefix=Prefix function Configuration (This : in out Root) - return Crate_Configuration.Global_Config; + return access Crate_Configuration.Global_Config; -- Returns the global configuration for the root and dependencies. This -- configuration is computed the first time it is requested. @@ -298,8 +308,11 @@ package Alire.Roots is Profiles : Crate_Configuration.Profile_Maps.Map); -- Give explicit profiles per crate. These are always overriding. - procedure Generate_Configuration (This : in out Root); - -- Generate or re-generate the crate configuration files + procedure Generate_Configuration (This : in out Root; + Full : Boolean); + -- Generate or re-generate the crate configuration files. If Full, + -- overwrite even if existing (so `alr update` can deal with any + -- corner case). procedure Print_Nested_Crates (Path : Any_Path); -- Look for nested crates below the given path and print a summary of diff --git a/src/alire/alire-solutions.ads b/src/alire/alire-solutions.ads index 285a6496..ca6a1e49 100644 --- a/src/alire/alire-solutions.ads +++ b/src/alire/alire-solutions.ads @@ -307,7 +307,9 @@ package Alire.Solutions is function Releases (This : Solution) return Release_Map; -- Returns the proper releases in the solution (regular and detected - -- externals). This also includes releases found at a linked folder. + -- externals). This also includes releases found at a linked folder. Since + -- this is a map name -> release, for provided releases there will be two + -- entries: the provider release and the provided dependency. function Required (This : Solution) return State_Map renames All_Dependencies; diff --git a/src/alire/alire-utils-text_files.adb b/src/alire/alire-utils-text_files.adb index b69dde4c..358c2b87 100644 --- a/src/alire/alire-utils-text_files.adb +++ b/src/alire/alire-utils-text_files.adb @@ -56,6 +56,20 @@ package body Alire.Utils.Text_Files is return access AAA.Strings.Vector is (This.Lines'Access); + ------------ + -- Create -- + ------------ + + function Create (Name : Any_Path) return File + is (Ada.Finalization.Limited_Controlled with + Length => Name'Length, + Backup_Len => 0, + Name => Name, + Backup => False, + Backup_Dir => "", + Lines => <>, + Orig => <>); + ---------- -- Load -- ---------- diff --git a/src/alire/alire-utils-text_files.ads b/src/alire/alire-utils-text_files.ads index 4fee2a10..d1de4d3c 100644 --- a/src/alire/alire-utils-text_files.ads +++ b/src/alire/alire-utils-text_files.ads @@ -8,6 +8,8 @@ package Alire.Utils.Text_Files is type File (<>) is tagged limited private; + function Create (Name : Any_Path) return File; + function Load (From : Any_Path; Backup : Boolean := True; Backup_Dir : Any_Path := "") diff --git a/src/alr/alr-commands-build.adb b/src/alr/alr-commands-build.adb index 8616a53b..69efa6b1 100644 --- a/src/alr/alr-commands-build.adb +++ b/src/alr/alr-commands-build.adb @@ -62,13 +62,7 @@ package body Alr.Commands.Build is Reportaise_Wrong_Arguments ("Only one build profile can be selected"); end if; - -- Prevent premature update of dependencies, as the exact folders - -- will depend on the build hashes, which are yet unknown until - -- build profiles are applied. - Cmd.Requires_Workspace (Sync => Alire.Builds.Sandboxed_Dependencies); - -- For sandboxed dependencies we keep the legacy behavior. We can unify - -- behaviors when crate configuration is only generated per missing - -- crate. + Cmd.Requires_Workspace; -- Build profile in the command line takes precedence. The configuration -- will have been loaded at this time with all profiles found in diff --git a/testsuite/drivers/helpers.py b/testsuite/drivers/helpers.py index 353cd61e..a7813bcb 100644 --- a/testsuite/drivers/helpers.py +++ b/testsuite/drivers/helpers.py @@ -238,6 +238,15 @@ def md5sum(file): return file_hash.hexdigest() +def prepend_to_file(filename : str, lines : []) -> None: + """ + Prepend the given lines to a file + """ + old_contents = content_of(filename) + with open(filename, "wt") as file: + file.write("\n".join(lines) + "\n" + old_contents) + + def replace_in_file(filename : str, old : str, new : str): """ Replace all occurrences of a string in a file diff --git a/testsuite/tests/build/hashes/compiler-missing/test.py b/testsuite/tests/build/hashes/compiler-missing/test.py index 8a838a35..55e94329 100644 --- a/testsuite/tests/build/hashes/compiler-missing/test.py +++ b/testsuite/tests/build/hashes/compiler-missing/test.py @@ -12,6 +12,7 @@ from drivers.asserts import assert_match run_alr("config", "--set", "--global", "dependencies.shared", "true") # Init a crate without explicit compiler dependency +# This does not fail because hashes are not computed until build time init_local_crate("xxx") run_alr("with", "libhello") diff --git a/testsuite/tests/build_profile/alr_build_switches/test.py b/testsuite/tests/build_profile/alr_build_switches/test.py index 855a17ae..b97b559c 100644 --- a/testsuite/tests/build_profile/alr_build_switches/test.py +++ b/testsuite/tests/build_profile/alr_build_switches/test.py @@ -18,6 +18,18 @@ def check_config(path, profile): assert_match('.*Build_Profile : Build_Profile_Kind := "%s"' % profile, conf) + # Extra check for the compiler switches + profile = profile.lower() + if profile == 'development': + assert_match('.*"-Og"', conf) + elif profile == 'release': + assert_match('.*"-O3"', conf) + elif profile == 'validation': + assert_match('.*"-gnata"', conf) + else: + raise Exception("Unknown profile: %s" % profile) + + lib1_config = "../lib_1/config/lib_1_config.gpr" bin_config = "config/bin_1_config.gpr" diff --git a/testsuite/tests/crate_config/gen_control/test.py b/testsuite/tests/crate_config/gen_control/test.py index 8c12fb9b..f63f72e8 100644 --- a/testsuite/tests/crate_config/gen_control/test.py +++ b/testsuite/tests/crate_config/gen_control/test.py @@ -1,5 +1,5 @@ """ -Test basic crate configuration +Test selective disabling of configuration """ from drivers.alr import run_alr diff --git a/testsuite/tests/crate_config/no-rebuilds/test.py b/testsuite/tests/crate_config/no-rebuilds/test.py new file mode 100644 index 00000000..a6f81aa1 --- /dev/null +++ b/testsuite/tests/crate_config/no-rebuilds/test.py @@ -0,0 +1,25 @@ +""" +Ensure that no unnecessary rebuilds happend due to crate config generation +""" + +import os +from drivers.alr import alr_with, init_local_crate, run_alr +from drivers.asserts import assert_match +from drivers.helpers import prepend_to_file + +init_local_crate() +run_alr("build") # First build + +# Same build, nothing should be recompiled +p = run_alr("build", quiet=False) +assert_match('.*gprbuild: "xxx.*" up to date', p.out) + +# Switch to another profile and build must happen +p = run_alr("build", "--validation", quiet=False) +assert_match('.*\[link\] xxx.adb', p.out) + +# Use same profile, nothing should be recompiled +p = run_alr("build", "--validation", quiet=False) +assert_match('.*gprbuild: "xxx.*" up to date', p.out) + +print('SUCCESS') diff --git a/testsuite/tests/crate_config/no-rebuilds/test.yaml b/testsuite/tests/crate_config/no-rebuilds/test.yaml new file mode 100644 index 00000000..872fc127 --- /dev/null +++ b/testsuite/tests/crate_config/no-rebuilds/test.yaml @@ -0,0 +1,3 @@ +driver: python-script +indexes: + basic_index: {} diff --git a/testsuite/tests/get/external-tool-dependency/test.py b/testsuite/tests/get/external-tool-dependency/test.py index 32ef8028..6b7e2b94 100644 --- a/testsuite/tests/get/external-tool-dependency/test.py +++ b/testsuite/tests/get/external-tool-dependency/test.py @@ -30,6 +30,7 @@ compare(dir_content, ['main_1.0.0_filesystem/alire', 'main_1.0.0_filesystem/alire.toml', 'main_1.0.0_filesystem/alire/alire.lock', + 'main_1.0.0_filesystem/alire/build_hash_inputs', 'main_1.0.0_filesystem/alire/cache', 'main_1.0.0_filesystem/alire/cache/dependencies', make_dep_dir, diff --git a/testsuite/tests/get/git-local/test.py b/testsuite/tests/get/git-local/test.py index 24d1ff8e..d50781ac 100644 --- a/testsuite/tests/get/git-local/test.py +++ b/testsuite/tests/get/git-local/test.py @@ -19,6 +19,7 @@ compare(list(filter 'libfoo_1.0.0_9ddda32b/alire', 'libfoo_1.0.0_9ddda32b/alire.toml', 'libfoo_1.0.0_9ddda32b/alire/alire.lock', + 'libfoo_1.0.0_9ddda32b/alire/build_hash_inputs', 'libfoo_1.0.0_9ddda32b/alire/complete_copy', 'libfoo_1.0.0_9ddda32b/alire/config.toml', 'libfoo_1.0.0_9ddda32b/b', diff --git a/testsuite/tests/get/unpack-in-place/test.py b/testsuite/tests/get/unpack-in-place/test.py index 7ad1831f..3ed79143 100644 --- a/testsuite/tests/get/unpack-in-place/test.py +++ b/testsuite/tests/get/unpack-in-place/test.py @@ -12,6 +12,7 @@ compare(contents('libhello_1.0.0_filesystem'), ['libhello_1.0.0_filesystem/alire', 'libhello_1.0.0_filesystem/alire.toml', 'libhello_1.0.0_filesystem/alire/alire.lock', + 'libhello_1.0.0_filesystem/alire/build_hash_inputs', 'libhello_1.0.0_filesystem/alire/complete_copy', 'libhello_1.0.0_filesystem/alire/config.toml', 'libhello_1.0.0_filesystem/config', -- 2.39.5