From b60d08881bdfecb6b81d484d2591ac28acf1ba8a Mon Sep 17 00:00:00 2001 From: Alejandro R Mosteo Date: Thu, 19 Aug 2021 09:44:58 +0200 Subject: [PATCH] Move the lockfile inside `$crate/alire` (#789) * Lockfile silently migrated to $crate/alire * testsuite: new test for the silent migration * testsuite: fixes for existing tests * user-changes.md: document the new lock location --- doc/user-changes.md | 17 +++++++ src/alire/alire-lockfiles.adb | 4 +- src/alire/alire-lockfiles.ads | 2 + src/alire/alire-roots.adb | 35 +++++++++++++- testsuite/drivers/alr.py | 2 +- testsuite/drivers/helpers.py | 11 +++++ .../get/external-tool-dependency/test.py | 2 +- testsuite/tests/get/git-local/test.py | 2 +- testsuite/tests/get/unpack-in-place/test.py | 2 +- testsuite/tests/misc/bad-lockfile/test.py | 7 ++- testsuite/tests/misc/lockfile-hiding/test.py | 48 +++++++++++++++++++ .../tests/misc/lockfile-hiding/test.yaml | 3 ++ testsuite/tests/misc/sync-manual-edit/test.py | 4 +- testsuite/tests/pin/downgrade/test.py | 6 +-- testsuite/tests/pin/post-update/test.py | 6 +-- testsuite/tests/pin/unpin/test.py | 4 +- testsuite/tests/update/manual-once/test.py | 6 +-- .../tests/workflows/init-options/test.py | 11 +++-- 18 files changed, 143 insertions(+), 29 deletions(-) create mode 100644 testsuite/tests/misc/lockfile-hiding/test.py create mode 100644 testsuite/tests/misc/lockfile-hiding/test.yaml diff --git a/doc/user-changes.md b/doc/user-changes.md index c6861c8d..4c6c7ba5 100644 --- a/doc/user-changes.md +++ b/doc/user-changes.md @@ -4,6 +4,23 @@ This document is a development diary summarizing changes in `alr` that notably affect the user experience. It is intended as a one-stop point for users to stay on top of `alr` new features. +### Lockfile moved to `alire` folder + +PR [#789](https://github.com/alire-project/alire/pull/789) + +The lock file (`alire.lock`) is now a purely internal file, regenerated +as needed from scratch, and needs not be put under version control. Since, +furthermore, this file is not intended for user edition or inspection, it is +now created inside the `alire` folder of a crate. + +Existing lock files at the root of a crate will be automatically migrated to +their new location the first time an `alr` command that uses the lock file is +run inside a crate with the old situation. + +This change obsoletes the recommendation that accompanied PR +[#501](https://github.com/alire-project/alire/pull/501) about putting the lock +file under version control. + ## Release `1.1` ### Conflicting releases diff --git a/src/alire/alire-lockfiles.adb b/src/alire/alire-lockfiles.adb index 9e8eb80d..762f7771 100644 --- a/src/alire/alire-lockfiles.adb +++ b/src/alire/alire-lockfiles.adb @@ -2,6 +2,7 @@ with Ada.Directories; with Ada.Text_IO; with Alire.Directories; +with Alire.Paths; with Alire.TOML_Load; with TOML.File_IO; @@ -27,7 +28,7 @@ package body Alire.Lockfiles is --------------- function File_Name (Root_Dir : Any_Path) return Any_Path - is (Root_Dir / "alire.lock"); + is (Root_Dir / Paths.Working_Folder_Inside_Root / Simple_Name); --------------- -- From_TOML -- @@ -125,6 +126,7 @@ package body Alire.Lockfiles is begin Trace.Debug ("Dumping lockfile contents to " & Filename); + Directories.Create_Tree (Directories.Parent (Filename)); Create (File, Out_File, Filename); Put_Line (File, "# THIS IS A MACHINE-GENERATED FILE. DO NOT EDIT MANUALLY."); diff --git a/src/alire/alire-lockfiles.ads b/src/alire/alire-lockfiles.ads index f3800032..2e783596 100644 --- a/src/alire/alire-lockfiles.ads +++ b/src/alire/alire-lockfiles.ads @@ -6,6 +6,8 @@ with TOML; package Alire.Lockfiles is + Simple_Name : constant String := "alire.lock"; + type Validities is (Missing, Invalid, Valid); -- The lockfile stores persistent private information read/written by diff --git a/src/alire/alire-roots.adb b/src/alire/alire-roots.adb index a05f84f5..e8e290ec 100644 --- a/src/alire/alire-roots.adb +++ b/src/alire/alire-roots.adb @@ -722,6 +722,39 @@ package body Alire.Roots is end if; end Release_Base; + ---------------------- + -- Migrate_Lockfile -- + ---------------------- + -- This function is intended to migrate lockfiles in the old root location + -- to inside the alire folder. It could be conceivably removed down the + -- line during a major release. + function Migrate_Lockfile (This : Root; + Path : Any_Path) + return Any_Path + is + package Adirs renames Ada.Directories; + Old_Path : constant Any_Path := + Adirs.Containing_Directory + (Adirs.Containing_Directory (Path)) + / Lockfiles.Simple_Name; + begin + if Adirs.Exists (Old_Path) then + Directories.Backup_If_Existing (Old_Path, + Base_Dir => This.Working_Folder); + + if Adirs.Exists (Path) then + Put_Info ("Removing old lockfile at " & TTY.URL (Old_Path)); + Adirs.Delete_File (Old_Path); + else + Put_Info ("Migrating lockfile from " + & TTY.URL (Old_Path) & " to " & TTY.URL (Path)); + Adirs.Rename (Old_Path, Path); + end if; + end if; + + return Path; + end Migrate_Lockfile; + --------------- -- Lock_File -- --------------- @@ -729,7 +762,7 @@ package body Alire.Roots is function Lock_File (This : Root) return Absolute_Path is (if This.Lockfile /= "" then +This.Lockfile - else Lockfiles.File_Name (+This.Path)); + else Migrate_Lockfile (This, Lockfiles.File_Name (+This.Path))); ---------------- -- Crate_File -- diff --git a/testsuite/drivers/alr.py b/testsuite/drivers/alr.py index 75e86ff3..5e8fd7a3 100644 --- a/testsuite/drivers/alr.py +++ b/testsuite/drivers/alr.py @@ -211,7 +211,7 @@ def init_local_crate(name="xxx", binary=True, enter=True): def alr_lockfile(): - return "alire.lock" + return os.path.join("alire", "alire.lock") def alr_manifest(): diff --git a/testsuite/drivers/helpers.py b/testsuite/drivers/helpers.py index 6d4097da..bb5c322a 100644 --- a/testsuite/drivers/helpers.py +++ b/testsuite/drivers/helpers.py @@ -5,6 +5,7 @@ Assorted helpers that are reused by several tests. from subprocess import run from zipfile import ZipFile +import hashlib import re import os import platform @@ -152,3 +153,13 @@ def zip_dir(path, filename): for file in files: abs_file = os.path.join(dir, file) zip.write(abs_file, abs_file) + + +def md5sum(file): + """Return the hex md5 hash of a file""" + with open(file, "rb") as f: + file_hash = hashlib.md5() + while chunk := f.read(8192): + file_hash.update(chunk) + + return file_hash.hexdigest() diff --git a/testsuite/tests/get/external-tool-dependency/test.py b/testsuite/tests/get/external-tool-dependency/test.py index 811abb69..8b7009dd 100644 --- a/testsuite/tests/get/external-tool-dependency/test.py +++ b/testsuite/tests/get/external-tool-dependency/test.py @@ -28,8 +28,8 @@ for elt in dir_content: # Check folder contents compare(dir_content, ['main_1.0.0_filesystem/alire', - 'main_1.0.0_filesystem/alire.lock', 'main_1.0.0_filesystem/alire.toml', + 'main_1.0.0_filesystem/alire/alire.lock', 'main_1.0.0_filesystem/alire/cache', 'main_1.0.0_filesystem/alire/cache/dependencies', make_dep_dir, diff --git a/testsuite/tests/get/git-local/test.py b/testsuite/tests/get/git-local/test.py index 3c9bced3..fc99ec2c 100644 --- a/testsuite/tests/get/git-local/test.py +++ b/testsuite/tests/get/git-local/test.py @@ -17,8 +17,8 @@ compare(list(filter contents('libfoo_1.0.0_9ddda32b'))), ['libfoo_1.0.0_9ddda32b/a', 'libfoo_1.0.0_9ddda32b/alire', - 'libfoo_1.0.0_9ddda32b/alire.lock', 'libfoo_1.0.0_9ddda32b/alire.toml', + 'libfoo_1.0.0_9ddda32b/alire/alire.lock', 'libfoo_1.0.0_9ddda32b/b', 'libfoo_1.0.0_9ddda32b/b/x', 'libfoo_1.0.0_9ddda32b/b/y', diff --git a/testsuite/tests/get/unpack-in-place/test.py b/testsuite/tests/get/unpack-in-place/test.py index c274f595..76e598f0 100644 --- a/testsuite/tests/get/unpack-in-place/test.py +++ b/testsuite/tests/get/unpack-in-place/test.py @@ -10,8 +10,8 @@ from drivers.helpers import compare, contents run_alr('get', 'libhello=1.0.0-tarball') compare(contents('libhello_1.0.0_filesystem'), ['libhello_1.0.0_filesystem/alire', - 'libhello_1.0.0_filesystem/alire.lock', 'libhello_1.0.0_filesystem/alire.toml', + 'libhello_1.0.0_filesystem/alire/alire.lock', 'libhello_1.0.0_filesystem/config', 'libhello_1.0.0_filesystem/config/libhello_config.ads', 'libhello_1.0.0_filesystem/config/libhello_config.gpr', diff --git a/testsuite/tests/misc/bad-lockfile/test.py b/testsuite/tests/misc/bad-lockfile/test.py index 48d789fb..95b5accb 100644 --- a/testsuite/tests/misc/bad-lockfile/test.py +++ b/testsuite/tests/misc/bad-lockfile/test.py @@ -2,7 +2,7 @@ A test that a bad lockfile is recovered from (losing pins) """ -from drivers.alr import run_alr +from drivers.alr import run_alr, alr_lockfile import os import re @@ -14,16 +14,15 @@ run_alr('init', '--bin', 'xxx') os.chdir('xxx') BADLINE = "SHOULND'T BE HERE" -FILE = "alire.lock" -with open(FILE, "a") as myfile: +with open(alr_lockfile(), "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: +with open(alr_lockfile(), 'r') as myfile: for line in myfile: assert line != BADLINE, "Unexpected line found in file" diff --git a/testsuite/tests/misc/lockfile-hiding/test.py b/testsuite/tests/misc/lockfile-hiding/test.py new file mode 100644 index 00000000..db70b675 --- /dev/null +++ b/testsuite/tests/misc/lockfile-hiding/test.py @@ -0,0 +1,48 @@ +""" +Verify that lockfiles are properly [re]moved from $crate/ to $crate/alire/ +""" + +import os + +from drivers.alr import run_alr, init_local_crate, alr_lockfile, alr_with +# from drivers.asserts import assert_eq, assert_match +from drivers.helpers import content_of +from os.path import isfile + + +old_lockfile = "alire.lock" + +init_local_crate() +# Add a dependency so the lockfile is not the same as the default one +default_content = content_of(alr_lockfile()) +alr_with("libhello") +proper_content = content_of(alr_lockfile()) +assert default_content != proper_content, \ + "error setting up the test (contents should differ)" + + +def verify(): + """ + Check that the lockfile is only in the new location, with proper contents + """ + assert isfile(alr_lockfile()) and not isfile(old_lockfile), \ + "Unexpected lockfile placement" + assert content_of(alr_lockfile()) == proper_content, "bad contents" + + +# Case 1: lockfile is only in the old location, and it is moved to new location + +# First, get the current lockfile and put it in the old location: +os.rename(alr_lockfile(), old_lockfile) +run_alr("show") # Triggers migration +verify() + +# Case 2: have an old lockfile that should be discarded without being copied, +# as the new lockfile is already in place (at the end of Case 1): +with open(old_lockfile, "wt") as file: + file.write(default_content) +run_alr("show") +verify() + + +print('SUCCESS') diff --git a/testsuite/tests/misc/lockfile-hiding/test.yaml b/testsuite/tests/misc/lockfile-hiding/test.yaml new file mode 100644 index 00000000..872fc127 --- /dev/null +++ b/testsuite/tests/misc/lockfile-hiding/test.yaml @@ -0,0 +1,3 @@ +driver: python-script +indexes: + basic_index: {} diff --git a/testsuite/tests/misc/sync-manual-edit/test.py b/testsuite/tests/misc/sync-manual-edit/test.py index b926f7e3..fe355f20 100644 --- a/testsuite/tests/misc/sync-manual-edit/test.py +++ b/testsuite/tests/misc/sync-manual-edit/test.py @@ -5,7 +5,7 @@ other commands that require a valid workspace import os.path -from drivers.alr import run_alr +from drivers.alr import run_alr, alr_touch_manifest from shutil import rmtree # from drivers.asserts import assert_eq, assert_match @@ -25,7 +25,7 @@ for cmd in ['build', 'pin', 'run', 'show', 'with', 'printenv']: file.write('[[depends-on]]\nlibhello="1"') # Make the lockfile "older" (otherwise timestamp is identical) - os.utime('alire.lock', (0, 0)) + alr_touch_manifest() # Run the command run_alr(cmd) diff --git a/testsuite/tests/pin/downgrade/test.py b/testsuite/tests/pin/downgrade/test.py index 0e264667..736779c7 100644 --- a/testsuite/tests/pin/downgrade/test.py +++ b/testsuite/tests/pin/downgrade/test.py @@ -4,7 +4,7 @@ Test that a pin to a lower version downgrades and retrieves the new version import os -from drivers.alr import run_alr, alr_pin +from drivers.alr import run_alr, alr_pin, alr_lockfile from drivers.asserts import assert_eq, assert_match from drivers.helpers import check_line_in @@ -22,8 +22,8 @@ def check_child(version, output, pinned): output, flags=re.S) # Verify lockfile - check_line_in('alire.lock', 'name = "libchild"') - check_line_in('alire.lock', f'version = "{version}"') + check_line_in(alr_lockfile(), 'name = "libchild"') + check_line_in(alr_lockfile(), f'version = "{version}"') # Verify dependency folders assert os.path.exists('alire/cache/dependencies/libchild_' + version + diff --git a/testsuite/tests/pin/post-update/test.py b/testsuite/tests/pin/post-update/test.py index 898c9ee0..da7d7d3e 100644 --- a/testsuite/tests/pin/post-update/test.py +++ b/testsuite/tests/pin/post-update/test.py @@ -4,7 +4,7 @@ Test that a pinned release does not get updated import os -from drivers.alr import run_alr, alr_pin, alr_unpin +from drivers.alr import run_alr, alr_pin, alr_unpin, alr_lockfile from drivers.asserts import assert_eq, assert_match from drivers.helpers import check_line_in @@ -23,8 +23,8 @@ def check_child(version, output, pinned): output, flags=re.S) # Verify lockfile - check_line_in('alire.lock', 'name = "libchild"') - check_line_in('alire.lock', f'version = "{version}"') + check_line_in(alr_lockfile(), 'name = "libchild"') + check_line_in(alr_lockfile(), f'version = "{version}"') # Create a new "xxx" program project diff --git a/testsuite/tests/pin/unpin/test.py b/testsuite/tests/pin/unpin/test.py index 3baa6b3e..536435f6 100644 --- a/testsuite/tests/pin/unpin/test.py +++ b/testsuite/tests/pin/unpin/test.py @@ -4,7 +4,7 @@ Test unpinning import os -from drivers.alr import run_alr, alr_pin, alr_unpin +from drivers.alr import run_alr, alr_pin, alr_unpin, alr_lockfile from drivers.asserts import assert_eq from drivers.helpers import check_line_in @@ -27,7 +27,7 @@ p = run_alr('pin') assert_eq('libhello 1.0.0\n', p.out) # Delete lockfile and verify the pin has survived -os.remove('alire.lock') +os.remove(alr_lockfile()) p = run_alr('pin') assert_eq('libhello 1.0.0\n', p.out) diff --git a/testsuite/tests/update/manual-once/test.py b/testsuite/tests/update/manual-once/test.py index 50bb5fc8..3242c317 100644 --- a/testsuite/tests/update/manual-once/test.py +++ b/testsuite/tests/update/manual-once/test.py @@ -2,7 +2,7 @@ Verify that, after manually touching the manifest, updates happen only once """ -from drivers.alr import run_alr, init_local_crate +from drivers.alr import run_alr, init_local_crate, alr_touch_manifest from drivers.asserts import assert_eq, assert_match import os @@ -17,9 +17,7 @@ import os def prepare_crate(name): """Prepare a crate with outdated lockfile""" init_local_crate(name) - # Set the modification time of the lockfile behind that of the manifest - info = os.stat("alire.toml") - os.utime("alire.lock", (info.st_atime, info.st_mtime - 1)) + alr_touch_manifest() warning_text = "Synchronizing workspace" diff --git a/testsuite/tests/workflows/init-options/test.py b/testsuite/tests/workflows/init-options/test.py index 5ee8eb1a..ef80315d 100644 --- a/testsuite/tests/workflows/init-options/test.py +++ b/testsuite/tests/workflows/init-options/test.py @@ -19,8 +19,8 @@ assert_match(".*Identifiers must be.*", p.out) run_alr('init', '--bin', 'xxx') compare(contents('xxx'), ['xxx/.gitignore', 'xxx/alire', - 'xxx/alire.lock', 'xxx/alire.toml', + 'xxx/alire/alire.lock', 'xxx/config', 'xxx/config/xxx_config.gpr', 'xxx/src', @@ -33,8 +33,8 @@ run_alr('init', '--bin', 'aaa') compare(contents('aaa'), ['aaa/.gitignore', 'aaa/aaa.gpr', 'aaa/alire', - 'aaa/alire.lock', 'aaa/alire.toml', + 'aaa/alire/alire.lock', 'aaa/config', 'aaa/config/aaa_config.gpr', 'aaa/src', @@ -43,8 +43,9 @@ compare(contents('aaa'), ['aaa/.gitignore', # Init without skeleton run_alr('init', '--bin', '--no-skel', 'yyy') compare(contents('yyy'), ['yyy/alire', - 'yyy/alire.lock', - 'yyy/alire.toml']) + 'yyy/alire.toml', + 'yyy/alire/alire.lock' + ]) # Init with existing crate os.chdir('yyy') @@ -71,8 +72,8 @@ os.chdir('zzz') run_alr('init', '--bin', '--in-place', 'zzz') compare(contents('.'), ['./.gitignore', './alire', - './alire.lock', './alire.toml', + './alire/alire.lock', './config', './config/zzz_config.gpr', './src', -- 2.39.5