search

mdedwards / slippery-chicken

slippery chicken: algorithmic composition software in common lisp and clos
http://michael-edwards.org/sc
72 stars 3 forks source link

draft .asd file #81

Closed rubenphilipp closed 8 months ago

rubenphilipp commented 8 months ago

Draft an .asd-file for slippery chicken.

rubenphilipp commented 8 months ago

While working on the .asd-file, I noticed that a few modules of sc (list below) are not compiled but just loaded? Unfortunately, ASDF does not provide any functionality to "skip" compilation for single files (i.e. components) without using clumsy workarounds[^1]. Hence the question (@mdedwards), whether it is possible to also compile the respective files.

[^1]: For reference: https://asdf.common-lisp.dev/asdf/How-do-I-mark-a-source-file-to-be-loaded-only-and-not-compiled_003f.html

rubenphilipp commented 8 months ago

Okay. I see that wolfram.lsp cannot be compiled. Then I need to find a workaround for this issue… cm-load.lsp doesn't complain about being compiled, though I didn't test it yet, as I have to deal with some other issues…

mdedwards commented 8 months ago

I believe cm-load.lsp could not be compiled either, hence I only loaded it, but I guess if you find a solution for Wolfram.lsp then the same can be applied to cm-load

rubenphilipp commented 8 months ago

Alright. I think, I have found a way for this.

rubenphilipp commented 8 months ago

