From 02619765cb4ba04f710831e843cf349962012b65 Mon Sep 17 00:00:00 2001 From: Alejandro R Mosteo Date: Thu, 11 Jun 2020 13:34:57 +0200 Subject: [PATCH] Safeguards for changes in lockfile/crate format (#436) If the lockfile is not loadable we presume it is in an old format. In this case we recreate it from scratch, which entails the loss of any stored pins. If the alire/crate.toml file is not loadable we advice the user to recreate the workspace from scratch. Note that both crate.toml and crate.lock use the index format to store releases. Hence, any change in index format may impact these files. While the lockfile is not that problematic, for the crate.toml file we have no easy workaround since we cannot know to which crate the workspace belongs. There are possible mitigations but as long as we don't have stable releases the complexity/benefit ratio is unclear. If we eventually make the crate.toml file only manually editable that could also impact how to deal with this issue, so not doing anything about it yet. --- src/alire/alire-directories.adb | 13 +++++++++ src/alire/alire-directories.ads | 3 +++ src/alire/alire-lockfiles.adb | 25 +++++++++++++++++ src/alire/alire-lockfiles.ads | 5 ++++ src/alire/alire-root.adb | 9 ++++++- src/alr/alr-commands.adb | 27 ++++++++++++++----- testsuite/tests/misc/bad-lockfile/test.py | 30 +++++++++++++++++++++ testsuite/tests/misc/bad-lockfile/test.yaml | 1 + testsuite/tests/misc/bad-tomlfile/test.py | 28 +++++++++++++++++++ testsuite/tests/misc/bad-tomlfile/test.yaml | 1 + 10 files changed, 135 insertions(+), 7 deletions(-) create mode 100644 testsuite/tests/misc/bad-lockfile/test.py create mode 100644 testsuite/tests/misc/bad-lockfile/test.yaml create mode 100644 testsuite/tests/misc/bad-tomlfile/test.py create mode 100644 testsuite/tests/misc/bad-tomlfile/test.yaml diff --git a/src/alire/alire-directories.adb b/src/alire/alire-directories.adb index 6cfc48ab..26758d0e 100644 --- a/src/alire/alire-directories.adb +++ b/src/alire/alire-directories.adb @@ -6,6 +6,19 @@ with Alire.Paths; package body Alire.Directories is + ------------------------ + -- Backup_If_Existing -- + ------------------------ + + procedure Backup_If_Existing (File : Any_Path) is + use Ada.Directories; + begin + if Exists (File) then + Trace.Debug ("Backing up " & File); + Copy_File (File, File & ".prev", "mode=overwrite"); + end if; + end Backup_If_Existing; + ---------- -- Copy -- ---------- diff --git a/src/alire/alire-directories.ads b/src/alire/alire-directories.ads index 511246fa..c3f32b6a 100644 --- a/src/alire/alire-directories.ads +++ b/src/alire/alire-directories.ads @@ -14,6 +14,9 @@ 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 Copy (Src_Folder, Dst_Parent_Folder : String; Excluding : String := ""); diff --git a/src/alire/alire-lockfiles.adb b/src/alire/alire-lockfiles.adb index 948d10b7..966b3a87 100644 --- a/src/alire/alire-lockfiles.adb +++ b/src/alire/alire-lockfiles.adb @@ -41,6 +41,31 @@ package body Alire.Lockfiles is end; end Read; + -------------- + -- Validity -- + -------------- + + function Validity (File : Any_Path) return Validities is + begin + if not GNAT.OS_Lib.Is_Read_Accessible_File (File) then + return Missing; + end if; + + -- Try to load to assess validity + + declare + Unused : constant Solver.Solution := Read (File); + begin + return Valid; + end; + + exception + when E : others => + Trace.Debug ("Exception while loading lockfile is: "); + Log_Exception (E, Debug); + return Invalid; + end Validity; + ----------- -- Write -- ----------- diff --git a/src/alire/alire-lockfiles.ads b/src/alire/alire-lockfiles.ads index a0898295..c6677dfa 100644 --- a/src/alire/alire-lockfiles.ads +++ b/src/alire/alire-lockfiles.ads @@ -3,6 +3,8 @@ with Alire.Solver; package Alire.Lockfiles is + type Validities is (Missing, Invalid, Valid); + -- A crate lockfile stores the dependency solution in use. This permanent -- storage, in /crate_name.lock, is the basis for orderly uploads -- and pinning. This file is autogenerated and manipulated via alr @@ -16,6 +18,9 @@ package Alire.Lockfiles is function Read (Filename : Any_Path) return Solver.Solution; -- Read a solution from the given lockfile + function Validity (File : Any_Path) return Validities; + -- Check if given file is a valid lockfile + procedure Write (Solution : Solver.Solution; Environment : Properties.Vector; Filename : Any_Path); diff --git a/src/alire/alire-root.adb b/src/alire/alire-root.adb index 5f17d6d0..e522ab59 100644 --- a/src/alire/alire-root.adb +++ b/src/alire/alire-root.adb @@ -20,7 +20,14 @@ package body Alire.Root is (TOML_Index.Load_Release_From_File (File), Path); exception - when E : Checked_Error => + when E : others => + Trace.Debug ("Exception while loading crate file is:"); + Log_Exception (E, Debug); + + Trace.Warning ("Could not load crate information from " & File); + Trace.Warning ("If this workspace was created with a previous" + & " alr version you may need to recreate it."); + return Roots.New_Invalid_Root.With_Reason ("Failed to load " & File & ": " & Errors.Get (E)); end; diff --git a/src/alr/alr-commands.adb b/src/alr/alr-commands.adb index fbea8c98..a7204b24 100644 --- a/src/alr/alr-commands.adb +++ b/src/alr/alr-commands.adb @@ -3,6 +3,7 @@ with AAA.Text_IO; with Ada.Characters.Handling; use Ada.Characters.Handling; with Ada.Command_Line; +with Ada.Directories; with Ada.Text_IO; use Ada.Text_IO; with Alire_Early_Elaboration; @@ -445,6 +446,8 @@ package body Alr.Commands is ---------------------------- procedure Requires_Valid_Session is + use Alire; + Checked : constant Alire.Roots.Root := Alire.Roots.Check_Valid (Root.Current); begin @@ -453,15 +456,27 @@ package body Alr.Commands is ("Cannot continue with invalid session: " & Checked.Invalid_Reason); end if; - -- For workspaces created pre-lockfiles, we create one on the fly - - if OS_Lib.Is_Regular_File (Checked.Lock_File) then - return; - end if; + -- For workspaces created pre-lockfiles, or with older format, recreate: + + case Lockfiles.Validity (Checked.Lock_File) is + when Lockfiles.Valid => + Trace.Debug ("Lockfile at " & Checked.Lock_File & " is valid"); + return; -- OK + when Lockfiles.Invalid => + Trace.Warning + ("This workspace was created with a previous alr version." + & " 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); + Ada.Directories.Delete_File (Checked.Lock_File); + when Lockfiles.Missing => + Trace.Debug ("Workspace has no lockfile at " & Checked.Lock_File); + end case; -- Solve current root dependencies to create the lock file - Trace.Debug ("Missing lockfile, generating it on the fly..."); + Trace.Debug ("Generating lockfile on the fly..."); declare Solution : constant Alire.Solutions.Solution := diff --git a/testsuite/tests/misc/bad-lockfile/test.py b/testsuite/tests/misc/bad-lockfile/test.py new file mode 100644 index 00000000..b6d1fd62 --- /dev/null +++ b/testsuite/tests/misc/bad-lockfile/test.py @@ -0,0 +1,30 @@ +""" +A test that a bad lockfile is recovered from (losing pins) +""" + +from drivers.alr import run_alr + +import os +import re + +# Create a new crate +run_alr('init', '--bin', 'xxx') + +# And muck its lockfile +os.chdir('xxx') + +BADLINE = "SHOULND'T BE HERE" +FILE = "alire/xxx.lock" + +with open(FILE, "a") as myfile: + myfile.write(BADLINE) + +# Run a command that requires the lockfile +p = run_alr('show') + +# Now the lockfile should be recreated and proper, so check no bad line: +with open(FILE, 'r') as myfile: + for line in myfile: + assert line != BADLINE, "Unexpected line found in file" + +print('SUCCESS') diff --git a/testsuite/tests/misc/bad-lockfile/test.yaml b/testsuite/tests/misc/bad-lockfile/test.yaml new file mode 100644 index 00000000..32c747b3 --- /dev/null +++ b/testsuite/tests/misc/bad-lockfile/test.yaml @@ -0,0 +1 @@ +driver: python-script diff --git a/testsuite/tests/misc/bad-tomlfile/test.py b/testsuite/tests/misc/bad-tomlfile/test.py new file mode 100644 index 00000000..96e8e51c --- /dev/null +++ b/testsuite/tests/misc/bad-tomlfile/test.py @@ -0,0 +1,28 @@ +""" +Check that a bad crate file is warned about +""" + +from drivers.alr import run_alr +from drivers.asserts import assert_match + +import os +import re + +# Create a new crate +run_alr('init', '--bin', 'xxx') + +# And muck its tomlfile +os.chdir('xxx') + +with open("alire/xxx.toml", "a") as myfile: + myfile.write("SHOULND'T BE HERE") + +# Verify that the expected warning is given +p = run_alr('show', complain_on_error=False, quiet=False) # Let warn through + +assert_match('.*Could not load crate information from.*' + 'If this workspace was created with a' + ' previous alr version you may need to recreate it.*', + p.out, flags=re.S) + +print('SUCCESS') diff --git a/testsuite/tests/misc/bad-tomlfile/test.yaml b/testsuite/tests/misc/bad-tomlfile/test.yaml new file mode 100644 index 00000000..32c747b3 --- /dev/null +++ b/testsuite/tests/misc/bad-tomlfile/test.yaml @@ -0,0 +1 @@ +driver: python-script -- 2.39.5