lisp-modules-new: strip any unrelated defsystem from the .asd

[hraban]

Description of changes

Strip any (asdf:defsystem .. that is not for the specific system we want to build. Fixes at least cl-async, which has a .asd with multiple defsystems, which causes problems as the wrong (non-precompiled) package is loaded at build time, which then tries to compile after the fact, putting a .fasl in a read-only location (the store), breaking the system.

See #155851 (comment)

Things done

I WOULD LOVE A SECOND PAIR OF EYES ON THIS! Please sanity check this! I am worried it might have false positives.

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 22.11 Release Notes (or backporting 22.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
  • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
• Fits CONTRIBUTING.md.

[hraban]

PS is master the right base branch for this?

[7c6f434c]

I am not good enough at AWK… does it check for slashy-ness?

Also, we still need to build some of the slashy systems then? (Probably not all because of tests with too many deps?)

[hraban]

I am not good enough at AWK… does it check for slashy-ness?

Also, we still need to build some of the slashy systems then? (Probably not all because of tests with too many deps?)

Upon further reflection I realize I was so focused on fixing this bug, I didn't stop to think about why it exists in the first place. The real question is why are these packages imported top-level?

[7c6f434c]

Upon further reflection I realize I was so focused on fixing this bug, I didn't stop to think about why it exists in the first place. The real question is why are these packages imported top-level?

Well, why wouldn't a rev-dep do that, I guess?…

[hraban]

I'm not familiar with how the graph is construced, but when I look at the quicklisp package dump "http://beta.quicklisp.org/dist/quicklisp/2022-07-08/releases.txt" I see the following entries for cl-async:

cl-async http://beta.quicklisp.org/archive/cl-async/2021-10-20/cl-async-20211020-git.tgz 57447 0e0cd11758e93a91b39ad726fb1051cc f32ba3f52003d06900086ec3a998bdc1d54816ea cl-async-20211020-git cl-async-repl.asd cl-async-ssl.asd cl-async-test.asd cl-async.asd
cl-async-await http://beta.quicklisp.org/archive/cl-async-await/2020-10-16/cl-async-await-20201016-git.tgz 14628 66d91f98068c3e9c57274f291bbf13fd 0728371bd39642b3d703378c21477c85a6bbfa4a cl-async-await-20201016-git cl-async-await.asd
cl-async-future http://beta.quicklisp.org/archive/cl-async-future/2015-01-13/cl-async-future-20150113-git.tgz 5719 961dbcb0bad3515ac7170f96dfd626ef 50751c2b573e0323f4c4687427df1d15b901cc38 cl-async-future-20150113-git cl-async-future.asd

The cl-util and cl-base packages are not loadable via quicklisp, because they're essentially internal to cl-async. You also can't (ql:quickload "cl-async-base").

It seems like we're fighting quicklisp and the individual packages by delving into their internal package structure and managing it with nix, too. I understand the dependency tracking, but maybe we should only expose top-level packages which are importable by quicklisp, too? I suspect nobody actually wants to import those internal packages in actual code, and they're only here to follow the (internal) dependency chain?

I wonder if this might be the source of our worries with mucking about with the .asd files, too (the renaming etc).

E.g., this very module we're commenting on, lisp-modules-new, also contains lisp code. It defines many systems, one for each file, which all include each other. They're not defined in individual .asd files, but top-level per package, but that seems like an implementation detail?

But this is all conjecture, I'd be very curious to learn from the original author.

[hraban]

For context, here are the cl-async packages exposed through lisp-modules-new:

  cl-async = {
  cl-async-await = {
  cl-async-base = {
  cl-async-future = {
  cl-async-repl = {
  cl-async-ssl = {
  cl-async-test = {
  cl-async-util = {

In this PR I work around a problem this introduces, but before we merge it I'd say the real question is why do we do this?

[7c6f434c]

In this PR I work around a problem this introduces, but before we merge it I'd say the real question is why do we do this?

1. Upstream Quicklisp system index does this. 2. Sometimes these additional systems are needed. 3. Sometimes they are actively not needed because they have too many extra deps, e.g. some tests. 3.1. Yes, there are some cyclical depednencies between ASD _files_ even though not between ASDF systems.

[hraban]

I understand the tracking (maybe), and you're right I see that in QL, too:

* (ql:system-apropos "cl-async")
#<SYSTEM cl-async / cl-async-20211020-git / quicklisp 2022-07-08>
#<SYSTEM cl-async-await / cl-async-await-20201016-git / quicklisp 2022-07-08>
#<SYSTEM cl-async-base / cl-async-20211020-git / quicklisp 2022-07-08>
#<SYSTEM cl-async-future / cl-async-future-20150113-git / quicklisp 2022-07-08>
#<SYSTEM cl-async-repl / cl-async-20211020-git / quicklisp 2022-07-08>
#<SYSTEM cl-async-ssl / cl-async-20211020-git / quicklisp 2022-07-08>
#<SYSTEM cl-async-test / cl-async-20211020-git / quicklisp 2022-07-08>
#<SYSTEM cl-async-util / cl-async-20211020-git / quicklisp 2022-07-08>

But QL makes a distinction between systems it tracks, and systems you can include top-level:

* (ql:quickload "cl-async-util")
To load "cl-async-util":
  Load 6 ASDF systems:
    bordeaux-threads cffi cl-libuv cl-ppcre fast-io vom
  Install 1 Quicklisp release:
    cl-async
; Loading "cl-async-util"

debugger invoked on a ASDF/FIND-COMPONENT:MISSING-COMPONENT in thread
#<THREAD "main thread" RUNNING {10046081E3}>:
  Component "cl-async-util" not found

Type HELP for debugger help, or (SB-EXT:EXIT) to exit from SBCL.

restarts (invokable by number or by possibly-abbreviated name):
  0: [RETRY                        ] Retry ASDF operation.
  1: [CLEAR-CONFIGURATION-AND-RETRY] Retry ASDF operation after resetting the
                                     configuration.
  2:                                 Retry ASDF operation.
  3:                                 Retry ASDF operation after resetting the
                                     configuration.
  4: [ABORT                        ] Give up on "cl-async-util"
  5: [REGISTER-LOCAL-PROJECTS      ] Register local projects and try again.
  6:                                 Exit debugger, returning to top level.

((:METHOD ASDF/OPERATE:OPERATE (SYMBOL T)) ASDF/LISP-ACTION:LOAD-OP "cl-async-util" :VERBOSE NIL) [fast-method]
; File has been modified since compilation:
;   SYS:CONTRIB;ASDF;ASDF.LISP.NEWEST
; Using form offset instead of character position.

   source: (ERROR 'MISSING-COMPONENT :REQUIRES COMPONENT)
0]

This seems like a sensible choice, given that cl-async doesn't intend for that system to be included in the first place. If nothing else, at the very least we're exposing internals.

More poignantly, the problem that this PR addresses, is precisely because of cl-async-base and cl-async-utils being separate Nix systems, with their own .asd files, etc. That is not something QL does.

Why do we need to expose those individual packages, unlike QL? Instead of offering one single top-level nix package per QL package (cl-async), pre-compiling all its "private" packages (cl-async-util, cl-async-base) as necessary, and let ASDF automatically find those dependent systems in that top-level package? If I understand it correctly, that's how QL does it.

I imagine it would obviate the need for this PR, and maybe even all that .asd-file rewriting code that currently lives atop imported.nix (the code which this PR extends).

@Uthar might have more thoughts on this

[Uthar]

Sorry my brain is already fixed on how I wrote it, it's hard to see another way after that. Maybe someone from outside can review.
To me there is no problem currently, everything works

[hraban]

Sorry my brain is already fixed on how I wrote it, it's hard to see another way after that. Maybe someone from outside can review. To me there is no problem currently, everything works

That's totally fair. I'll dig in and see what I can find, and come back to you with more specific questions or ideas once I have a better grasp of the system.

👍

[hraban]

This patch is obsolete with the new lispmodules merge, right? I haven't followed the implementation in a while. I've also created my own exploration of this in "https://github.com/hraban/cl-nix-lite" which attacks the problem from a different angle.

[Uthar]

Yes, recently a major rewrite of lisp modules was merged. But there are still such files with "unused" system definitions. Even though that is the case, they load fine. I guess it would be cool to remove them, but is not strictly necessary. I remember I attempted it with regexps, but it turned out too brittle. I also know almost nothing about AWK.

Thanks for the link!