On 11/4/12 Nov 4 -11:01 AM, Luís Oliveira wrote:

Hello Mark,

On Wed, Oct 24, 2012 at 1:04 PM, Mark Cox <markcox80@gmail.com> wrote:

...

There is an issue with the ASDF component of CFFI-GROVEL when running on ECL. The problem stems from ASDF:OUTPUT-FILES returning two items. The attached patch hopes to handle this situation.

I haven't touched this code in quite while and I don't remember exactly what's going on. Why is ASDF:OUTPUT-FILES returning two items?

ASDF:OUTPUT-FILES is allowed to return a second value which, if true, indicates that the output files should NOT be relocated.

From the ASDF manual:

@item @code{output-files} The @code{output-files} method determines where the method will put its files. It returns two values, a list of pathnames, and a boolean. If the boolean is @code{T} then the pathnames are marked not be translated by enclosing @code{:around} methods. If the boolean is @code{NIL} then enclosing @code{:around} methods may translate these pathnames, e.g. to ensure object files are somehow stored in some implementation-dependent cache. Best, Robert

Hi Luis, On 05/11/2012, at 3:01 AM, Luís Oliveira wrote:

Hello Mark,

On Wed, Oct 24, 2012 at 1:04 PM, Mark Cox <markcox80@gmail.com> wrote:

...

There is an issue with the ASDF component of CFFI-GROVEL when running on ECL. The problem stems from ASDF:OUTPUT-FILES returning two items. The attached patch hopes to handle this situation.

I haven't touched this code in quite while and I don't remember exactly what's going on. Why is ASDF:OUTPUT-FILES returning two items?

