From dcc53a8caab8dbeb38b4b48b24271609bbb29981 Mon Sep 17 00:00:00 2001 From: Alejandro R Mosteo Date: Thu, 20 Feb 2025 13:33:57 +0100 Subject: [PATCH] feat: allow pinning a crate in a subdir of a repo (#1857) * Load monorepo pin from manifest * Add --subdir to `alr with` * Add `--subdir` to `alr pin` * Test * Self-review * User changes * Explicity typing in specs --- BREAKING.md | 1 + doc/catalog-format-spec.md | 5 +- doc/user-changes.md | 13 ++++ src/alire/alire-roots-editable.adb | 24 ++++-- src/alire/alire-roots-editable.ads | 5 +- src/alire/alire-user_pins.adb | 59 +++++++++++++-- src/alire/alire-user_pins.ads | 24 +++++- src/alire/alire.ads | 4 +- src/alr/alr-commands-pin.adb | 18 ++++- src/alr/alr-commands-pin.ads | 4 +- src/alr/alr-commands-withing.adb | 16 +++- src/alr/alr-commands-withing.ads | 4 +- testsuite/tests/pin/remote-subdir/test.py | 82 +++++++++++++++++++++ testsuite/tests/pin/remote-subdir/test.yaml | 4 + 14 files changed, 239 insertions(+), 24 deletions(-) create mode 100644 testsuite/tests/pin/remote-subdir/test.py create mode 100644 testsuite/tests/pin/remote-subdir/test.yaml diff --git a/BREAKING.md b/BREAKING.md index 70818fd0..6af4fb20 100644 --- a/BREAKING.md +++ b/BREAKING.md @@ -8,6 +8,7 @@ Log of breaking changes in index or alr. ### alr 2.1.0 + index 1.4.0 - index: git remotes in origins are recognized even without `git+` prefix +- manifest: remote pins accept a new `subdir` field to specify a subdirectory ### alr 2.0.0 + index 1.3.0 diff --git a/doc/catalog-format-spec.md b/doc/catalog-format-spec.md index 421cbf81..d426e3cf 100644 --- a/doc/catalog-format-spec.md +++ b/doc/catalog-format-spec.md @@ -759,11 +759,14 @@ The specific pin kinds and their attributes are: * Pins to git repositories: the repository will be cloned locally and its directory will be used as in the previous case. This pin may optionally include a commit to fix the checkout to be used, or a branch to track. Otherwise, the default branch will be used. Running `alr update` will refresh the checkout. - * `url`: the URL of a git repository + * `url`: the URL of a git repository. * `commit` (optional): a complete git commit hash. + * `branch` (optional, mutually exclusive with commit): a branch to track on `alr update`. + * `subdir`: (optional): relative path that indicates where the crate is located when not at the repository root. * `crate_name = { url = "https://my/repo.git" } # Updatable pin to default branch` * `crate_name = { url = "https://my/repo.git", branch="feature" } # Updatable pin` * `crate_name = { url = "https://my/repo.git", commit="abcdef..." } # Fixed pin` + * `crate_name = { url = "https://my/repo.git", subdir="mycrate"} # Crate located in a subdirectory` ### Using pins for crate testing diff --git a/doc/user-changes.md b/doc/user-changes.md index 0b37a415..93e5a566 100644 --- a/doc/user-changes.md +++ b/doc/user-changes.md @@ -6,6 +6,19 @@ stay on top of `alr` new features. ## Release `2.1` +### Allow pinning a crate in a subdirectory of a git repository + +PR [#1857](https://github.com/alire-project/alire/pull/1857) + +Until now, monorepos were supported in origins of indexed crates but not in user +pins. A workaround was to manually clone a repository and pin the appropriate +local path. This can now be achieved entirely within Alire, e.g.: + +`alr with --use=https://github.com/myuser/mymonorepo --subdir=mycrate` + +This way, `alr update` works as expected and it removes the need to manually +update these repositories. + ### Configurable trusted sites list for Git repositories PR [#1819](https://github.com/alire-project/alire/pull/1819) diff --git a/src/alire/alire-roots-editable.adb b/src/alire/alire-roots-editable.adb index ab8817ea..1cd78e6e 100644 --- a/src/alire/alire-roots-editable.adb +++ b/src/alire/alire-roots-editable.adb @@ -372,7 +372,8 @@ package body Alire.Roots.Editable is Crate : Alire.Optional.Crate_Name; Origin : URL; Ref : String := ""; - Branch : String := "") + Branch : String := ""; + Subdir : Relative_Path := "") is --------------------------- @@ -387,7 +388,11 @@ package body Alire.Roots.Editable is if Commit /= "" then Put_Info ("Using commit " & TTY.Emph (Commit) & " for reference " & TTY.Emph (Ref)); - This.Add_Remote_Pin (Crate, Origin, Commit, Branch); + This.Add_Remote_Pin (Crate, + Origin, + Ref => Commit, + Branch => Branch, + Subdir => Subdir); else Raise_Checked_Error ("Requested remote reference " & TTY.Emph (Ref) @@ -427,14 +432,22 @@ package body Alire.Roots.Editable is -- We can proceed as if it where a local pin now declare + use Directories.Operators; + + Crate_Path : constant Absolute_Path := + (if Subdir /= "" + then Temp_Pin.Filename / Subdir + else Temp_Pin.Filename); + Crate : constant Crate_Name := Add_Pin_Preparations (This, Add_Remote_Pin.Crate, - Temp_Pin.Filename); + Crate_Path); New_Pin : User_Pins.Pin := User_Pins.New_Remote (URL => Origin, Commit => Ref, - Branch => Branch); + Branch => Branch, + Subdir => Subdir); Destination : constant Absolute_Path := New_Pin.Deploy_Path (Crate, This.Edit.Pins_Dir); @@ -462,7 +475,8 @@ package body Alire.Roots.Editable is Crate, User_Pins.New_Remote (URL => Origin, Commit => Ref, - Branch => Branch)); + Branch => Branch, + Subdir => Subdir)); This.Reload_Manifest; -- And update lockfile. We need to call Deploy on the pin (although diff --git a/src/alire/alire-roots-editable.ads b/src/alire/alire-roots-editable.ads index 8fea2cfb..e685d976 100644 --- a/src/alire/alire-roots-editable.ads +++ b/src/alire/alire-roots-editable.ads @@ -99,7 +99,8 @@ package Alire.Roots.Editable is Crate : Optional.Crate_Name; Origin : URL; Ref : String := ""; - Branch : String := "") + Branch : String := ""; + Subdir : Relative_Path := "") with Pre => (not (Ref /= "" and then Branch /= "") or else raise Checked_Error with @@ -109,7 +110,7 @@ package Alire.Roots.Editable is or else This.Can_Be_Pinned (Crate.Element)); -- Add a pin to a remote repo, with optional Ref xor Branch. If Ref is -- not a Commit, it will be converted to one using `git ls-remote`. If - -- Crate.Is_Empty then Path must point to an Alire workspace for which it + -- Crate.Is_Empty then Origin must point to an Alire workspace for which it -- can be deduced. If Crate.Has_Element, the crates should match. If the -- root does not depend already on the crate, a dependency will be added. diff --git a/src/alire/alire-user_pins.adb b/src/alire/alire-user_pins.adb index cca4c3b6..5fab51ee 100644 --- a/src/alire/alire-user_pins.adb +++ b/src/alire/alire-user_pins.adb @@ -23,6 +23,7 @@ package body Alire.User_Pins is Commit : constant String := "commit"; Internal : constant String := "lockfiled"; Path : constant String := "path"; + Subdir : constant String := "subdir"; URL : constant String := "url"; Version : constant String := "version"; end Keys; @@ -47,14 +48,16 @@ package body Alire.User_Pins is -- New_Remote -- ---------------- - function New_Remote (URL : Alire.URL; + function New_Remote (URL : Alire.URL; Commit : String := ""; - Branch : String := "") + Branch : String := ""; + Subdir : Alire.Relative_Path := "") return Pin is (Kind => To_Git, URL => +URL, Commit => +Commit, Branch => +Branch, + Subdir => +Subdir, Checkout_Path => <>); ----------- @@ -93,11 +96,15 @@ package body Alire.User_Pins is "path='" & VFS.Attempt_Portable (Path (This)) & "'", when To_Git => "url='" & (+This.URL) & "'" + & (if This.Subdir /= "" + then ", subdir='" & (+This.Subdir) & "'" + else "") & (if This.Branch /= "" then ", branch='" & (+This.Branch) & "'" elsif This.Commit /= "" then ", commit='" & (+This.Commit) & "'" - else "")) + else "") + ) & " }"); --------------- @@ -348,7 +355,7 @@ package body Alire.User_Pins is declare Root : Roots.Optional.Root := - Roots.Optional.Detect_Root (Destination); + Roots.Optional.Detect_Root (This.Path); begin -- Check crate name mismatch @@ -410,6 +417,7 @@ package body Alire.User_Pins is function Path (This : Pin) return Absolute_Path is + use Alire.Directories.Operators; -- Having this as an expression function causes CE2021 to return a -- corrupted string some times. begin @@ -418,7 +426,11 @@ package body Alire.User_Pins is return +This.Local_Path; when To_Git => if +This.Checkout_Path /= "" then - return +This.Checkout_Path; + if +This.Subdir /= "" then + return (+This.Checkout_Path) / (+This.Subdir); + else + return +This.Checkout_Path; + end if; else raise Program_Error with "Undeployed pin"; end if; @@ -488,6 +500,11 @@ package body Alire.User_Pins is Result.Branch := +This.Checked_Pop (Keys.Branch, TOML_String).As_String; end if; + + if This.Contains (Keys.Subdir) then + Result.Subdir := + +This.Checked_Pop (Keys.Subdir, TOML_String).As_String; + end if; end return; else @@ -562,6 +579,7 @@ package body Alire.User_Pins is TOML_String).As_String, Branch => <>, Commit => <>, + Subdir => <>, Checkout_Path => <>); begin if This.Contains (Keys.Branch) @@ -585,6 +603,15 @@ package body Alire.User_Pins is "branch cannot be the empty string"); end if; + -- Subdir + + if This.Contains (Keys.Subdir) then + Result.Subdir := + +This.Checked_Pop (Keys.Subdir, TOML_String).As_String; + This.Assert (+Result.Subdir in Alire.Relative_Path, + "invalid subdir : " & (+Result.Subdir)); + end if; + -- TEST: empty branch value This.Report_Extra_Keys; @@ -665,10 +692,28 @@ package body Alire.User_Pins is Table.Set (Keys.Branch, Create_String (Branch (This).Element.Ptr.all)); end if; + + if Subdir (This).Has_Element then + Table.Set (Keys.Subdir, + Create_String (Subdir (This).Element.Ptr.all)); + end if; end if; - Table.Set (Keys.Path, - Create_String (VFS.Attempt_Portable (Path (This)))); + -- Path; we store separately checkout and subdir path, like in the user + -- manifest. + + case This.Kind is + when To_Path => + Table.Set + (Keys.Path, + Create_String (VFS.Attempt_Portable (+This.Local_Path))); + when To_Git => + Table.Set + (Keys.Path, + Create_String (VFS.Attempt_Portable (+This.Checkout_Path))); + when To_Version => + null; + end case; Table.Set (Keys.Internal, Create_Boolean (True)); diff --git a/src/alire/alire-user_pins.ads b/src/alire/alire-user_pins.ads index 984a1daf..c5e0d295 100644 --- a/src/alire/alire-user_pins.ads +++ b/src/alire/alire-user_pins.ads @@ -66,9 +66,10 @@ package Alire.User_Pins is -- Remote pins - function New_Remote (URL : Alire.URL; + function New_Remote (URL : Alire.URL; Commit : String := ""; - Branch : String := "") + Branch : String := ""; + Subdir : Alire.Relative_Path := "") return Pin with Pre => Commit = "" or else VCSs.Git.Is_Valid_Commit (Commit), @@ -83,6 +84,12 @@ package Alire.User_Pins is function Commit (This : Pin) return Optional.String with Pre => This.Is_Remote; + function Subdir (This : Pin) return Optional.String with + Pre => This.Is_Remote, + Post => + (if Subdir'Result.Has_Element + then not Check_Absolute_Path (Subdir'Result.Value)); + function TTY_URL_With_Reference (This : Pin; Detailed : Boolean := False) return String @@ -142,7 +149,9 @@ private Branch : UString; -- Optional Commit : UString; -- Optional Checkout_Path : Unbounded_Absolute_Path; - -- Empty until the pin is locally deployed + -- Empty until the repo is locally deployed + Subdir : Unbounded_Relative_Path; + -- For monorepos, subdir in which the crate is found when To_Path => Local_Path : Unbounded_Absolute_Path; when To_Version => @@ -168,6 +177,15 @@ private then Optional.Strings.Empty else Optional.Strings.Unit (+This.Commit)); + ------------ + -- Subdir -- + ------------ + + function Subdir (This : Pin) return Optional.String + is (if +This.Subdir = "" + then Optional.Strings.Empty + else Optional.Strings.Unit (+This.Subdir)); + --------------- -- Is_Remote -- --------------- diff --git a/src/alire/alire.ads b/src/alire/alire.ads index f18ebeca..069c664c 100644 --- a/src/alire/alire.ads +++ b/src/alire/alire.ads @@ -139,8 +139,8 @@ package Alire with Preelaborate is -- To clarify constants/functions declared herein: function Check_Absolute_Path (Path : Any_Path) return Boolean; - -- Return True if the string Path represent an absolute path on the - -- platform. + -- Returns True if the string Path represent an absolute path on the + -- platform, False otherwise. subtype Directory_Path is Any_Path; diff --git a/src/alr/alr-commands-pin.adb b/src/alr/alr-commands-pin.adb index 651da414..bbe79c92 100644 --- a/src/alr/alr-commands-pin.adb +++ b/src/alr/alr-commands-pin.adb @@ -190,10 +190,19 @@ package body Alr.Commands.Pin is (Crate => Optional_Crate, Origin => Cmd.URL.all, Ref => Cmd.Commit.all, - Branch => Cmd.Branch.all); + Branch => Cmd.Branch.all, + Subdir => Cmd.Subdir.all); else + -- Ensure no subdir for local pins + + if Cmd.Subdir.all /= "" then + Reportaise_Wrong_Arguments + ("Pins to local directories do not accept the " + & TTY.Terminal ("--subdir") & " switch"); + end if; + -- Pin to dir, with a warning if it doesn't look like a path -- and a subsequent confirmation prompt if it doesn't exist. @@ -312,6 +321,13 @@ package body Alr.Commands.Pin is Argument => "REF", Help => "Reference to be retrieved from repository"); + Define_Switch + (Config => Config, + Output => Cmd.Subdir'Access, + Long_Switch => "--subdir=", + Argument => "REL_PATH", + Help => "Relative path to crate inside repository"); + Define_Switch (Config => Config, Output => Cmd.URL'Access, diff --git a/src/alr/alr-commands-pin.ads b/src/alr/alr-commands-pin.ads index 8a9d8803..e91700ce 100644 --- a/src/alr/alr-commands-pin.ads +++ b/src/alr/alr-commands-pin.ads @@ -30,7 +30,8 @@ package Alr.Commands.Pin is overriding function Usage_Custom_Parameters (Cmd : Command) return String is ("[[crate[=]]" - & " | crate --use= [--commit=REF] [--branch=NAME]" + & " | crate --use=" + & " [--commit=REF] [--branch=NAME] [--subdir=REL_PATH]" & " | --all]"); private @@ -38,6 +39,7 @@ private type Command is new Commands.Command with record Branch : aliased GNAT.Strings.String_Access; Commit : aliased GNAT.Strings.String_Access; + Subdir : aliased GNAT.Strings.String_Access; Pin_All : aliased Boolean; Unpin : aliased Boolean; URL : aliased GNAT.Strings.String_Access; diff --git a/src/alr/alr-commands-withing.adb b/src/alr/alr-commands-withing.adb index cfa157a2..c4ae5240 100644 --- a/src/alr/alr-commands-withing.adb +++ b/src/alr/alr-commands-withing.adb @@ -206,7 +206,8 @@ package body Alr.Commands.Withing is (Crate => Crate, Origin => Cmd.URL.all, Ref => Cmd.Commit.all, - Branch => Cmd.Branch.all); + Branch => Cmd.Branch.all, + Subdir => Cmd.Subdir.all); else @@ -225,6 +226,12 @@ package body Alr.Commands.Withing is & "branch or commit was specified."); end if; + if Cmd.Subdir.all /= "" then + Reportaise_Wrong_Arguments + ("Pins to local directories do not accept the " + & TTY.Terminal ("--subdir") & " switch"); + end if; + if not Alire.Utils.User_Input.Approve_Dir (Path) then Trace.Info ("Abandoned by user."); return; @@ -435,6 +442,13 @@ package body Alr.Commands.Withing is Argument => "REF", Help => "Commit to retrieve from repository"); + Define_Switch + (Config => Config, + Output => Cmd.Subdir'Access, + Long_Switch => "--subdir=", + Argument => "REL_PATH", + Help => "Relative path to crate inside repository"); + Define_Switch (Config => Config, Output => Cmd.URL'Access, diff --git a/src/alr/alr-commands-withing.ads b/src/alr/alr-commands-withing.ads index a89cd7da..6f2579c8 100644 --- a/src/alr/alr-commands-withing.ads +++ b/src/alr/alr-commands-withing.ads @@ -29,7 +29,8 @@ package Alr.Commands.Withing is overriding function Usage_Custom_Parameters (Cmd : Command) return String is ("[{ [--del] [versions]..." & " | --from ..." - & " | [versions] --use [--commit REF] [--branch NAME]} ]" + & " | [versions] --use " + & "[--commit REF] [--branch NAME] [--subdir REL_PATH]} ]" & " | --solve | --tree | --versions"); private @@ -37,6 +38,7 @@ private type Command is new Commands.Command with record Branch : aliased GNAT.Strings.String_Access; Commit : aliased GNAT.Strings.String_Access; + Subdir : aliased GNAT.Strings.String_Access; Del : aliased Boolean := False; From : aliased Boolean := False; Graph : aliased Boolean := False; diff --git a/testsuite/tests/pin/remote-subdir/test.py b/testsuite/tests/pin/remote-subdir/test.py new file mode 100644 index 00000000..5e3b7277 --- /dev/null +++ b/testsuite/tests/pin/remote-subdir/test.py @@ -0,0 +1,82 @@ +""" +Check pinning to a crate in a subdir of a remote, cleanup and redeploy +""" + +import os +import shutil + +from drivers.alr import run_alr, init_local_crate +from drivers.helpers import init_git_repo +from drivers.asserts import assert_eq, assert_substring + + +# Ensure the "remote" looks like a git repo +URL = "git+file:" + os.path.join(os.getcwd(), "upstream") + + +def verify(head=""): # Either head or branch /= "" + # Check that the linked dir exists at the expected location + pin_path = (f"alire/cache/pins/crate" + + ("" if head == "" else f"_{head[:8]}") + + "/crate") + assert os.path.isdir(pin_path), \ + f"Expected path not found: {pin_path}\n" \ + f"Contents of alire/cache/pins: {os.listdir('alire/cache/pins')}\n" + + # Verify info reported by alr + p = run_alr("pin") + assert_eq(f"crate file:{pin_path} {URL}" + + ("" if head == "" else f"#{head[0:8]}") + "\n", + p.out) + + # Verify building with pinned dependency + run_alr("build") + + # Verify removal of cached download + run_alr("clean", "--cache") + assert not os.path.isdir(pin_path) + + # Verify automatic redownload when needed + run_alr("build") + + # Prepare for next test + run_alr("with", "--del", "crate") # Remove dependency + p = run_alr("pin") + assert_eq(f"There are no pins\n", p.out) # Ensure pin is gone + shutil.rmtree("alire") # Total cleanup outside of alr + + +# Initialize a git repo that will act as the "online" remote +os.mkdir("upstream") +os.chdir("upstream") +init_local_crate(name="crate", binary=False, enter=False) +head = init_git_repo(".") # The repo is rooted at "upstream", the crate is at "upstream/crate" +os.chdir("..") + +# Initialize a client crate that will use the remote +init_local_crate() # This leaves us inside the new crate + +# Add using with directly +run_alr("with", "--use", URL, "--commit", head, "--subdir", "crate") +verify(head) + +# Add using with, without head commit +run_alr("with", "--use", URL, "--subdir", "crate") +verify() + +# Pin afterwards, with commit +run_alr("with", "crate", force=True) # force, as it is unsolvable +run_alr("pin", "crate", "--use", URL, "--commit", head, "--subdir", "crate") +verify(head) + +# Pin afterwards, without commit +run_alr("with", "crate", force=True) +run_alr("pin", "crate", "--use", URL, "--subdir", "crate") +verify() + +# BONUS: verify that linking to a local folder with --subdir is rejected +p = run_alr("with", "crate", "--use", "../upstream", "--subdir", "crate", + complain_on_error=False) +assert_substring("Pins to local directories do not accept the --subdir switch", p.out) + +print('SUCCESS') diff --git a/testsuite/tests/pin/remote-subdir/test.yaml b/testsuite/tests/pin/remote-subdir/test.yaml new file mode 100644 index 00000000..e2c5af22 --- /dev/null +++ b/testsuite/tests/pin/remote-subdir/test.yaml @@ -0,0 +1,4 @@ +driver: python-script +indexes: + basic_index: {} + compiler_only_index: {} -- 2.39.5