From 35d197c56dd6b67049ee0c810a45d10f7ec16f97 Mon Sep 17 00:00:00 2001
From: Alejandro R Mosteo <alejandro@mosteo.com>
Date: Fri, 25 Jun 2021 12:16:08 +0200
Subject: [PATCH] Detect circularities in link pins (#755)

* Detect circularities in link pins

* Test for self-pinning and circularities

* Fix platform-dependent path on Windows
---
 src/alire/alire-roots.adb                 | 33 +++++++++++++++-
 testsuite/tests/pin/circularity/test.py   | 47 +++++++++++++++++++++++
 testsuite/tests/pin/circularity/test.yaml |  3 ++
 3 files changed, 81 insertions(+), 2 deletions(-)
 create mode 100644 testsuite/tests/pin/circularity/test.py
 create mode 100644 testsuite/tests/pin/circularity/test.yaml

diff --git a/src/alire/alire-roots.adb b/src/alire/alire-roots.adb
index 6d80f16b..861ecb14 100644
--- a/src/alire/alire-roots.adb
+++ b/src/alire/alire-roots.adb
@@ -288,8 +288,10 @@ package body Alire.Roots is
         Containers.Crate_Name_Sets.Empty_Set)
    is
 
-      Sol      : Solutions.Solution := This.Solution;
-      Pins_Dir : constant Any_Path := This.Pins_Dir;
+      Sol        : Solutions.Solution := This.Solution;
+      Pins_Dir   : constant Any_Path := This.Pins_Dir;
+      Linkers    : Containers.Crate_Name_Sets.Set;
+      --  We store here crates that contain link pins, to detect cycles
 
       --------------
       -- Add_Pins --
@@ -331,6 +333,20 @@ package body Alire.Roots is
             use type User_Pins.Pin;
          begin
 
+            --  Store the requester of this link to be able to detect cycles,
+            --  and check that the requested link is not already in the list
+            --  of requesters (which would imply circularity).
+
+            Linkers.Include (This.Name);
+
+            if Linkers.Contains (Crate) then
+               Raise_Checked_Error
+                 ("Pin circularity detected when adding pin "
+                  & TTY.Name (This.Name) & " --> " & TTY.Name (Crate)
+                  & ASCII.LF & "Last manifest in the cycle is "
+                  & TTY.URL (This.Crate_File));
+            end if;
+
             --  Just in case this is a remote pin, deploy it. Deploy is
             --  conservative (unless Online), but it will detect local
             --  inexpensive changes like a missing checkout, changed commit
@@ -422,6 +438,8 @@ package body Alire.Roots is
                Pin   :          User_Pins.Pin := Element (I);
             begin
 
+               --  Avoid obvious self-pinning
+
                Trace.Debug ("Crate " & TTY.Name (This.Name)
                             & " adds pin for crate " & TTY.Name (Crate));
 
@@ -470,6 +488,17 @@ package body Alire.Roots is
          Trace.Detail ("Local pins updated and committed to lockfile");
          This.Set (Solution => Sol);
       end if;
+
+   exception
+      when others =>
+         --  In the event that the manifest contains bad pins, we ensure the
+         --  lockfile is outdated so the manifest is not ignored on next run.
+         if Ada.Directories.Exists (This.Lock_File) then
+            Trace.Debug ("Removing lockfile because of bad pins in manifest");
+            Ada.Directories.Delete_File (This.Lock_File);
+         end if;
+
+         raise;
    end Sync_Pins_From_Manifest;
 
    ---------------
diff --git a/testsuite/tests/pin/circularity/test.py b/testsuite/tests/pin/circularity/test.py
new file mode 100644
index 00000000..c1f1dc0d
--- /dev/null
+++ b/testsuite/tests/pin/circularity/test.py
@@ -0,0 +1,47 @@
+"""
+Verify pin circularity detection
+"""
+
+from drivers.alr import run_alr, alr_pin, alr_unpin, init_local_crate
+from drivers.asserts import assert_eq, assert_match
+from drivers.helpers import on_windows, dir_separator
+
+import os
+import re
+import shutil
+
+# Obvious self-pinning detection
+init_local_crate()
+alr_pin("xxx", path=".", update=False)
+p = run_alr("pin", complain_on_error=False)
+assert_match(".*"
+             "ERROR: Pin circularity detected when adding pin xxx --> xxx:\n"
+             "ERROR:    Last manifest in the cycle is .*\n",
+             p.out)
+
+
+# A real cycle detection
+os.chdir("..")    # back to top-level
+shutil.rmtree("xxx")  # and restart from scratch
+
+# Prepare a cycle
+init_local_crate("yyy", enter=False)
+init_local_crate("zzz")
+alr_pin("yyy", path="../yyy", update=False)
+os.chdir("..")
+os.chdir("yyy")
+alr_pin("zzz", path="../zzz", update=False)
+os.chdir("..")
+init_local_crate("xxx")
+alr_pin("yyy", path="../yyy", update=False)
+
+# At this point, xxx --> yyy --> zzz --> yyy
+p = run_alr("pin", complain_on_error=False)
+s = re.escape(dir_separator())
+assert_match(
+    ".*"
+    "ERROR: Pin circularity detected when adding pin zzz --> yyy:\n"
+    f"ERROR:    Last manifest in the cycle is .*{s}zzz{s}alire.toml\n",
+    p.out)
+
+print('SUCCESS')
diff --git a/testsuite/tests/pin/circularity/test.yaml b/testsuite/tests/pin/circularity/test.yaml
new file mode 100644
index 00000000..872fc127
--- /dev/null
+++ b/testsuite/tests/pin/circularity/test.yaml
@@ -0,0 +1,3 @@
+driver: python-script
+indexes:
+    basic_index: {}
-- 
2.39.5