radian-software / straight.el

🍀 Next-generation, purely functional package manager for the Emacs hacker.
MIT License
2.71k stars 150 forks source link

Don't install packages when :if condition fails in use-package #235

Open Luis-Henriquez-Perez opened 6 years ago

Luis-Henriquez-Perez commented 6 years ago

I noticed when using the use-package :if keyword to conditionally load a package and making sure the condition results as nil (ie. (use-package 'ivy :if nil)) the package was still installed. I could tell because the autoloaded commands were available. With straight-use-package-by-default set to t I run (macroexpand '(use-package ivy)) and get:

(progn
  (straight-use-package 'ivy)
  (defvar use-package--warning138
    (function
     (lambda (keyword err)
       (let ((msg (format "%s/%s: %s"
                          'ivy
                          keyword
                          (error-message-string err))))
         (display-warning 'use-package msg :error)))))
  (condition-case-unless-debug err
      nil
    (error (funcall use-package--warning138 :catch err))))

With this sexp calling use-package will always install ivy regardless of whether you use the :if or :disable keyword. It also seems that this may slow down startup because all packages are installed with straight immediately upon startup. Only the configuration and settings added by use-package are deferred but not the package itself. Ideally, straight-use-package should be called just before use-package tries to configure the package (maybe in between :init and :config).

I've been thinking about these three ways of fixing it: 1) create a wrapper around use-package - would involve duplicating a lot of the work use-package does 2) create a wrapper around a function that use-package uses - possibly best solution? 3) change the order of straight in use-package-keywords - risky

xuchunyang commented 6 years ago

I noticed when using the use-package :if keyword to conditionally load a package and making sure the condition results as nil (ie. (use-package 'ivy :if nil)) the package was still installed.

Maybe it is desired?

:straight servers the same purpose as use-package's built-in :ensure and it does the same thing, i.e., (use-package foo :if nil :ensure t) will install foo. I also find it is strange and reported last year in https://github.com/jwiegley/use-package/issues/415.

Luis-Henriquez-Perez commented 6 years ago

Oh ok. I can see how for some crucial packages like helm or ivy that would be desirable. However, is it possible to defer the package installation until needed? For instance, I wouldn't want (straight-use-package 'clojure-mode) to be called on startup because I might never use clojure in my emacs session.

Perhaps manually specifying the load-path would do it?

(use-package clojure-mode :load-path "~/.emacs.d/straight/build/clojure-mode")

raxod502 commented 6 years ago

This was previously tracked in #45, but figuring that out from the contents of the issue would probably have been quite difficult, so let's make this issue the canonical one.


There are three use cases here:

  1. Install package unconditionally, but don't configure it when the :if fails.
  2. Don't install package when the :if fails, but still register its recipe with straight.el.
  3. Don't install package or register recipe with straight.el when the :if fails.

Currently, case (1) happens when you do this:

(use-package foo
  :straight t
  :if <cond>)

And you can get case (2) by doing this:

(if <cond>
    (use-package foo
      :straight t)
  (straight-register-package 'foo))

Case (3) looks as follows:

(when <cond>
  (use-package foo
    :straight t))

Let me start out by saying that case (3) is problematic and you probably should never do it. The reason is that writing accurate version lockfiles relies on straight.el knowing what packages are part of your configuration, even if they are not all installed. You should always register all packages that are part of your configuration, even if you use lazy or conditional installation.

Now I'd be open to making case (2) what happens when you write the code for case (1). In that case, you could recover the behavior of case (1) by doing:

(straight-use-package 'foo)
(use-package foo
  :if <cond>)

Thoughts on which case should be the default?


However, is it possible to defer the package installation until needed? For instance, I wouldn't want (straight-use-package 'clojure-mode) to be called on startup because I might never use clojure in my emacs session.

This is lazy installation, which is an entirely different question than your original comment (which dealt with conditional installation). I have plans to add direct support for lazy installation to straight.el, but this requires a nontrivial amount of additional infrastructure. See #197.

If you want to implement a low-budget version yourself, it looks like this:

(straight-use-package-lazy 'clojure-mode)
(defun clojure-mode ()
  (interactive)
  (fmakunbound 'clojure-mode)
  (straight-use-package 'clojure-mode)
  (clojure-mode))

In fact, straight.el's use-package integration used to have support for doing this automatically for all functions declared in :commands, :bind, :mode, etc. But it required a lot of machinery on the use-package side. When @jwiegley refactored use-package, he rightfully decided to remove said machinery, which was kind of a hack, until such time as it could be re-added in a cleaner and more systematic way. Thus my new plan is to implement deferred installation support almost entirely on the straight.el side. I think it will be both cleaner and more useful.


(use-package clojure-mode :load-path "~/.emacs.d/straight/build/clojure-mode")

No, this is a really really bad idea. Never use :load-path with straight.el.

Luis-Henriquez-Perez commented 6 years ago

Thoughts on which case should be the default?

I personally would prefer making case (2) happen when writing the code for case (1) the default. It seems to make more intuitive and cleaner to me. Even recovering case (1) seems after changing the default seems simpler than getting case (2) currently.

I appreciate the detailed answers.

raxod502 commented 6 years ago

Let's leave this open in order to track making case (2) happen by default.