The result of ASDF:OUTPUT-FILES on cffi/src/utils.lisp is the following when using ECL (see code at the end of the email). (#P"/tmp/quicklisp/dists/quicklisp/software/cffi_0.10.7.1/src/utils.o" #P"/tmp/quicklisp/dists/quicklisp/software/cffi_0.10.7.1/src/utils.fas") T As you can see, ECL's ASDF:OUTPUT-FILES on CL source files returns a list with more than one element. The first item, utils.o, is an object file created using COMPILE-FILE with :SYSTEM-P T. The second is a FASl file created using COMPILE-FILE with no keywords. From line 4411 of [1], "having both of them allows us to later on reuse the object files for bundles, libraries and standalone executables." More information on the distinction between the two outputs can be found in [2].

Could you also explain why :SYSTEM-P T is no longer always passed on to COMPILE-FILE?

:SYSTEM-P is only required to create the object file. Upon inspection of ASDF sources, they avoid the second compilation by (re)using the object file to create the FASl file. See COMPILE-FILE-KEEPING-OBJECT in [1]. I think there are two ways to proceed. The first is to stick with the assumption that the list returned by ASDF:OUTPUT-FILES only contains one item, but process that single item according to the current lisp machine. The second is to assume that ASDF:OUTPUT-FILES can return any number of items. The later is the approach my patch took with the introduction of %COMPILE-FILE-TO-PATHNAME. The problem with the later is that it is not specified how one creates the output file from the input source file. I am not sure if Juan reads this list. He may have a better idea on how to achieve this approach. I will wait for your advice on how to proceed. Thanks Mark [1] asdf.lisp from ASDF or ecl/contrib/asdf/asdf.lisp. [2] http://ecls.sourceforge.net/new-manual/ch32s02.html (defun ecl-test () (let* ((cffi-src (asdf:find-component (asdf:find-system "cffi") "src")) (utils-component (asdf:find-component cffi-src "utils"))) ;; utils-component is the asdf component representing cffi/src/utils.lisp (asdf:output-files (make-instance 'asdf:compile-op) utils-component)))

On 11/5/12 Nov 5 -4:37 PM, Luís Oliveira wrote:

Hello Mark,

On Mon, Nov 5, 2012 at 11:57 AM, Mark Cox <markcox80@gmail.com> wrote:

...

I think there are two ways to proceed. The first is to stick with the assumption that the list returned by ASDF:OUTPUT-FILES only contains one item, but process that single item according to the current lisp machine. The second is to assume that ASDF:OUTPUT-FILES can return any number of items. The later is the approach my patch took with the introduction of %COMPILE-FILE-TO-PATHNAME. The problem with the later is that it is not specified how one creates the output file from the input source file. I am not sure if Juan reads this list. He may have a better idea on how to achieve this approach.

Thanks for your clear explanation. After reading it, then re-reading your patch, I reached the following conclusions:

1. Your patch is processing the grovel file n times, where n is the number of items ASDF:OUTPUT-FILES returns. That doesn't make sense, since we only need to process the grovel file once to produce a Lisp file.

2. In this case, since GROVEL-FILE inherits from ASDF:CL-SOURCE-FILE, ASDF:OUTPUT-FILES is computing what the result of compiling the intermediate lisp file should be. We pass it to PROCESS-GROVEL-FILE just to figure out where we should be placing the intermediate lisp file. (Perhaps there's a better way to do that, I don't know.)

3. It would be nice if we didn't have to do Lisp compilation/loading ourselves. Instead, maybe we could delegate that task to the methods specialized on ASDF:CL-SOURCE-FILE.

What do you think about something along these lines?

(defmethod asdf:perform ((op asdf:compile-op) (c grovel-file)) (let* ((fasl-path (first (asdf:output-files op c)))) (lisp-file (process-grovel-file (asdf:component-pathname c) fasl-path))) (setf (slot-value c 'asdf::absolute-pathname) lisp-file) (call-next-method)))

Let me see if I understand this: you have a grovel file, and you have defined it as producing a .o file and a .fasl file, correct? Now the groveling produces a lisp file from the grovel file? If that's the case, what about defining a new operation, ASDF:GROVEL-OP, that takes a fasl file and performs PROCESS-GROVEL-FILE on it? I.e., when you PERFORM a GROVEL-OP, you get a new lisp file? I.e., there would be a lisp-file as the sole OUTPUT-FILE of the grovel-op. Then you could use the standard ASDF machinery to compile the LISP file afterwards, instead of having to do that mess with the ASDF::ABSOLUTE-PATHNAME, etc. For whatever file this is, you would have the following dependency: GROVEL-FILE --GROVEL-OP--> CL-SOURCE-FILE --COMPILE-OP--> COMPILED-FILE I'm afraid I don't understand exactly what CFFI-GROVEL is supposed to do, so I may be getting this all wrong. best, r

On 06/11/2012, at 9:21 AM, Luís Oliveira wrote:

On Mon, Nov 5, 2012 at 11:04 PM, Robert Goldman <rpgoldman@sift.info> wrote:

...

If that's the case, what about defining a new operation, ASDF:GROVEL-OP, that takes a fasl file and performs PROCESS-GROVEL-FILE on it? I.e., when you PERFORM a GROVEL-OP, you get a new lisp file? I.e., there would be a lisp-file as the sole OUTPUT-FILE of the grovel-op.

This sounds perfect. Thanks Robert!

Mark, do you feel like taking a stab at this?

Sure, I will have a go. Mark

Hello Mark, On Sat, Nov 10, 2012 at 8:34 AM, Mark Cox <markcox80@gmail.com> wrote:

I have attached a patch that changes CFFI-GROVEL's ASDF code to use the default ASDF operations for compiling and loading processed grovel and wrapper files. The implementation is based on suggestions made by Robert Goldman and Juan Jose Garcia-Ripoll.

This code looks much more idiomatic! Nicely done. Unfortunately I can't seem to be able to load Osicat using it. (I'll have to poke some more to figure out what exactly is going on.) Osicat is what I usually use to test cffi-grovel, since we don't have a proper test suite. What did you test it with? Cheers, -- Luís Oliveira http://r42.eu/~luis/

On Mon, Dec 3, 2012 at 4:24 AM, Mark Cox <markcox80@gmail.com> wrote:

I tested it by loading IOLIB using SBCL and ECL.

Is this code now part of the master branch in cffi? I tried today and did not manage to load iolib. Juanjo -- Instituto de Física Fundamental, CSIC c/ Serrano, 113b, Madrid 28006 (Spain) http://juanjose.garciaripoll.googlepages.com

Hi Stelian, On 03/12/2012, at 11:56 PM, Stelian Ionescu wrote:

On Mon, 2012-12-03 at 13:24 +1000, Mark Cox wrote:

...

Hi Luis,

On 27/11/2012, at 2:40 AM, Luís Oliveira wrote:

...

Hello Mark,

On Sat, Nov 10, 2012 at 8:34 AM, Mark Cox <markcox80@gmail.com> wrote:

...

I have attached a patch that changes CFFI-GROVEL's ASDF code to use the default ASDF operations for compiling and loading processed grovel and wrapper files. The implementation is based on suggestions made by Robert Goldman and Juan Jose Garcia-Ripoll.

This code looks much more idiomatic! Nicely done.

Unfortunately I can't seem to be able to load Osicat using it. (I'll have to poke some more to figure out what exactly is going on.) Osicat is what I usually use to test cffi-grovel, since we don't have a proper test suite. What did you test it with?

I tested it by loading IOLIB using SBCL and ECL.

What version of IOlib ? The current git HEAD doesn't use cffi-grovel

IOLIB 0.7.3 Mark

-- Stelian Ionescu a.k.a. fe[nl]ix Quidquid latine dictum sit, altum videtur. http://common-lisp.net/project/iolib

_______________________________________________ cffi-devel mailing list cffi-devel@common-lisp.net http://lists.common-lisp.net/cgi-bin/mailman/listinfo/cffi-devel

On Mon, Dec 3, 2012 at 3:24 AM, Mark Cox <markcox80@gmail.com> wrote:

I can perform ASDF:TEST-SYSTEM on OSICAT if I delete the OSICAT files from ~/.cache/.

Right, it seems that for some reason ASDF is not picking up basix-unixint's compile-time dependency on packages.lisp. Does that seem like a plausible (if incomplete) explanation? -- Luís Oliveira http://r42.eu/~luis/

On Fri, Dec 7, 2012 at 7:37 PM, Mark Cox <markcox80@gmail.com> wrote:

...

Right, it seems that for some reason ASDF is not picking up basix-unixint's compile-time dependency on packages.lisp. Does that seem like a plausible (if incomplete) explanation?

Yes you are correct. The problem can be seen with ASDF::TRAVERSE returning the wrong plan:

...

(asdf::traverse (make-instance 'asdf:load-op) (asdf:find-system "osicat"))

((#<ASDF:LOAD-OP NIL {100793C8D3}> . #<ASDF:CL-SOURCE-FILE "osicat" "osicat-sys" "osicat-sys">) (#<ASDF:LOAD-OP NIL {100793C8D3}> . #<ASDF:MODULE "osicat" "osicat-sys">) (#<CFFI-GROVEL::PROCESS-OP NIL {10076A7263}> . #<CFFI-GROVEL:GROVEL-FILE "osicat" "posix" "basic-unixint">) (#<ASDF:LOAD-OP NIL {10077E2133}> . #<ASDF:CL-SOURCE-FILE "osicat" "posix" "packages">) (#<ASDF:COMPILE-OP NIL {1007609953}> . #<CFFI-GROVEL:GROVEL-FILE "osicat" "posix" "basic-unixint">) ....

For some reason there exists a PROCESS-OP on "basic-unixint" before the LOAD-OP on "packages".

Did you define a method on component-depends-on, so that process-op will depend on the load-op? Something like that might do: (defmethod component-depends-on ((o process-op) (c grovel-file)) `((load-op ,@(component-load-dependencies c)) ,@(call-next-method))) You might even skip the call-next-method. —♯ƒ • François-René ÐVB Rideau •Reflection&Cybernethics• http://fare.tunes.org Don't have good ideas if you aren't willing to be responsible for them. — Alan Perlis

...

Did you define a method on component-depends-on, so that process-op will depend on the load-op?

Something like that might do:

(defmethod component-depends-on ((o process-op) (c grovel-file)) `((load-op ,@(component-load-dependencies c)) ,@(call-next-method)))

You might even skip the call-next-method.

No I had not. Adding this method solves the problem that Luis encountered. I have attached a new patch incorporating this method. Thank you for your help.

Note that I've just fixed a long-standing bug in ASDF 2.26.9, whereby in older versions of ASDF this will cause excessive recompilation, and would have to instead use component-do-first (which I have just removed). Don't be alarmed if you see too much recompilation (but tell me, I like my hypotheses confirmed) — and instead upgrade to ASDF 2.26.9, or ASDF 2.27 when it's out later this month. —♯ƒ • François-René ÐVB Rideau •Reflection&Cybernethics• http://fare.tunes.org If mice were the ultimate input device, humans would be born with one arm and three fingers.

Hello Mark, On Sat, Dec 8, 2012 at 6:04 AM, Mark Cox <markcox80@gmail.com> wrote:

Let me know if there are any other issues.

I have finally tested and applied your patch. Thanks. Also, congratulations on your newborn! Cheers, -- Luís Oliveira http://r42.eu/~luis/