From c9cd2d9eeb7217f9cb3fdecafdd875ada80f0ff6 Mon Sep 17 00:00:00 2001 From: Alejandro R Mosteo Date: Fri, 19 Jun 2020 19:12:46 +0200 Subject: [PATCH] Fix a bug in the dependency checkout logic (#440) This bug would manifest when a regular release other than the root one in the solution depends on a linked dependency. With the proposed change we should be covered for any future dependency kind that is not fulfilled by a regular release. --- src/alire/alire-workspace.adb | 31 +++++++-------- .../indirect-link/my_index/index/index.toml | 1 + .../my_index/index/ti/tier1.toml | 10 +++++ .../my_index/index/ti/tier2.toml | 10 +++++ .../my_index/index/ti/tier3.toml | 10 +++++ testsuite/tests/get/indirect-link/test.py | 39 +++++++++++++++++++ testsuite/tests/get/indirect-link/test.yaml | 4 ++ 7 files changed, 89 insertions(+), 16 deletions(-) create mode 100644 testsuite/tests/get/indirect-link/my_index/index/index.toml create mode 100644 testsuite/tests/get/indirect-link/my_index/index/ti/tier1.toml create mode 100644 testsuite/tests/get/indirect-link/my_index/index/ti/tier2.toml create mode 100644 testsuite/tests/get/indirect-link/my_index/index/ti/tier3.toml create mode 100644 testsuite/tests/get/indirect-link/test.py create mode 100644 testsuite/tests/get/indirect-link/test.yaml diff --git a/src/alire/alire-workspace.adb b/src/alire/alire-workspace.adb index dac19382..d2336638 100644 --- a/src/alire/alire-workspace.adb +++ b/src/alire/alire-workspace.adb @@ -14,8 +14,6 @@ with GNATCOLL.VFS; package body Alire.Workspace is - package States renames Dependencies.States; - use type Conditional.Dependencies; ------------------------- @@ -42,20 +40,17 @@ package body Alire.Workspace is Alire.Lockfiles.Write (Solution, Root.Lock_File); - -- Mark any unsolved / hinted dependencies as already deployed (in - -- practice, we don't have to deploy them). - - for Dep of Solution.Dependencies_That (States.Is_Missing'Access) - loop - Deployed.Include (Dep.Crate); - end loop; + -- Mark any dependencies without a corresponding regular release as + -- already deployed (in practice, we don't have to deploy them, and + -- dependents don't need to wait for their deployment). - for Dep of Solution.Dependencies_That (States.Is_Hinted'Access) - loop - Deployed.Include (Dep.Crate); + for Dep of Solution.Required loop + if not Dep.Is_Solved then + Deployed.Include (Dep.Crate); + end if; end loop; - -- Deploy resolved dependencies: + -- Deploy regular resolved dependencies: while not Pending.Is_Empty loop Round := Round + 1; @@ -71,9 +66,10 @@ package body Alire.Workspace is for Rel of Pending loop - -- Identify releases that don't have undeployed dependencies. - -- Until we track who introduces what dependency, we fall back - -- to enumerating all dependencies of a release on platform. + -- In the 1st step of each round we identify releases that + -- don't have undeployed dependencies. Until we track who + -- introduces what dependency, we fall back to enumerating + -- all dependencies of a release on platform. if (for some Dep of Enum (Rel.Dependencies (Env)) => not Deployed.Contains (Dep.Crate)) @@ -95,6 +91,9 @@ package body Alire.Workspace is end if; end loop; + -- In the 2nd step of each round we mark as deployed all releases + -- that were deployed in the 1st step of the round. + if To_Remove.Is_Empty then raise Program_Error with "No release checked-out in round" & Round'Img; diff --git a/testsuite/tests/get/indirect-link/my_index/index/index.toml b/testsuite/tests/get/indirect-link/my_index/index/index.toml new file mode 100644 index 00000000..7c969026 --- /dev/null +++ b/testsuite/tests/get/indirect-link/my_index/index/index.toml @@ -0,0 +1 @@ +version = "0.2" diff --git a/testsuite/tests/get/indirect-link/my_index/index/ti/tier1.toml b/testsuite/tests/get/indirect-link/my_index/index/ti/tier1.toml new file mode 100644 index 00000000..aa550a58 --- /dev/null +++ b/testsuite/tests/get/indirect-link/my_index/index/ti/tier1.toml @@ -0,0 +1,10 @@ +[general] +description = "top-level crate" +maintainers = ["john@doe.com"] +maintainers-logins = ["mylogin"] +licenses = [] + +depends-on.tier2 = "*" + +['1.0'] +origin = "file://." diff --git a/testsuite/tests/get/indirect-link/my_index/index/ti/tier2.toml b/testsuite/tests/get/indirect-link/my_index/index/ti/tier2.toml new file mode 100644 index 00000000..cc943018 --- /dev/null +++ b/testsuite/tests/get/indirect-link/my_index/index/ti/tier2.toml @@ -0,0 +1,10 @@ +[general] +description = "2nd-level crate" +maintainers = ["john@doe.com"] +maintainers-logins = ["mylogin"] +licenses = [] + +depends-on.tier3 = "*" + +['1.0'] +origin = "file://." diff --git a/testsuite/tests/get/indirect-link/my_index/index/ti/tier3.toml b/testsuite/tests/get/indirect-link/my_index/index/ti/tier3.toml new file mode 100644 index 00000000..cc943018 --- /dev/null +++ b/testsuite/tests/get/indirect-link/my_index/index/ti/tier3.toml @@ -0,0 +1,10 @@ +[general] +description = "2nd-level crate" +maintainers = ["john@doe.com"] +maintainers-logins = ["mylogin"] +licenses = [] + +depends-on.tier3 = "*" + +['1.0'] +origin = "file://." diff --git a/testsuite/tests/get/indirect-link/test.py b/testsuite/tests/get/indirect-link/test.py new file mode 100644 index 00000000..b1a61186 --- /dev/null +++ b/testsuite/tests/get/indirect-link/test.py @@ -0,0 +1,39 @@ +""" +Check that an indirect dependency on a linked dir is deployed correctly +""" + +import re +import os + +from drivers.alr import run_alr +from drivers.asserts import assert_match + +# The crux of this test is to have a dependency that in turn depends on a +# linked dir: root -> tier1 -> tier2, where tier2 is linked or missing. +# For the bug to manifest, root must already depend on tier2 when tier1 is +# added. So the test adds root -> tier2, and later root -> tier1, causing: +# root -> tier1 -> tier2 +# └----------------^ +# This indeed does not work with alr versions pre- this PR. + + +# Initialize root workspace +run_alr('init', '--bin', 'xxx') +os.chdir('xxx') + +# Depend on tier2, as a linked idr +run_alr('with', 'tier2', '--use', '/') # path is irrelevant + +# Add tier1 (this is where the bug manifests pre- fix) +run_alr('with', 'tier1') + +# Verify the solution graph looks as expected +p = run_alr('with', '--solve') +assert_match('.*Dependencies \(graph\):\n' + ' tier1=1\.0\.0 --> tier2\*.*' + ' xxx=0\.0\.0 --> tier1=1\.0\.0.*' + ' xxx=0\.0\.0 --> tier2\*.*', + p.out, flags=re.S) + + +print('SUCCESS') diff --git a/testsuite/tests/get/indirect-link/test.yaml b/testsuite/tests/get/indirect-link/test.yaml new file mode 100644 index 00000000..0a859639 --- /dev/null +++ b/testsuite/tests/get/indirect-link/test.yaml @@ -0,0 +1,4 @@ +driver: python-script +indexes: + my_index: + in_fixtures: false -- 2.39.5