From 0c95742bc70a4d144f5d406c41a29e81727410df Mon Sep 17 00:00:00 2001
From: Francois-Rene Rideau <fare@tunes.org>
Date: Sun, 11 Sep 2016 18:06:54 -0400
Subject: [PATCH] Commment on the bridges between systems and CL modules
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The code was not clear enough why string-downcase is used at places,
and this notably confused Daniel Kochmański. See discussion at
https://gitlab.common-lisp.net/asdf/asdf/merge_requests/13
---
 operate.lisp | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/operate.lisp b/operate.lisp
index 755e997fc..0d994817b 100644
--- a/operate.lisp
+++ b/operate.lisp
@@ -198,21 +198,31 @@ the implementation's REQUIRE rather than by internal ASDF mechanisms."))
       (require module)))
 
   (defmethod resolve-dependency-combination (component (combinator (eql :require)) arguments)
-    (unless (length=n-p arguments 1)
-      (error (compatfmt "~@<Bad dependency ~S for ~S. ~S takes only one argument~@:>")
+    (unless (and (length=n-p arguments 1)
+		 (typep (car argument) '(or string (and symbol (not null)))))
+      (error (compatfmt "~@<Bad dependency ~S for ~S. ~S takes one argument, a string or non-null symbol~@:>")
              (cons combinator arguments) component combinator))
+    ;; :require must be prepared for some implementations providing modules using ASDF,
+    ;; as SBCL used to do, and others may might do. Thus, the system provided in the end
+    ;; would be a downcased name as per module-provide-asdf above. For the same reason,
+    ;; we cannot assume that the system in the end will be of type require-system,
+    ;; but must check whether we can use find-system and short-circuit cl:require.
+    ;; Otherwise, calling cl:require could result in nasty reentrant calls between
+    ;; cl:require and asdf:operate that could potentially blow up the stack.
     (let* ((module (car arguments))
            (name (string-downcase module))
            (system (find-system name nil)))
       (assert module)
-      ;;(unless (typep system '(or null require-system))
-      ;;  (warn "~S depends on ~S but ~S is registered as a ~S"
-      ;;        component (cons combinator arguments) module (type-of system)))
       (or system (let ((system (make-instance 'require-system :name name)))
                    (register-system system)
                    system))))
 
   (defun module-provide-asdf (name)
+    ;; We must use string-downcase, because modules are traditionally specified as symbols,
+    ;; that implementations traditionally normalize as uppercase, for which we seek a system
+    ;; with a name that is traditionally in lowercase. Case is lost along the way. That's fine.
+    ;; We could make complex, non-portable rules to try to preserve case, and just documenting
+    ;; them would be a hell that it would be a disservice to inflict on users.
     (let ((module (string-downcase name)))
       (unless (member module *modules-being-required* :test 'equal)
         (let ((*modules-being-required* (cons module *modules-being-required*))
-- 
GitLab