This is very interesting. It did seem like more lookups started happening when I switched to the -SNAPSHOT convention.

While we’re on the subject, I don’t like the idea of constantly reloading artifacts from maven, especially when, AFAIUI, ABCL doesn’t currently support reloading classes from jars. I guess the question is should we resolve the maven artifact at compile time or load time? Probably not both. Well, it probably should be some custom ASDF component-type that has a resolve-op, but that’s an exercise for another day.

In the meantime, how about something like this:

diff --git i/contrib/abcl-asdf/abcl-asdf.lisp w/contrib/abcl-asdf/abcl-asdf.lisp
index aace30e..da430bd 100644
--- i/contrib/abcl-asdf/abcl-asdf.lisp
+++ w/contrib/abcl-asdf/abcl-asdf.lisp
@@ -15,6 +15,7 @@ (defclass mvn (iri)
   ((group-id :initarg :group-id :initform nil)
    (artifact-id :initarg :artifact-id :initform nil)
    (repository :initform "http://repo1.maven.org/maven2/") ;;; XXX unimplmented
+   (resolved-path :initform nil :accessor resolved-path)
    (classname :initarg :classname :initform nil)
    (alternate-uri :initarg :alternate-uri :initform nil)
    ;; inherited from ASDF:COMPONENT ??? what are the CL semantics on overriding -- ME 2012-04-01
@@ -27,12 +28,13 @@ (defmethod find-component ((component iri) path)
 
 ;;; We intercept compilation to ensure that load-op will succeed
 (defmethod perform ((op compile-op) (c mvn))
-  (abcl-asdf:resolve   
-   (ensure-parsed-mvn c)))
+  (unless (resolved-path c)
+    (setf (resolved-path c)
+          (abcl-asdf:resolve   
+           (ensure-parsed-mvn c)))))
 
 (defmethod perform ((operation load-op) (c mvn))
-  (let ((resolved-path 
-         (abcl-asdf:resolve (ensure-parsed-mvn c))))
+  (let ((resolved-path (resolved-path c)))
     (when (stringp resolved-path)
       (java:add-to-classpath (abcl-asdf:as-classpath resolved-path)))))
 
This has us only resolving the the maven artifact once.

thanks,

Cyrus

On Feb 23, 2014, at 2:14 PM, Arnaud Bailly <arnaud.oqube@gmail.com> wrote:

Hi,
I know much more about maven than asdf and actually maven knows much more about the version than what you say. 
 - SNAPSHOT is not an arbitrary keyword (eg. a classifier in maven parlance) but something that is baked in the artefact resolution and repository definition of maven. Snapshot versions can be updated and new versions looked for in a remote repository whereas "release" versions are never updated
 - if the version number follows maven conventions (X.Y.Z numbers) then actually you can define version ranges in dependencies (http://books.sonatype.com/mvnref-book/reference/pom-relationships-sect-project-dependencies.html),
 - moreover, there is a notion of ordering which is maintained in a repository metadata so that one can request the LATEST version of some artefact.

My 2 cts

Regards,

--
Arnaud Bailly
FoldLabs Associate: http://foldlabs.com


On Sun, Feb 23, 2014 at 5:02 PM, Mark Evenson <evenson@panix.com> wrote:
On 2/23/14, 1:27, Cyrus Harmon wrote:
>
> Whoops. I attached the wrong patch.
>
> Correct one (hopefully) attached:
>
>
>
>
>
>
> On Feb 22, 2014, at 5:21 PM, Cyrus Harmon <ch-lisp@bobobeach.com> wrote:
>
>>
>> The attached patch adds support for maven snapshot versions with the following defsystem component syntax:
>>
>>    (:mvn "org.openscience.cdk/cdk-bundle" :version "1.5.6" :snapshot t)
>>
>> I’m open to other suggestions to solving this, but this works for my immediate needs.

No, that isn't a sufficiently generalized solution for my tastes, but
congratulations on hacking it to work for you!

As far as I understand Maven, the "-SNAPSHOT" is merely a suffix to the
version coordinate and is merely a convention, not supported by the
underlying Maven infrastructure as the version in Maven is merely an
opaque string without any meaningful ordering operations between
different versions (i.e. there is no sure way to tell that one version
in Maven is greater or lesser than another).  So, making an keyword
argument for "-SNAPSHOT" but not "-rc-1" and whatever else gets stuffed
in here is the wrong way forward, especially as we can see that your
diff is mostly about "carrying the extra arg" through the various method
calls which would have to be duplicated for every additional suffix that
one encounters.

ASDF tries to be much stricter about its version component, which
probably means that trying to use the ASDF VERSION component to
represent both Maven's and ASDF's notion of a version is probably a
losing battle.  Currently, the ASDF  on abcl-1.3.0-dev signals a
non-intelligible error about ASDF/SYSTEM:BUILTIN-SYSTEM-P when ASDF
encounters a VERSION that is not strictly three integers separated by
two periods.

Fortunately, the current version of ASDF and ABCL-ASDF accepts the
following form:

   (:mvn "org.openscience.cdk/cdk-bundle/1.5.6-SNAPSHOT")

which should do what you want without patching the API.  This is where
the ENSURE-PARSED-MVN function that you recently patched comes in,
"fixing up" the MVN component to contain the right values.  This works,
since in spite of requiring that ASDF versions can be meaningfully
compared, ASDF itself does not use this requirement for anything other
than possibly upgrading its own components.

I couldn't find your artifact on the standard Maven repo, but tested it
with the following artifact:

    (:mvn "org.mod4j.com.google.inject/guice/1.0-XTEXT-PATCHED")

Could you test that this will work for your case?

Longer term, the way forward is to clearly not use the ASDF VERSION
field to store the MVN version at all.

--
"A screaming comes across the sky.  It has happened before, but there
is nothing to compare to it now."