From 98648092e2427867f4f18bd12f702f6e846f3f2e Mon Sep 17 00:00:00 2001 From: Alejandro R Mosteo Date: Tue, 21 Nov 2023 11:13:56 +0100 Subject: [PATCH] Offer uninstalled compiler as fallback with warning (#1508) * Allow solving with unistalled tools as fallback * Fix precedence of installed compiler over remote ones * Add warning in solution diff about toolchain download * Fallback on installed compiler over uninstalled one This is after checking that the preferred compiler cannot fulfill the compiler dependency. * Tweaks to restore former behaviors * Fix test that inadvertently used virtual crate It should have been real all along without alternatives * Rebase tweaks * Reword information message --- src/alire/alire-solutions-diffs.adb | 41 +++++- src/alire/alire-solutions.adb | 22 +++ src/alire/alire-solver.adb | 139 ++++++++++++------ src/alire/alire-toolchains.adb | 36 ++++- .../crate_non_virt/crate_non_virt-1.0.0.toml | 9 ++ .../gnat_external/gnat_external-external.toml | 6 +- .../tests/build/hashes/compiler-input/test.py | 8 +- .../tests/solver/compiler-installed/test.py | 25 ++-- .../tests/solver/compiler-priorities/test.py | 8 +- testsuite/tests/solver/forbids/test.py | 2 +- .../tests/toolchain/select-dontmix/test.py | 2 - 11 files changed, 223 insertions(+), 75 deletions(-) create mode 100644 testsuite/fixtures/toolchain_index/cr/crate_non_virt/crate_non_virt-1.0.0.toml diff --git a/src/alire/alire-solutions-diffs.adb b/src/alire/alire-solutions-diffs.adb index 377fdd64..7d4a8958 100644 --- a/src/alire/alire-solutions-diffs.adb +++ b/src/alire/alire-solutions-diffs.adb @@ -23,7 +23,8 @@ package body Alire.Solutions.Diffs is Unpinned, -- A release being unpinned Unchanged, -- An unchanged dependency/release Missing, -- A missing dependency - Binary -- A binary, system or external release + Binary, -- A binary, system or external release + Info -- General info icon ); ---------- @@ -42,7 +43,8 @@ package body Alire.Solutions.Diffs is when Unpinned => TTY.Emph (U ("🎈")), -- alts: 𐩒🎈 when Unchanged => TTY.OK (U ("=")), when Missing => TTY.Error (U ("❗")), -- alts: ⚠️❗‼️ - when Binary => TTY.Warn (U ("📦"))) + when Binary => TTY.Warn (U ("📦")), + when Info => TTY.Emph (U ("🛈"))) else (case Change is when Added => U ("+"), @@ -54,7 +56,8 @@ package body Alire.Solutions.Diffs is when Unpinned => U ("o"), when Unchanged => U ("="), when Missing => U ("!"), - when Binary => U ("b") + when Binary => U ("b"), + when Info => U ("i") )); -- This type is used to summarize every detected change @@ -391,6 +394,35 @@ package body Alire.Solutions.Diffs is Table : Utils.Tables.Table; Changed : Boolean := False; + ----------------------------- + -- Warn_Toolchain_Download -- + ----------------------------- + -- If the solution requires downloading a new toolchain, warn about it + procedure Warn_Toolchain_Download is + begin + for Rel of This.Latter.Releases loop + if Toolchains.Is_Tool (Rel) + and then not Toolchains.Available.Contains (Rel) + then + Trace.Log (Prefix, Level); + Trace.Log + (Prefix & Icon (Info) + & " The solution requires a toolchain that is"); + Trace.Log + (Prefix + & " not yet installed. Accepting the solution"); + Trace.Log + (Prefix + & " will download and install this toolchain."); + return; + end if; + end loop; + end Warn_Toolchain_Download; + + -------------------------------------- + -- Warn_Unsatisfiable_GNAT_External -- + -------------------------------------- + procedure Warn_Unsatisfiable_GNAT_External is begin for Dep of This.Latter.All_Dependencies loop @@ -470,7 +502,10 @@ package body Alire.Solutions.Diffs is if Changed then Table.Print (Level); + Warn_Toolchain_Download; Warn_Unsatisfiable_GNAT_External; + -- Only one of those two can happen and emit their warning, so the + -- order doesn't matter. else Trace.Log (Prefix & "No changes between former and new solution.", Level); diff --git a/src/alire/alire-solutions.adb b/src/alire/alire-solutions.adb index 3654acc3..d8e811e4 100644 --- a/src/alire/alire-solutions.adb +++ b/src/alire/alire-solutions.adb @@ -9,6 +9,7 @@ with Alire.Index; with Alire.Milestones; with Alire.Root; with Alire.Solutions.Diffs; +with Alire.Toolchains; with Alire.Utils.Tables; with Alire.Utils.Tools; with Alire.Utils.TTY; @@ -507,6 +508,27 @@ package body Alire.Solutions is -- TODO: instead of using the first discrepancy, we should count all -- differences and see which one is globally "newer". + -- Prefer one with an installed compiler + + for Rel_L of This.Releases loop + if Rel_L.Provides (GNAT_Crate) then + for Rel_R of Than.Releases loop + if Rel_R.Provides (GNAT_Crate) then + if Toolchains.Available.Contains (Rel_L) + xor Toolchains.Available.Contains (Rel_R) + then + return (if Toolchains.Available.Contains (Rel_L) + then Better + else Worse); + else + exit; -- No need to keep checking, 1 compiler in sol + end if; + end if; + end loop; + exit; -- No need to keep checking, only 1 compiler in sol + end if; + end loop; + -- Check releases in both only for Rel of This.Releases loop diff --git a/src/alire/alire-solver.adb b/src/alire/alire-solver.adb index 7adb6ac3..c82392ca 100644 --- a/src/alire/alire-solver.adb +++ b/src/alire/alire-solver.adb @@ -181,6 +181,18 @@ package body Alire.Solver is return Boolean is (Resolve (Deps, Props, Pins, Options).Is_Complete); + --------------------- + -- Culprit_Is_Toolchain -- + --------------------- + -- Say if the reason for the solution to be incomplete is that it requires + -- a tool from the toolchain that is not already installed + function Culprit_Is_Toolchain (Sol : Solutions.Solution) return Boolean is + begin + return + (for all Dep of Sol.Hints => + Toolchains.Tools.Contains (Dep.Crate)); + end Culprit_Is_Toolchain; + ------------- -- Resolve -- ------------- @@ -346,6 +358,36 @@ package body Alire.Solver is end if; end Ask_User_To_Continue; + ------------------------------ + -- Contains_All_Satisfiable -- + ------------------------------ + -- A solution may be incomplete but also may be only missing + -- impossible dependencies. In that case we can finish already, as + -- if the solution were complete. Otherwise, an e.g. missing crate + -- may force exploring all the combos of the rest of crates just + -- because it doesn't exist. + function Contains_All_Satisfiable + (Solution : Alire.Solutions.Solution) + return Boolean is + begin + for Crate of Solution.Crates loop + if Solution.State (Crate).Fulfilment in Missed | Hinted + -- So the dependency is not solved, but why? + and then + not Unavailable_Crates.Contains (Crate) + -- Because it does not exist at all, so "complete" + and then + not Unavailable_Direct_Deps.Contains + (Solution.Dependency (Crate)) + -- Because no release fulfills it, so "complete" + then + return False; + end if; + end loop; + + return True; + end Contains_All_Satisfiable; + ------------- -- Partial -- ------------- @@ -478,41 +520,52 @@ package body Alire.Solver is Trace.Debug ("SOLVER: gnat PASS " & Boolean' - (Specific_GNAT (St.Remaining).Value.Crate = R.Name)'Img + (R.Satisfies (Specific_GNAT (St.Remaining).Value))'Img & " for " & R.Milestone.TTY_Image & " due to compiler already in dependencies: " & Specific_GNAT (State.Remaining).Value.TTY_Image); - return Specific_GNAT (State.Remaining).Value.Crate = R.Name; + return R.Satisfies (Specific_GNAT (St.Remaining).Value); - elsif Toolchains.Tool_Is_Configured (GNAT_Crate) then + elsif Toolchains.Tool_Is_Configured (GNAT_Crate) + and then Options.Completeness = First_Complete + -- When we cannot find a complete solution in the first + -- completeness level, this means we need a compiler that + -- is not installed, and then we avoid this branch which + -- forces the selected compiler even if unavailable. + then -- There is a preferred compiler that we must use, as there -- is no overriding reason not to Trace.Debug ("SOLVER: gnat PASS " & Boolean' - (Toolchains - .Tool_Dependency (GNAT_Crate).Crate = R.Name)'Img + (R.Satisfies + (Toolchains.Tool_Dependency (GNAT_Crate)))'Img & " for " & R.Milestone.TTY_Image & " due to configured compiler: " & Toolchains.Tool_Dependency (GNAT_Crate).TTY_Image); - return Toolchains - .Tool_Dependency (GNAT_Crate).Crate = R.Name; + return R.Satisfies (Toolchains.Tool_Dependency (GNAT_Crate)); elsif Dep.Crate = GNAT_Crate then - -- For generic dependencies on gnat, we do not want to use - -- a compiler that is not already installed. + -- For generic dependencies on gnat, we do not want to use a + -- compiler that is not already installed, unless we failed + -- on the First_Complete level. Trace.Debug ("SOLVER: gnat PASS " & Boolean' - (Tools.Contains (R))'Image + (Tools.Contains (R) + or else Options.Completeness > First_Complete)'Image & " for " & R.Milestone.TTY_Image & " due to installed compiler availability."); - return Tools.Contains (R); + -- On first attempt we prefer only installed GNATs, but + -- we allow a not-installed available one if no complete + -- solution could be found otherwise. + return Tools.Contains (R) + or else Options.Completeness > First_Complete; else @@ -925,7 +978,8 @@ package body Alire.Solver is (R.Satisfies (Dep) and then (Dep.Crate /= GNAT_Crate or else - Tools.Contains (R))); + Tools.Contains (R) or else + Options.Completeness > First_Complete)); Check (R, Is_Reused => False); end Consider; @@ -1056,35 +1110,6 @@ package body Alire.Solver is -------------------- procedure Store_Finished (Solution : Alire.Solutions.Solution) is - - ------------------------------ - -- Contains_All_Satisfiable -- - ------------------------------ - -- A solution may be incomplete but also may be only missing - -- impossible dependencies. In that case we can finish already, as - -- if the solution were complete. Otherwise, an e.g. missing crate - -- may force exploring all the combos of the rest of crates just - -- because it doesn't exist. - function Contains_All_Satisfiable return Boolean is - begin - for Crate of Solution.Crates loop - if Solution.State (Crate).Fulfilment in Missed | Hinted - -- So the dependency is not solved, but why? - and then - not Unavailable_Crates.Contains (Crate) - -- Because it does not exist at all, so "complete" - and then - not Unavailable_Direct_Deps.Contains - (Solution.Dependency (Crate)) - -- Because no release fulfills it, so "complete" - then - return False; - end if; - end loop; - - return True; - end Contains_All_Satisfiable; - Pre_Length : constant Count_Type := Solutions.Length; begin Trace.Debug ("SOLVER: tree FULLY expanded as: " @@ -1103,7 +1128,7 @@ package body Alire.Solver is Progress_Report; -- As we found a new solution if Options.Completeness = First_Complete - and then Contains_All_Satisfiable + and then Contains_All_Satisfiable (Solution) then raise Solution_Found; -- break recursive search end if; @@ -1291,15 +1316,30 @@ package body Alire.Solver is -- on options, there must exist at least one incomplete solution, or we -- can retry with a larger solution space. - if Solutions.Is_Empty then - if Options.Completeness <= All_Complete then + if Solutions.Is_Empty + or else not Solutions.First_Element.Is_Complete + then + + -- Inform that no complete solution was found, only when the culprit + -- is not a tool from the toolchain (as that is expected when an + -- uninstalled compiler is needed). + + if Options.Completeness <= All_Complete + and then not Solutions.Is_Empty + and then not Culprit_Is_Toolchain (Solutions.First_Element) + then Put_Warning ("Spent " & TTY.Emph (Timer.Image) & " seconds " & "exploring complete solutions"); end if; + -- Now downgrade options to look for more solutions, if allowed and + -- if it makes sense. + if Options.Completeness < All_Incomplete and then Options.Exhaustive and then User_Answer_Continue /= No + and then (Solutions.Is_Empty or else + not Contains_All_Satisfiable (Solutions.First_Element)) then Trace.Detail ("No solution found with completeness policy of " @@ -1338,11 +1378,20 @@ package body Alire.Solver is elsif User_Answer_Continue = Always then Continue else Options.On_Timeout)))); - else + elsif Solutions.Is_Empty then raise Query_Unsuccessful with Errors.Set ("Solver failed to find any solution to fulfill dependencies " & "after " & Timer.Image); end if; + end if; + + -- In case of finding any solution, we always want to go through this + -- final step of marking transitivity and reporting: + + if Solutions.Is_Empty then + raise Query_Unsuccessful with Errors.Set + ("Solver failed to find any solution to fulfill dependencies " + & "after " & Timer.Image); else -- Mark direct/indirect dependencies post-hoc diff --git a/src/alire/alire-toolchains.adb b/src/alire/alire-toolchains.adb index afdb74a8..0c6388a1 100644 --- a/src/alire/alire-toolchains.adb +++ b/src/alire/alire-toolchains.adb @@ -22,6 +22,11 @@ with Semantic_Versioning.Extended; package body Alire.Toolchains is use type Ada.Containers.Count_Type; + use type Milestones.Milestone; + + use Directories.Operators; + + procedure Invalidate_Available_Cache; -------------- -- Any_Tool -- @@ -377,6 +382,8 @@ package body Alire.Toolchains is Install (Release); end loop; + Invalidate_Available_Cache; + end Assistant; ---------------------- @@ -405,6 +412,8 @@ package body Alire.Toolchains is (Level, Key => Tool_Key (Release.Name, For_Is_External), Value => not Release.Origin.Is_Index_Provided); + + Invalidate_Available_Cache; end Set_As_Default; ----------------------------- @@ -447,7 +456,9 @@ package body Alire.Toolchains is exception when E : Constraint_Error => Log_Exception (E); - Raise_Checked_Error ("Requested tool configured but not installed: " + Raise_Checked_Error ("Requested tool configured as " + & Tool_Milestone (Crate).TTY_Image + & " but not installed: " & Utils.TTY.Name (Crate)); end Tool_Release; @@ -476,11 +487,20 @@ package body Alire.Toolchains is end if; end; end if; + + Invalidate_Available_Cache; end Unconfigure; - use Directories.Operators; + Available_Cached : Releases.Containers.Release_Set; - use type Milestones.Milestone; + -------------------------------- + -- Invalidate_Available_Cache -- + -------------------------------- + + procedure Invalidate_Available_Cache is + begin + Available_Cached.Clear; + end Invalidate_Available_Cache; --------------- -- Available -- @@ -524,6 +544,13 @@ package body Alire.Toolchains is end Detect; begin + -- Early exit with cached available toolchains. Looking for toolchains + -- on disk is expensive. We invalidate the cache on toolchain changes. + + if not Available_Cached.Is_Empty then + return Available_Cached; + end if; + if Ada.Directories.Exists (Path) then Directories.Traverse_Tree (Start => Path, @@ -546,6 +573,7 @@ package body Alire.Toolchains is end loop; end loop; + Available_Cached := Result; return Result; end Available; @@ -632,6 +660,8 @@ package body Alire.Toolchains is Fail_If_Unset => False); Toolchains.Unconfigure (Release.Name, Config.Local, Fail_If_Unset => False); + + Invalidate_Available_Cache; end if; if not Confirm or else Query diff --git a/testsuite/fixtures/toolchain_index/cr/crate_non_virt/crate_non_virt-1.0.0.toml b/testsuite/fixtures/toolchain_index/cr/crate_non_virt/crate_non_virt-1.0.0.toml new file mode 100644 index 00000000..3d2c273f --- /dev/null +++ b/testsuite/fixtures/toolchain_index/cr/crate_non_virt/crate_non_virt-1.0.0.toml @@ -0,0 +1,9 @@ +description = "A crate with no providers elsewhere" +name = "crate_non_virt" +version = "1.0.0" +maintainers = ["alejandro@mosteo.com"] +maintainers-logins = ["mylogin"] + +[origin] +url = "file:../../../crates/libhello_1.0.0.tgz" +hashes = ["sha512:99fa3a55540d0655c87605b54af732f76a8a363015f183b06e98aa91e54c0e69397872718c5c16f436dd6de0fba506dc50c66d34a0e5c61fb63cb01fa22f35ac"] diff --git a/testsuite/fixtures/toolchain_index/gn/gnat_external/gnat_external-external.toml b/testsuite/fixtures/toolchain_index/gn/gnat_external/gnat_external-external.toml index f1171a6b..df9d9fd5 100644 --- a/testsuite/fixtures/toolchain_index/gn/gnat_external/gnat_external-external.toml +++ b/testsuite/fixtures/toolchain_index/gn/gnat_external/gnat_external-external.toml @@ -6,7 +6,7 @@ maintainers-logins = ["mosteo"] [[external]] kind = "version-output" -# We look for make instead that should be always installed. -version-command = ["make", "--version"] -version-regexp = ".*Make ([\\d\\.]+).*" +# We mock a 3.3.3 version as external compiler +version-command = ["echo", "3.3.3"] +version-regexp = "([\\d\\.]+).*" provides = "gnat" diff --git a/testsuite/tests/build/hashes/compiler-input/test.py b/testsuite/tests/build/hashes/compiler-input/test.py index 14a7f4c9..a22231b4 100644 --- a/testsuite/tests/build/hashes/compiler-input/test.py +++ b/testsuite/tests/build/hashes/compiler-input/test.py @@ -9,12 +9,14 @@ from drivers.asserts import assert_match from drivers.builds import clear_builds_dir, hash_input from drivers import builds +target_crate = "crate_non_virt" + def check_hash(signature: str) -> None: """ Check that the given signature is present in the hash inputs """ - assert_match(f".*{signature}.*", hash_input("crate_real")) + assert_match(f".*{signature}.*", hash_input(target_crate)) # The first test is to check that the external compiler is used when no @@ -25,7 +27,7 @@ run_alr("toolchain", "--disable-assistant") # Init a crate without explicit compiler dependency init_local_crate("xxx") -alr_with("crate_real") # A regular crate in the index +alr_with(target_crate) # A regular crate in the index builds.sync() # Ensure the hash inputs are written to disk # Check the external compiler is in the hash inputs @@ -47,8 +49,8 @@ check_hash("version:gnat_native=8888.0.0") # Next, check with an explicit compiler in the dependencies. Note that we give # the virtual dependency, but the actual native one is used for the hash. -clear_builds_dir() alr_with("gnat=7777") # Downgrade the compiler with an explicit dependency +clear_builds_dir() builds.sync() # Check the expected compiler is in the hash inputs diff --git a/testsuite/tests/solver/compiler-installed/test.py b/testsuite/tests/solver/compiler-installed/test.py index e5c16fca..260de5ff 100644 --- a/testsuite/tests/solver/compiler-installed/test.py +++ b/testsuite/tests/solver/compiler-installed/test.py @@ -1,6 +1,6 @@ """ -Check that, for generic gnat dependencies, no compilers are installed (only a -locally available one is used). +Check that, for generic gnat dependencies, uninstalled compilers are used only +as last resort, and a warning is shown """ import re @@ -18,10 +18,9 @@ assert_match(".*\n" # Headers "gnat_external.*Available.*Detected.*\n", p.out) -# Capture version -version = re.search("gnat_external ([0-9.]+)", p.out, re.MULTILINE).group(1) +# We know the external version of the compiler (3.3.3) +version = "3.3.3" -print(version) # When no compiler is selected, since the external one is available, it should # be used before offering to download a new compiler. @@ -33,13 +32,17 @@ alr_with("gnat") match_solution(f"gnat={version} (gnat_external)", escape=True) # Check that requesting a version different to the one externally available -# results in missing compiler, as Alire won't try to install one. +# results in a complete solution but with installation warning alr_with("gnat", delete=True, manual=False) -alr_with(f"gnat/={version}") -match_solution(f"gnat/={version} (direct,hinted)", escape=True) -# Hinted because we know the crate exists as external - -# Now, if the user installs a cross compiler, it will be used +p = run_alr("with", f"gnat/={version}", quiet=False) +assert_match(".*solution requires a toolchain", p.out) +match_solution(f"gnat=8888.0.0 (gnat_native) (origin: binary_archive)", + escape=True) + +# Now, if the user installs a cross compiler, it will be used in preference to +# the 8888 newer one, because it's installed (but we need to uninstall the 8888 +# one first) +run_alr("toolchain", "--uninstall", "gnat_native=8888") run_alr("toolchain", "--install", "gnat_cross_2") run_alr("update") diff --git a/testsuite/tests/solver/compiler-priorities/test.py b/testsuite/tests/solver/compiler-priorities/test.py index 9b469d60..d9ee27fd 100644 --- a/testsuite/tests/solver/compiler-priorities/test.py +++ b/testsuite/tests/solver/compiler-priorities/test.py @@ -4,10 +4,10 @@ Check compiler priorities in the solver. These priorities are: - An externally available compiler - Newest installed native compiler - Newest installed cross-compiler - - Newest uninstalled native compiler - - Newest uninstalled cross-compiler -Generic dependencies on gnat= never cause compiler installation. Those only -match installed or externally available compilers. + - Newest uninstalled explicit native compiler + - Newest uninstalled explicit cross-compiler +Generic dependencies on gnat= cause compiler installation as a last resort. +Those prefer first a native compiler. This case is out of scope of this test. """ import subprocess diff --git a/testsuite/tests/solver/forbids/test.py b/testsuite/tests/solver/forbids/test.py index 88103f3e..c5d0c17a 100644 --- a/testsuite/tests/solver/forbids/test.py +++ b/testsuite/tests/solver/forbids/test.py @@ -12,7 +12,7 @@ from re import escape as e # This test relies on three crates in the toolchain_index: # crate_conflict=1.2.3 conflicts with crate_lone* and crate_virtual* # crate_lone is a regular crate -# crate_virtual has no releases, but is provided by crate_conflict_1 +# crate_virtual has no releases, but is provided by crate_conflict=1.2.3 # Crate conflict cannot appear with any of the others in a solution, because of # its [forbids] table. diff --git a/testsuite/tests/toolchain/select-dontmix/test.py b/testsuite/tests/toolchain/select-dontmix/test.py index 2a8dd21b..74c55564 100644 --- a/testsuite/tests/toolchain/select-dontmix/test.py +++ b/testsuite/tests/toolchain/select-dontmix/test.py @@ -13,7 +13,6 @@ ver = re.search("gnat_external ([0-9.]+)", p.out, re.MULTILINE).group(1) # First, see that trying to use a native with an external is reported p = run_alr("toolchain", "--select", "gnat_external", "gprbuild=8888", complain_on_error=False) -assert p.status != 0, "Expected error didn't happend" assert_match(".*Use --force to override compatibility checks.*", p.out) # Same thing works if forced @@ -29,7 +28,6 @@ assert_match(f".*gnat_external.*{ver}.*Default.*", p.out) # Likewise, picking first a native and requesting a second external tool fails p = run_alr("toolchain", "--select", "gnat_native", "gprbuild/=8888", complain_on_error=False) -assert p.status != 0, "Expected error didn't happend" assert_match(".*Use --force to override compatibility checks.*", p.out) # But leaving free choice of gprbuild will result in the native being chosen -- 2.39.5