From 88ad24e07c176b531ce73b086236b12fbbe06000 Mon Sep 17 00:00:00 2001 From: Alejandro R Mosteo Date: Mon, 24 Feb 2025 15:08:06 +0100 Subject: [PATCH] fix: detect changes in manifests of linked dependencies (#1860) * Test that triggers bug * Check linked manifests for changes * Self-review * Fix test --- src/alire/alire-roots.adb | 63 +++++++++++++++-- src/alire/alire-roots.ads | 18 ++++- testsuite/drivers/helpers.py | 9 +++ .../misc/sync-manual-edit-indirect/test.py | 67 +++++++++++++++++++ .../misc/sync-manual-edit-indirect/test.yaml | 4 ++ 5 files changed, 154 insertions(+), 7 deletions(-) create mode 100644 testsuite/tests/misc/sync-manual-edit-indirect/test.py create mode 100644 testsuite/tests/misc/sync-manual-edit-indirect/test.yaml diff --git a/src/alire/alire-roots.adb b/src/alire/alire-roots.adb index 9995735c..9a3a2b4c 100644 --- a/src/alire/alire-roots.adb +++ b/src/alire/alire-roots.adb @@ -1556,12 +1556,23 @@ package body Alire.Roots is end; elsif This.Solution.State (Crate).Is_Linked then return This.Solution.State (Crate).Link.Path; + -- Usage doesn't matter in this case, the crate is where it is else raise Program_Error with "release must be either solved or linked"; end if; end Release_Base; + ---------------------- + -- Release_Manifest -- + ---------------------- + + function Release_Manifest (This : in out Root; + Crate : Crate_Name; + Usage : Usages) + return Absolute_File + is (This.Release_Base (Crate, Usage) / Paths.Crate_File_Name); + ---------------------- -- Migrate_Lockfile -- ---------------------- @@ -1703,6 +1714,48 @@ package body Alire.Roots is File_Time_Stamp (This.Crate_File) > File_Time_Stamp (This.Lock_File); end Is_Lockfile_Outdated; + -------------------- + -- Outdated_Links -- + -------------------- + + function Has_Outdated_Links (This : in out Root) return Boolean is + use GNAT.OS_Lib; + Changed : AAA.Strings.Set; + begin + + -- If we do not even have a lockfile, for sure we must update + + if not This.Has_Lockfile then + return True; + end if; + + -- Otherwise, check manifests of linked dependencies. Note that we do + -- not care about their lockfiles, which may well be outdated; we care + -- that the user has modified the dependency info in the manifest and + -- thus *we* need updating. The lockfile in the dependency will be + -- automatically updated if it ever is used as the root. + + for Dep of This.Solution.All_Dependencies loop + if Dep.Is_Linked and then Dep.Has_Release + and then + File_Time_Stamp (This.Release_Manifest (Dep.Crate, For_Build)) > + File_Time_Stamp (This.Lock_File) + then + Trace.Debug ("Changes detected in pinned dependency: " + & Dep.Crate.TTY_Image); + Changed.Include (Dep.Crate.As_String); + end if; + end loop; + + if Changed.Is_Empty then + return False; + else + Put_Info ("Changes detected in pinned dependencies: " + & Changed.To_Vector.Flatten (", ")); + return True; + end if; + end Has_Outdated_Links; + ------------- -- Is_Root -- ------------- @@ -1731,12 +1784,10 @@ package body Alire.Roots is Force : Boolean := False) is begin - if Force or else This.Is_Lockfile_Outdated then - -- TODO: we may want to recursively check manifest timestamps of - -- linked crates to detect changes in these manifests and re-resolve. - -- Otherwise a manual `alr update` is needed to detect these changes. - -- This would imply to store the timestamps in our lockfile for - -- linked crates with a manifest. + if Force + or else This.Is_Lockfile_Outdated + or else This.Has_Outdated_Links + then Put_Info ("Synchronizing workspace..."); diff --git a/src/alire/alire-roots.ads b/src/alire/alire-roots.ads index 65ec0d09..2893efec 100644 --- a/src/alire/alire-roots.ads +++ b/src/alire/alire-roots.ads @@ -125,7 +125,11 @@ package Alire.Roots is Post => Release'Result.Provides (Crate); -- Retrieve a release, that can be either the root or any in the solution - type Usages is (For_Deploy, For_Build); + type Usages is (For_Deploy, + -- Deployments are to the vault, a specific write-once dir + For_Build + -- Builds take place in a copy located in the builds dir + ); function Release_Parent (This : in out Root; Rel : Releases.Release; @@ -141,6 +145,12 @@ package Alire.Roots is return Absolute_Path; -- Find the base folder in which a release can be found for the given root + function Release_Manifest (This : in out Root; + Crate : Crate_Name; + Usage : Usages) + return Absolute_File; + -- Return the full path to the manifest of a release in the solution + function Requires_Build_Sync (This : in out Root; Rel : Releases.Release) return Boolean @@ -176,6 +186,12 @@ package Alire.Roots is -- conceivably we could use checksums to make it more robust against -- automated changes within the same second. + function Has_Outdated_Links (This : in out Root) return Boolean; + -- Check whether any linked dependency has a more recent manifest than + -- ours. If so, that means the user has edited a linked dependency and + -- we need to update. TL;DR: True if we need to update because a linked + -- dependency has changed. + function Is_Root_Release (This : in out Root; Dep : Dependencies.States.State) return Boolean; diff --git a/testsuite/drivers/helpers.py b/testsuite/drivers/helpers.py index 70ec398f..9ae92a69 100644 --- a/testsuite/drivers/helpers.py +++ b/testsuite/drivers/helpers.py @@ -135,6 +135,15 @@ def host_os(): host_os = 'macos' return host_os + +def offset_timestamp(file, seconds): + """ + Add offset to the modification time of a file + """ + os.utime(file, (os.path.getatime(file), + os.path.getmtime(file) + seconds)) + + # Add a 'with "something";' at the top of a project file def with_project(file, project): with open(file, 'r+') as f: diff --git a/testsuite/tests/misc/sync-manual-edit-indirect/test.py b/testsuite/tests/misc/sync-manual-edit-indirect/test.py new file mode 100644 index 00000000..230bbe1a --- /dev/null +++ b/testsuite/tests/misc/sync-manual-edit-indirect/test.py @@ -0,0 +1,67 @@ +""" +Verify that manual changes to the manifest of a linked dependency result in +an automatic update before other commands that require a valid workspace. This +was bug https://github.com/alire-project/alire/issues/1662 +""" + +import os.path + +from drivers.alr import alr_with, init_local_crate, run_alr +from drivers.asserts import assert_substring +from drivers.helpers import offset_timestamp, prepend_to_file +from shutil import rmtree + + +MAIN_CRATE = "main" +DEP_CRATE = "dep" + +def set_up(): + initial_dir = os.getcwd() + + # Ensure clean state + rmtree(MAIN_CRATE, ignore_errors=True) + rmtree(DEP_CRATE, ignore_errors=True) + + # Indirect crate we will edit + init_local_crate(DEP_CRATE, enter=False) + + # Main crate + init_local_crate(MAIN_CRATE) + alr_with(DEP_CRATE, path=f"../{DEP_CRATE}", update=True) + + # Add new dependency to the linked crate + os.chdir(f"../{DEP_CRATE}") + alr_with("libhello") + prepend_to_file(f"src/{DEP_CRATE}.adb", + ["with Libhello;"]) + + # Some OSes have a granularity of 1 second in file timestamps, so move the + # manifest timestamp of the dependency crate forward in time to ensure + # change detection. + offset_timestamp(file=f"../{DEP_CRATE}/alire.toml", seconds=2.0) + + # Back to the root directory + os.chdir(initial_dir) + + +# After editing the manifest of a linked dependency, run commands that require a +# synchronized workspace. Before the fix, this usually caused errors in the +# logic of dependency deployment (as we were missing dependencies that should be +# in the solution). +for cmd in ['build', 'pin', 'run', 'show', 'with', 'printenv']: + + # Prepare faulty condition + set_up() + + # Run the command in the main crate folder + os.chdir(MAIN_CRATE) + p = run_alr(cmd, quiet=False) + + # If no error was reported, then we should be okay. Still, check that the + # update happened as expected: + assert_substring("Changes detected in pinned dependencies", p.out) + + # Go to where we started + os.chdir("..") + +print('SUCCESS') diff --git a/testsuite/tests/misc/sync-manual-edit-indirect/test.yaml b/testsuite/tests/misc/sync-manual-edit-indirect/test.yaml new file mode 100644 index 00000000..e2c5af22 --- /dev/null +++ b/testsuite/tests/misc/sync-manual-edit-indirect/test.yaml @@ -0,0 +1,4 @@ +driver: python-script +indexes: + basic_index: {} + compiler_only_index: {} -- 2.39.5