Sly fails to start due to package variance issue.

[mfiano]

SBCL has the ability to error on package variance instead of warn, which is incredibly useful during development. However, when updating SLY today, I ran into the following error in my inferior lisp buffer after M-x sly:

CL-USER> ; compiling file "/home/mfiano/.cache/emacs/straight/repos/sly/slynk/slynk-backend.lisp" (written 17 DEC 2021 02:36:24 AM):

; wrote /home/mfiano/.cache/common-lisp/sbcl-2.2.2.24-a5543b4c2-linux-x64/home/mfiano/.cache/emacs/straight/repos/sly/slynk/slynk-backend-tmp319UV2V8.fasl
; compilation finished in 0:00:00.336

debugger invoked on a SB-INT:PACKAGE-AT-VARIANCE-ERROR in thread
#<THREAD "main thread" RUNNING {1002640003}>:
  SLYNK-BACKEND also exports the following symbols:
  (SLYNK-BACKEND:BACKGROUND-SAVE-IMAGE
   SLYNK-BACKEND:CALL-WITH-DEBUGGING-ENVIRONMENT SLYNK-BACKEND:UNPROFILE
   SLYNK-BACKEND:INSTALL-DEBUGGER-GLOBALLY SLYNK-BACKEND:DEFAULT-DIRECTORY
   SLYNK-BACKEND:MAKE-INPUT-STREAM SLYNK-BACKEND:THREAD-ID
   SLYNK-BACKEND:ADD-FD-HANDLER SLYNK-BACKEND:MAKE-APROPOS-MATCHER
   SLYNK-BACKEND:FRAME-RESTARTABLE-P SLYNK-BACKEND:LISP-IMPLEMENTATION-PROGRAM
   SLYNK-BACKEND:MAKE-AUTO-FLUSH-THREAD ...)
See also:
  The ANSI Standard, Macro DEFPACKAGE

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

restarts (invokable by number or by possibly-abbreviated name):
  0: [DROP-THEM                    ] Unexport them.
  1: [KEEP-THEM                    ] Keep exporting them.
  2: [TRY-RECOMPILING              ] Recompile slynk-backend and try loading it again
  3: [RETRY                        ] Retry
                                     loading FASL for #<CL-SOURCE-FILE "slynk" "slynk-backend">.
  4: [ACCEPT                       ] Continue, treating
                                     loading FASL for #<CL-SOURCE-FILE "slynk" "slynk-backend">
                                     as having been successful.
  5:                                 Retry ASDF operation.
  6: [CLEAR-CONFIGURATION-AND-RETRY] Retry ASDF operation after resetting the
                                     configuration.
  7: [ABORT                        ] Exit debugger, returning to top level.

