From 5641f848ce98be6f4b9060162b6627cbc3be753d Mon Sep 17 00:00:00 2001
From: Alejandro R Mosteo <amosteo@unizar.es>
Date: Wed, 13 Sep 2023 14:58:58 +0200
Subject: [PATCH] Fix bug in dependency traversal order (#1445)

* Fix bug in solution traversal related to linked releases

* New test

* CI fixes
---
 src/alire/alire-solutions.adb                 | 15 +++++++--
 .../tests/crate_config/no-rebuilds/test.py    |  1 -
 testsuite/tests/solver/traverse-order/test.py | 33 +++++++++++++++++++
 .../tests/solver/traverse-order/test.yaml     |  3 ++
 4 files changed, 48 insertions(+), 4 deletions(-)
 create mode 100644 testsuite/tests/solver/traverse-order/test.py
 create mode 100644 testsuite/tests/solver/traverse-order/test.yaml

diff --git a/src/alire/alire-solutions.adb b/src/alire/alire-solutions.adb
index ce1274cd3..7fce6794f 100644
--- a/src/alire/alire-solutions.adb
+++ b/src/alire/alire-solutions.adb
@@ -1433,6 +1433,7 @@ package body Alire.Solutions is
       Root  : Alire.Releases.Containers.Optional :=
         Alire.Releases.Containers.Optional_Releases.Empty)
    is
+      Rels    : constant Release_Map := This.Releases;
       Pending : State_Map := This.Dependencies;
       Visited : Containers.Crate_Name_Sets.Set;
       Round   : Natural := 0;
@@ -1485,16 +1486,24 @@ package body Alire.Solutions is
 
             for Dep of Pending loop
 
-               if not Dep.Is_Solved then
-                  Trace.Debug ("Round" & Round'Img & ": NOOP "
+               if Dep.Is_Missing then
+                  --  This leaves solved/linked to visit
+                  Trace.Debug ("Round" & Round'Img & ": VISIT ready (missing) "
                                & Dep.Release.Milestone.Image);
 
                   To_Remove.Insert (Dep.Crate, Dep);
 
                elsif
+                 --  Some dependency is still unvisited, either under its own
+                 --  name or through some alias. These nested fors may merit
+                 --  optimization in the future?
                  (for some Rel_Dep of Dep.Release.Flat_Dependencies
                     (Alire.Root.Platform_Properties) =>
-                        not Visited.Contains (Rel_Dep.Crate))
+                       not Visited.Contains (Rel_Dep.Crate)
+                       and then
+                       not (for some Rel of Rels =>
+                              Visited.Contains (Rel.Name)
+                              and then Rel.Provides (Rel_Dep.Crate)))
                then
                   Trace.Debug ("Round" & Round'Img & ": SKIP not-ready " &
                                  Dep.Release.Milestone.Image);
diff --git a/testsuite/tests/crate_config/no-rebuilds/test.py b/testsuite/tests/crate_config/no-rebuilds/test.py
index 29b8c16a3..7715f9c24 100644
--- a/testsuite/tests/crate_config/no-rebuilds/test.py
+++ b/testsuite/tests/crate_config/no-rebuilds/test.py
@@ -2,7 +2,6 @@
 Ensure that no unnecessary rebuilds happend due to crate config generation
 """
 
-import os
 from drivers.alr import alr_with, init_local_crate, run_alr
 from drivers.asserts import assert_match
 from drivers.helpers import prepend_to_file
diff --git a/testsuite/tests/solver/traverse-order/test.py b/testsuite/tests/solver/traverse-order/test.py
new file mode 100644
index 000000000..dec303163
--- /dev/null
+++ b/testsuite/tests/solver/traverse-order/test.py
@@ -0,0 +1,33 @@
+"""
+Check that a linked dependency is considered during solution traversal
+"""
+
+from glob import glob
+import re
+from drivers.alr import alr_with, init_local_crate, run_alr
+from drivers.asserts import assert_eq, assert_match
+
+run_alr("get", "libhello")
+init_local_crate()
+run_alr("with", f"--use={glob('../libhello_*')[0]}")
+alr_with("hello") # Libhello is already linked
+
+# Verify the solution is as expected
+p = run_alr("with", "--solve", quiet=False)
+assert_match(".*" + re.escape("""\
+Dependencies (solution):
+   hello=1.0.1 (origin: filesystem)
+   libhello=1.0.0 (pinned) (origin: ../libhello_1.0.0_filesystem)
+"""),
+             p.out)
+
+# Verify that linked libhello prevents visiting hello too soon. This depends on
+# debug output which is not very nice but there's no simple alternative.
+p = run_alr("-vv", "build", quiet=False)
+
+assert_match(".*Round 1: SKIP not-ready hello=1.0.1"
+             ".*Round 1: VISIT ready libhello=1.0.0"
+             ".*Round 2: VISIT ready hello=1.0.1",
+             p.out)
+
+print('SUCCESS')
diff --git a/testsuite/tests/solver/traverse-order/test.yaml b/testsuite/tests/solver/traverse-order/test.yaml
new file mode 100644
index 000000000..872fc1274
--- /dev/null
+++ b/testsuite/tests/solver/traverse-order/test.yaml
@@ -0,0 +1,3 @@
+driver: python-script
+indexes:
+    basic_index: {}