Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Straight doesn't reload a versioned dependency if an older version of the dependency is already loaded #822

Open
rmolinari opened this issue Jul 23, 2021 · 6 comments

Comments

@rmolinari
Copy link

rmolinari commented Jul 23, 2021

What's wrong

The bufler package (https://github.com/alphapapa/bufler.el) depends on version 2.1 (or higher) of map. My emacs, v27.2, ships with v 2.0 of map. Straight correctly resolves bufler's dependency and clones map from the ELPA mirror, but map is loaded by several other packages and, if one of these is loaded before bufler, the built-in version of map is loaded before straight can update the load-path.

  • M-x emacs-version: GNU Emacs 27.2 (build 1, x86_64-apple-darwin19.6.0, Carbon Version 162 AppKit 1894.6) of 2021-03-27
  • M-x straight-version: prerelease (HEAD -> develop, origin/develop) 72a1ce9 2021-07-06

Directions to reproduce

I have not been able to reproduce cleanly with straight-bug-report because its machinery itself triggers a "too-early" loading of map, apparently via a gnutls > network-stream > nsm > map dependency chain. Here is a ~/.emacs.d/init.el file that demonstrates the issue against an otherwise empty ~/.emacs.d:

(setq straight-repository-branch "develop")

(defvar bootstrap-version)
(let ((bootstrap-file
       (expand-file-name "straight/repos/straight.el/bootstrap.el" user-emacs-directory))
      (bootstrap-version 5))
  (unless (file-exists-p bootstrap-file)
    (with-current-buffer
        (url-retrieve-synchronously
         "https://raw.githubusercontent.com/raxod502/straight.el/develop/install.el"
         'silent 'inhibit-cookies)
      (goto-char (point-max))
      (eval-print-last-sexp)))
  (load bootstrap-file nil 'nomessage))

(require 'json)
(straight-use-package 'bufler)

Now execute M-x bufler to see the error Symbol's value as variable is void: max-width.

Loading json triggers Emacs to load the built-in version of map, which defeats straight's efforts to add to load-path the version it installs from the ELPA mirror on the next line. So bufler gets v2.0 of map and fails.

This problem can be avoided by telling straight about map before anything else tries to load it. This works as ~/.emacs.d/init.el once straight has been bootstrapped:

(load-file "~/.emacs.d/straight/repos/straight.el/bootstrap.el")
(straight-use-package 'map)
(require 'json)
(straight-use-package 'bufler)

and now M-x bufler works. This what I am doing now in my real init.el. But note that straight's bootstrapping code itself loads map and so this approach doesn't work in a completely clean environment.

It would be nice if straight noticed that the wrong version of map is loaded when it is handling bufler and reloaded the right version. I don't know if this is a reasonable expectation of straight.

Version information

  • Emacs version: GNU Emacs 27.2 (build 1, x86_64-apple-darwin19.6.0, Carbon Version 162 AppKit 1894.6) of 2021-03-27
  • Operating system: MacOS Catalina 10.15.7
@progfolio
Copy link
Contributor

progfolio commented Jul 24, 2021

If I understand correctly, the proper version of map is cloned and built. Bufler is compiled in a child Emacs process with this (correct) version of map. However, an older version of map is loaded prior to bufler being loaded.

I'll have to think on this to see if there's a good solution we can implement.
In the meantime, i think unloading the map feature prior to installing bufler might work. e.g.

(unload-feature 'map t)
(straight-use-package 'bufler)

This way the proper version of map should be on the load-path for whoever requires map next.
Unfortunately, there's no hook or macro that ties into require just before loading a package. That would be better as we could localize the effect of unloading map more. I wouldn't recommend this, but you could advise require to do just that:

(define-advice require (:before (&rest args) "bufler-reload-map.el")
  (when (and (eq (car args) 'bufler)
             (featurep 'map))
    (unload-feature 'map t)))

Again, not a great solution because we pay the penalty of that check for every call to require.
I'll do some more research to see if Emacs natively supports any such machinery.

@rmolinari
Copy link
Author

Yes, this works

(unload-feature 'map t)
(straight-use-package 'bufler)

It lets me keep the special handling for map close to the bufler section of my init.el. Thanks!

It is feasible for straight to warn about this sort of situation? "Oh, package foo requires v 3 of bar - which has been installed - but v 2 is already loaded."

@progfolio
Copy link
Contributor

Glad that works for you.

It is feasible for straight to warn about this sort of situation? "Oh, package foo requires v 3 of bar - which has been installed - but v 2 is already loaded."

I will look into implementing something like this.

@raxod502
Copy link
Member

raxod502 commented Jul 24, 2021

Oh no, that's terrible! I hoped something like this wouldn't happen. See #236 for previous discussion of the problem of built-in packages. The thing that's new here is that straight.el itself is loading map. I think for this special case using unload-feature would be not unreasonable, and we might even consider special-casing it to happen automatically for this specific package. Might be going too far, though.

I rather want to avoid bringing version numbers into it, though. For the last four and a half years, straight.el has gotten along pretty well without ever having a notion of version constraints for dependencies, which simplifies a whole bunch of things. Really it shouldn't matter whether the new version of map.el is required, you'd expect the version you requested to be loaded whether it's required or not.

@alphapapa
Copy link

we might even consider special-casing it to happen automatically for this specific package. Might be going too far, though.

Not to tell you what to do, but I think that would be going too far--if for no other reason than that it's not the only package of mine that requires map 2.1 or greater for the pcase patterns. :)

I don't know exactly how well unload-feature is intended to or expected to work, but in my own personal config, I've found it to work pretty well, as long as a dependency chain isn't interrupted. I use this function when I upgrade certain packages, which saves me from having to restart Emacs: https://github.com/alphapapa/unpackaged.el#reload-a-packages-features

@raxod502
Copy link
Member

raxod502 commented Oct 3, 2021

Oh, sorry, when I said "special-casing it to happen automatically for this specific package", by "this specific package" I meant map.el, not your bufler package that depends on map.el :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

4 participants