G'day,
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.
Thanks Mark
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? Could you also explain why :SYSTEM-P T is no longer always passed on to COMPILE-FILE?
Thanks,
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
On Sun, Nov 4, 2012 at 7:03 PM, Robert Goldman rpgoldman@sift.info wrote:
ASDF:OUTPUT-FILES is allowed to return a second value which, if true, indicates that the output files should NOT be relocated.
I believe Mark is saying that ASDF:OUTPUT-FILES is returning a list with with two items, rather than two values.
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)))
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)))
It'd be nice if someone with more ASDF-fu could chime in. :-)
Cheers,
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:
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.
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.)
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 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?
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
G'day,
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.
The patch introduces two new classes, PROCESS-OP and PROCESS-OP-INPUT. The class PROCESS-OP represents an ASDF operation that generates a lisp source file from a PROCESS-OP-INPUT component. A PROCESS-OP-INPUT component acts like a ASDF:CL-SOURCE-FILE by redirecting COMPILE-OP and LOAD-SOURCE-OP to the generated lisp source file.
The previously defined GROVEL-FILE and WRAPPER-FILE components now inherit from PROCESS-OP-INPUT.
Thanks 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,
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.
I can perform ASDF:TEST-SYSTEM on OSICAT if I delete the OSICAT files from ~/.cache/.
SBCL: 1.1.1 ECL: 93be6ce0b OSICAT: from quicklisp 2012-10-13 CFFI: 588b96ff2 with my patch applied. Operating System: OSX 10.7.5
Mark
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
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
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 Tue, Dec 4, 2012 at 12:12 AM, Mark Cox markcox80@gmail.com wrote:
What version of IOlib ? The current git HEAD doesn't use cffi-grovel
The version in quicklisp has indeed this problem because of the dependency.
Juanjo
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?
Hi Luis,
This is just an update. My son was born recently and I haven't had much time to work on this.
On 05/12/2012, at 9:28 AM, Luís Oliveira wrote:
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?
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".
I am still trying to ascertain how this occurs though. I am unable to reproduce the incorrect plan with my simpler MY-LOAD-OP, MY-COMPILE-OP, MY-CL-SOURCE-FILE, MY-PROCESS-OP and MY-PROCESS-OP-INPUT definitions.
I will keep hunting.
Have a good weekend. Mark
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
Hi Faré,
On 08/12/2012, at 2:23 PM, Faré wrote:
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.
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.
Let me know if there are any other issues.
Thanks Mark
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.
On 09/12/2012, at 1:31 AM, Faré wrote:
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.
I can confirm this. Thank you for fixing this.
Mark
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,
On Mon, Nov 5, 2012 at 12:57 PM, Mark Cox markcox80@gmail.com wrote:
I am not sure if Juan reads this list. He may have a better idea on how to achieve this approach.
I am afraid I do not know what grovel is doing, but all what you mentioned in your email is correct. ECL needs both files because that is the only correct way to build a shared library out of an ASDF definition. In the past we only kept the FASL and threw away the object file, which implied a second recompilation for every new operation and this led to unwanted changes in the order of ASDF -- the resulting libraries were not identical to the process of loading a clean and fresh ASDF library.
What I do not understand is why cffi-grovel is dealing with the output of a compile-op operation. I would have implemented http://common-lisp.net/project/cffi/manual/html_node/Groveller-ASDF-Integrat... the following ASDF dependency rule (method on depends-on or something like that)
load-op grovel-file -> grovel-op grovel-file & load-op resulting-lisp-file compile-op grovel-file -> grovel-op grovel-file & compile-op resulting-lisp-file
This parallel rule would ensure that the intermediate lisp is always generated (via grovel-op) and the grovel-file does not have to deal with compilation itself, which is delegated to the load/compile-op on the resulting file.
Juanjo
-
Faré -
Juan Jose Garcia-Ripoll -
Luís Oliveira -
Mark Cox -
Robert Goldman -
Stelian Ionescu