(SB-IMPL::NOTE-PACKAGE-VARIANCE :FORMAT-CONTROL "~A also exports the following symbols:~%  ~S" :FORMAT-ARGUMENTS ("SLYNK-BACKEND" (SLYNK-BACKEND:BACKGROUND-SAVE-IMAGE SLYNK-BACKEND:CALL-WITH-DEBUGGING-ENVIRONMENT SLYNK-BACKEND:UNPROFILE SLYNK-BACKEND:INSTALL-DEBUGGER-GLOBALLY SLYNK-BACKEND:DEFAULT-DIRECTORY SLYNK-BACKEND:MAKE-INPUT-STREAM SLYNK-BACKEND:THREAD-ID SLYNK-BACKEND:ADD-FD-HANDLER SLYNK-BACKEND:MAKE-APROPOS-MATCHER SLYNK-BACKEND:FRAME-RESTARTABLE-P SLYNK-BACKEND:LISP-IMPLEMENTATION-PROGRAM SLYNK-BACKEND:MAKE-AUTO-FLUSH-THREAD ...)) :PACKAGE #<PACKAGE "SLYNK-BACKEND">)

I admit, it's been quite a while since I updated SLY, but it appears during this period, something in SLY has been changed that tries exporting the same symbols multiple times. I could bisect etc if you need more info.

To reproduce, add this to your sbclrc file, as documented here:

(setf sb-ext:*on-package-variance* '(:warn nil :error t))

and then try starting SLY.

[mfiano]

It seems this problem was introduced in 8074da031ed9d677abc77fb3dab4bb42c6be76c3 so pinging @daewok

[daewok]

The package variance has been there for quite a while. You didn't notice it before because this (mostly) hid it:

#+sbcl
(defmethod operate :around ((o load-op) (c (eql (find-system :slynk))) &key &allow-other-keys)
  (let ((asdf:*compile-file-failure-behaviour* :warn)
        (sb-ext:*on-package-variance* '(:warn t)))
    (call-next-method)))

Unfortunately, it only hid it for (asdf:load-system :slynk). It was not hidden if Slynk was loaded as a dependency of another package. That's what #492 and #502 fixed. They muffle all compilation warnings and override the user's choice of sb-ext:*on-package-variance* when compiling Slynk, no matter if it's pulled in directly or indirectly.

I missed that the old behavior also overrode sb-ext:*on-package-variance* at load time, not just compile time. Unfortunately, ASDF does not provide an around load hook, so it's not trivial to fix. These are some options I see:

1. Fix the package variance issues instead of hiding them. I looked into this briefly. The major thing that dynamically exports symbols is the definterface macro. It's exported from the slynk-backend package. So if anyone uses it outside of the SLY codebase, changing it will likely break their code.
2. Readd the :around method on asdf:operate. This will fix things for people like @mfiano, but only if Slynk is loaded directly and not as a dependency.
3. Make a custom ASDF component class for Slynk files that has an :around method for performing a load-op that overrides the value of sb-ext:*on-package-variance* when loading the file. This would ensure the issues remain hidden for everyone.
4. Say that Slynk is incompatible with (setf sb-ext:*on-package-variance* '(:warn nil :error t)). Tell people to use (setf sb-ext:*on-package-variance* '(:warn (:slynk :slynk-backend) :error t)) instead.

[joaotavora]

1. Fix the package variance issues instead of hiding them. I looked into this briefly. The major thing that dynamically exports symbols is the definterface macro. It's exported from the slynk-backend package. So if anyone uses it outside of the SLY codebase, changing it will likely break their code.

I don't know any such code from the top of my head, but it's not unthinkable that a SLY contrib would use definterface to provide platform-specific behaviour.

This is tricky, but it needs a fix.

SBCL has the ability to error on package variance instead of warn, which is incredibly useful during development.

@mfiano, hmmm why is it so useful to you? And if its "during development", shouldn't it be set after the development environment (in your case SLY) has been loaded. Then you wouldn't have a problem, right?

[mfiano]

4. Say that Slynk is incompatible with (setf sb-ext:*on-package-variance* '(:warn nil :error t)). Tell people to use (setf sb-ext:*on-package-variance* '(:warn (:slynk :slynk-backend) :error t)) instead

I had completely forgot about the alternate syntax, despite linking to the manual above. This will likely be a good work-around for me, if the SLY codebase would take some time to be fixed, not that it needs to be fixed -- using cl:export explicitly is a valid thing to do.

@mfiano, hmmm why is it so useful to you? And if its "during development", shouldn't it be set after the development environment (in your case SLY) has been loaded. Then you wouldn't have a problem, right?

It is useful to me, because the default behavior issues a warning when altering a package form, which triggers SLY to show the compilation log buffer/window, and the source buffer is annotated with compilation issues, which I have to sly-remove-notes to hide. Additionally, this will continue to happen every time the package definition is recompiled thereafter. Closing the compilation log window, removing notes, and repeatedly doing so is an unnecessary inconvenience during development.

With the error t method, the debugger is opened with a restart to drop the old symbols from being exported, which is almost always what I want, because the problem is then not repeated on subsequent package definition recompiles.

As to why I cannot set it after the development environment has been loaded, that is because I have a custom core image, with useful things already defined. This is not as good of an excuse as the above reasoning as to why I prefer it to error, but it's just one more item in my workflow for efficiency.

That said, I think solution 4 above will be acceptable. I will change that and rebuild my core image to see how it goes.

[daewok]

I've got a local branch that mostly fixes the variance. I'll work on finishing it and submit it for discussion.

[mfiano]

Ok, I set (setf sb-ext:*on-package-variance* '(:warn (:slynk :slynk-backend :slynk-api) :error t)) to be compiled into my core image. Of note is the addition of :slynk-api that is also required to get around this issue, for anyone else stumbling upon this issue.