From 99e579cf5402fb5e74401c61bdfca59927196429 Mon Sep 17 00:00:00 2001 From: Alejandro R Mosteo Date: Tue, 20 Jul 2021 18:08:18 +0200 Subject: [PATCH] Deploy dependencies atomically and related cleanup subcommand (#768) * Use a temporary to fetch & deploy releases This is done to avoid an interrupted download (for example with Ctrl-C) to mistake our deployer, which considers an existing directory as a successfully deployed dependency. * Cmd.Clean: implement --temp option To remove any dangling temporaries * Improve clean --cache to avoid useless downloads * Test for `alr clean --temp` --- src/alire/alire-directories.adb | 29 +++-- src/alire/alire-directories.ads | 5 + src/alire/alire-origins-deployers.adb | 30 ++++- src/alire/alire-paths.ads | 2 + src/alire/alire-roots.adb | 2 +- src/alr/alr-commands-clean.adb | 134 +++++++++++++++++++-- src/alr/alr-commands-clean.ads | 3 +- testsuite/tests/clean/temp-files/test.py | 36 ++++++ testsuite/tests/clean/temp-files/test.yaml | 3 + 9 files changed, 219 insertions(+), 25 deletions(-) create mode 100644 testsuite/tests/clean/temp-files/test.py create mode 100644 testsuite/tests/clean/temp-files/test.yaml diff --git a/src/alire/alire-directories.adb b/src/alire/alire-directories.adb index d9332a28..9cc4d3cf 100644 --- a/src/alire/alire-directories.adb +++ b/src/alire/alire-directories.adb @@ -375,12 +375,7 @@ package body Alire.Directories is -- TEMP FILES -- ---------------- - ---------------- - -- Initialize -- - ---------------- - - overriding - procedure Initialize (This : in out Temp_File) is + function Temp_Name (Length : Positive := 8) return String is subtype Valid_Character is Character range 'a' .. 'z'; package Char_Random is new Ada.Numerics.Discrete_Random (Valid_Character); @@ -388,10 +383,24 @@ package body Alire.Directories is begin Char_Random.Reset (Gen); - This.Name := +"alr-XXXX.tmp"; - for I in 5 .. 8 loop - UStrings.Replace_Element (This.Name, I, Char_Random.Random (Gen)); - end loop; + 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; + + ---------------- + -- Initialize -- + ---------------- + + overriding + procedure Initialize (This : in out Temp_File) is + + begin + This.Name := +Temp_Name; -- Try to use our alire folder to hide temporaries; return an absolute -- path in any case to avoid problems with the user of the tmp file diff --git a/src/alire/alire-directories.ads b/src/alire/alire-directories.ads index b98313bb..8e561607 100644 --- a/src/alire/alire-directories.ads +++ b/src/alire/alire-directories.ads @@ -98,6 +98,11 @@ package Alire.Directories is -- Temporary files -- --------------------- + function Temp_Name (Length : Positive := 8) return String + with Pre => Length >= 5; + -- Return a filename such as "alr-sdrv.tmp". Length refers to the name + -- without .tmp. The alr- prefix is fixed. + -- TEMP_FILE: obtain a temporary name with optional cleanup type Temp_File is tagged limited private; diff --git a/src/alire/alire-origins-deployers.adb b/src/alire/alire-origins-deployers.adb index 80223838..df655310 100644 --- a/src/alire/alire-origins-deployers.adb +++ b/src/alire/alire-origins-deployers.adb @@ -1,5 +1,6 @@ with Ada.Directories; +with Alire.Directories; with Alire.Origins.Deployers.External; with Alire.Origins.Deployers.Filesystem; with Alire.Origins.Deployers.Git; @@ -57,24 +58,47 @@ package body Alire.Origins.Deployers is function Deploy_Steps (From : Origin; Folder : String) return Outcome is + use Directories.Operators; + Temp_Dir : Directories.Temp_File := + Directories.With_Name + (Ada.Directories.Containing_Directory (Folder) + / Directories.Temp_Name); + -- We use a temporary location to fetch and verify, as otherwise any + -- failure before final deployment may result in considering a crate + -- already deployed. + The_Deployer : constant Deployer'Class := New_Deployer (From); Result : Outcome; begin -- 1. Fetch sources - Result := The_Deployer.Fetch (Folder); + Result := The_Deployer.Fetch (Temp_Dir.Filename); if not Result.Success then return Result; end if; -- 2. Verify sources - Result := The_Deployer.Verify_Hashes (Folder); + Result := The_Deployer.Verify_Hashes (Temp_Dir.Filename); if not Result.Success then return Result; end if; -- 3. Deploy final sources - return The_Deployer.Deploy (Folder); + The_Deployer.Deploy (Temp_Dir.Filename).Assert; + + -- 4. Rename into final location. This is always in the same drive (as + -- we created the temporary as a sibling of the final location) so it + -- should be an instant operation. We check for the folder existence + -- as some deployers may not need one (like system packages). + Temp_Dir.Keep; + if Ada.Directories.Exists (Temp_Dir.Filename) then + Trace.Debug ("Renaming into place " & TTY.URL (Temp_Dir.Filename) + & " as " & TTY.URL (Folder)); + Ada.Directories.Rename (Old_Name => Temp_Dir.Filename, + New_Name => Folder); + end if; + + return Outcome_Success; exception when E : others => Log_Exception (E); diff --git a/src/alire/alire-paths.ads b/src/alire/alire-paths.ads index 857e297f..e4daa062 100644 --- a/src/alire/alire-paths.ads +++ b/src/alire/alire-paths.ads @@ -5,6 +5,8 @@ package Alire.Paths with Preelaborate is Crate_File_Name : constant String := "alire.toml"; -- Name of the manifest file in a regular workspace + Cache_Folder_Inside_Working_Folder : constant Relative_Path := "cache"; + Temp_Folder_Inside_Working_Folder : constant Relative_Path := "tmp"; function Working_Folder_Inside_Root return Relative_Path diff --git a/src/alire/alire-roots.adb b/src/alire/alire-roots.adb index 43e34205..918b90fb 100644 --- a/src/alire/alire-roots.adb +++ b/src/alire/alire-roots.adb @@ -721,7 +721,7 @@ package body Alire.Roots is --------------- function Cache_Dir (This : Root) return Absolute_Path - is (This.Working_Folder / "cache"); + is (This.Working_Folder / Paths.Cache_Folder_Inside_Working_Folder); ---------------------- -- Dependencies_Dir -- diff --git a/src/alr/alr-commands-clean.adb b/src/alr/alr-commands-clean.adb index 00ca98c7..6cf5c275 100644 --- a/src/alr/alr-commands-clean.adb +++ b/src/alr/alr-commands-clean.adb @@ -1,3 +1,8 @@ +with Ada.Directories; + +with Alire.Directories; +with Alire.Paths; +with Alire.TTY; with Alire.Utils; with Alr.Spawn; @@ -5,6 +10,86 @@ with Alr.Platform; package body Alr.Commands.Clean is + ----------------------- + -- Delete_Temp_Files -- + ----------------------- + + procedure Delete_Temp_Files is + + ------------ + -- Delete -- + ------------ + + procedure Delete (Path : String) + is + begin + Trace.Detail ("Deleting " & Alire.TTY.URL (Path)); + Alire.Directories.Force_Delete (Path); + end Delete; + + Targets : Alire.Utils.String_Set; + + ---------------- + -- Add_Target -- + ---------------- + + procedure Add_Target (Item : Ada.Directories.Directory_Entry_Type; + Unused_Stop : in out Boolean) + is + use Ada.Directories; + use Alire.Utils; + Name : constant String := Simple_Name (Item); + begin + if Starts_With (Name, "alr-") and then Ends_With (Name, ".tmp") then + Targets.Include (Ada.Directories.Full_Name (Item)); + end if; + end Add_Target; + + package TTY renames Alire.TTY; + begin + Alire.Directories.Traverse_Tree + (Start => ".", + Doing => Add_Target'Access, + Recurse => True); + + for Target of Targets loop + Delete (Target); + end loop; + + if Targets.Is_Empty then + Trace.Info ("No temporaries found."); + elsif Targets.Length in 1 then + Trace.Info ("Deleted " & TTY.Emph ("1") & " temporary."); + else + Trace.Info ("Deleted" & TTY.Emph (Targets.Length'Image) + & " temporaries."); + end if; + end Delete_Temp_Files; + + ---------------- + -- Find_Cache -- + ---------------- + -- Return the cache dir, or "" if not found + function Find_Cache return String is + use Ada.Directories; + use Alire.Directories.Operators; + Root : constant String := Alire.Directories.Detect_Root_Path; + begin + if Root /= "" then + if Exists (Root + / Alire.Paths.Working_Folder_Inside_Root + / Alire.Paths.Cache_Folder_Inside_Working_Folder) + then + return + Root + / Alire.Paths.Working_Folder_Inside_Root + / Alire.Paths.Cache_Folder_Inside_Working_Folder; + end if; + end if; + + return ""; + end Find_Cache; + ------------- -- Execute -- ------------- @@ -13,9 +98,9 @@ package body Alr.Commands.Clean is procedure Execute (Cmd : in out Command) is use Alire.Utils; begin - Cmd.Requires_Valid_Session; - if not Cmd.Cache then + if not (Cmd.Cache or else Cmd.Temp) then + Cmd.Requires_Valid_Session; Cmd.Root.Export_Build_Environment; Trace.Detail ("Cleaning project and dependencies..."); @@ -32,17 +117,36 @@ package body Alr.Commands.Clean is Scenario.As_Command_Line, Understands_Verbose => True); end loop; + + return; end if; if Cmd.Cache then - if OS_Lib.Is_Folder (Cmd.Root.Cache_Dir) then - Trace.Detail ("Deleting working copy cache..."); - Alire.Directories.Force_Delete (Cmd.Root.Cache_Dir); - else - Trace.Detail ("Cache folder not present"); - -- This is expected if the crate has no dependencies - end if; + + -- We do not want to use Cmd.Root here, as it will check for a valid + -- root, in turn deploying any missing dependencies (which we want to + -- delete). This might result in that running two `alr clean --cache` + -- in a row would redownload everything, and delete it again. So we + -- go lower level and use more basic parts of Alire. + + declare + Cache_Dir : constant String := Find_Cache; + begin + if Cache_Dir /= "" then + Trace.Detail ("Deleting working copy cache..."); + Alire.Directories.Force_Delete (Cache_Dir); + Trace.Info ("Cache folder deleted."); + else + Trace.Info ("Cache folder not present."); + -- This is expected if the crate has no dependencies + end if; + end; + end if; + + if Cmd.Temp then + Delete_Temp_Files; end if; + end Execute; ---------------------- @@ -58,7 +162,13 @@ package body Alr.Commands.Clean is & " build environment.") .New_Line .Append ("--cache:") - .Append (" All downloaded dependencies will be deleted.")); + .Append (" All downloaded dependencies will be deleted.") + .New_Line + .Append ("--temp:") + .Append (" All alr-???.tmp files in the subtree will be deleted." + & " These files may remain when alr is interrupted via" + & " Ctrl-C or other forceful means.") + ); -------------------- -- Setup_Switches -- @@ -74,6 +184,10 @@ package body Alr.Commands.Clean is Cmd.Cache'Access, Long_Switch => "--cache", Help => "Delete cache of releases"); + Define_Switch (Config, + Cmd.Temp'Access, + Long_Switch => "--temp", + Help => "Delete dangling temporary files"); end Setup_Switches; end Alr.Commands.Clean; diff --git a/src/alr/alr-commands-clean.ads b/src/alr/alr-commands-clean.ads index dfd94320..b09eb6e6 100644 --- a/src/alr/alr-commands-clean.ads +++ b/src/alr/alr-commands-clean.ads @@ -20,12 +20,13 @@ package Alr.Commands.Clean is overriding function Usage_Custom_Parameters (Cmd : Command) return String - is (""); + is ("[--cache] [--temp]"); private type Command is new Commands.Command with record Cache : aliased Boolean := False; + Temp : aliased Boolean := False; end record; end Alr.Commands.Clean; diff --git a/testsuite/tests/clean/temp-files/test.py b/testsuite/tests/clean/temp-files/test.py new file mode 100644 index 00000000..3c1d03b4 --- /dev/null +++ b/testsuite/tests/clean/temp-files/test.py @@ -0,0 +1,36 @@ +""" +Verify that `alr clean --temp` works properly +""" + +from drivers.alr import run_alr +from drivers.asserts import assert_eq, assert_match + +import e3 +import os + +os.mkdir("test") +os.chdir("test") + +# We create a temp file above us, one at the current dir, and one below. +# The one above us should not be cleaned. Also, a file not conforming to +# "alr-????.tmp" should not be cleaned either. + +e3.os.fs.touch("../alr-0001.tmp") +e3.os.fs.touch("alr-0002.tmp") +os.mkdir("nested") +e3.os.fs.touch("nested/alr-0003.tmp") +e3.os.fs.touch("alien.tmp") + +p = run_alr("clean", "--temp") + +assert os.path.exists("../alr-0001.tmp"), "unexpected deletion" +assert os.path.exists("alien.tmp"), "unexpected deletion" +assert not os.path.exists("alr-0002.tmp"), "unexpected file" +assert not os.path.exists("nested/alr-0003.tmp"), "unexpected file" + +# Finally verify that running again in a clean environment does nothing + +p = run_alr("clean", "--temp", quiet=False) +assert_eq("No temporaries found.\n", p.out) + +print('SUCCESS') diff --git a/testsuite/tests/clean/temp-files/test.yaml b/testsuite/tests/clean/temp-files/test.yaml new file mode 100644 index 00000000..872fc127 --- /dev/null +++ b/testsuite/tests/clean/temp-files/test.yaml @@ -0,0 +1,3 @@ +driver: python-script +indexes: + basic_index: {} -- 2.39.5