From 8c5dea22ab4de2568022859e17e98cd074311081 Mon Sep 17 00:00:00 2001 From: Alejandro R Mosteo Date: Wed, 24 Mar 2021 11:22:58 +0100 Subject: [PATCH] Transitively add pinned dirs (#713) * Store relative pins when given as such * Ensure pinned dirs use portable paths when relative * Adjustments in testsuite for new relative links * Recursively add links when linking crates * Compound relative paths when adding links * Restore inadvertently removed comment * Relax too restrictive checks In the case of linked releases, it doesn't matter if these contain expressions as we don't write them to disk. As for the spurious warning during loading of links, we may be transitively loading and the assumption on the current directory is faulty in that case. The check must be performed elsewhere. * Require approval for links to non-existent dirs * Show broken links in dependency image * Testsuite: new test and minimal tweaks * Code review fixes * Softlinks: report absolute project paths --- src/alire/alire-dependencies-states.ads | 8 +++ src/alire/alire-externals-softlinks.adb | 6 -- src/alire/alire-externals-softlinks.ads | 6 +- src/alire/alire-solutions.adb | 68 +++++++++++++++++++ src/alire/alire-solutions.ads | 20 ++---- src/alire/alire-utils-user_input.adb | 24 +++++++ src/alire/alire-utils-user_input.ads | 7 ++ src/alr/alr-commands-pin.adb | 5 ++ src/alr/alr-commands-withing.adb | 9 +++ .../my_index/crates/crate_1234/alire.lock | 2 - testsuite/tests/with/pin-transitive/test.py | 47 +++++++++++++ testsuite/tests/with/pin-transitive/test.yaml | 1 + testsuite/tests/with/tree-switch/test.py | 2 +- 13 files changed, 180 insertions(+), 25 deletions(-) delete mode 100644 testsuite/tests/printenv/linked-paths/my_index/crates/crate_1234/alire.lock create mode 100644 testsuite/tests/with/pin-transitive/test.py create mode 100644 testsuite/tests/with/pin-transitive/test.yaml diff --git a/src/alire/alire-dependencies-states.ads b/src/alire/alire-dependencies-states.ads index 77107574..874ac3fa 100644 --- a/src/alire/alire-dependencies-states.ads +++ b/src/alire/alire-dependencies-states.ads @@ -268,6 +268,10 @@ private & Utils.To_Lower_Case (This.Fulfilled.Fulfillment'Img) & (if This.Fulfilled.Fulfillment = Linked then ",pin=" & This.Fulfilled.Target.Get.Path + & (if GNAT.OS_Lib.Is_Directory + (This.Fulfilled.Target.Get.Path) + then "" + else "," & TTY.Error ("broken")) & (if This.Has_Release then ",release" else "") @@ -469,6 +473,10 @@ private & (if This.Fulfilled.Fulfillment = Linked then "," & TTY.Emph ("pin") & "=" & TTY.URL (This.Fulfilled.Target.Get.Path) + & (if GNAT.OS_Lib.Is_Directory + (This.Fulfilled.Target.Get.Path) + then "" + else "," & TTY.Error ("broken")) & (if This.Has_Release then "," & TTY.OK ("release") else "") diff --git a/src/alire/alire-externals-softlinks.adb b/src/alire/alire-externals-softlinks.adb index 39a69658..4974303b 100644 --- a/src/alire/alire-externals-softlinks.adb +++ b/src/alire/alire-externals-softlinks.adb @@ -1,5 +1,3 @@ -with Ada.Directories; - with Alire.URI; with Alire.Utils.TTY; @@ -47,10 +45,6 @@ package body Alire.Externals.Softlinks is declare Path : constant Any_Path := URI.Local_Path (From); begin - if not GNAT.OS_Lib.Is_Directory (Path) then - Trace.Warning ("Given path does not exist: " - & Utils.TTY.Emph (Path)); - end if; -- Store the path as a minimal relative path, so cloning a monorepo -- will work as-is, when originally given as a relative path diff --git a/src/alire/alire-externals-softlinks.ads b/src/alire/alire-externals-softlinks.ads index 78a6ecc3..4c35bdbb 100644 --- a/src/alire/alire-externals-softlinks.ads +++ b/src/alire/alire-externals-softlinks.ads @@ -1,3 +1,5 @@ +with Ada.Directories; + with Alire.Interfaces; with Alire.TOML_Adapters; private with Alire.VFS; @@ -89,6 +91,8 @@ private ------------------- function Project_Paths (This : External) return Utils.String_Vector - is (Utils.To_Vector (This.Path)); + is (Utils.To_Vector (Ada.Directories.Full_Name (This.Path))); + -- As the path may be relative, we make it absolute to avoid duplicates + -- with absolute paths reported by a Release.Project_Paths. end Alire.Externals.Softlinks; diff --git a/src/alire/alire-solutions.adb b/src/alire/alire-solutions.adb index 87f98d4d..6441eeaa 100644 --- a/src/alire/alire-solutions.adb +++ b/src/alire/alire-solutions.adb @@ -6,6 +6,8 @@ with Alire.Dependencies.Containers; with Alire.Dependencies.Diffs; with Alire.Dependencies.Graphs; with Alire.Index; +with Alire.OS_Lib; +with Alire.Roots.Optional; with Alire.Root; with Alire.Solutions.Diffs; with Alire.Utils.Tables; @@ -287,6 +289,72 @@ package body Alire.Solutions is end case; end Is_Better; + ------------- + -- Linking -- + ------------- + + function Linking (This : Solution; + Crate : Crate_Name; + Link : Externals.Softlinks.External) + return Solution + is + use Alire.OS_Lib.Operators; + + ---------- + -- Join -- + ---------- + + function Join (Parent, Child : Any_Path) return Any_Path + is (if Check_Absolute_Path (Child) + then Child + else Parent / Child); + + Linked_Root : constant Roots.Optional.Root := + Roots.Optional.Detect_Root (Link.Path); + begin + + -- Recursively find any other links + + return Result : Solution := (Solved => True, + Dependencies => + This.Dependencies.Including + (This.State (Crate).Linking (Link))) + do + if Linked_Root.Is_Valid and then Linked_Root.Value.Has_Lockfile then + declare + Linked_Solution : Solution renames Linked_Root.Value.Solution; + begin + + -- Go through any links in the linked release + + for Dep of Linked_Solution.Links loop + declare + + -- Create the new link for our own solution, composing + -- relative paths when possible. + + New_Link : constant Externals.Softlinks.External := + Externals.Softlinks.New_Softlink + (Join + (Parent => Link.Path, + Child => Linked_Solution.State + (Dep.Crate).Link.Path)); + begin + + -- We may or not already depend on the transitively + -- linked release. Just in case, we add the dependency + -- before the link. + + Result := Result.Depending_On (Dep) + .Linking (Crate => Dep.Crate, + Link => New_Link); + end; + end loop; + end; + end if; + end return; + end Linking; + ------------------ -- New_Solution -- ------------------ diff --git a/src/alire/alire-solutions.ads b/src/alire/alire-solutions.ads index e4f88c31..99d5076c 100644 --- a/src/alire/alire-solutions.ads +++ b/src/alire/alire-solutions.ads @@ -311,10 +311,12 @@ package Alire.Solutions is overriding function To_TOML (This : Solution) return TOML.TOML_Value with Pre => (for all Release of This.Releases => - Release.Dependencies.Is_Unconditional and then - Release.Properties.Is_Unconditional); + This.State (Release.Name).Is_Linked + or else (Release.Dependencies.Is_Unconditional + and then Release.Properties.Is_Unconditional)); -- Requires releases not to have dynamic expressions. This is currently - -- guaranteed by the states storing static versions of releases. + -- guaranteed by the states storing static versions of releases. We do not + -- store linked releases, so in that case it does not matter. --------------- -- Utilities -- @@ -451,18 +453,6 @@ private -- Linking -- ------------- - function Linking (This : Solution; - Crate : Crate_Name; - Link : Externals.Softlinks.External) - return Solution - is (Solved => True, - Dependencies => - This.Dependencies.Including (This.State (Crate).Linking (Link))); - - ------------- - -- Linking -- - ------------- - function Linking (This : Solution; Crate : Crate_Name; Path : Any_Path) diff --git a/src/alire/alire-utils-user_input.adb b/src/alire/alire-utils-user_input.adb index 9fdd8349..add7a6e6 100644 --- a/src/alire/alire-utils-user_input.adb +++ b/src/alire/alire-utils-user_input.adb @@ -1,6 +1,8 @@ with Ada.Text_IO; with Ada.Characters.Handling; +with GNAT.OS_Lib; + with Interfaces.C_Streams; with Alire.Utils.TTY; @@ -25,6 +27,28 @@ package body Alire.Utils.User_Input is when No => "no", when Always => "always"); + ----------------- + -- Approve_Dir -- + ----------------- + + function Approve_Dir (Dir : Any_Path; + Force : Boolean := Alire.Force) + return Boolean + is + begin + if not GNAT.OS_Lib.Is_Directory (Dir) then + return Query + (Question => TTY.Error (if TTY.Color_Enabled then "⚠" else "!") + & " Given path does not exist: " & TTY.URL (Dir) + & ASCII.LF & "Do you want to continue anyway?", + Valid => (Yes | No => True, others => False), + Default => (if Force then Yes else No)) + = Yes; + end if; + + return True; + end Approve_Dir; + ------------ -- Is_TTY -- ------------ diff --git a/src/alire/alire-utils-user_input.ads b/src/alire/alire-utils-user_input.ads index 2324de94..f225454c 100644 --- a/src/alire/alire-utils-user_input.ads +++ b/src/alire/alire-utils-user_input.ads @@ -76,4 +76,11 @@ package Alire.Utils.User_Input is -- True when the user answers positively. Defaults to Yes when the new -- solution is complete, or when Alire.Force. + function Approve_Dir (Dir : Any_Path; + Force : Boolean := Alire.Force) + return Boolean; + -- Some commands receive a path from the user (e.g., pinning). If such path + -- does not exist, we allow to continue only after user confirmation (or + -- forcing). Returns whether to proceed. + end Alire.Utils.User_Input; diff --git a/src/alr/alr-commands-pin.adb b/src/alr/alr-commands-pin.adb index bbf487bf..e3439414 100644 --- a/src/alr/alr-commands-pin.adb +++ b/src/alr/alr-commands-pin.adb @@ -177,6 +177,11 @@ package body Alr.Commands.Pin is -- Pin to dir + if not Alire.Utils.User_Input.Approve_Dir (Cmd.URL.all) then + Trace.Info ("Abandoned by user."); + return; + end if; + Cmd.Requires_Full_Index; -- Next statement recomputes a solution New_Sol := Alire.Pinning.Pin_To diff --git a/src/alr/alr-commands-withing.adb b/src/alr/alr-commands-withing.adb index 75473a6e..0e4c71f7 100644 --- a/src/alr/alr-commands-withing.adb +++ b/src/alr/alr-commands-withing.adb @@ -100,6 +100,15 @@ package body Alr.Commands.Withing is New_Dep : constant Alire.Dependencies.Dependency := Alire.Dependencies.From_Milestones (Requested); begin + -- Confirm target dir + + if not Alire.Utils.User_Input.Approve_Dir (Cmd.URL.all) then + Trace.Info ("Abandoned by user."); + return; + end if; + + -- Prepare new solution + declare use Alire; use type Conditional.Dependencies; diff --git a/testsuite/tests/printenv/linked-paths/my_index/crates/crate_1234/alire.lock b/testsuite/tests/printenv/linked-paths/my_index/crates/crate_1234/alire.lock deleted file mode 100644 index 0f075c0d..00000000 --- a/testsuite/tests/printenv/linked-paths/my_index/crates/crate_1234/alire.lock +++ /dev/null @@ -1,2 +0,0 @@ -[solution] -solved = true diff --git a/testsuite/tests/with/pin-transitive/test.py b/testsuite/tests/with/pin-transitive/test.py new file mode 100644 index 00000000..2037cba0 --- /dev/null +++ b/testsuite/tests/with/pin-transitive/test.py @@ -0,0 +1,47 @@ +""" +Check transitive links (a pinned dir that contains pinned dirs itself) +""" + +import os +import re + +from drivers.alr import run_alr, init_local_crate +from drivers.asserts import assert_eq + +# The test will create ./indirect, ./direct, and ./nest/base crates. +# Then they are pinned as base -> direct -> indirect. As "base" has a different +# relative path to "indirect" that "direct", this is also checked. + +init_local_crate(name="indirect", binary=False, enter=False) + +# Now create "direct" and pin to "indirect" +init_local_crate(name="direct", binary=False) +run_alr("with", "--use=../indirect") + +# Now create "base" and pin to "direct" +os.chdir("..") +os.mkdir("nest") +os.chdir("nest") +init_local_crate(name="base") +run_alr("with", "--use=../../direct") + +s = os.sep + +# Verify created pins +p = run_alr("pin") +assert_eq("direct file:.." + s + ".." + s + "direct \n" + "indirect file:.." + s + ".." + s + "indirect\n", + p.out) + +# Check pin removal +os.chdir("../../direct") +run_alr("with", "--del", "indirect") +os.chdir("../nest/base") +run_alr("update") + +p = run_alr("pin") +assert_eq("direct file:.." + s + ".." + s + "direct\n", + p.out) + + +print('SUCCESS') diff --git a/testsuite/tests/with/pin-transitive/test.yaml b/testsuite/tests/with/pin-transitive/test.yaml new file mode 100644 index 00000000..32c747b3 --- /dev/null +++ b/testsuite/tests/with/pin-transitive/test.yaml @@ -0,0 +1 @@ +driver: python-script diff --git a/testsuite/tests/with/tree-switch/test.py b/testsuite/tests/with/tree-switch/test.py index 4fa278be..2ea7ff88 100644 --- a/testsuite/tests/with/tree-switch/test.py +++ b/testsuite/tests/with/tree-switch/test.py @@ -21,7 +21,7 @@ run_alr('with', 'hello^1') run_alr('with', 'superhello') # Add more dependencies, without a proper release -run_alr('with', 'wip', '--use', '/fake') +run_alr('with', 'wip', '--use', '/fake', '--force') # force bc dir is missing run_alr('with', 'unobtanium', '--force') # Verify printout (but for test-dependent path) -- 2.39.5