Use-package: `:if` keyword only work if package was installed previously

[caadar]

Let's test it e.g. with async package.

emacs -Q:

(require 'package)
(package-initialize)

(package-installed-p 'async)

=> nil, yes async not installed.

(use-package async
  :if nil
  :ensure t)

(package-installed-p 'async)

=> t, but nil expected cuz :if nil.
(featurep 'async)
=> t, so why it was loaded unconditionally?

After emacs restarted use-package does not load async in case of :t nil, it's ok.

Is this intended behavior?

[thomasf]

Yes..

edit: I'm not sure anymore by looking at the code..The code has changed a lot since i last read it.

[jwiegley]

Note that the intended way of disabling a configuration is not :if nil (which is dynamic), but :disabled t (which is static, and is equivalent to having never written the config block at all).

[caadar]

Well, :if nil here is just a false condition.

Actually needed ability to conditionally install and load a package, say

(when window-system
  (use-package async
  :ensure t))

always prevent package install/loading when window system not present despite of async
was installed or not.

Feel free to close the issue if this not a bug.

[jwiegley]

Since I didn't write the :ensure logic, I don't know what its semantics are. I don't know if it installs each package as it is encountered during loadup, in which case I would have expected :if nil to do what you expect.

[caadar]

This is not a bug. Logic may be changed by use-package-keywords tuning: just set :ensure keyword after :if :when :unless ones. It change keywords sorting so conditions will be checked before :ensure part.

May be it's worth to set this as dafault for use-package-keywords? Current keywords order seems misleading. Feel free to close the issue if it is not desirable by some reason.

[jwiegley]

This sounds like a good solution. Anyone disagree?

[caadar]

According to use-package docstring:

:preface Code to be run before everything except ‘:disabled’; this can be used to define functions for use in ‘:if’...

so consider to place :if :when :unless keywords right after the :preface one.

[raxod502]

Playing devil's advocate, one might also want a package to be installed unconditionally, but only configured and/or loaded :if some condition holds. This is currently possible by using :if vs. wrapping in a when block. Under the proposed change, there would be no way to do that.

Since I don't use :if, I have no opinion on this matter. Just thought I should point it out.

[raxod502]

@caadar FYI, I believe that package.el may cause some features to be loaded during initial installation (due to byte-compilation), so that's why you observed (featurep 'async) to be non-nil in the third test. There's no way around that AFAIK without modifying package.el, and any changes to package.el are going to have to go through the mailing list.

[caadar]

@raxod502 thanx for explonation about package.el, currently I satisfied without touching it.

This is currently possible by using :if vs. wrapping in a when block. Under the proposed change, there would be no way to do that.

Why? Just set it back (setq use-package-keywords '(... :ensure :if :when :unless ...)) if really needed.

The question is about sane default value.

[raxod502]

@caadar IMO, having the user set use-package-keywords directly is an unacceptable solution under any circumstances. If you put something like that in your config, it will introduce subtle bugs every time use-package upstream changes the keyword ordering, and you also have to keep track of the side effects of every other package that modifies use-package-keywords.

Not that I'm saying it doesn't make sense to provide a customization option for whether :if affects :ensure—but having use-package-keywords be that option is not a good solution.

[caadar]

If you put something like that in your config, it will introduce subtle bugs every time use-package upstream changes the keyword ordering, and you also have to keep track of the side effects of every other package that modifies use-package-keywords.

The reason I lobbing the change :)

BTW, wrapping use-package in a when block seems visually crude for me.

[caadar]

having the user set use-package-keywords directly is an unacceptable solution under any circumstances.

use-package-keywords avalable for customize though.

[raxod502]

@caadar

The reason I lobbing the change :)

BTW, wrapping use-package in a when block seems visually crude for me.

I'm not disagreeing with your goal; I'm just saying that a mechanism based on end-user modification of use-package-keywords is not an appropriate way to achieve that goal. It would be much better to have a boolean variable use-package-if-affects-ensure-p that can be customized to get either behavior.

I mean, what would the documentation look like? "If you want :if to affect :ensure, override use-package-keywords to move this keyword from here to over there"? What happens when we get more options like this? It would be a combinatorial explosion of possible values for use-package-keywords, which would not be future-proof to changes upstream.

Of course, if we think that the current behavior is just plain wrong, and nobody should be allowed to use it, then your proposal makes perfect sense: it's only if we want to support both use cases that there would be a problem.

use-package-keywords avalable for customize though.

I think modifying use-package-keywords via Custom is a terrible idea: it will not interact correctly with other packages wishing to modify the variable. Nobody should be setting use-package-keywords directly, only adding and removing things from it.

Maybe @jwiegley can elaborate on how use-package-keywords is intended to be used by the end user, in which case his knowledge obviously will supersede mine.

[caadar]

Agreed completely!

[thomasf]

Well,. subtle errors form upgrades are possible with any package which has a defcustom.

If you really want to be bothered with it you could do something in your emacs init after loading use-package which checks if the default value has changed. I haven't really tested this code properly but it should be about right:

(unless
    (equal
     (cadr (car (get 'use-package-keywords 'standard-value)))
     '(:disabled :preface :pin :defer-install :ensure :if :when :unless :requires :load-path :no-require :bind :bind* :bind-keymap :bind-keymap* :interpreter :mode :magic :magic-fallback :commands :defines :functions :defer :init :after :demand :config :diminish :delight))
  (warn "use-package-keywords default value has changed!"))

You can also load the list value and rearrange it in elisp code instead of setting it to a static value.

[raxod502]

Well,. subtle errors form upgrades are possible with any package which has a defcustom.

Yes, so I think defcustoms should be kept as simple as possible, and defcustoms should not be created for things which will cause subtle bugs if the default values are changed.

In this case, the user should not actually be customizing use-package-keywords. They should only be customizing 5% of use-package-keywords, so a better solution would be to have a single boolean value that can be customized, such that customizing this value will have the same effect as reassigning use-package-keywords in the appropriate way.

If you really want to be bothered with it you could do something like this in your emacs init

I would actually use el-patch-defvar from my package el-patch, which is designed precisely for this use case and does basically what you describe. But el-patch should only be necessary when a package has been designed poorly, IMO (it was basically created as a power tool for making internal changes to packages without forking them).

You can also load the list value and rearrange it in elisp code instead of setting it to a static value.

Yes, that's what packages which extend use-package do. And it's the right thing to do. But it's a big hassle for the end user to deal with, when we could instead provide a nice boolean defcustom with a docstring that they could set.

[thomasf]

I agree that it probably should't be a defcustom but I don't think it's a generally solvable problem.
Conflicts can easily occur when the user and one or more packages tries to be smart about how to rearrange the keywords using relative positions to other keywords (like before adding something before :ensure for an non obvious reason) . There will always be the possibility of cascading changes and thus still be able to cause subtle errors.

[raxod502]

Agreed. It's definitely not a generally soluble problem.

However, I think that if we make it so that the user never has to mess with use-package-keywords, then at least we have eliminated one factor, which always helps.

I am currently experimenting with a more general weighted-priority middleware model for use-package-style keywords in my with-feature package; see in particular the with-feature-defmiddleware declarations. Just thought I would mention this because one of my goals with it is to make it easier to write future-proof extensions that add keywords and modify or remove existing behavior.

[jwiegley]

use-package-keywords was created to allow other package authors to extend or alter the behavior of use-package. It was never intended to be used by any but the most advanced users (who would know how to debug issues arising from divergence from the default value).

[shackra]

I recently got bitten by this bug (?)

Weechat package won't work on Windows because when it is byte compiled the error dbus-call-method: peculiar error: "Emacs not compiled with dbus support" will stop my configuration from loading.

Even if I use :if (executable-find "weechat") which evals to nil the package will be downloaded and byte compiled anyway.

Maybe a workaround for me is to set :disabled (not (executable-find "weechat")), I don't know if the code gets evaluated for that keyword, but is a shame :if keyword don't behaves like his brother :disabled

[raxod502]

@shackra :disabled does not do what you think it does: see https://github.com/jwiegley/use-package/issues/387.

The correct workaround is to wrap your use-package form in a when block.

[shackra]

I see, thanks @raxod502