[asdf-devel] Re: Mistake (?) in grammar of defsystem

[Faré]

On Sat, Feb 22, 2014 at 2:56 PM, Robert P. Goldman <rpgoldman at sift.info> wrote:
> I see that the grammar of dependencies in the manual specifies that
> :version appears as a keyword symbol, but feature appears as *not* a
> keyword symbol.
>
> Is this a documentation bug?  I think so, but you know the actual
> execution semantics better.
>
> If it is a doc bug, I'll fix it right away.
>
Beware that there are two "feature" features

1- In a depends-on specification, as defined in find-component.lisp,
you can write (:feature <feature-expression> <dependency-spec>), where
the dependency link is valid if <feature-expression> is true as per
featurep. This feature was hidden in the ASDF 1 code, but I'm not
convinced this feature worked before ASDF 3, when I fixed it for good.
Therefore, I'm pretty sure noone uses this feature.

2- In a component-depends-on method or in-order-to specification,
(feature foo) instead of (operation components...) then a missing
dependency error is raised. This was used in conjunction with the
ill-designed :if-component-dep-fails feature to provide conditional
dependency. This later feature was removed (minus a thin partial
compatibility layer for old SBCLs), because it was not compatible with
the fixed ASDF3 dependency model.

I suggest both features should probably be deprecated and removed, and
the :if-feature feature introduced in ASDF3, that actually works. If
any system anywhere uses these features, they should be fixed ASAP.

We should probably run a cl-test-grid test with these features
disabled. See patch attached.

NB: I obviously cannot commit this patch unless the maintainer wants
me to. Robert, this baby is yours. The diff above also doesn't remove
the misfeatures from the tests and documentation.

—♯ƒ • François-René ÐVB Rideau •Reflection&Cybernethics• http://fare.tunes.org
The program isn't debugged until the last user is dead.
-------------- next part --------------
diff --git a/action.lisp b/action.lisp
index 431dc6c..59810e7 100644
--- a/action.lisp
+++ b/action.lisp
@@ -122,12 +122,6 @@ You can put together sentences using this phrase."))
         a component name or a component object.  Also note that, the degenerate
         case of (<operation>) is a no-op.]
 
-      or
-
-      (FEATURE <feature>), which means that the component depends
-        on the <feature> expression satisfying FEATUREP.
-        (This is DEPRECATED -- use :IF-FEATURE instead.)
-
     Methods specialized on subclasses of existing component types
     should usually append the results of CALL-NEXT-METHOD to the list."))
   (define-convenience-action-methods component-depends-on (operation component))
diff --git a/backward-internals.lisp b/backward-internals.lisp
index 17070e3..314c846 100644
--- a/backward-internals.lisp
+++ b/backward-internals.lisp
@@ -49,27 +49,6 @@
     (%remove-component-inline-methods component)
     (%define-component-inline-methods component rest)))
 
-;;;; PARTIAL SUPPORT ONLY for the :if-component-dep-fails component attribute
-;; and the companion asdf:feature pseudo-dependency.
-;; This won't recurse into dependencies to accumulate feature conditions.
-;; Therefore it will accept the SB-ROTATE-BYTE of an old SBCL
-;; (older than 1.1.2.20-fe6da9f) but won't suffice to load an old nibbles.
-(with-upgradability ()
-  (defun %resolve-if-component-dep-fails (if-component-dep-fails component)
-    (asdf-message "The system definition for ~S uses deprecated ~
-                  ASDF option :IF-COMPONENT-DEP-FAILS. ~
-                  Starting with ASDF 3, please use :IF-FEATURE instead"
-                  (coerce-name (component-system component)))
-    ;; This only supports the pattern of use of the "feature" seen in the wild
-    (check-type component parent-component)
-    (check-type if-component-dep-fails (member :fail :ignore :try-next))
-    (unless (eq if-component-dep-fails :fail)
-      (loop :with o = (make-operation 'compile-op)
-            :for c :in (component-children component) :do
-              (loop* :for (feature? feature) :in (component-depends-on o c)
-                     :when (eq feature? 'feature) :do
-                     (setf (component-if-feature c) feature))))))
-
 (when-upgrading (:when (fboundp 'make-sub-operation))
   (defun make-sub-operation (c o dep-c dep-o)
     (declare (ignore c o dep-c dep-o)) (asdf-upgrade-error)))
diff --git a/find-component.lisp b/find-component.lisp
index 89108ae..77bd3ce 100644
--- a/find-component.lisp
+++ b/find-component.lisp
@@ -119,8 +119,8 @@
            (cons combinator arguments) component))
 
   (defmethod resolve-dependency-combination (component (combinator (eql :feature)) arguments)
-    (when (featurep (first arguments))
-      (resolve-dependency-spec component (second arguments))))
+    (error (compatfmt "~@<system ~A uses unsupported :feature feature~@>")
+           (component-system component)))
 
   (defmethod resolve-dependency-combination (component (combinator (eql :version)) arguments)
     (resolve-dependency-name component (first arguments) (second arguments)))) ;; See lp#527788
diff --git a/operation.lisp b/operation.lisp
index 0c6dbb2..9521bcc 100644
--- a/operation.lisp
+++ b/operation.lisp
@@ -7,7 +7,8 @@
   (:export
    #:operation
    #:operation-original-initargs #:original-initargs ;; backward-compatibility only. DO NOT USE.
-   #:*operations* #:make-operation #:find-operation #:feature))
+   #:*operations* #:make-operation #:find-operation
+   #:feature)) ;; TODO: stop exporting the deprecated FEATURE feature.
 (in-package :asdf/operation)
 
 ;;; Operation Classes
@@ -49,9 +50,7 @@
   (defmethod find-operation ((context t) (spec operation))
     spec)
   (defmethod find-operation (context (spec symbol))
-    (unless (member spec '(nil feature))
-      ;; NIL designates itself, i.e. absence of operation
-      ;; FEATURE is the ASDF1 misfeature that comes with IF-COMPONENT-DEP-FAILS
+    (when spec ;; NIL designates itself, i.e. absence of operation
       (apply 'make-operation spec (operation-original-initargs context))))
   (defmethod operation-original-initargs ((context symbol))
     (declare (ignorable context))
diff --git a/parse-defsystem.lisp b/parse-defsystem.lisp
index 80c1141..a6f3d45 100644
--- a/parse-defsystem.lisp
+++ b/parse-defsystem.lisp
@@ -202,7 +202,10 @@
         (setf (component-sideway-dependencies component) depends-on)
         (%refresh-component-inline-methods component rest)
         (when if-component-dep-fails
-          (%resolve-if-component-dep-fails if-component-dep-fails component))
+          (error "The system definition for ~S uses deprecated ~
+            ASDF option :IF-COMPONENT-DEP-FAILS. ~
+            Starting with ASDF 3, please use :IF-FEATURE instead"
+           (coerce-name (component-system component))))
         component)))
 
   (defun register-system-definition