Issues with package location information on SBCL

[Eric Timmons]

I've started testing ASDF 3.3.1.3 with my group's code on SBCL 1.4.4
and noticed some issues with uiop:define-package (due to commit
8281e011).

First, when compiling ASDF I get 456 compilation notes
(https://pastebin.com/NnRUKGWe). I get the same notes when using
uiop:define-package in our code as well. I honestly think this is an
issue of SBCL being over aggressive. It's also odd because if the
recording of the source location is removed, then the notes aren't
produced. It appears this started happening in SBCL 1.4.1 (potentially
due to commit af5e2ed1e).

The volume of notes (especially when using package-inferred-system)
can drown out real issues. I'll likely report this as an issue to SBCL
since the keywords *should* be constant and the source transform for
apply doesn't seem to be preserving that. However, it could also be
fixed in ASDF by changing the apply to a funcall in the define-package
expansion (patch 2).

Second, the comma on (sb-c:source-location) seems to cause the source
location for the package to always point to the body of the
define-package macro. Patch 1 changes this so that form is evaluated
after the macro is expanded. I didn't test this on anything earlier
than SBCL 1.4.4, but I don't believe this behavior is version
dependent.

-Eric

[Faré]

Thanks a lot for fixing this issue! I opened a MR based on it:
https://gitlab.common-lisp.net/asdf/asdf/merge_requests/92

—♯ƒ • François-René ÐVB Rideau •Reflection&Cybernethics• http://fare.tunes.org
To be an anarchist only means that you believe that aggression is not
justified, and that states necessarily employ aggression. And, therefore,
that states, and the aggression they necessarily employ, are unjustified.
It's quite simple, really. It's an ethical view, so no surprise
it confuses utilitarians.
—- N. Stephan Kinsella

Post by Eric Timmons
I've started testing ASDF 3.3.1.3 with my group's code on SBCL 1.4.4
and noticed some issues with uiop:define-package (due to commit
8281e011).
First, when compiling ASDF I get 456 compilation notes
(https://pastebin.com/NnRUKGWe). I get the same notes when using
uiop:define-package in our code as well. I honestly think this is an
issue of SBCL being over aggressive. It's also odd because if the
recording of the source location is removed, then the notes aren't
produced. It appears this started happening in SBCL 1.4.1 (potentially
due to commit af5e2ed1e).
The volume of notes (especially when using package-inferred-system)
can drown out real issues. I'll likely report this as an issue to SBCL
since the keywords *should* be constant and the source transform for
apply doesn't seem to be preserving that. However, it could also be
fixed in ASDF by changing the apply to a funcall in the define-package
expansion (patch 2).
Second, the comma on (sb-c:source-location) seems to cause the source
location for the package to always point to the body of the
define-package macro. Patch 1 changes this so that form is evaluated
after the macro is expanded. I didn't test this on anything earlier
than SBCL 1.4.4, but I don't believe this behavior is version
dependent.
-Eric

[Faré]

Glad to help! I've also opened the following bug on SBCL to let them know
about it: https://bugs.launchpad.net/sbcl/+bug/1750466.

Thanks!

Also, I checked that nothing else in ASDF uses `parse-define-package-form`,
but I somehow missed until just now that it's actually exported from uiop
=/. Is there any concern about another library using it? If so, I can try to
fix it in such a way that the output of `parse-define-package-form` is
unchanged.

I wouldn't worry about that: grepping through quicklisp systems reveals no user.

—♯ƒ • François-René ÐVB Rideau •Reflection&Cybernethics• http://fare.tunes.org
It costs the nation millions to keep Gandhi living in poverty — Sarojini Naidu

[Robert Goldman]

This does seem to illustrate an issue with the current "export
everything that's in UIOP" strategy.

Should we consider changing this policy?

Or, if we have "internal" functions that we don't want to be visible
through `UIOP/DRIVER`, should we simply not export them from the
sub-packages and use `:import-from` to move them among the sub-packages?

Best,
r

Post by Faré

Glad to help! I've also opened the following bug on SBCL to let them know
about it: https://bugs.launchpad.net/sbcl/+bug/1750466.

Thanks!

Also, I checked that nothing else in ASDF uses
`parse-define-package-form`,
but I somehow missed until just now that it's actually exported from uiop
=/. Is there any concern about another library using it? If so, I can try to
fix it in such a way that the output of `parse-define-package-form` is
unchanged.

I wouldn't worry about that: grepping through quicklisp systems reveals no user.
—♯ƒ • François-René ÐVB Rideau •Reflection&Cybernethics•
http://fare.tunes.org
It costs the nation millions to keep Gandhi living in poverty —
Sarojini Naidu

[Faré]

This does seem to illustrate an issue with the current "export everything
that's in UIOP" strategy.
Should we consider changing this policy?

1- UIOP reexports everything that individual subpackages of UIOP
export. I believe that's a simple and good policy. If a helper must
remain private, don't export it in the subpackage.
2- UIOP/PACKAGE is special, in that it is defined by defpackage (for
bootstrap reasons), and that therefore to portably ensure
upgradability, the list of symbols it exports MAY NEVER CHANGE, EVER.
No adds, no deletes, no renames. No change. If you need any change,
define and export a different package. Or use defpackage to define an
empty package or constant package, then define-package to import-from
and reexport from it.

Or, if we have "internal" functions that we don't want to be visible through
UIOP/DRIVER, should we simply not export them from the sub-packages and use
:import-from to move them among the sub-packages?

Yes, that's the idea so far.

Also, we've moved symbols within UIOP in the past, and the
UIOP/subpackage names aren't stable. If you're using a utility in
UIOP, use UIOP: as a symbol prefix, don't use the symbol from its
current subpackage.

—♯ƒ • François-René ÐVB Rideau •Reflection&Cybernethics• http://fare.tunes.org
Statism is the secular version of salvation through faith: it doesn't
matter what bureaucrats do, only that they do it with good intentions.

[Eric Timmons]

https://bugs.launchpad.net/sbcl/+bug/1750466.

This bug has been fixed upstream and should probably appear in SBCL 1.4.7.

Since they got that fixed before ASDF 3.3.2 got out, is it worth
reverting commit 21e3a85? That would keep the return of
`parse-define-package-form` unchanged and the warnings would only show
up in SBCL 1.4.1-1.4.6 inclusive.

-Eric