From c12d28d9d3a69b403e99258e20f06f499dc4a5a3 Mon Sep 17 00:00:00 2001 From: Alejandro R Mosteo Date: Fri, 2 Jul 2021 09:38:57 +0200 Subject: [PATCH] Fix corner case when pinning to a not available version (#767) * Less confusing user feedback for existing pin * Typo in comment * Fix for corner case in which no solution was found The solution is impossible, but still an incomplete solution should be returned. In essence, when the user requested an impossible situation by applying a pin to a non-existent version for a crate already in the dependencies, the solver reused the old version assuming it would succeed, which was not the case if the pin is impossible. * Better pin info separation * Extra test case for pinning to unavailable version --- src/alire/alire-dependencies-states.ads | 12 + src/alire/alire-roots-editable.adb | 30 ++- src/alire/alire-roots-editable.ads | 17 +- src/alire/alire-roots-optional.ads | 2 +- src/alire/alire-roots.adb | 241 ++++++++---------- src/alire/alire-roots.ads | 10 +- src/alire/alire-solutions.adb | 29 ++- src/alire/alire-solutions.ads | 3 + src/alire/alire-solver.adb | 137 +++++++--- src/alire/alire-solver.ads | 7 +- testsuite/tests/pin/dir-mismatch/test.py | 4 +- testsuite/tests/pin/version/test.py | 14 + testsuite/tests/with/pin-dir-mismatch/test.py | 2 +- 13 files changed, 305 insertions(+), 203 deletions(-) diff --git a/src/alire/alire-dependencies-states.ads b/src/alire/alire-dependencies-states.ads index d8573a77..edf18016 100644 --- a/src/alire/alire-dependencies-states.ads +++ b/src/alire/alire-dependencies-states.ads @@ -117,6 +117,9 @@ package Alire.Dependencies.States is function Pin_Version (This : State) return Semantic_Versioning.Version with Pre => This.Is_Pinned; + function User_Pin (This : State) return User_Pins.Pin + with Pre => This.Is_User_Pinned; + function Release (This : State) return Releases.Release with Pre => This.Has_Release; @@ -523,4 +526,13 @@ private Pinning => (Pinned => False), Transitivity => Base.Transitivity); + -------------- + -- User_Pin -- + -------------- + + function User_Pin (This : State) return User_Pins.Pin + is (if This.Is_Linked + then This.Link + else User_Pins.New_Version (This.Pin_Version)); + end Alire.Dependencies.States; diff --git a/src/alire/alire-roots-editable.adb b/src/alire/alire-roots-editable.adb index 99a8ee9d..a95b5548 100644 --- a/src/alire/alire-roots-editable.adb +++ b/src/alire/alire-roots-editable.adb @@ -24,6 +24,7 @@ package body Alire.Roots.Editable is return Result : Root do Result.Orig := Original; Result.Edit := Roots.Root (Original.Temporary_Copy); + Result.Edit.Sync_Pins_From_Manifest (Exhaustive => False); end return; end New_Root; @@ -98,7 +99,7 @@ package body Alire.Roots.Editable is .Dependencies (This.Edit.Environment) and Dep, Props => This.Edit.Environment, - Current => This.Edit.Solution); + Pins => This.Edit.Pins); begin if Sol.State (Dep.Crate).Has_Release then return @@ -122,9 +123,7 @@ package body Alire.Roots.Editable is -- Do not add if already a direct dependency - if (for some Existing_Dep of Release (This.Edit).Flat_Dependencies => - Existing_Dep.Crate = Dep.Crate) - then + if Release (This.Edit).Depends_On (Dep.Crate, This.Edit.Environment) then raise Checked_Error with Errors.Set (TTY.Name (Dep.Crate) & " is already a direct dependency."); end if; @@ -157,9 +156,7 @@ package body Alire.Roots.Editable is -- If dependency is not among dependencies at all, nothing to do - if not (for some Dep of Release (This.Edit).Flat_Dependencies => - Dep.Crate = Crate) - then + if not Release (This.Edit).Depends_On (Crate) then Raise_Checked_Error ("Requested crate is not among direct dependencies."); end if; @@ -197,6 +194,10 @@ package body Alire.Roots.Editable is Version : Semver.Version) is begin + + -- If nothing in the solution depends on the pinned crate, add it as a + -- direct dependency. + if not This.Solution.Depends_On (Crate) then This.Add_Dependency (Dependencies.New_Dependency @@ -215,7 +216,7 @@ package body Alire.Roots.Editable is User_Pins.New_Version (Version)); This.Reload_Manifest; - This.Edit.Set (This.Solution.Pinning (Crate, Version)); + This.Edit.Set (This.Solution.Resetting (Crate).Pinning (Crate, Version)); end Add_Version_Pin; -------------------------- @@ -298,6 +299,7 @@ package body Alire.Roots.Editable is -- Since link pins can bring in more dependencies, we must also Update. -- Changes will be shown afterwards on the call to Confirm_And_Commit. + This.Edit.Update (Allow_All_Crates, Silent => True, Interact => False); @@ -390,7 +392,7 @@ package body Alire.Roots.Editable is if Adirs.Exists (Destination) then -- Remove a previous pin deployment, which may be obsolete - Adirs.Delete_Tree (Destination); + Directories.Delete_Tree (Destination); end if; Adirs.Rename (Old_Name => Temp_Pin.Filename, @@ -405,8 +407,8 @@ package body Alire.Roots.Editable is Branch => Branch)); This.Reload_Manifest; - -- And update lockfile. We need to "deploy" the pin (it's already - -- deployed) so the pin becomes aware of its own path. + -- And update lockfile. We need to call Deploy on the pin (although + -- it is already deployed) so the pin becomes aware of its own path. New_Pin.Deploy (Crate, This.Edit.Pins_Dir, Online => False); This.Edit.Set (This.Solution.Linking (Crate, New_Pin)); @@ -414,6 +416,7 @@ package body Alire.Roots.Editable is -- Since link pins can bring in more dependencies, we must -- also Update. Changes will be shown afterwards on the call -- to Confirm_And_Commit. + This.Edit.Update (Allow_All_Crates, Silent => True, Interact => False); @@ -427,12 +430,11 @@ package body Alire.Roots.Editable is procedure Remove_Pin (This : in out Root; Crate : Crate_Name) is begin - if This.Solution.Depends_On (Crate) - and then This.Solution.State (Crate).Is_User_Pinned - then + if Release (This.Edit).Pins.Contains (Crate) then Alire.Manifest.Remove_Pin (This.Edit.Crate_File, Crate); This.Edit.Set (This.Solution.User_Unpinning (Crate)); + This.Reload_Manifest; end if; end Remove_Pin; diff --git a/src/alire/alire-roots-editable.ads b/src/alire/alire-roots-editable.ads index f2672ff3..9f9ff67b 100644 --- a/src/alire/alire-roots-editable.ads +++ b/src/alire/alire-roots-editable.ads @@ -28,6 +28,9 @@ package Alire.Roots.Editable is -- A few proxies so useful predicates can be kept + function Old (This : Root) return Roots.Root; + -- The original root this editable copy was made from + function Name (This : Root) return Crate_Name; function Solution (This : in out Root) return Solutions.Solution; @@ -54,12 +57,11 @@ package Alire.Roots.Editable is function Can_Be_Pinned (This : in out Root; Crate : Crate_Name) return Boolean - is (not This.Solution.Depends_On (Crate) - or else not This.Solution.State (Crate).Is_User_Pinned + is (not Release (This.Old).Pins.Contains (Crate) or else Force or else raise Checked_Error with Errors.Set - (TTY.Name (Crate) & " is already pinned to " - & This.Solution.State (Crate).Image)); + (TTY.Name (Crate) & " is already pinned with pin " + & Release (This.Old).Pins.Element (Crate).Image (User => False))); -- Says if a pin can be added: not already a pin, or Force. As an -- exception, the body is here as this function is intended to serve as -- a precondition, an hence serve as documentation. @@ -119,6 +121,13 @@ private function Name (This : Root) return Crate_Name is (This.Edit.Name); + --------- + -- Old -- + --------- + + function Old (This : Root) return Roots.Root + is (This.Orig); + -------------- -- Solution -- -------------- diff --git a/src/alire/alire-roots-optional.ads b/src/alire/alire-roots-optional.ads index 861619eb..59b5294a 100644 --- a/src/alire/alire-roots-optional.ads +++ b/src/alire/alire-roots-optional.ads @@ -59,7 +59,7 @@ package Alire.Roots.Optional is return Dependencies.Dependency with Pre => This.Is_Valid; -- If This.Is_Valid, get the corresponding updatable - -- dependency (e.g., ^1.2, ~0.1.2). Otherwise, return "any". + -- dependency (e.g., ^1.2, ~0.1.2). private diff --git a/src/alire/alire-roots.adb b/src/alire/alire-roots.adb index aa508fd8..43e34205 100644 --- a/src/alire/alire-roots.adb +++ b/src/alire/alire-roots.adb @@ -289,8 +289,8 @@ package body Alire.Roots is Containers.Crate_Name_Sets.Empty_Set) is - Sol : Solutions.Solution := This.Solution; - Pins_Dir : constant Any_Path := This.Pins_Dir; + Top_Root : Root renames This; + Pins_Dir : constant Any_Path := This.Pins_Dir; Linked : Containers.Crate_Name_Sets.Set; -- And we use this to avoid re-processing the same link target @@ -304,6 +304,8 @@ package body Alire.Roots is -- hence attempting to link to an upstream means a cycle in the graph. is + Pins : Solutions.Solution renames Top_Root.Pins; + --------------------- -- Add_Version_Pin -- --------------------- @@ -311,21 +313,24 @@ package body Alire.Roots is procedure Add_Version_Pin (Crate : Crate_Name; Pin : User_Pins.Pin) is use type Semver.Version; begin - if not Sol.Depends_On (Crate) then - Sol := Sol.Depending_On - (Dependencies.New_Dependency (Crate, Pin.Version)); - end if; - - if Sol.State (Crate).Is_Pinned - and then - Sol.State (Crate).Pin_Version /= Pin.Version + if Pins.Depends_On (Crate) + and then Pins.State (Crate).Is_Pinned + and then Pins.State (Crate).Pin_Version /= Pin.Version then Put_Warning ("Incompatible version pins requested for crate " & TTY.Name (Crate) & "; fix versions or override with a link pin."); end if; - Sol := Sol.Resetting (Crate).Pinning (Crate, Pin.Version); + if not Pins.Depends_On (Crate) then + Pins := Pins.Depending_On + (Release (Top_Root) + .Dependency_On (Crate) + .Or_Else + (Dependencies.New_Dependency (Crate, Pin.Version))); + end if; + + Pins := Pins.Pinning (Crate, Pin.Version); end Add_Version_Pin; ------------------ @@ -363,16 +368,16 @@ package body Alire.Roots is -- At this point, we can detect that a link is conflicting with -- another one. - if Sol.Depends_On (Crate) - and then Sol.State (Crate).Is_Linked - and then Sol.State (Crate).Link /= Pin + if Pins.Depends_On (Crate) + and then Pins.State (Crate).Is_Linked + and then Pins.State (Crate).Link /= Pin then Raise_Checked_Error ("Conflicting pin links for crate " & TTY.Name (Crate) & ": Crate " & TTY.Name (Release (This).Name) & " wants to link " & TTY.URL (Pin.Image (User => True)) & ", but a previous link exists to " - & TTY.URL (Sol.State (Crate).Link.Image (User => True))); + & TTY.URL (Pins.State (Crate).Link.Image (User => True))); end if; -- If the link target has already been seen, we do not need to @@ -390,6 +395,7 @@ package body Alire.Roots is declare use Containers.Crate_Name_Sets; + use Semver.Extended; Target : constant Optional.Root := Optional.Detect_Root (Pin.Path); begin @@ -411,20 +417,14 @@ package body Alire.Roots is ("No crate found at pin location " & Pin.Relative_Path); end if; - -- Add the best dependency we can find for the link if the user - -- hasn't given one in the manifest. - - if not Sol.Depends_On (Crate) then - Sol := Sol.Depending_On - (if Target.Is_Valid - then Target.Updatable_Dependency - else Dependencies.New_Dependency - (Crate, Semantic_Versioning.Extended.Any)); - end if; - - Sol := Sol - .Resetting (Crate) - .Linking (Crate, Pin); + Pins := + Pins.Depending_On + (Release (Top_Root).Dependency_On (Crate) + .Or_Else (if Target.Is_Valid + then Target.Updatable_Dependency + else Dependencies.New_Dependency + (Crate, Any))) + .Linking (Crate, Pin); -- Add possible pins at the link target @@ -436,7 +436,7 @@ package body Alire.Roots is end; end Add_Link_Pin; - Pins : constant User_Pins.Maps.Map := Release (This).Pins; + New_Pins : constant User_Pins.Maps.Map := Release (This).Pins; begin @@ -445,7 +445,7 @@ package body Alire.Roots is -- process, so they're available for use immediately. All link pins -- have a proper path once this process completes. - for I in Pins.Iterate loop + for I in New_Pins.Iterate loop declare use all type User_Pins.Kinds; use User_Pins.Maps.Pin_Maps; @@ -466,45 +466,24 @@ package body Alire.Roots is end case; Trace.Detail ("Crate " & TTY.Name (This.Name) - & " adds pin " & Sol.State (Crate).TTY_Image); + & " adds pin " & Pins.State (Crate).TTY_Image); end; end loop; end Add_Pins; - ---------------- - -- Prune_Pins -- - ---------------- - - procedure Prune_Pins is - begin - for Dep of Sol.User_Pins loop - Sol := Sol.User_Unpinning (Dep.Value.Crate); - end loop; - end Prune_Pins; - - use type Solutions.Solution; - begin -- Remove any existing pins in the stored solution, to avoid conflicts -- between old and new definitions of the same pin, and to discard -- removed pins. - Prune_Pins; + This.Pins := Solutions.Empty_Valid_Solution; -- Recursively add all pins from this workspace and other linked ones Add_Pins (This, Upstream => Containers.Crate_Name_Sets.To_Set (This.Name)); - if Sol /= This.Solution then - Solutions.Diffs.Between (This.Solution, Sol).Print - (Changed_Only => True, - Level => Trace.Detail); - Trace.Detail ("Local pins updated and committed to lockfile"); - This.Set (Solution => Sol); - end if; - exception when others => -- In the event that the manifest contains bad pins, we ensure the @@ -662,6 +641,7 @@ package body Alire.Roots is Path => +Path, Release => Containers.To_Release_H (R), Cached_Solution => <>, + Pins => <>, Lockfile => <>, Manifest => <>); @@ -812,7 +792,6 @@ package body Alire.Roots is Interact : Boolean; Force : Boolean := False) is - Old_Solution : constant Solutions.Solution := This.Solution; begin if Force or else This.Is_Lockfile_Outdated then -- TODO: we may want to recursively check manifest timestamps of @@ -828,8 +807,7 @@ package body Alire.Roots is -- a non-exhaustive sync of pins, that will anyway detect evident -- changes (new/removed pins, changed explicit commits). - This.Sync_Dependencies (Old => Old_Solution, - Silent => Silent, + This.Sync_Dependencies (Silent => Silent, Interact => Interact); -- Don't ask for confirmation as this is an automatic update in -- reaction to a manually edited manifest, and we need the lockfile @@ -894,7 +872,6 @@ package body Alire.Roots is Silent : Boolean; Interact : Boolean) is - Old : constant Solutions.Solution := This.Solution; begin This.Sync_Pins_From_Manifest (Exhaustive => True, Allowed => Allowed); @@ -905,7 +882,6 @@ package body Alire.Roots is This.Sync_Dependencies (Allowed => Allowed, - Old => Old, Silent => Silent, Interact => Interact and not Alire.Utils.User_Input.Not_Interactive); end Update; @@ -924,7 +900,6 @@ package body Alire.Roots is is use type Conditional.Dependencies; - Old : constant Solutions.Solution := This.Solution; Deps : Conditional.Dependencies := Release (This).Dependencies (This.Environment); begin @@ -932,7 +907,7 @@ package body Alire.Roots is -- Identify crates that must be held back if not Allowed.Is_Empty then - for Release of Old.Releases loop + for Release of This.Solution.Releases loop if not Allowed.Contains (Release.Name) then Trace.Debug ("Forcing release in solution: " & Release.Version.Image); @@ -941,10 +916,16 @@ package body Alire.Roots is end loop; end if; + -- Ensure we have complete pin information + + This.Sync_Pins_From_Manifest (Exhaustive => False); + + -- And solve + return Solver.Resolve (Deps => Deps, Props => This.Environment, - Current => Old, + Pins => This.Pins, Options => Options); end Compute_Update; @@ -956,97 +937,83 @@ package body Alire.Roots is (This : in out Root; Silent : Boolean; -- Do not output anything Interact : Boolean; -- Request confirmation from the user - Old : Solutions.Solution := Solutions.Empty_Invalid_Solution; Options : Solver.Query_Options := Solver.Default_Options; Allowed : Containers.Crate_Name_Sets.Set := Alire.Containers.Crate_Name_Sets.Empty_Set) is + Old : constant Solutions.Solution := This.Solution; begin - declare - -- Shadow the argument with the one we want to use everywhere. Note - -- that this old is only used for comparison, as the stored solution - -- may already include changes caused by pin preparations, and - -- furthermore the stored root is the one we need to pass to the - -- solver (as it contains the pins). - Old : constant Solutions.Solution := - (if Sync_Dependencies.Old.Is_Attempted - then Sync_Dependencies.Old - else This.Solution); + -- Ensure requested crates are in solution first. - begin + for Crate of Allowed loop + if not Old.Depends_On (Crate) then + Raise_Checked_Error ("Requested crate is not a dependency: " + & TTY.Name (Crate)); + end if; - -- Ensure requested crates are in solution first. + if Old.Pins.Contains (Crate) then + -- The solver will never update a pinned crate, so we may allow + -- this to be attempted but it will have no effect. + Recoverable_Error + ("Requested crate is pinned and cannot be updated: " + & Alire.Utils.TTY.Name (Crate)); + end if; + end loop; - for Crate of Allowed loop - if not Old.Depends_On (Crate) then - Raise_Checked_Error ("Requested crate is not a dependency: " - & TTY.Name (Crate)); - end if; + declare + Needed : constant Solutions.Solution := This.Compute_Update + (Allowed, Options); + Diff : constant Solutions.Diffs.Diff := Old.Changes (Needed); + begin + -- Early exit when there are no changes - if Old.Pins.Contains (Crate) then - -- The solver will never update a pinned crate, so we may allow - -- this to be attempted but it will have no effect. - Recoverable_Error - ("Requested crate is pinned and cannot be updated: " - & Alire.Utils.TTY.Name (Crate)); + if not Alire.Force and not Diff.Contains_Changes then + if not Needed.Is_Complete then + Trace.Warning + ("There are missing dependencies" + & " (use `alr with --solve` for details)."); end if; - end loop; - - declare - Needed : constant Solutions.Solution := This.Compute_Update - (Allowed, Options); - Diff : constant Solutions.Diffs.Diff := Old.Changes (Needed); - begin - -- Early exit when there are no changes - - if not Alire.Force and not Diff.Contains_Changes then - if not Needed.Is_Complete then - Trace.Warning - ("There are missing dependencies" - & " (use `alr with --solve` for details)."); - end if; - - This.Sync_Manifest_And_Lockfile_Timestamps; - -- In case manual changes in manifest do not modify the - -- solution. - if not Silent then - Trace.Info ("Nothing to update."); - end if; - - else + This.Sync_Manifest_And_Lockfile_Timestamps; + -- In case manual changes in manifest do not modify the + -- solution. - -- Show changes and optionally ask user to apply them - - if not Interact then - declare - Level : constant Trace.Levels := - (if Silent then Debug else Info); - begin - Trace.Log - ("Dependencies automatically updated as follows:", - Level); - Diff.Print (Level => Level); - end; - elsif not Utils.User_Input.Confirm_Solution_Changes (Diff) then - Trace.Detail ("Update abandoned."); - return; - end if; + if not Silent then + Trace.Info ("Nothing to update."); + end if; + else + + -- Show changes and optionally ask user to apply them + + if not Interact then + declare + Level : constant Trace.Levels := + (if Silent then Debug else Info); + begin + Trace.Log + ("Dependencies automatically updated as follows:", + Level); + Diff.Print (Level => Level); + end; + elsif not Utils.User_Input.Confirm_Solution_Changes (Diff) then + Trace.Detail ("Update abandoned."); + return; end if; - -- Apply the update. We do this even when no changes were - -- detected, as pin evaluation may have temporarily stored - -- unsolved dependencies which have been re-solved now. + end if; - This.Set (Solution => Needed); - This.Deploy_Dependencies; + -- Apply the update. We do this even when no changes were + -- detected, as pin evaluation may have temporarily stored + -- unsolved dependencies which have been re-solved now. - -- Update/Create configuration files - This.Generate_Configuration; + This.Set (Solution => Needed); + This.Deploy_Dependencies; - Trace.Detail ("Update completed"); - end; + -- Update/Create configuration files + This.Generate_Configuration; + + Trace.Detail ("Update completed"); end; end Sync_Dependencies; @@ -1137,6 +1104,10 @@ package body Alire.Roots is (This.Crate_File, Manifest.Local, Strict => True)); + + -- And our pins + + This.Sync_Pins_From_Manifest (Exhaustive => False); end Reload_Manifest; end Alire.Roots; diff --git a/src/alire/alire-roots.ads b/src/alire/alire-roots.ads index 820dfe3a..46f880ad 100644 --- a/src/alire/alire-roots.ads +++ b/src/alire/alire-roots.ads @@ -167,15 +167,12 @@ package Alire.Roots is (This : in out Root; Silent : Boolean; -- Do not output anything Interact : Boolean; -- Request confirmation from the user - Old : Solutions.Solution := Solutions.Empty_Invalid_Solution; Options : Solver.Query_Options := Solver.Default_Options; Allowed : Containers.Crate_Name_Sets.Set := Alire.Containers.Crate_Name_Sets.Empty_Set); -- Resolve and update all or given crates in a root, and regenerate -- configuration. When Silent, run as in non-interactive mode as this is an - -- automatically-triggered update. Old_Sol is used to present differences, - -- and when left at the default invalid argument value, This.Solution will - -- be used as old solution. + -- automatically-triggered update. procedure Sync_Pins_From_Manifest (This : in out Root; @@ -239,6 +236,9 @@ private Release : Containers.Release_H; Cached_Solution : Cached_Solutions.Cache; + Pins : Solutions.Solution; + -- Closure of all pins that are recursively found + -- These values, if different from "", mean this is a temporary root Manifest : Unbounded_Absolute_Path; Lockfile : Unbounded_Absolute_Path; @@ -263,7 +263,7 @@ private -- Obtain a temporary copy of This root, in the sense that it uses temp -- names for the manifest and lockfile. The cache is shared, so any -- pins/dependencies added to the temporary copy are ready if the copy is - -- commited (see Commit call). The intended use is to be able to modify + -- committed (see Commit call). The intended use is to be able to modify -- the temporary manifest, and finally compare the solutions between This -- and its copy. This way, no logic remains in `alr with`/`alr pin`, for -- example, as they simply edit the manifest as if the user did it by hand. diff --git a/src/alire/alire-solutions.adb b/src/alire/alire-solutions.adb index c7913a87..47aa45b7 100644 --- a/src/alire/alire-solutions.adb +++ b/src/alire/alire-solutions.adb @@ -327,6 +327,22 @@ package body Alire.Solutions is end return; end New_Solution; + --------------- + -- Link_Pins -- + --------------- + + function Link_Pins (This : Solution) return Conditional.Dependencies is + begin + return Dependencies : Conditional.Dependencies do + for Dep of This.Dependencies loop + if Dep.Is_Linked then + Dependencies + .Append (Conditional.New_Dependency (Dep.As_Dependency)); + end if; + end loop; + end return; + end Link_Pins; + ---------- -- Pins -- ---------- @@ -350,16 +366,11 @@ package body Alire.Solutions is -- User_Pins -- --------------- - function User_Pins (This : Solution) return Conditional.Dependencies is + function User_Pins (This : Solution) return Conditional.Dependencies + is + use type Conditional.Dependencies; begin - return Dependencies : Conditional.Dependencies := This.Pins do - for Dep of This.Dependencies loop - if Dep.Is_Linked then - Dependencies - .Append (Conditional.New_Dependency (Dep.As_Dependency)); - end if; - end loop; - end return; + return This.Pins and This.Link_Pins; end User_Pins; ---------- diff --git a/src/alire/alire-solutions.ads b/src/alire/alire-solutions.ads index 85b8fafb..41127c56 100644 --- a/src/alire/alire-solutions.ads +++ b/src/alire/alire-solutions.ads @@ -249,6 +249,9 @@ package Alire.Solutions is function Links (This : Solution) return Dependency_Map; -- Return crates that are solved with a softlink + function Link_Pins (This : Solution) return Conditional.Dependencies; + -- Return dependencies of linked crates in the solution + function Misses (This : Solution) return Dependency_Map; -- Return crates for which there is neither hint nor proper versions diff --git a/src/alire/alire-solver.adb b/src/alire/alire-solver.adb index 7833a7e0..e862b571 100644 --- a/src/alire/alire-solver.adb +++ b/src/alire/alire-solver.adb @@ -113,10 +113,10 @@ package body Alire.Solver is function Is_Resolvable (Deps : Types.Abstract_Dependencies; Props : Properties.Vector; - Current : Solution; + Pins : Solution; Options : Query_Options := Default_Options) return Boolean - is (Resolve (Deps, Props, Current, Options).Is_Complete); + is (Resolve (Deps, Props, Pins, Options).Is_Complete); ------------- -- Resolve -- @@ -124,7 +124,7 @@ package body Alire.Solver is function Resolve (Deps : Alire.Types.Abstract_Dependencies; Props : Properties.Vector; - Current : Solution; + Pins : Solution; Options : Query_Options := Default_Options) return Solution is @@ -371,6 +371,82 @@ package body Alire.Solver is end if; end Expand_Missing; + ----------------------- + -- Check_Version_Pin -- + ----------------------- + -- Specific checks for a version pin that narrow down the search + procedure Check_Version_Pin is + Pin_Version : constant Semver.Version := + Pins.State (Dep.Crate).Pin_Version; + begin + + -- For a version pin release, we try only a release with the + -- exact version of the pin, to speed up the solving. If the + -- pin version is incompatible with the dependency, this branch + -- cannot succeed though. + + if Semver.Extended.Is_In (Pin_Version, Dep.Versions) then + + -- The pin is compatible with the dependency, go ahead + + if Index.Exists (Dep.Crate, Pin_Version) then + + -- There is a valid crate for this pin and dependency + + Trace.Debug ("SOLVER short-cutting due to version pin" + & " with valid release in index"); + Check (Index.Find (Dep.Crate, Pin_Version)); + + -- The check may still fail, so we must attempt this one: + + Trace.Debug + ("SOLVER: marking crate " & Dep.Image + & " MISSING in case pinned version available in index " + & TTY.Version (Pin_Version.Image) + & " is incompatible with other dependencies" + & " when the search tree was " + & Tree'(Expanded + and Target + and Remaining).Image_One_Line); + + Expand_Missing (Dep); + + else + + -- There is no release for this pin + + Trace.Debug + ("SOLVER: marking crate " & Dep.Image + & " MISSING because index LACKS pinned version " + & TTY.Version (Pin_Version.Image) + & " when the search tree was " + & Tree'(Expanded + and Target + and Remaining).Image_One_Line); + + Expand_Missing (Dep); + + end if; + + else + + -- The pin contradicts the dependency + + Trace.Debug + ("SOLVER: marking crate " & Dep.Image + & " MISSING because version pin " + & TTY.Version (Pin_Version.Image) & " cannot satisfy " + & Dep.TTY_Image + & " when the search tree was " + & Tree'(Expanded + and Target + and Remaining).Image_One_Line); + + Expand_Missing (Dep); + + end if; + end Check_Version_Pin; + Satisfiable : Boolean := False; -- Mark that the dependency is satisfiable. When we refactor the -- solver from recursive to priority queue (I guess we eventually @@ -379,8 +455,8 @@ package body Alire.Solver is begin - if Current.Depends_On (Dep.Crate) and then - Current.State (Dep.Crate).Is_Linked + if Pins.Depends_On (Dep.Crate) and then + Pins.State (Dep.Crate).Is_Linked then -- The dependency is softlinked in the starting solution, hence @@ -388,20 +464,20 @@ package body Alire.Solver is Trace.Debug ("SOLVER: dependency LINKED to " & - Current.State (Dep.Crate).Link.Path & + Pins.State (Dep.Crate).Link.Path & " when tree is " & Tree'(Expanded and Target and Remaining).Image_One_Line); Expand (Expanded => Expanded and Dep, Target => Remaining and - (if Current.State (Dep.Crate).Has_Release - then Current.State (Dep.Crate) - .Release.Dependencies (Props) + (if Pins.State (Dep.Crate).Has_Release + then Pins.State (Dep.Crate) + .Release.Dependencies (Props) else Empty), Remaining => Empty, Solution => Solution.Linking (Dep.Crate, - Current.State (Dep.Crate).Link)); + Pins.State (Dep.Crate).Link)); elsif Solution.Releases.Contains (Dep.Crate) then @@ -410,18 +486,13 @@ package body Alire.Solver is Check (Solution.Releases.Element (Dep.Crate)); - elsif - Current.Depends_On (Dep.Crate) and then - Current.State (Dep.Crate).Is_Pinned and then - Current.State (Dep.Crate).Is_Solved + elsif Pins.Depends_On (Dep.Crate) and then + Pins.State (Dep.Crate).Is_Pinned then - -- For an existing pinned release, we try first to reuse the - -- stored release instead of looking for another release with - -- the same version (which will be the same one anyway for a - -- same index). + -- Specific pin checks that can speed up the search - Check (Current.Releases.Element (Dep.Crate)); + Check_Version_Pin; elsif Index.Exists (Dep.Crate) then @@ -503,6 +574,15 @@ package body Alire.Solver is if not Satisfiable or else Options.Completeness = All_Incomplete then + + Trace.Debug + ("SOLVER: marking crate " & Dep.Image + & " MISSING with Satisfiable=" & Satisfiable'Image + & " when the search tree was " + & Tree'(Expanded + and Target + and Remaining).Image_One_Line); + Expand_Missing (Dep); end if; @@ -676,7 +756,7 @@ package body Alire.Solver is if not Index.Exists (Dep.Value.Crate) then -- Crate totally unavailable Unavailable_Crates.Include (Dep.Value.Crate); - Trace.Detail ("Direct dependency is not a known crate: " + Trace.Debug ("Direct dependency is not a known crate: " & TTY.Name (Dep.Value.Crate)); else -- Pre-populate external releases @@ -688,7 +768,7 @@ package body Alire.Solver is -- No valid releases for the crate Unavailable_Deps.Include (Dep.Value.Image); - Trace.Detail + Trace.Debug ("Direct dependency has no fulfilling releases: " & TTY.Name (Dep.Value.Image)); end if; @@ -706,11 +786,11 @@ package body Alire.Solver is procedure Trace_Pins is begin - if (for some State of Current.All_Dependencies => + if (for some State of Pins.All_Dependencies => State.Is_User_Pinned) then Trace.Detail ("User pins to apply:"); - for State of Current.All_Dependencies loop + for State of Pins.All_Dependencies loop if State.Is_User_Pinned then Trace.Detail (" " & State.TTY_Image); end if; @@ -721,10 +801,9 @@ package body Alire.Solver is end Trace_Pins; Full_Dependencies : constant Conditional.Dependencies := - Tree'(Current.User_Pins and Deps).Evaluate (Props); - -- Include pins before other dependencies. This ensures their dependency - -- can only be solved with the pinned version, and they are attempted - -- first to avoid wasteful trial-and-error with other versions. + Tree'(Pins.User_Pins and Deps).Evaluate (Props); + -- Include pins before other dependencies. This makes their dependency + -- show in solutions explicitly. Solution : constant Alire.Solutions.Solution := Alire.Solutions.Empty_Valid_Solution; @@ -785,7 +864,7 @@ package body Alire.Solver is return Resolve (Deps => Deps, Props => Props, - Current => Current, + Pins => Pins, Options => (Query_Options' (Age => Options.Age, @@ -809,7 +888,7 @@ package body Alire.Solver is declare Best_Solution : Alire.Solutions.Solution := - Solutions.First_Element.With_Pins (Current); + Solutions.First_Element.With_Pins (Pins); begin -- Mark pins as direct dependencies diff --git a/src/alire/alire-solver.ads b/src/alire/alire-solver.ads index 1ed15d88..00617947 100644 --- a/src/alire/alire-solver.ads +++ b/src/alire/alire-solver.ads @@ -131,16 +131,17 @@ package Alire.Solver is function Resolve (Deps : Alire.Types.Abstract_Dependencies; Props : Properties.Vector; - Current : Solution; + Pins : Solution; Options : Query_Options := Default_Options) return Solution; -- Exhaustively look for a solution to the given dependencies, under the -- given platform properties and lookup options. Pins can be supplied to - -- override Deps. May raise No_Solution_Error. + -- override Deps. May raise No_Solution_Error when not using Exhaustive + -- options. function Is_Resolvable (Deps : Types.Abstract_Dependencies; Props : Properties.Vector; - Current : Solution; + Pins : Solution; Options : Query_Options := Default_Options) return Boolean; -- Simplified call to Resolve, discarding result diff --git a/testsuite/tests/pin/dir-mismatch/test.py b/testsuite/tests/pin/dir-mismatch/test.py index 78d9d580..bdb61bbf 100644 --- a/testsuite/tests/pin/dir-mismatch/test.py +++ b/testsuite/tests/pin/dir-mismatch/test.py @@ -27,14 +27,14 @@ run_alr('with', 'nothello', '--use', '..') # Detect a repin is rejected unless forced p = run_alr('with', 'nothello', '--use', '..' + dir_separator() + target, complain_on_error=False) -assert_match(".*nothello is already pinned to.*", p.out) +assert_match(".*nothello is already pinned with pin .*", p.out) # Try to repin to a dir with valid crate metadata p = run_alr('with', 'nothello', '--use', '..' + dir_separator() + target, complain_on_error=False, force=True) # Expected error -assert_match('.*crate mismatch: expected nothello but found hello at.*', p.out) +assert_match('.*expected nothello but found hello.*', p.out) # skip test-specific path print('SUCCESS') diff --git a/testsuite/tests/pin/version/test.py b/testsuite/tests/pin/version/test.py index 82fbfbf4..d9334d83 100644 --- a/testsuite/tests/pin/version/test.py +++ b/testsuite/tests/pin/version/test.py @@ -26,4 +26,18 @@ assert_eq("ERROR: Plain crate name or crate=version argument expected" " for pinning, but got: hello^1.0\n", p.out) +# Test that pinning to a non-existent version is doable but solution is missing +p = run_alr("pin", "hello=7.7.7", force=True) +p = run_alr("pin") +assert_eq("hello 7.7.7\n", p.out) +p = run_alr("with", "--solve") +assert_eq("""\ +Dependencies (direct): + hello* +Dependencies (external): + hello=7.7.7 (direct,missed,pin=7.7.7) (pinned) +Dependencies (graph): + xxx=0.0.0 --> hello* +""", p.out) + print('SUCCESS') diff --git a/testsuite/tests/with/pin-dir-mismatch/test.py b/testsuite/tests/with/pin-dir-mismatch/test.py index 373561af..c88efae4 100644 --- a/testsuite/tests/with/pin-dir-mismatch/test.py +++ b/testsuite/tests/with/pin-dir-mismatch/test.py @@ -23,7 +23,7 @@ p = run_alr('with', 'nothello', '--use', '..' + dir_separator() + target, complain_on_error=False) # Expected error -assert_match('.*crate mismatch: expected nothello but found hello at.*', p.out) +assert_match('.*expected nothello but found hello.*', p.out) # skip test-specific path print('SUCCESS') -- 2.39.5