From 39a61704b8f9b4c27e38670346b4d8187ed13f99 Mon Sep 17 00:00:00 2001 From: Alejandro R Mosteo Date: Sat, 8 Feb 2025 15:52:45 +0100 Subject: [PATCH] fix: don't crash on trying to re-add a pin (#1838) * Fix attempt to use unavailable pin path * Test * Reformat post refactor --- src/alire/alire-user_pins.adb | 72 ++++++++++++++---------- src/alire/alire-user_pins.ads | 14 +++-- testsuite/tests/with/pin-twice/test.py | 34 +++++++++++ testsuite/tests/with/pin-twice/test.yaml | 3 + 4 files changed, 88 insertions(+), 35 deletions(-) create mode 100644 testsuite/tests/with/pin-twice/test.py create mode 100644 testsuite/tests/with/pin-twice/test.yaml diff --git a/src/alire/alire-user_pins.adb b/src/alire/alire-user_pins.adb index 653a66d3..cca4c3b6 100644 --- a/src/alire/alire-user_pins.adb +++ b/src/alire/alire-user_pins.adb @@ -40,8 +40,8 @@ package body Alire.User_Pins is -------------- function New_Path (Path : Any_Path) return Pin - is (Kind => To_Path, - Path => +Path); + is (Kind => To_Path, + Local_Path => +Path); ---------------- -- New_Remote -- @@ -51,11 +51,11 @@ package body Alire.User_Pins is Commit : String := ""; Branch : String := "") return Pin - is (Kind => To_Git, - URL => +URL, - Commit => +Commit, - Branch => +Branch, - Local_Path => <>); + is (Kind => To_Git, + URL => +URL, + Commit => +Commit, + Branch => +Branch, + Checkout_Path => <>); ----------- -- Image -- @@ -64,11 +64,13 @@ package body Alire.User_Pins is function Image (This : Pin; User : Boolean) return String is (case This.Kind is when To_Version => "version=" & TTY.Version (This.Version.Image), - when To_Path => "path=" & TTY.URL (if User - then VFS.Attempt_Portable (+This.Path) - else +This.Path), + when To_Path => "path=" + & TTY.URL + (if User + then VFS.Attempt_Portable (+This.Local_Path) + else +This.Local_Path), when To_Git => - (if Path (This) /= "" + (if This.Has_Path and then This.Path /= "" then "path=" & TTY.URL ((if User then VFS.Attempt_Portable (Path (This)) else Path (This))) & "," @@ -253,7 +255,7 @@ package body Alire.User_Pins is (for all Path of Paths => AAA.Strings.Has_Prefix (Path, "alire/") or else AAA.Strings.Has_Prefix (Path, "config/")); - -- 'git status' yields '/' separated paths, even on Windows. + -- 'git status' yields '/' separated paths, even on Windows Question : constant String := "Updating the pin '" @@ -308,7 +310,7 @@ package body Alire.User_Pins is return; end if; - This.Local_Path := +Destination; + This.Checkout_Path := +Destination; -- Don't check out an already existing commit pin, or a non-update -- branch pin @@ -392,6 +394,16 @@ package body Alire.User_Pins is then "#" & TTY.Emph (+This.Branch) else "")); + -------------- + -- Has_Path -- + -------------- + + function Has_Path (This : Pin) return Boolean + is (This.Kind = To_Path + or else + (This.Kind = To_Git + and then +This.Checkout_Path /= "")); + ---------- -- Path -- ---------- @@ -403,10 +415,10 @@ package body Alire.User_Pins is begin case This.Kind is when To_Path => - return +This.Path; + return +This.Local_Path; when To_Git => - if +This.Local_Path /= "" then - return +This.Local_Path; + if +This.Checkout_Path /= "" then + return +This.Checkout_Path; else raise Program_Error with "Undeployed pin"; end if; @@ -465,7 +477,7 @@ package body Alire.User_Pins is +This.Checked_Pop (Keys.URL, TOML_String).As_String; - Result.Local_Path := + Result.Checkout_Path := +Utils.User_Input.To_Absolute_From_Portable (This.Checked_Pop (Keys.Path, TOML_String).As_String); @@ -483,14 +495,14 @@ package body Alire.User_Pins is -- Just a local pin return Result : Pin := (Kind => To_Path, others => <>) do - Result.Path := + Result.Local_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 + if not GNAT.OS_Lib.Is_Directory (+Result.Local_Path) then This.Recoverable_Error ("Pin path is not a valid directory: " - & (+Result.Path)); + & (+Result.Local_Path)); end if; end return; end if; @@ -503,7 +515,7 @@ package body Alire.User_Pins is function Load_To_Path return Pin is Result : Pin := (Kind => To_Path, - Path => <>); + Local_Path => <>); User_Path : constant String := This.Checked_Pop (Keys.Path, TOML_String).As_String; @@ -523,16 +535,16 @@ package body Alire.User_Pins is -- Make the path absolute if not already, and store it - Result.Path := + Result.Local_Path := +Utils.User_Input.To_Absolute_From_Portable (User_Path => User_Path, Error_When_Relative_Native => "Pin relative paths must use forward slashes " & " to be portable"); - if not GNAT.OS_Lib.Is_Directory (+Result.Path) then + if not GNAT.OS_Lib.Is_Directory (+Result.Local_Path) then This.Recoverable_Error ("Pin path is not a valid directory: " - & (+Result.Path)); + & (+Result.Local_Path)); end if; return Result; @@ -545,12 +557,12 @@ package body Alire.User_Pins is function Load_Remote return Pin is use Ada.Strings.Unbounded; Result : Pin := - (Kind => To_Git, - URL => +This.Checked_Pop (Keys.URL, - TOML_String).As_String, - Branch => <>, - Commit => <>, - Local_Path => <>); + (Kind => To_Git, + URL => +This.Checked_Pop (Keys.URL, + TOML_String).As_String, + Branch => <>, + Commit => <>, + Checkout_Path => <>); begin if This.Contains (Keys.Branch) and then This.Contains (Keys.Commit) diff --git a/src/alire/alire-user_pins.ads b/src/alire/alire-user_pins.ads index ab90332c..984a1daf 100644 --- a/src/alire/alire-user_pins.ads +++ b/src/alire/alire-user_pins.ads @@ -49,6 +49,10 @@ package Alire.User_Pins is function Is_Broken (This : Pin) return Boolean with Pre => This.Kind in Kinds_With_Path; + function Has_Path (This : Pin) return Boolean; + -- True if Path will return a path for a local or deployed link, False if + -- Path will raise. + function Path (This : Pin) return Absolute_Path with Pre => This.Kind in Kinds_With_Path; -- May raise if a Git pin hasn't been yet deployed (see Deploy proc). Even @@ -134,13 +138,13 @@ private type Pin (Kind : Kinds) is tagged record case Kind is when To_Git => - URL : UString; - Branch : UString; -- Optional - Commit : UString; -- Optional - Local_Path : Unbounded_Absolute_Path; + URL : UString; + Branch : UString; -- Optional + Commit : UString; -- Optional + Checkout_Path : Unbounded_Absolute_Path; -- Empty until the pin is locally deployed when To_Path => - Path : Unbounded_Absolute_Path; + Local_Path : Unbounded_Absolute_Path; when To_Version => Version : Semantic_Versioning.Version; end case; diff --git a/testsuite/tests/with/pin-twice/test.py b/testsuite/tests/with/pin-twice/test.py new file mode 100644 index 00000000..8e260bae --- /dev/null +++ b/testsuite/tests/with/pin-twice/test.py @@ -0,0 +1,34 @@ +""" +Fix for issue https://github.com/alire-project/alire/issues/1801 +Ensure that the proper error is given when trying to pin a crate twice +""" + +from drivers.alr import init_local_crate, run_alr +from drivers.asserts import assert_substring +from drivers.helpers import init_git_repo + +# We can trigger the bug condition locally by using a local git repo as the +# remote pin. We are going to pin twice a nested crate (the nesting is +# irrelevant). + +TARGET = 'nested' + +# Our main crate +init_local_crate() + +# Create the nested crate and initialize a repo there +init_local_crate(TARGET, enter=False) +init_git_repo(TARGET) + +# Pin first time +run_alr("with", TARGET, f"--use=git+file:{TARGET}") + +# Verify pin is correct +assert_substring(f"git+file:{TARGET}", run_alr("pin").out) + +# Verify proper error on 2nd attempt +assert_substring(f"{TARGET} is already pinned with pin url=git+file:{TARGET}", + run_alr("with", TARGET, f"--use=git+file:{TARGET}", + complain_on_error=False).out) + +print('SUCCESS') diff --git a/testsuite/tests/with/pin-twice/test.yaml b/testsuite/tests/with/pin-twice/test.yaml new file mode 100644 index 00000000..fa855459 --- /dev/null +++ b/testsuite/tests/with/pin-twice/test.yaml @@ -0,0 +1,3 @@ +driver: python-script +indexes: + compiler_only_index: {} -- 2.39.5