Re: A CFFI -ASDF integration bug
Thanks for the explanation. process-op does not really need a generated .so file, only its name. I'll try moving compilation of .c files to compile-op and see if it causes any problems. It seems it will be an easy thing to do.
Re: A CFFI -ASDF integration bug
On Tue, Aug 4, 2020 at 6:13 PM Ilya Perminov wrote: > The files are the same. compile-op does not touch them at all. They > are just its fake output-files. Is it a good idea to compile a .c to > .o/.so in a compile-op? Its doc string says ""Operation for compiling > a Lisp file to a FASL". I don't see why there's any issue compiling a .c to a .o in a compile-op, the operation is for compiling in general. Maybe the doc string can be amended to "Operation for compiling source files or preprocessed source files to object files, fasls, etc.". If they are not otherwise needed at a previous step, I would compile the files during compile-op, as a defmethod perform :before (or :after) on the suitable class. If needed at a previous step, I would copy them to a different filename. Apologies for likely being the author of this bug. —♯ƒ • François-René ÐVB Rideau •Reflection• http://fare.tunes.org Social peace will come when the socialists will love the poor more than they hate the rich. — Not Golda Meir
Re: A CFFI -ASDF integration bug
On 4 Aug 2020, at 17:00, Ilya Perminov wrote: I think there is a problem with the :around method approach - it is not safe for an ASDF extension to define methods on ASDF's operations/components, because other ASDF extensions may want to do the same thing. Imagine there is CFFI* and I want to use both CFFI and CFFI*. That is true -- that's why I always try to have either my own operation class or my own component class, or both. So to do this, you would want to have your own subclass of `system` that you wish to have work this way (e.g., `cffi-extended-system`) -- you generally do not want to add methods to the standard classes in ASDF, both because of potential collisions, and because you don't know what else they might interfere with. On Tue, Aug 4, 2020 at 2:35 PM Robert Goldman wrote: On 4 Aug 2020, at 16:18, Ilya Perminov wrote: The around method will look something like this (defmethod input-files :around ((o asdf/bundle::gather-operation) (c system)) (unless (eq (asdf/bundle::bundle-type o) :no-output-file) (append (call-next-method) (remove-if-not (asdf/bundle::pathname-type-equal-function (asdf/bundle::bundle-pathname-type (asdf/bundle::gather-type o))) (mappend (lambda (c) (output-files 'process-op c)) (asdf/component:sub-components c :type 'wrapper-file)) To my taste it knows too much about ASDF guts, but maybe it is not too bad. I'm looking at the code now, and bundle-type is exported from asdf/bundle (but not asdf) (it could use a docstring if you can propose one) So is bundle-pathname-type pathname-type-equal-function is not exported, but it's a trivial combination of pathname-type and equalp gather-type is the only "really internal" function. (A block comment would be nice here, too.) gather-operation is not exported, but I think that's probably wrong on our part: I don't believe there should be hidden operations. If I get around to it, I will probably export it from the asdf (really asdf/interface) package, unless someone has a good argument for it continuing to be internal. So I don't think that is a lot of ASDF internals, especially not for customizing the ASDF protocol.
Re: A CFFI -ASDF integration bug
I think there is a problem with the :around method approach - it is not safe for an ASDF extension to define methods on ASDF's operations/components, because other ASDF extensions may want to do the same thing. Imagine there is CFFI* and I want to use both CFFI and CFFI*.
Re: A CFFI -ASDF integration bug
The files are the same. compile-op does not touch them at all. They are just its fake output-files. Is it a good idea to compile a .c to .o/.so in a compile-op? Its doc string says ""Operation for compiling a Lisp file to a FASL".
Re: A CFFI -ASDF integration bug
On Fri, Jul 31, 2020 at 1:18 PM Ilya Perminov wrote: > As a result inputs and outputs of the ops look like this: > process-op: > input: wrapper-file > output: bindings-file.lisp file.c FILE.O FILE.SO > > compile-op: > input: bindings-file.lisp > output: bindings-file.fasl FILE.O FILE.SO > Are these FILE.O and FILE.SO two different files with the same name, or the same file reported twice? In BOTH cases, you'll want to have DIFFERENT NAMES for the two sets of outputs. If the files are the same, then either you should compile them only during the compile-op, or, if not possible, you should copy the output to the new file with a different name. —♯ƒ • François-René ÐVB Rideau •Reflection• http://fare.tunes.org The world is a comedy to those that think; a tragedy to those that feel. — Horatio Walpole, 4th Earl of Orford (1717–1797)
Re: A CFFI -ASDF integration bug
Actually, a quick aside: `gather-operation` is an unpleasing name, since most of our other operation subclasses are named *foo*-`op`. Is there some way to make `gather-op` a new name, while keeping `gather-operation` as a (deprecated) alias for the class? Ideally permitting users' method dispatch and `make-instance` usage to continue to function? In the best of all worlds, I would like to make that name change, and then export `gather-op` (and *only* `gather-op`). On 4 Aug 2020, at 16:35, Robert Goldman wrote: On 4 Aug 2020, at 16:18, Ilya Perminov wrote: The around method will look something like this ``` (defmethod input-files :around ((o asdf/bundle::gather-operation) (c system)) (unless (eq (asdf/bundle::bundle-type o) :no-output-file) (append (call-next-method) (remove-if-not (asdf/bundle::pathname-type-equal-function (asdf/bundle::bundle-pathname-type (asdf/bundle::gather-type o))) (mappend (lambda (c) (output-files 'process-op c)) (asdf/component:sub-components c :type 'wrapper-file)) ``` To my taste it knows too much about ASDF guts, but maybe it is not too bad. I'm looking at the code now, and * `bundle-type` is exported from `asdf/bundle` (but not `asdf`) (it could use a docstring if you can propose one) * So is `bundle-pathname-type` * `pathname-type-equal-function` is not exported, but it's a trivial combination of `pathname-type` and `equalp` * `gather-type` is the only "really internal" function. (A block comment would be nice here, too.) * `gather-operation` is *not* exported, but I think that's probably wrong on our part: I don't believe there should be hidden operations. If I get around to it, I will probably export it from the `asdf` (really `asdf/interface`) package, unless someone has a good argument for it continuing to be internal. So I don't think that is a lot of ASDF internals, especially not for customizing the ASDF protocol.
Re: A CFFI -ASDF integration bug
On 4 Aug 2020, at 16:18, Ilya Perminov wrote: The around method will look something like this ``` (defmethod input-files :around ((o asdf/bundle::gather-operation) (c system)) (unless (eq (asdf/bundle::bundle-type o) :no-output-file) (append (call-next-method) (remove-if-not (asdf/bundle::pathname-type-equal-function (asdf/bundle::bundle-pathname-type (asdf/bundle::gather-type o))) (mappend (lambda (c) (output-files 'process-op c)) (asdf/component:sub-components c :type 'wrapper-file)) ``` To my taste it knows too much about ASDF guts, but maybe it is not too bad. I'm looking at the code now, and * `bundle-type` is exported from `asdf/bundle` (but not `asdf`) (it could use a docstring if you can propose one) * So is `bundle-pathname-type` * `pathname-type-equal-function` is not exported, but it's a trivial combination of `pathname-type` and `equalp` * `gather-type` is the only "really internal" function. (A block comment would be nice here, too.) * `gather-operation` is *not* exported, but I think that's probably wrong on our part: I don't believe there should be hidden operations. If I get around to it, I will probably export it from the `asdf` (really `asdf/interface`) package, unless someone has a good argument for it continuing to be internal. So I don't think that is a lot of ASDF internals, especially not for customizing the ASDF protocol.
Re: A CFFI -ASDF integration bug
The around methed will look something like this (defmethod input-files :around ((o asdf/bundle::gather-operation) (c system)) (unless (eq (asdf/bundle::bundle-type o) :no-output-file) (append (call-next-method) (remove-if-not (asdf/bundle::pathname-type-equal-function (asdf/bundle::bundle-pathname-type (asdf/bundle::gather-type o))) (mappend (lambda (c) (output-files 'process-op c)) (asdf/component:sub-components c :type 'wrapper-file)) To my taste it knows too much about ASDF guts, but maybe it is not too bad.
Re: A CFFI -ASDF integration bug
On Tue, Aug 4, 2020 at 12:51 PM Robert Goldman wrote: > I don't claim to understand this process, but wouldn't it be possible for you > to make your own input-files :around method for gather-operation that would > collect the outputs from the process-op's and add them to what you want? I think it is a reasonable option, at least I like it more than what CFFI does now. input-files for gather-operation is defined on SYSTEM, so the around method will have to fetch all WRAPPER-FILEs from a system. gather-type and probably some other functions, that the around method will need, are not exported from ASDF though. > That said, I can think of a simpler, and easier method, and that would be to > override the operation-done-p method for the compile-op so that it knows that > the .o and .so files are generated by the compile-op, and only pays attention > to the relationship between bindings-file.lisp and bindings-file.fasl. I do not think it will work, because up-to-date check happens outside of operation-done-p. compute-action-stamp: (and all-present ;; if all filesystem effects are up-to-date up-to-date-p (operation-done-p o c) ;; and there's no invalidating reason. > One thing you didn't say was what it means that the compile-op is done over > and over -- is it only compiling the bindings-file, or is it doing something > to the object files? I don't believe it should change the .o and .so files, > because those are already compiled by process-op, right? Yes, only the lisp file gets recompiled. compile-op does not do anything to the .o and .so files, they are just its fake outputs.
Re: A CFFI -ASDF integration bug
On 4 Aug 2020, at 12:02, Ilya Perminov wrote: I think protobuf and CFFI structure their operations in a very similar way - process-op is analogous to proto-to-lisp, it takes a "specification" file and generates a lisp file and some other files. protobuf generates lisp(fasl) files only, so it does not need to do anything special to support bundle operations. CFFI's process-op generates some .o and .so files that a bundle operation may need. The current implementation adds .o and .so files to outputs of compile-op and it causes the problem I described. I do not know what methods need to be defined on process-op to make bundle operations to pick up its output files. From my very limited understanding of ASDF I do not think there is a way to do it. Method "component-depends-on ((o gather-operation) (s system))" determines input-files of a bundle-op. The method returns dependencies of one operation only (e.g. compile-op), but in case of CFFI's wrapper-file we need output files of two operations: process-op and compile-op. I don't claim to understand this process, but wouldn't it be possible for you to make your own `input-files :around` method for `gather-operation` that would collect the outputs from the `process-op`'s and add them to what you want? Here's the existing definition of what I think is the relevant `input-files` method: ``` (defmethod input-files ((o gather-operation) (c system)) (unless (eq (bundle-type o) :no-output-file) (direct-dependency-files o c :key 'output-files :test (pathname-type-equal-function (bundle-pathname-type (gather-type o)) ``` This invokes `map-direct-dependencies` which invokes the `component-depends-on` method for `gather-operation` on the system which ... I don't really understand, but I believe it's the `compile-op`'s. I think it would be easiest to write your own method that collects up all of the `process-op` outputs, drops any `.lisp` files (which will be superseded by the `.fasl` files), and adds them to the return value of `call-next-method`. If you do that, and drop the `.o` and `.so` files from the `output-files` of `compile-op`, I think that would get what you want: you would collect the `.o` and `.so` files from the `process-op`, and you wouldn't get ASDF trying to regenerate the files when it's not necessary. That said, I can think of a simpler, and easier method, and that would be to override the `operation-done-p` method for the `compile-op` so that it knows that the `.o` and `.so` files are generated by the `compile-op`, and only pays attention to the relationship between `bindings-file.lisp` and `bindings-file.fasl`. One thing you didn't say was what it means that the compile-op is done over and over -- is it only compiling the bindings-file, or is it doing something to the object files? I don't believe it should change the `.o` and `.so` files, because those are already compiled by `process-op`, right?
Re: A CFFI -ASDF integration bug
On Tue, Aug 4, 2020 at 1:03 PM Ilya Perminov wrote: > > I think protobuf and CFFI structure their operations in a very similar way - > process-op is analogous to proto-to-lisp, it takes a "specification" file and > generates a lisp file and some other files. protobuf generates lisp(fasl) > files only, so it does not need to do anything special to support bundle > operations. CFFI's process-op generates some .o and .so files that a bundle > operation may need. The current implementation adds .o and .so files to > outputs of compile-op and it causes the problem I described. > I do not know what methods need to be defined on process-op to make bundle > operations to pick up its output files. From my very limited understanding of > ASDF I do not think there is a way to do it. Method "component-depends-on ((o > gather-operation) (s system))" determines input-files of a bundle-op. The > method returns dependencies of one operation only (e.g. compile-op), but in > case of CFFI's wrapper-file we need output files of two operations: > process-op and compile-op. > The intended design is that you only take the output of compile-op, and that process-op is implicitly involved as a dependency to compute said output, but that process-op's own outputs are not included. If process-op does the compilation and you want its outputs, you lose. That said, I haven't looked at the code recently, nor do I intend to, being too busy trying to make my startup survive and feed my family. —♯ƒ • François-René ÐVB Rideau •Reflection• http://fare.tunes.org A word for the epoch of free software and universal publishing: voluntocracy n 1. governance by those who do the work. 2. the volunteers who do the work. — Aubrey Jaffer, http://swissnet.ai.mit.edu/~jaffer/
Re: A CFFI -ASDF integration bug
I think protobuf and CFFI structure their operations in a very similar way - process-op is analogous to proto-to-lisp, it takes a "specification" file and generates a lisp file and some other files. protobuf generates lisp(fasl) files only, so it does not need to do anything special to support bundle operations. CFFI's process-op generates some .o and .so files that a bundle operation may need. The current implementation adds .o and .so files to outputs of compile-op and it causes the problem I described. I do not know what methods need to be defined on process-op to make bundle operations to pick up its output files. From my very limited understanding of ASDF I do not think there is a way to do it. Method "component-depends-on ((o gather-operation) (s system))" determines input-files of a bundle-op. The method returns dependencies of one operation only (e.g. compile-op), but in case of CFFI's wrapper-file we need output files of two operations: process-op and compile-op.
