From 9240af16b5c378bc2b2b5ec722f0649bd312952b Mon Sep 17 00:00:00 2001 From: Alejandro R Mosteo Date: Tue, 1 Sep 2020 20:29:45 +0200 Subject: [PATCH] Store backed up files inside `alire` folder (#500) Instead of creating the ".prev" backup files in place, we move them into the alire folder so they don't pollute the user workspace. --- src/alire/alire-directories.adb | 37 ++++++++++--------- src/alire/alire-directories.ads | 22 ++++++----- src/alire/alire-manifest.adb | 11 +++++- src/alire/alire-utils-text_files.adb | 20 ++++++---- src/alire/alire-utils-text_files.ads | 20 ++++++---- src/alire/alire-workspace.adb | 19 +++++++--- src/alr/alr-commands.adb | 5 ++- src/alr/alr-utils-auto_gpr_with.adb | 6 ++- .../tests/get/backup-user-manifest/test.py | 6 ++- 9 files changed, 92 insertions(+), 54 deletions(-) diff --git a/src/alire/alire-directories.adb b/src/alire/alire-directories.adb index c9d07f4e..5e2132c1 100644 --- a/src/alire/alire-directories.adb +++ b/src/alire/alire-directories.adb @@ -11,12 +11,18 @@ package body Alire.Directories is -- Backup_If_Existing -- ------------------------ - procedure Backup_If_Existing (File : Any_Path) is + procedure Backup_If_Existing (File : Any_Path; + Base_Dir : Any_Path := "") + is use Ada.Directories; + Dst : constant String := (if Base_Dir /= "" + then Base_Dir / Simple_Name (File) & ".prev" + else File & ".prev"); begin if Exists (File) then - Trace.Debug ("Backing up " & File); - Copy_File (File, File & ".prev", "mode=overwrite"); + Trace.Debug ("Backing up " & File + & " with base dir: " & Base_Dir); + Copy_File (File, Dst, "mode=overwrite"); end if; end Backup_If_Existing; @@ -286,7 +292,6 @@ package body Alire.Directories is overriding procedure Finalize (This : in out Temp_File) is use Ada.Directories; - use Ada.Exceptions; begin if This.Keep then return; @@ -301,12 +306,6 @@ package body Alire.Directories is Delete_Tree (This.Filename); end if; end if; - exception - when E : others => - Trace.Debug - ("Temp_File.Finalize: unexpected exception: " & - Exception_Name (E) & ": " & Exception_Message (E) & " -- " & - Exception_Information (E)); end Finalize; ------------------- @@ -369,14 +368,17 @@ package body Alire.Directories is -- New_Replacement -- --------------------- - function New_Replacement (File : Any_Path; - Backup : Boolean := True) + function New_Replacement (File : Any_Path; + Backup : Boolean := True; + Backup_Dir : Any_Path := "") return Replacer is begin - return This : constant Replacer := (Length => File'Length, - Original => File, - Backup => Backup, - Temp_Copy => <>) + return This : constant Replacer := (Length => File'Length, + Backup_Len => Backup_Dir'Length, + Original => File, + Backup => Backup, + Backup_Dir => Backup_Dir, + Temp_Copy => <>) do Ada.Directories.Copy_File (File, This.Temp_Copy.Filename); end return; @@ -387,12 +389,11 @@ package body Alire.Directories is ------------- procedure Replace (This : in out Replacer) is - Backup : constant Any_Path := This.Original & ".prev"; begin -- Copy around, so never ceases to be a valid manifest in place if This.Backup then - Ada.Directories.Copy_File (This.Original, Backup); + Backup_If_Existing (This.Original, This.Backup_Dir); end if; Ada.Directories.Copy_File (This.Editable_Name, This.Original); diff --git a/src/alire/alire-directories.ads b/src/alire/alire-directories.ads index f8250d36..f09f4da8 100644 --- a/src/alire/alire-directories.ads +++ b/src/alire/alire-directories.ads @@ -14,8 +14,10 @@ package Alire.Directories is function "/" (L, R : String) return String renames Directories."/"; end Operators; - procedure Backup_If_Existing (File : Any_Path); - -- If File exists, move to file.prev + procedure Backup_If_Existing (File : Any_Path; + Base_Dir : Any_Path := ""); + -- If File exists, copy to file.prev. If Base_Dir /= "", it is instead + -- copied to Base_Dir / Simple_Name (file) & ".prev" procedure Copy (Src_Folder, Dst_Parent_Folder : String; @@ -105,12 +107,13 @@ package Alire.Directories is -- modified and can be tested as the client sees fit. 3) If the new file is -- proper, the old one is renamed to .prev and the new one takes its place. - function New_Replacement (File : Any_Path; - Backup : Boolean := True) + function New_Replacement (File : Any_Path; + Backup : Boolean := True; + Backup_Dir : Any_Path := "") return Replacer; -- Receives a file to be modified, and prepares a copy in a temporary. If -- Backup, once the replacement is performed, the original file is kept as - -- ".prev". + -- ".prev". Backup_Dir works as in Alire.Directories.Backup_If_Existing function Editable_Name (This : Replacer) return Any_Path; -- Obtain the editable copy @@ -151,10 +154,11 @@ private overriding procedure Finalize (This : in out Temp_File); - type Replacer (Length : Positive) is tagged limited record - Original : Any_Path (1 .. Length); - Temp_Copy : Temp_File; - Backup : Boolean := True; + type Replacer (Length, Backup_Len : Natural) is tagged limited record + Original : Any_Path (1 .. Length); + Temp_Copy : Temp_File; + Backup : Boolean := True; + Backup_Dir : Any_Path (1 .. Backup_Len); end record; end Alire.Directories; diff --git a/src/alire/alire-manifest.adb b/src/alire/alire-manifest.adb index 1bc05925..4744ea33 100644 --- a/src/alire/alire-manifest.adb +++ b/src/alire/alire-manifest.adb @@ -2,6 +2,7 @@ with Ada.Text_IO; use Ada.Text_IO; with Alire.Directories; with Alire.Errors; +with Alire.Paths; with Alire.Releases; with Alire.TOML_Keys; with Alire.Utils.Text_Files; @@ -17,7 +18,10 @@ package body Alire.Manifest is procedure Append (Name : Any_Path; Deps : Dependencies.Containers.List) is Replacer : constant Directories.Replacer := - Directories.New_Replacement (Name); + Directories.New_Replacement + (Name, + Backup => True, + Backup_Dir => Paths.Working_Folder_Inside_Root); File : File_Type; begin if Deps.Is_Empty then @@ -235,7 +239,10 @@ package body Alire.Manifest is end Remove; Replacer : constant Directories.Replacer := - Directories.New_Replacement (Name); + Directories.New_Replacement + (Name, + Backup => True, + Backup_Dir => Paths.Working_Folder_Inside_Root); begin if Deps.Is_Empty then return; diff --git a/src/alire/alire-utils-text_files.adb b/src/alire/alire-utils-text_files.adb index 5b1e0728..cc035edf 100644 --- a/src/alire/alire-utils-text_files.adb +++ b/src/alire/alire-utils-text_files.adb @@ -19,7 +19,9 @@ package body Alire.Utils.Text_Files is declare Replacer : Directories.Replacer := - Directories.New_Replacement (This.Name, This.Backup); + Directories.New_Replacement (This.Name, + This.Backup, + This.Backup_Dir); begin Open (File, Out_File, Replacer.Editable_Name); for Line of This.Lines loop @@ -41,17 +43,21 @@ package body Alire.Utils.Text_Files is -- Load -- ---------- - function Load (From : Any_Path; - Backup : Boolean := True) + function Load (From : Any_Path; + Backup : Boolean := True; + Backup_Dir : Any_Path := "") return File is F : File_Type; begin return This : File := (Ada.Finalization.Limited_Controlled with - Length => From'Length, - Name => From, - Backup => Backup, - others => <>) + Length => From'Length, + Backup_Len => Backup_Dir'Length, + Name => From, + Backup => Backup, + Backup_Dir => Backup_Dir, + Lines => <>, + Orig => <>) do Open (F, In_File, From); while not End_Of_File (F) loop diff --git a/src/alire/alire-utils-text_files.ads b/src/alire/alire-utils-text_files.ads index 4c59f458..6233e0f1 100644 --- a/src/alire/alire-utils-text_files.ads +++ b/src/alire/alire-utils-text_files.ads @@ -6,22 +6,26 @@ package Alire.Utils.Text_Files is type File (<>) is tagged limited private; - function Load (From : Any_Path; - Backup : Boolean := True) + function Load (From : Any_Path; + Backup : Boolean := True; + Backup_Dir : Any_Path := "") return File; -- Load a text file into memory. If Backup, when saving takes place the - -- original is renamed to ".prev". + -- original is renamed to ".prev". Backup_Dir optionally designates where + -- the backup file will be moved. function Lines (This : aliased in out File) return access String_Vector; private - type File (Length : Natural) is new Ada.Finalization.Limited_Controlled + type File (Length, Backup_Len : Natural) is + new Ada.Finalization.Limited_Controlled with record - Name : Any_Path (1 .. Length); - Lines : aliased String_Vector; -- The final contents - Orig : String_Vector; -- The original contents - Backup : Boolean := True; + Name : Any_Path (1 .. Length); + Lines : aliased String_Vector; -- The final contents + Orig : String_Vector; -- The original contents + Backup : Boolean := True; + Backup_Dir : Any_Path (1 .. Backup_Len); end record; overriding diff --git a/src/alire/alire-workspace.adb b/src/alire/alire-workspace.adb index 3da02fd8..540ca79d 100644 --- a/src/alire/alire-workspace.adb +++ b/src/alire/alire-workspace.adb @@ -8,6 +8,7 @@ with Alire.Lockfiles; with Alire.Manifest; with Alire.Origins.Deployers; with Alire.OS_Lib; +with Alire.Paths; with Alire.Properties.Actions.Executor; with Alire.Roots; with Alire.Solutions.Diffs; @@ -206,15 +207,21 @@ package body Alire.Workspace is Working_Dir : Guard (Enter (Release.Unique_Folder)) with Unreferenced; begin + Ada.Directories.Create_Path (Paths.Working_Folder_Inside_Root); + if GNAT.OS_Lib.Is_Regular_File (Roots.Crate_File_Name) then Trace.Debug ("Backing up bundled manifest file as *.upstream"); declare Upstream_File : constant String := - Roots.Crate_File_Name & ".upstream"; + Paths.Working_Folder_Inside_Root / + (Roots.Crate_File_Name & ".upstream"); begin - Alire.Directories.Backup_If_Existing (Upstream_File); - Ada.Directories.Rename (Old_Name => Roots.Crate_File_Name, - New_Name => Upstream_File); + Alire.Directories.Backup_If_Existing + (Upstream_File, + Base_Dir => Paths.Working_Folder_Inside_Root); + Ada.Directories.Rename + (Old_Name => Roots.Crate_File_Name, + New_Name => Upstream_File); end; end if; end; @@ -266,7 +273,9 @@ package body Alire.Workspace is & Release.Milestone.Image & " with" & Release.Dependencies.Leaf_Count'Img & " dependencies"); - Directories.Backup_If_Existing (Root.Crate_File); + Directories.Backup_If_Existing + (Root.Crate_File, + Base_Dir => Paths.Working_Folder_Inside_Root); Release.To_File (Root.Crate_File, Manifest.Local); end Generate_Manifest; diff --git a/src/alr/alr-commands.adb b/src/alr/alr-commands.adb index 63471b1e..65f87e1c 100644 --- a/src/alr/alr-commands.adb +++ b/src/alr/alr-commands.adb @@ -12,6 +12,7 @@ with Alire.Config; with Alire.Errors; with Alire.Features.Index; with Alire.Lockfiles; +with Alire.Paths; with Alire.Platforms; with Alire.Roots.Optional; with Alire.Solutions; @@ -447,7 +448,9 @@ package body Alr.Commands is & " Internal data is going to be updated and, as a result," & " any existing pins will be unpinned and will need to be" & " manually recreated."); - Alire.Directories.Backup_If_Existing (Checked.Lock_File); + Alire.Directories.Backup_If_Existing + (Checked.Lock_File, + Base_Dir => Alire.Paths.Working_Folder_Inside_Root); Ada.Directories.Delete_File (Checked.Lock_File); when Lockfiles.Missing => diff --git a/src/alr/alr-utils-auto_gpr_with.adb b/src/alr/alr-utils-auto_gpr_with.adb index fa1130bf..b8e3eee6 100644 --- a/src/alr/alr-utils-auto_gpr_with.adb +++ b/src/alr/alr-utils-auto_gpr_with.adb @@ -3,8 +3,8 @@ with Ada.Directories; with Alire.Directories; with Alire.Utils.User_Input; -with Alire.Config; with Alire.Config.Edit; +with Alire.Paths; package body Alr.Utils.Auto_GPR_With is @@ -118,7 +118,9 @@ package body Alr.Utils.Auto_GPR_With is Close (In_File); Close (Out_File); - Alire.Directories.Backup_If_Existing (GPR_File); + Alire.Directories.Backup_If_Existing + (GPR_File, + Base_Dir => Alire.Paths.Working_Folder_Inside_Root); Ada.Directories.Copy_File (Tmp.Filename, GPR_File); end Update; diff --git a/testsuite/tests/get/backup-user-manifest/test.py b/testsuite/tests/get/backup-user-manifest/test.py index 321a4644..80e6b259 100644 --- a/testsuite/tests/get/backup-user-manifest/test.py +++ b/testsuite/tests/get/backup-user-manifest/test.py @@ -10,13 +10,15 @@ from os import chdir, path run_alr('get', 'crate') chdir('crate_1.0.0_filesystem') +upstream = path.join('alire', 'alire.toml.upstream') + # Verify that the manifest has been properly renamed -assert path.isfile('alire.toml.upstream'), "Expected backup file missing" +assert path.isfile(upstream), "Expected backup file missing" # Verify that contents are as expected in the generated and backed up manifests assert "badproperty" not in content_of("alire.toml"), \ "Unexpected contents present in manifest file" -assert "badproperty" in content_of("alire.toml.upstream"), \ +assert "badproperty" in content_of(upstream), \ "Unexpected contents missing in upstream manifest file" print('SUCCESS') -- 2.39.5