From 4c85c717e265b1ab9b26e8e774ff14482dd959a5 Mon Sep 17 00:00:00 2001
From: Francois-Rene Rideau <tunes@google.com>
Date: Fri, 31 Jan 2014 00:37:31 -0500
Subject: [PATCH] Update some comments and docstrings, proofreading Robert's
 edits. Remove some trailing spaces.

---
 action.lisp        | 20 ++++++++++----------
 component.lisp     |  3 +--
 lisp-action.lisp   |  3 +--
 operation.lisp     |  2 --
 plan.lisp          | 13 +++++++------
 test/test-asdf.asd |  2 +-
 upgrade.lisp       |  2 +-
 7 files changed, 21 insertions(+), 24 deletions(-)

diff --git a/action.lisp b/action.lisp
index a1001994..049176b4 100644
--- a/action.lisp
+++ b/action.lisp
@@ -119,8 +119,8 @@ You can put together sentences using this phrase."))
 
         [Note: an <operation> is an operation designator -- it can be either an
         operation name or an operation object.  Similarly, a <component> may be
-        a component name or a component object.  Finally, the degenerate case of
-        (<operation>) is treated as a no-op.]
+        a component name or a component object.  Also note that, the degenerate
+        case of (<operation>) is a no-op.]
 
       or
 
@@ -199,9 +199,9 @@ then the action (O . C) of O on component C depends on each (S . C) for S in L.
 E.g. before a component may be loaded by LOAD-OP, it must have been compiled by COMPILE-OP.
 A operation-designator designates a singleton list of the designated operation;
 a list of operation-designators designates the list of designated operations;