I have just implemented a first version of our new system definition (slippery-chicken.asd). It works quite fine on my system[^3] and the RTs (did not check the full tests yet) run smoothly except the two fails caused by the different cm-version (2.12.0 instead of 2.6.0) which we discovered this morning. Of course, there are some notes to be made:

  1. As mentioned above, I found a workaround for the files (in ASDF-parlance "components") that have been loaded instead of compiled in the previous "version" of sc.
    • cm-load.lsp seems to be compilable
    • wolfram.lsp can also be compiled, but needs (interestingly) to be loaded once before compilation
  2. For opaque reasons, lsp-files containing (sc)clm instrument-definitions do not successfully compile via asdf (via asdf::perform-lisp-compilation) by default. Thus, I compile them (via (cl-user::compile-clm-ins)) with the time-tested, classical method before calling ASDF's default compilation function via the (compile-op :before hook (cf. slippery-chicken.asd). This prevents the ASDF-compilation procedure from re-compiling an already existing and valid fasl (or whatever).
  3. All binary files are stored in ASDF's default cache directory (cf. (asdf::apply-output-translations) et al.). Thus, the bin directory will be obsolete and should be removed (after discussion).
  4. I transferred most required/relevant functions and parameter definitions from all.lsp to slippery-chicken.asd. Though, there are some exceptions and/or modifications to the code.
    1. cl-user::+slippery-chicken-src-path+ is now located in globals.lsp. This is necessary, as the path-mechanism of ASDF works differently than the one previously used, as it relates to the pathname specifications of the asdf-system. Obviously, globals.lsp is not an ideal place to define something outside of the scope of sc, though, this location was -- at least to me -- quite suggestive for the moment. We might also want to move the form to the .asd file (didn't test whether this works or does cause similar issues as depicted in 6.) or to a different location (cf. 5.).
    2. Similarly, cl-user::+slippery-chicken-home-dir+ is also located in globals.lsp.
    3. The function cl-user::sc-compile-and-load is in essence not required as ASDF does this job for us now (at least I thought so for one hour). But as cm.lsp (re-)uses this function in order to refresh the diapason, I (re-)implemented it in our .asd file with some major modifications[^2]. Different from the previous version, cl-user::sc-compile-and-load now uses ASDF's function to determine the output location and extension of the binaries.
    4. …this implies the reason why I deemed cl-user::+slippery-chicken-fasl-extension+ to be deprecated and to be omitted in the .asd file.
    5. I copied the following lines of code to the .asd without being completely confident about its raison d'être: 
      #+(and (or sbcl ccl) (or linux darwin))
(pushnew :sc-auto-open *features*)
  5. Generally (related to 4.), ASDF encourages moving lisp code from the .asd definitions to dedicated components.[^2] Thus we should consider to move the respective "non-asdf-"forms to new modules (e.g. "init" and "provide") in src/.
  6. I was not able to export the symbols defined in slippery chicken just by adding the known (from all.lsp) form at the bottom of the .asd, as the non-asdf-forms seem to be evaluated in a rather oblique way (i.e. it seems not to be discernible whether the code is reliably evaluated before or after the system definition). Thus, I felt the need to add a specific component/module (src/export-symbols.lsp) to export the symbols defined in the slippery-chicken package loaded at the very end of the definition of the asdf-components. Maybe this could later also be included in the "provide" module (cf. 5).
  7. I needed to slightly alter the order by which modules/components are loaded/compiled, first, because some require the global vars defined now in globals.lsp (cf. 4), second, because control-wave.lsp complained about not knowing about the related instrument. This is related to the (sane, I think) :serial t definition. I included a list of the original order which at the bottom for reference which is, of course, commented out and to be deleted after discussion.
  8. We definitely need to think about how to test the system. Currently, (run-tests) works as usual, but in the ASDF-world, there exists (asdf:test-system ...). The common practice in ASDF is to exclude RTs from the actual system/package.[^1] Thus, it might be desirable (and not too complicated) to do something similar to our RTs.
  9. There has been a possible bug in control-wave.lsp which I fixed on the quicklisp branch, but which needs further inspection. (cf. #85).
  10. What about all.lsp after this manoeuvre? Removing it (I'd suggest) or keeping (and maintaining!) it for non-asdf users?

Cheers, Ruben

[^1]: As the following comment made me reluctant to solving the problem this way, I double-checked whether the used symbols are actually exported from ASDF. They are, so I am more or less tranquillised… "When you use internals, you MUST notify the ASDF maintainers, and request that they either export the symbol and support the API, or offer an alternative API more to their liking for the same functionality. Otherwise, they will be justified in modifying internals that no one is supposed to use, and they will blame you when they later change or remove these internals and your software breaks." (cf. https://github.com/fare/asdf/blob/master/doc/best_practices.md#using-asdf-internals).

[^2]: Particularly remarkable is this paragraph: "ASDF currently allows arbitrary Lisp code in a .asd file. I would like to deprecate that in the future to make ASDF more declarative. In the meantime, here are some guidelines for how to write such code properly." (cf. https://github.com/fare/asdf/blob/master/doc/best_practices.md#code-in-asd-files) Even though I didn't try to identify the persona of this prose, I feel inclined to follow her/his prophecies…

[^3]: MacOS 13.6.4, SBCL 2.3.0

mdedwards commented 8 months ago

Great Ruben, thanks.

I still have a visceral hatred of ASDF et al (lots of fighting to get this to work on my end...e.g. where the hell is the Alexandria package that CM 'requires' but doesn't tell us about?) but I'm holding my nose and gagging myself as I fight the good fight to make you young 'uns happy ;-)

I have just implemented a first version of our new system definition (slippery-chicken.asd). It works quite fine on my system1 and the RTs (did not check the full tests yet) run smoothly except the two fails caused by the different cm-version (2.12.0 instead of 2.6.0) which we discovered this morning.

OK they were trivial really. I've fixed them and checked in.

  1. As mentioned above, I found a workaround for the files (in ASDF-parlance "components") that have been loaded instead of compiled in the previous "version" of sc.

Nice.

  • cm-load.lsp seems to be compilable
  • wolfram.lsp can also be compiled, but needs (interestingly) to be loaded once before compilation

Something to do with macros and defun being called in them? Some form of (eval-when ) might also have taken care of that.

  1. For opaque reasons, lsp-files containing (sc)clm instrument-definitions do not successfully compile via asdf (via asdf::perform-lisp-compilation) by default. Thus, I compile them (via (cl-user::compile-clm-ins)) with the time-tested, classical method before calling ASDF's default compilation function via the (compile-op :before hook (cf. slippery-chicken.asd). This prevents the ASDF-compilation procedure from re-compiling an already existing and valid fasl (or whatever).

OK

  1. All binary files are stored in ASDF's default cache directory (cf. (asdf::apply-output-translations) et al.). Thus, the bin directory will be obsolete and should be removed (after discussion).

Excellent. However bin still contains clm compilation files so we might want to retain or jiggle the descins macro so that it writes in another dir?

  1. cl-user::+slippery-chicken-src-path+ is now located in globals.lsp. This is necessary, as the path-mechanism of ASDF works differently than the one previously used, as it relates to the pathname specifications of the asdf-system.

ok

Obviously, globals.lsp is not an ideal place to define something outside of the scope of sc, though, this location was -- at least to me -- quite suggestive for the moment.

It's fine for me.

  1. Similarly, cl-user::+slippery-chicken-home-dir+ is also located in globals.lsp.

ok

  1. The function cl-user::sc-compile-and-load is in essence not required as ASDF does this job for us now (at least I thought so for one hour). But as cm.lsp (re-)uses this function in order to refresh the diapason, I (re-)implemented it in our .asd file with some major modifications2. Different from the previous version, cl-user::sc-compile-and-load now uses ASDF's function to determine the output location and extension of the binaries.

ok

  1. …this implies the reason why I deemed cl-user::+slippery-chicken-fasl-extension+ to be deprecated and to be omitted in the .asd file.

fine

  1. I copied the following lines of code to the .asd without being completely confident about its raison d'être:

    
#+(and (or sbcl ccl) (or linux darwin))
(pushnew :sc-auto-open *features*)

that's just there so that auto-open capable systems do indeed auto-open by default. Are you familiar with the common-lisp-standard features list?


5. Generally (related to 4.), ASDF encourages moving lisp code from the .asd definitions to dedicated components.[2](#user-content-fn-2-7f1ba2725d1f4d3b10e2ffc5f01fd4dd) Thus we should consider to move the respective "non-asdf-"forms to new modules (e.g. "init" and "provide") in `src/`.

I'm happy either way. I am still an asdf detractor i.e. far from an asdf purist ;-)

  1. I was not able to export the symbols defined in slippery chicken just by adding the known (from all.lsp) form at the bottom of the .asd, as the non-asdf-forms seem to be evaluated in a rather oblique way (i.e. it seems not to be discernible whether the code is reliably evaluated before or after the system definition). Thus, I felt the need to add a specific component/module (src/export-symbols.lsp) to export the symbols defined in the slippery-chicken package loaded at the very end of the definition of the asdf-components. Maybe this could later also be included in the "provide" module (cf. 5).

fine either way with me

  1. I needed to slightly alter the order by which modules/components are loaded/compiled,

yes, that's to be expected

first, because some require the global vars defined now in globals.lsp (cf. 4), second, because control-wave.lsp complained about not knowing about the related instrument. This is related to the (sane, I think) :serial t definition.

yeah, makes sense

I included a list of the original order which at the bottom for reference which is, of course, commented out and to be deleted after discussion.

delete away to your heart's content

  1. We definitely need to think about how to test the system. Currently, (run-tests) works as usual, but in the ASDF-world, there exists (asdf:test-system ...). The common practice in ASDF is to exclude RTs from the actual system/package.3 Thus, it might be desirable (and not too complicated) to do something similar to our RTs.

well if one of you can get test-system to essentially to do (run-tests) that's fine. The RTs were originally in their own repo but I integrated them into the main sc repo because I would encourage people to run them themselves if they're developing. But if we farm them out to a separate repo again, doesn't that make any sort of asdf:test-system even more tricky?

  1. There has been a possible bug in control-wave.lsp which I fixed on the quicklisp branch, but which needs further inspection. (cf. control-wave.lsp fails to build #85).

That's fine: the nil vs integer problem, yes?

  1. What about all.lsp after this manoeuvre? Removing it (I'd suggest) or keeping (and maintaining!) it for non-asdf users?

I would suggest retaining all.lsp for those who want to go via that route instead of asdf but we could add a note that asdf is now the way to use sc, and warn that we won't be maintaining all.lsp in the future. This of course is dependent on whether you can really convince me to go asdf all the way ;-)

Thanks again and best, Michael

mdedwards commented 8 months ago

update: sc-test-full.lsp passing on my end. pull yourselves some fully-functioning code

Leon-Focker commented 8 months ago

Hey, great work! On my Linux everything seems to be perfect. I have only tried run-tests for now but everything passes. On Windows however there is an annoying problem: directory-namestring does not return the same on windows as it does on linux (I'm using sbcl 2.3.0 on Windows, sbcl 2.0.1 on Linux) Linux:

(directory-namestring "e:/code/test/")
"e:/code/test/"

Windows:

(directory-namestring "e:/code/test/")
"/code/test/"

Which leads to fun problems... For example when running the tests on windows, I get this error:

(WAVELAB-TO-AUDACITY-MARKER-FILE "e:/code/slippery-chicken/tests/24-7loops3.mrk" 44100)

Error opening #P"c:/code/slippery-chicken/tests/24-7loops3.txt":
  Das System kann den angegebenen Pfad nicht finden.

Because WAVELAB-TO-AUDACITY-MARKER-FILE calls directory-namestring. I'm not sure why these pop up now and not when I ran tests earlier. Also I'm not sure how to handle these, because (to me) it seems like the only alternative is make-pathname. Which is tedious...

rubenphilipp commented 8 months ago

@mdedwards. Great! Thanks for the work. I've just pulled the latest commit and run the tests (just run-tests for now) and

where the hell is the Alexandria package that CM 'requires' but doesn't tell us about?

That's the raison d'être of quicklisp, when using ql:quickload :slippery-chicken, non-local dependencies are automatically fetched via the dist repo. But I agree with you that error reporting in this case could be more transparent, especially as nested dependencies (as in our case) make these things really annoying some times.

Something to do with macros and defun being called in them? Some form of (eval-when ) might also have taken care of that.

Shall we change wolfram.lsp then or keep it like this?

Excellent. However bin still contains clm compilation files so we might want to retain or jiggle the descins macro so that it writes in another dir?

I'd prefer keeping all binaries together and "getting rid" of bin. I've created an issue for this question open for discussion: https://github.com/mdedwards/slippery-chicken/issues/87.

It's fine for me.

D'accord.

Are you familiar with the common-lisp-standard features list?

Yeah, of course. ASDF also uses this mechanism e.g. for :if-feature which is used quite extensively in our .asd. I was probably too focused on a different aspect of this project to get a proper interpretation of the respective lines.

I'm happy either way. I am still an asdf detractor i.e. far from an asdf purist ;-)

I see. :) I also agree with this solution as a lot of asd packages out there work this way, so I deem the scenario pointed out in the footnote to be negligible.

delete away to your heart's content

Thank git, it's conserved…

well if one of you can get test-system to essentially to do (run-tests) that's fine. The RTs were originally in their own repo but I integrated them into the main sc repo because I would encourage people to run them themselves if they're developing. But if we farm them out to a separate repo again, doesn't that make any sort of asdf:test-system even more tricky?

It is not necessary (still less desirable) to move the tests to a different repo. You can define multiple systems residing in the same repo. I have drafted a (non-functional) blueprint, commented out, in the .asd. I will take care of this and created a separate issue: https://github.com/mdedwards/slippery-chicken/issues/88

That's fine: the nil vs integer problem, yes?

Oui.

I would suggest retaining all.lsp for those who want to go via that route instead of asdf but we could add a note that asdf is now the way to use sc, and warn that we won't be maintaining all.lsp in the future.

Then, we will definitely have to warn users, that the all.lsp is not supposed to be working as we have already taken some steps to bind sc to asdf (cf. the new sc-path mechanism and https://github.com/mdedwards/slippery-chicken/issues/87). Of course, we can try to make sure that things will also work without asdf by adding, for example, a feature test #-asdf(or similar) when declaring variables like cl-user::+slippery-chicken-src-path+, but this double tracked approach makes things maybe too complicated to maintain?

This of course is dependent on whether you can really convince me to go asdf all the way ;-)

I hope to be able to do so… ;)

@Leon-Focker. Good to hear that the program also works on your Linux machine. The issue seems strange to me. Would you mind creating a new issue for this, so we can focus here on the design of the .asd? Thanks!

Leon-Focker commented 8 months ago

Sure thing

rubenphilipp commented 8 months ago

@mdedwards By the way: The full tests also work on my system.

mdedwards commented 8 months ago

Hello

where the hell is the Alexandria package that CM 'requires' but doesn't tell us about?

That's the raison d'être of quicklisp, when using ql:quickload :slippery-chicken, non-local dependencies are automatically fetched via the dist repo.

Of course and in the end I was forced to use quicklisp obviously, but not happily. This is what still makes me a little nervous about the whole thing. I believe we should be able to do a quicklisp-free asdf-only installation. That should be easy but as always it wasn't. Until I ignored all the online recommendations to use asdf:central-registry and discovered that that approach is deprecated. I'll write another issue about alexandria.

Something to do with macros and defun being called in them? Some form of (eval-when ) might also have taken care of that.

Shall we change wolfram.lsp then or keep it like this?

If it works and it's not ugly, let's keep it the way it is.

Excellent. However bin still contains clm compilation files so we might want to retain or jiggle the descins macro so that it writes in another dir?

I'd prefer keeping all binaries together and "getting rid" of bin. I've created an issue for this question open for discussion: #87.

Oh for sure: that needs to go. defscins was merely a macro to take care of the placement of compilation files. As with the bin directory in general, SC programmed its own way to keep the src folder free of .fasl clutter. ASDF seems to have caught up ;-)

It is not necessary (still less desirable) to move the tests to a different repo. You can define multiple systems residing in the same repo. I have drafted a (non-functional) blueprint, commented out, in the .asd. I will take care of this and created a separate issue: #88

Nice, thanks.

I would suggest retaining all.lsp for those who want to go via that route instead of asdf but we could add a note that asdf is now the way to use sc, and warn that we won't be maintaining all.lsp in the future.

Then, we will definitely have to warn users, that the all.lsp is not supposed to be working as we have already taken some steps to bind sc to asdf (cf. the new sc-path mechanism and #87). Of course, we can try to make sure that things will also work without asdf by adding, for example, a feature test #-asdf(or similar) when declaring variables like cl-user::+slippery-chicken-src-path+, but this double tracked approach makes things maybe too complicated to maintain?

I think it is. Who knows what other incompatibilities between cm 2.6 and 2.12 are lurking in the bowels of the system? I already hit one with midi files auto-opening (independently from sc-auto-open) and that would be the least of our problems. Sc 1.0.12 will be the last non-asdf version. After finally biting the bullet and facing the asdf demon we're going to use it, damn it ;-)

Best, Michael

mdedwards commented 8 months ago

Some of these issues are being dealt with elsewhere so am closing now.