From 67d7a8e065300e0b885d8885179d72d3ae6da06c Mon Sep 17 00:00:00 2001 From: "Alejandro R. Mosteo" Date: Fri, 14 Jun 2024 17:36:50 +0200 Subject: [PATCH] Fix bug in pin loading from out-of-root paths --- src/alire/alire-directories.adb | 14 ++++++--- src/alire/alire-lockfiles.adb | 10 +++++-- src/alire/alire-lockfiles.ads | 8 ++--- src/alire/alire-manifest.adb | 7 +++-- src/alire/alire-manifest.ads | 9 ++++-- src/alire/alire-publish.adb | 18 +++++++---- src/alire/alire-releases.adb | 10 ++++++- src/alire/alire-releases.ads | 8 +++-- src/alire/alire-roots-optional.adb | 8 +++-- src/alire/alire-roots.adb | 28 +++++++++++++---- src/alire/alire-roots.ads | 3 +- src/alire/alire-user_pins.adb | 6 ++++ src/alire/alire-version.ads | 2 +- src/alr/alr-commands.adb | 2 +- testsuite/tests/printenv/out-of-root/test.py | 30 +++++++++++++++++++ .../tests/printenv/out-of-root/test.yaml | 4 +++ 16 files changed, 132 insertions(+), 35 deletions(-) create mode 100644 testsuite/tests/printenv/out-of-root/test.py create mode 100644 testsuite/tests/printenv/out-of-root/test.yaml diff --git a/src/alire/alire-directories.adb b/src/alire/alire-directories.adb index 4c8142ce6..7db13a7eb 100644 --- a/src/alire/alire-directories.adb +++ b/src/alire/alire-directories.adb @@ -641,10 +641,16 @@ package body Alire.Directories is function Temp_Name (Length : Positive := 8) return String is Result : String (1 .. Length + 4); begin - Result (1 .. 4) := "alr-"; - Result (Length + 1 .. Result'Last) := ".tmp"; - Tempfile_Support.Next_Name (Result (5 .. Length)); - return Result; + + Char_Random.Reset (Gen, To_Integer (Seed)); + + return Result : String (1 .. Length + 4) do + Result (1 .. 4) := "alr-"; + Result (Length + 1 .. Result'Last) := ".tmp"; + for I in 5 .. Length loop + Result (I) := Char_Random.Random (Gen); + end loop; + end return; end Temp_Name; ---------------- diff --git a/src/alire/alire-lockfiles.adb b/src/alire/alire-lockfiles.adb index 20f60109a..3982d1762 100644 --- a/src/alire/alire-lockfiles.adb +++ b/src/alire/alire-lockfiles.adb @@ -57,7 +57,11 @@ package body Alire.Lockfiles is -- Read -- ---------- - function Read (Filename : Absolute_Path) return Contents is + function Read (Root, Filename : Absolute_Path) return Contents is + -- Enter the workspace root for this lockfile, so any + -- relative pin paths can be properly resolved. + use Alire.Directories; + CWD : Guard (Enter (Root)) with Unreferenced; begin Trace.Debug ("Reading persistent contents from " & Filename); @@ -93,7 +97,7 @@ package body Alire.Lockfiles is -- Validity -- -------------- - function Validity (File : Any_Path) return Validities is + function Validity (Root, File : Absolute_Path) return Validities is begin if not GNAT.OS_Lib.Is_Read_Accessible_File (File) then return Missing; @@ -102,7 +106,7 @@ package body Alire.Lockfiles is -- Try to load to assess validity declare - Unused : constant Contents := Read (File); + Unused : constant Contents := Read (Root, File); begin return Valid; end; diff --git a/src/alire/alire-lockfiles.ads b/src/alire/alire-lockfiles.ads index 01a219d62..37dde7730 100644 --- a/src/alire/alire-lockfiles.ads +++ b/src/alire/alire-lockfiles.ads @@ -27,11 +27,11 @@ package Alire.Lockfiles is -- Return the location /path/to/crate/dir/alire.lock, filename included, -- given the root directory where the crate is deployed. - function Read (Filename : Absolute_Path) return Contents; - -- Read contents from the given lockfile + function Read (Root, Filename : Absolute_Path) return Contents; + -- Read contents from the given lockfile, for a crate rooted at Root - function Validity (File : Any_Path) return Validities; - -- Check if given file is a valid lockfile + function Validity (Root, File : Absolute_Path) return Validities; + -- Check if given file is a valid lockfile, for a crate at Root procedure Write (Contents : Lockfiles.Contents; Filename : Absolute_Path); diff --git a/src/alire/alire-manifest.adb b/src/alire/alire-manifest.adb index 3a229294e..19a77fc2b 100644 --- a/src/alire/alire-manifest.adb +++ b/src/alire/alire-manifest.adb @@ -53,11 +53,14 @@ package body Alire.Manifest is -- Is_Valid -- -------------- - function Is_Valid (Name : Any_Path; Source : Sources) return Boolean is + function Is_Valid (Name : Any_Path; + Source : Sources; + Root : Any_Path := "") + return Boolean is begin -- Check we are able to load the manifest file if Releases.From_Manifest - (Name, Source, Strict => False).Version_Image /= "" + (Name, Source, Strict => False, Root_Path => Root).Version_Image /= "" then Trace.Debug ("Checked valid manifest at " & Name); return True; diff --git a/src/alire/alire-manifest.ads b/src/alire/alire-manifest.ads index c7c1f7699..8709195e0 100644 --- a/src/alire/alire-manifest.ads +++ b/src/alire/alire-manifest.ads @@ -27,7 +27,12 @@ package Alire.Manifest is -- As removal of dependencies, but for pins. If the pin is not found, or -- it cannot be safely removed, it will raise. - function Is_Valid (Name : Any_Path; Source : Sources) return Boolean; - -- Check that the given Name is a loadable manifest + function Is_Valid (Name : Any_Path; + Source : Sources; + Root : Any_Path := "") + return Boolean + with Pre => Source /= Local or else Check_Absolute_Path (Root); + -- Check that the given Name is a loadable manifest. For a local manifest + -- that may contain links, we need the root path. end Alire.Manifest; diff --git a/src/alire/alire-publish.adb b/src/alire/alire-publish.adb index 0edcc4b2f..907b751fa 100644 --- a/src/alire/alire-publish.adb +++ b/src/alire/alire-publish.adb @@ -41,6 +41,7 @@ with TOML_Slicer; package body Alire.Publish is + package Adirs renames Ada.Directories; package Semver renames Semantic_Versioning; use all type UString; @@ -390,7 +391,8 @@ package body Alire.Publish is Check_Release (Releases.From_Manifest (Starting_Manifest (Context), Alire.Manifest.Local, - Strict => True)); + Strict => True, + Root_Path => Adirs.Full_Name (+Context.Path))); -- Will have raised if the release is not loadable or incomplete else declare @@ -612,7 +614,8 @@ package body Alire.Publish is procedure Prepare_Archive (Context : in out Data) with Pre => Alire.Manifest.Is_Valid (Context.Options.Manifest, - Alire.Manifest.Local); + Alire.Manifest.Local, + Adirs.Full_Name (+Context.Path)); -- Prepare a tar file either using git archive (if git repo detected) or -- plain tar otherwise. @@ -622,9 +625,11 @@ package body Alire.Publish is Target_Dir : constant Relative_Path := Paths.Working_Folder_Inside_Root / "archives"; Release : constant Releases.Release := - Releases.From_Manifest (Context.Options.Manifest, - Alire.Manifest.Local, - Strict => True); + Releases.From_Manifest + (Context.Options.Manifest, + Alire.Manifest.Local, + Strict => True, + Root_Path => Adirs.Full_Name (+Context.Path)); Milestone : constant String := TOML_Index.Manifest_File (Release.Name, Release.Version, @@ -788,7 +793,8 @@ package body Alire.Publish is .From_Manifest (Packaged_Manifest (Context), Alire.Manifest.Local, - Strict => True) + Strict => True, + Root_Path => Adirs.Full_Name (+Context.Path)) .Replacing (Origin => Context.Origin); begin Check_Release (Release); diff --git a/src/alire/alire-releases.adb b/src/alire/alire-releases.adb index f616c4e0d..9751795c4 100644 --- a/src/alire/alire-releases.adb +++ b/src/alire/alire-releases.adb @@ -966,9 +966,17 @@ package body Alire.Releases is function From_Manifest (File_Name : Any_Path; Source : Manifest.Sources; - Strict : Boolean) + Strict : Boolean; + Root_Path : Any_Path := "") return Release is + -- Move to file base dir, as relative paths in pins are resolved during + -- loading relative to CWD. + CWD : Directories.Guard + (if Root_Path /= "" then + Directories.Enter (Root_Path) + else + Directories.Stay) with Unreferenced; begin return From_TOML (TOML_Adapters.From diff --git a/src/alire/alire-releases.ads b/src/alire/alire-releases.ads index bf679b7fe..f0ed1a32a 100644 --- a/src/alire/alire-releases.ads +++ b/src/alire/alire-releases.ads @@ -323,8 +323,12 @@ package Alire.Releases is function From_Manifest (File_Name : Any_Path; Source : Manifest.Sources; - Strict : Boolean) - return Release; + Strict : Boolean; + Root_Path : Any_Path := "") + return Release + with Pre => Source in Manifest.Index or else Root_Path in Absolute_Path; + -- When loading a manifest for a workspace, it may contain pins that we + -- must resolve relative to Root_Path. function From_TOML (From : TOML_Adapters.Key_Queue; Source : Manifest.Sources; diff --git a/src/alire/alire-roots-optional.adb b/src/alire/alire-roots-optional.adb index 0d3134e41..84d27ba78 100644 --- a/src/alire/alire-roots-optional.adb +++ b/src/alire/alire-roots-optional.adb @@ -51,9 +51,11 @@ package body Alire.Roots.Optional is return This : constant Root := Outcome_Success (Roots.New_Root - (R => Releases.From_Manifest (Crate_File, - Manifest.Local, - Strict => True), + (R => Releases.From_Manifest + (Crate_File, + Manifest.Local, + Strict => True, + Root_Path => Directories.Current), Path => Directories.Current, Env => Alire.Root.Platform_Properties)) do diff --git a/src/alire/alire-roots.adb b/src/alire/alire-roots.adb index be9ea1d43..5b60531da 100644 --- a/src/alire/alire-roots.adb +++ b/src/alire/alire-roots.adb @@ -1177,7 +1177,9 @@ package body Alire.Roots is return "Expected ordinary manifest file but found a: " & Kind (This.Crate_File)'Img; - elsif not Alire.Manifest.Is_Valid (This.Crate_File, Alire.Manifest.Local) + elsif not Alire.Manifest.Is_Valid (This.Crate_File, + Alire.Manifest.Local, + Path (This)) then return "Manifest is not loadable: " & This.Crate_File; end if; @@ -1201,6 +1203,11 @@ package body Alire.Roots is ------------------------------ procedure Export_Build_Environment (This : in out Root) is + CWD : Directories.Guard (Directories.Enter (Path (This))) + with Unreferenced; + -- Required as this function gets called sometimes directly from + -- commands that may not have relocated to the crate root. + Context : Alire.Environment.Context; begin Alire.Environment.Loading.Load (Context, This); @@ -1342,6 +1349,15 @@ package body Alire.Roots is function Solution (This : in out Root) return Solutions.Solution is + -- Enter the lockfile parent dir, which will be the crate root, so any + -- relative pin paths can be properly resolved, if the lockfile is not + -- yet loaded. + use Alire.Directories; + CWD : Guard (if This.Cached_Solution.Has_Element + then Stay + else Enter (Parent (Parent (This.Lock_File)))) + with Unreferenced; + Result : constant Cached_Solutions.Cached_Info := This.Cached_Solution.Element (This.Lock_File); begin @@ -1650,9 +1666,10 @@ package body Alire.Roots is -- speeds up things greatly and both should be in sync if things -- are as they should. or else - (if Check_Valid - then Lockfiles.Validity (This.Lock_File) in Lockfiles.Valid - else Ada.Directories.Exists (This.Lock_File))); + (if Check_Valid then + Lockfiles.Validity (Path (This), This.Lock_File) in Lockfiles.Valid + else + Ada.Directories.Exists (This.Lock_File))); -------------------------- -- Is_Lockfile_Outdated -- @@ -2023,7 +2040,8 @@ package body Alire.Roots is (Releases.From_Manifest (This.Crate_File, Manifest.Local, - Strict => True)); + Strict => True, + Root_Path => Path (This))); -- And our pins diff --git a/src/alire/alire-roots.ads b/src/alire/alire-roots.ads index 47d79308d..65ec0d097 100644 --- a/src/alire/alire-roots.ads +++ b/src/alire/alire-roots.ads @@ -374,7 +374,8 @@ private -- triggered when doing This.Configuration here. function Load_Solution (Lockfile : Absolute_Path) return Solutions.Solution - is (Lockfiles.Read (Lockfile).Solution); + is (Lockfiles.Read (Directories.Current, Lockfile).Solution); + -- Note that this function requires CWD to already be the crate root procedure Write_Solution (Solution : Solutions.Solution; Lockfile : Absolute_Path); diff --git a/src/alire/alire-user_pins.adb b/src/alire/alire-user_pins.adb index ea0c9ffc3..adf22e83d 100644 --- a/src/alire/alire-user_pins.adb +++ b/src/alire/alire-user_pins.adb @@ -408,6 +408,12 @@ package body Alire.User_Pins is Result.Path := +Utils.User_Input.To_Absolute_From_Portable (This.Checked_Pop (Keys.Path, TOML_String).As_String); + + if not GNAT.OS_Lib.Is_Directory (+Result.Path) then + This.Recoverable_Error + ("Pin path is not a valid directory: " + & (+Result.Path)); + end if; end return; end if; end From_Lockfile; diff --git a/src/alire/alire-version.ads b/src/alire/alire-version.ads index 441a826dc..a9112dc84 100644 --- a/src/alire/alire-version.ads +++ b/src/alire/alire-version.ads @@ -16,7 +16,7 @@ private -- be replaced by `alr build` with the current commit, and appended with -- "_or_later" after build. - Current_Str : constant String := "2.1-dev"; + Current_Str : constant String := "2.1-dev+bd25511f_or_later"; -- 2.0.0: alr settings refactor and minor fixes -- 2.0.0-rc1: release candidate for 2.0 -- 2.0.0-b1: first public release on the 2.0 branch diff --git a/src/alr/alr-commands.adb b/src/alr/alr-commands.adb index 43a50e3fa..9ce783564 100644 --- a/src/alr/alr-commands.adb +++ b/src/alr/alr-commands.adb @@ -363,7 +363,7 @@ package body Alr.Commands is -- For workspaces created pre-lockfiles, or with older format, -- recreate: - case Lockfiles.Validity (Checked.Lock_File) is + case Lockfiles.Validity (Checked.Path, Checked.Lock_File) is when Lockfiles.Valid => Trace.Debug ("Lockfile at " & Checked.Lock_File & " is valid"); diff --git a/testsuite/tests/printenv/out-of-root/test.py b/testsuite/tests/printenv/out-of-root/test.py new file mode 100644 index 000000000..6edc36078 --- /dev/null +++ b/testsuite/tests/printenv/out-of-root/test.py @@ -0,0 +1,30 @@ +""" +Verify that printenv output is correct for regular and pinned crates when +invoked not from the root of the workspace. +""" + +import os +import re +from drivers.alr import alr_pin, init_local_crate, run_alr +from drivers.asserts import assert_eq, assert_match + +parent = os.getcwd() + +init_local_crate("base") +init_local_crate("pinned", enter=False) +alr_pin("pinned", path="pinned") + +os.chdir("src") # Enter a subfolder +p = run_alr("printenv") + +# Verify root crate proper path in GPR_PROJECT_PATH +assert_match(r".*GPR_PROJECT_PATH[^\n]+" + + re.escape(os.path.join(parent, "base")) + + "(:|\")", p.out) + +# Verify pinned crate proper path in GPR_PROJECT_PATH +assert_match(r".*GPR_PROJECT_PATH[^\n]+" + + re.escape(os.path.join(parent, "base", "pinned")) + + "(:|\")", p.out) + +print("SUCCESS") diff --git a/testsuite/tests/printenv/out-of-root/test.yaml b/testsuite/tests/printenv/out-of-root/test.yaml new file mode 100644 index 000000000..702010525 --- /dev/null +++ b/testsuite/tests/printenv/out-of-root/test.yaml @@ -0,0 +1,4 @@ +driver: python-script +build_mode: both +indexes: + compiler_only_index: {}