-NIL is not a valid operation designator in that context.  Note that orderings between 
-the operations in a list of SELWARD-OPERATION should be indicated separately in order
-that they be scheduled properly."))
+NIL is not a valid operation designator in that context.  Note that any dependency
+ordering between the operations in a list of SELFWARD-OPERATION should be specified separately
+in the respective operation's COMPONENT-DEPENDS-ON methods so that they be scheduled properly."))
   (defun selfward-operation-depends-on (o c)
     (loop :for op :in (ensure-list (selfward-operation o)) :collect `(,op ,c)))
   (defmethod component-depends-on ((o selfward-operation) (c component))
@@ -405,11 +405,11 @@ in some previous image, or T if it needs to be done.")
 
 ;;; Generic build operation
 (with-upgradability ()
-  ;; build op was intended to be the master, default operation on a system
-  ;; (LOAD-OP typically serves that function now).  This feature has not yet
-  ;; been fully implemented yet.
-  ;; This is a path forward, but is not backwardly compatible, and is not used
-  ;; yet. [2014/01/26:rpg]
+  ;; BUILD-OP was intended to be the default "master" operation to invoke on a system in ASDF3
+  ;; (LOAD-OP typically serves that function now).
+  ;; This feature has not yet been fully implemented yet, and is not used by anyone yet.
+  ;; This is a path forward, and its default below is to backward compatibly depend on LOAD-OP.
+  ;; [2014/01/26:rpg]
   (defclass build-op (non-propagating-operation) ())
   (defmethod component-depends-on ((o build-op) (c component))
     `((,(or (component-build-operation c) 'load-op) ,c))))
diff --git a/component.lisp b/component.lisp
index f129b105..33578ab2 100644
--- a/component.lisp
+++ b/component.lisp
@@ -138,8 +138,7 @@ another pathname in a degenerate way."))
 
   (defun component-find-path (component)
     "Return a path from a root system to the COMPONENT.
-The return value is a list of component NAMES; a list of
-strings."
+The return value is a list of component NAMES; a list of strings."
     (check-type component (or null component))
     (reverse
      (loop :for c = component :then (component-parent c)
diff --git a/lisp-action.lisp b/lisp-action.lisp
index 1e435867..0622c42e 100644
--- a/lisp-action.lisp
+++ b/lisp-action.lisp
@@ -39,8 +39,7 @@
 (with-upgradability ()
   (defclass prepare-op (upward-operation sideway-operation)
     ((sideway-operation :initform 'load-op :allocation :class))
-    (:documentation "Load the necessary dependencies for the COMPONENT to which we apply
-the PREPARE-OP."))
+    (:documentation "Load dependencies necessary for COMPILE-OP or LOAD-OP of a given COMPONENT."))
   (defclass load-op (basic-load-op downward-operation selfward-operation)
     ;; NB: even though compile-op depends on prepare-op it is not needed-in-image-p,
     ;; so we need to directly depend on prepare-op for its side-effects in the current image.
diff --git a/operation.lisp b/operation.lisp
index d2e38a64..0c6dbb26 100644
--- a/operation.lisp
+++ b/operation.lisp
@@ -57,5 +57,3 @@
     (declare (ignorable context))
     nil))
 
-
-
diff --git a/plan.lisp b/plan.lisp
index d564aec5..eb972f64 100644
--- a/plan.lisp
+++ b/plan.lisp
@@ -322,17 +322,18 @@ the action of OPERATION on COMPONENT in the PLAN"))
 
   (defmethod traverse-action (plan operation component needed-in-image-p)
     (block nil
-      ;; ACTION-VALID-P among other things, handles forcing logic, including
-      ;; FORCE-NOT.
+      ;; ACTION-VALID-P among other things, handles forcing logic, including FORCE-NOT,
+      ;; and IF-FEATURE filtering.
       (unless (action-valid-p plan operation component) (return nil))
-      ;; the following is needed by POIU, which tracks a dependency graph,
+      ;; the following hook is needed by POIU, which tracks a full dependency graph,
       ;; instead of just a dependency order as in vanilla ASDF
       (plan-record-dependency plan operation component)
       ;; needed in image distinguishes b/w things that must happen in the
-      ;; current image and those things that simply need to have been done.
-      (let* ((aniip (needed-in-image-p operation component))
-             ;; effectively needed in image
+      ;; current image and those things that simply need to have been done in a previous one.
+      (let* ((aniip (needed-in-image-p operation component)) ; action-specific needed-in-image
+             ;; effective niip: meaningful for the action and required by the plan as traversed
              (eniip (and aniip needed-in-image-p))
+             ;; status: have we traversed that action previously, and if so what was its status?
              (status (plan-action-status plan operation component)))
         (when (and status (or (action-done-p status) (action-planned-p status) (not eniip)))
           (return (action-stamp status))) ; Already visited with sufficient need-in-image level!
diff --git a/test/test-asdf.asd b/test/test-asdf.asd
index 0e9f97b0..fd99fc06 100644
--- a/test/test-asdf.asd
+++ b/test/test-asdf.asd
@@ -50,7 +50,7 @@
        (:file "does-not-exist"
         :in-order-to ((compile-op (feature (:not :common-lisp))))))
       :if-component-dep-fails :ignore)))))
-       
+
 
 (defsystem :test-asdf/test9-2
   :version "1.0"
diff --git a/upgrade.lisp b/upgrade.lisp
index b8a7c66f..c62363ff 100644
--- a/upgrade.lisp
+++ b/upgrade.lisp
@@ -51,7 +51,7 @@ You can compare this string with e.g.: (ASDF:VERSION-SATISFIES (ASDF:ASDF-VERSIO
   (defun upgrading-p ()
     (and *previous-asdf-versions* (not (equal *asdf-version* (first *previous-asdf-versions*)))))
   (defmacro when-upgrading ((&key (upgrading-p '(upgrading-p)) when) &body body)
-    "A wrapper macro for code that should only be run when upgrading a 
+    "A wrapper macro for code that should only be run when upgrading a
 previously-loaded version of ASDF."
     `(with-upgradability ()
        (when (and ,upgrading-p ,@(when when `(,when)))
-- 
GitLab