jwiegley / use-package

A use-package declaration for simplifying your .emacs
https://jwiegley.github.io/use-package
GNU General Public License v3.0
4.41k stars 261 forks source link

Issue with using the `use-package-always-ensure` option #190

Closed kaushalmodi closed 6 years ago

kaushalmodi commented 9 years ago

Hi ,

I tried out the new option and set it to t.

But it gives me this error:

Eager macro-expansion failure: (error "Package defuns-' is unavailable") package-compute-transaction: Packagedefuns-' is unavailable

I have the below in my emacs init:

(defun prepend-path ( my-path )
  (setq load-path (cons (expand-file-name my-path) load-path)))

(prepend-path (concat user-emacs-directory "/elisp"))
;; ...
(package-initialize)
;; ...
(eval-when-compile
  (require 'use-package)
  (setq use-package-always-ensure t))
(use-package bind-key)
(use-package defuns)

The defuns.el is present in that elisp/ path.

But is looks like defuns is being tried to be installed by the package manager (even though it is available in the load-path) when this new option is t (or probably because :ensure is t).

jwiegley commented 9 years ago

Can you run M-x toggle-debug-on-error and paste the full backtrace of the error? Also can you show me the evaluation of:

(pp-to-string (macroexpand '(use-package defuns)))
kaushalmodi commented 9 years ago

Sure.

Here is the output of emacs --debug-init

Debugger entered--Lisp error: (error "Package `defuns-' is unavailable")
  signal(error ("Package `defuns-' is unavailable"))
  error("Package `%s-%s' is unavailable" defuns "")
  package-compute-transaction(nil ((defuns)))
  package-install(defuns)
  (progn (package-install package))
  (if (not (package-installed-p package)) (progn (package-install package)))
  use-package-ensure-elpa(defuns)
  (progn (require (quote package)) (use-package-ensure-elpa package-name))
  (if package-name (progn (require (quote package)) (use-package-ensure-elpa package-name)))
  (let ((package-name (or (and (eq ensure t) name-symbol) ensure))) (if package-name (progn (require (quote package)) (use-package-ensure-elpa package-name))))
  (let ((body (use-package-process-keywords name-symbol rest state))) (let ((package-name (or (and (eq ensure t) name-symbol) ensure))) (if package-name (progn (require (quote package)) (use-package-ensure-elpa package-name)))) body)
  use-package-handler/:ensure(defuns :ensure t (:config (t)) nil)
  funcall(use-package-handler/:ensure defuns :ensure t (:config (t)) nil)
  (if (functionp handler-sym) (funcall handler-sym name-symbol keyword arg rest state) (use-package-error (format "Keyword handler not defined: %s" handler)))
  (let* ((handler (concat "use-package-handler/" (symbol-name keyword))) (handler-sym (intern handler))) (if (functionp handler-sym) (funcall handler-sym name-symbol keyword arg rest state) (use-package-error (format "Keyword handler not defined: %s" handler))))
  (let* ((keyword (car plist)) (arg (cadr plist)) (rest (cddr plist))) (if (keywordp keyword) nil (use-package-error (format "%s is not a keyword" keyword))) (let* ((handler (concat "use-package-handler/" (symbol-name keyword))) (handler-sym (intern handler))) (if (functionp handler-sym) (funcall handler-sym name-symbol keyword arg rest state) (use-package-error (format "Keyword handler not defined: %s" handler)))))
  (if (null plist) nil (let* ((keyword (car plist)) (arg (cadr plist)) (rest (cddr plist))) (if (keywordp keyword) nil (use-package-error (format "%s is not a keyword" keyword))) (let* ((handler (concat "use-package-handler/" (symbol-name keyword))) (handler-sym (intern handler))) (if (functionp handler-sym) (funcall handler-sym name-symbol keyword arg rest state) (use-package-error (format "Keyword handler not defined: %s" handler))))))
  use-package-process-keywords(defuns (:ensure t :config (t)))
  (macroexp-progn (use-package-process-keywords name-symbol args*))
  (let ((body (macroexp-progn (use-package-process-keywords name-symbol args*)))) (if use-package-debug (display-buffer (save-current-buffer (let ((buf (get-buffer-create "*use-package*"))) (save-current-buffer (set-buffer buf) (delete-region (point-min) (point-max)) (emacs-lisp-mode) (insert (pp-to-string body))) buf)))) body)
  (let* ((name-symbol (if (stringp name) (intern name) name)) (args0 (use-package-plist-maybe-put (use-package-normalize-plist name-symbol args) :config (quote (t)))) (args* (use-package-sort-keywords (if use-package-always-ensure (use-package-plist-maybe-put args0 :ensure use-package-always-ensure) args0)))) (if (and (boundp (quote byte-compile-current-file)) byte-compile-current-file) (setq args* (use-package-plist-cons args* :preface (cons (quote eval-when-compile) (append (mapcar (function ...) (plist-get args* :defines)) (list (list ... ... ... ...))))))) (let ((body (macroexp-progn (use-package-process-keywords name-symbol args*)))) (if use-package-debug (display-buffer (save-current-buffer (let ((buf ...)) (save-current-buffer (set-buffer buf) (delete-region ... ...) (emacs-lisp-mode) (insert ...)) buf)))) body))
  (if (member :disabled args) nil (let* ((name-symbol (if (stringp name) (intern name) name)) (args0 (use-package-plist-maybe-put (use-package-normalize-plist name-symbol args) :config (quote (t)))) (args* (use-package-sort-keywords (if use-package-always-ensure (use-package-plist-maybe-put args0 :ensure use-package-always-ensure) args0)))) (if (and (boundp (quote byte-compile-current-file)) byte-compile-current-file) (setq args* (use-package-plist-cons args* :preface (cons (quote eval-when-compile) (append (mapcar ... ...) (list ...)))))) (let ((body (macroexp-progn (use-package-process-keywords name-symbol args*)))) (if use-package-debug (display-buffer (save-current-buffer (let (...) (save-current-buffer ... ... ... ...) buf)))) body)))
  (lambda (name &rest args) "Declare an Emacs package by specifying a group of configuration options.\n\nFor full documentation, please see the README file that came with\nthis file.  Usage:\n\n  (use-package package-name\n     [:keyword [option]]...)\n\n:init          Code to run before PACKAGE-NAME has been loaded.\n:config        Code to run after PACKAGE-NAME has been loaded.  Note that if\n               loading is deferred for any reason, this code does not execute\n               until the lazy load has occurred.\n:preface       Code to be run before everything except `:disabled'; this can\n               be used to define functions for use in `:if', or that should be\n               seen by the byte-compiler.\n\n:mode          Form to be added to `auto-mode-alist'.\n:interpreter   Form to be added to `interpreter-mode-alist'.\n\n:commands      Define autoloads for commands that will be defined by the\n               package.  This is useful if the package is being lazily loaded,\n               and you wish to conditionally call functions in your `:init'\n               block that are defined in the package.\n\n:bind          Bind keys, and define autoloads for the bound commands.\n:bind*         Bind keys, and define autoloads for the bound commands,\n               *overriding all minor mode bindings*.\n:bind-keymap   Bind a key prefix to an auto-loaded keymap defined in the\n               package.  This is like `:bind', but for keymaps.\n:bind-keymap*  Like `:bind-keymap', but overrides all minor mode bindings\n\n:defer         Defer loading of a package -- this is implied when using\n               `:commands', `:bind', `:bind*', `:mode' or `:interpreter'.\n               This can be an integer, to force loading after N seconds of\n               idle time, if the package has not already been loaded.\n:demand        Prevent deferred loading in all cases.\n\n:if EXPR       Initialize and load only if EXPR evaluates to a non-nil value.\n:disabled      The package is ignored completely if this keyword is present.\n:defines       Declare certain variables to silence the byte-compiler.\n:functions     Declare certain functions to silence the byte-compiler.\n:load-path     Add to the `load-path' before attempting to load the package.\n:diminish      Support for diminish.el (if installed).\n:ensure        Loads the package using package.el if necessary.\n:pin           Pin the package to an archive." (if (member :disabled args) nil (let* ((name-symbol (if (stringp name) (intern name) name)) (args0 (use-package-plist-maybe-put (use-package-normalize-plist name-symbol args) :config (quote (t)))) (args* (use-package-sort-keywords (if use-package-always-ensure (use-package-plist-maybe-put args0 :ensure use-package-always-ensure) args0)))) (if (and (boundp (quote byte-compile-current-file)) byte-compile-current-file) (setq args* (use-package-plist-cons args* :preface (cons (quote eval-when-compile) (append ... ...))))) (let ((body (macroexp-progn (use-package-process-keywords name-symbol args*)))) (if use-package-debug (display-buffer (save-current-buffer (let ... ... buf)))) body))))(defuns)
  (use-package defuns)
  eval-buffer(#<buffer  *load*> nil "/home/kmodi/.emacs.d/init.el" nil t)  ; Reading at buffer position 4835
  load-with-code-conversion("/home/kmodi/.emacs.d/init.el" "/home/kmodi/.emacs.d/init.el" t t)
  load("/home/kmodi/.emacs.d/init" t t)
  #[0 "\205\262

If I try to eval pp-to-string in the same session, I get the same error as in the first post:

Eager macro-expansion failure: (error "Package `defuns-' is unavailable") [2 times]
package-compute-transaction: Package `defuns-' is unavailable

All of this was while use-package-always-ensure was set to t.

But in the same session, after I eval (setq use-package-always-ensure nil), the above pp-to-string statement you suggested evals to below:

"(if
    (not
     (require 'defuns nil t))
    (ignore
     (display-warning 'use-package
                      (format \"Could not load %s\" 'defuns)
                      :error)))
"
jwiegley commented 9 years ago

Setting use-package-always-ensure to t is the same as putting an :ensure t in that defuns form. I'm still not sure where things are going wrong.

npostavs commented 9 years ago

I'm still not sure where things are going wrong.

It's attempting to install a non-existent package defuns.

I think what @kaushalmodi meant in https://github.com/jwiegley/use-package/commit/0e87adababb0c289b22f9137b421eb948f75db2d#commitcomment-10344705 is that :load-path <foo-path> should imply :ensure nil, or :ensure (not (locate-library <foo> nil <foo-path>)), or perhaps :ensure t should check the result of (locate-library <foo> nil <foo-path>) before attempting any package operations.

kaushalmodi commented 9 years ago

@npostavs Right on!

:ensure t must not try to install a package using package manager if one of the below is true:

In the example of this defuns package. It can already be found by emacs as I added it to load-path:

(defun prepend-path ( my-path )
  (setq load-path (cons (expand-file-name my-path) load-path)))

(prepend-path (concat user-emacs-directory "/elisp"))

(My defuns.el lies in that elisp/ dir. So a plain old (require 'defuns) would work right-away.) In such a case, use-package must not try to install that package using the package manager even if :ensure t (either set explicitly or using the new option use-package-always-ensure).

npostavs commented 9 years ago

That package already exists on load-path

That could be problematic since the check happens at macroexpand time, while load-path might be changed at run time, e.g.

(add-to-list 'load-path "~/src/my-elisp/") ; assume "my-functions.el" is in "~/src/my-elisp"

(use-package my-functions
  :ensure t)

At macroexpand time, "~/src/my-elisp/" hasn't been added to load-path yet, so we try to package-install the my-functions package. So this may or may not work depending on how the eager macro expansion occurs (and it definitely won't work if you byte compile).

jwiegley commented 9 years ago

Could someone who relies on :ensure comment on whether its logic should happen at expansion time or at evaluation time?

kaushalmodi commented 9 years ago

Also it looks like :ensure t tries to install a package using the package manager even when I have specified :load-path (first condition listed in my above comment: https://github.com/jwiegley/use-package/issues/190#issuecomment-85173911)

I have a package de-ansi which I am loading using :load-path and I have set use-package-always-ensure to t.

Here is how I load it:

(use-package de-ansi
  :load-path "elisp/de-ansi"
  :commands (de-ansify)
  :init
  (progn
    ;; ...
))

But emacs --debug-init gives:

Debugger entered--Lisp error: (error "Package `de-ansi-' is unavailable")
  signal(error ("Package `de-ansi-' is unavailable"))
  error("Package `%s-%s' is unavailable" de-ansi "")
  package-compute-transaction(nil ((de-ansi)))
  package-install(de-ansi)
  (progn (package-install package))
  (if (not (package-installed-p package)) (progn (package-install package)))
  use-package-ensure-elpa(de-ansi)
  (progn (require (quote package)) (use-package-ensure-elpa package-name))
  (if package-name (progn (require (quote package)) (use-package-ensure-elpa package-name)))
  (let ((package-name (or (and (eq ensure t) name-symbol) ensure))) (if package-name (progn (require (quote package)) (use-package-ensure-elpa package-name))))
  (let ((body (use-package-process-keywords name-symbol rest state))) (let ((package-name (or (and (eq ensure t) name-symbol) ensure))) (if package-name (progn (require (quote package)) (use-package-ensure-elpa package-name)))) body)
  use-package-handler/:ensure(de-ansi :ensure t (:load-path ("/home/kmodi/.emacs.d/elisp/de-ansi") :commands (de-ansify) :init ((progn (bind-to-modi-map "d" de-ansify))) :config (t) :defer t) nil)
  funcall(use-package-handler/:ensure de-ansi :ensure t (:load-path ("/home/kmodi/.emacs.d/elisp/de-ansi") :commands (de-ansify) :init ((progn (bind-to-modi-map "d" de-ansify))) :config (t) :defer t) nil)
  (if (functionp handler-sym) (funcall handler-sym name-symbol keyword arg rest state) (use-package-error (format "Keyword handler not defined: %s" handler)))
  (let* ((handler (concat "use-package-handler/" (symbol-name keyword))) (handler-sym (intern handler))) (if (functionp handler-sym) (funcall handler-sym name-symbol keyword arg rest state) (use-package-error (format "Keyword handler not defined: %s" handler))))
  (let* ((keyword (car plist)) (arg (cadr plist)) (rest (cddr plist))) (if (keywordp keyword) nil (use-package-error (format "%s is not a keyword" keyword))) (let* ((handler (concat "use-package-handler/" (symbol-name keyword))) (handler-sym (intern handler))) (if (functionp handler-sym) (funcall handler-sym name-symbol keyword arg rest state) (use-package-error (format "Keyword handler not defined: %s" handler)))))
  (if (null plist) nil (let* ((keyword (car plist)) (arg (cadr plist)) (rest (cddr plist))) (if (keywordp keyword) nil (use-package-error (format "%s is not a keyword" keyword))) (let* ((handler (concat "use-package-handler/" (symbol-name keyword))) (handler-sym (intern handler))) (if (functionp handler-sym) (funcall handler-sym name-symbol keyword arg rest state) (use-package-error (format "Keyword handler not defined: %s" handler))))))
  use-package-process-keywords(de-ansi (:ensure t :load-path ("/home/kmodi/.emacs.d/elisp/de-ansi") :commands (de-ansify) :init ((progn (bind-to-modi-map "d" de-ansify))) :config (t) :defer t))
  (macroexp-progn (use-package-process-keywords name-symbol args*))
  (let ((body (macroexp-progn (use-package-process-keywords name-symbol args*)))) (if use-package-debug (display-buffer (save-current-buffer (let ((buf (get-buffer-create "*use-package*"))) (save-current-buffer (set-buffer buf) (delete-region (point-min) (point-max)) (emacs-lisp-mode) (insert (pp-to-string body))) buf)))) body)
  (let* ((name-symbol (if (stringp name) (intern name) name)) (args0 (use-package-plist-maybe-put (use-package-normalize-plist name-symbol args) :config (quote (t)))) (args* (use-package-sort-keywords (if use-package-always-ensure (use-package-plist-maybe-put args0 :ensure use-package-always-ensure) args0)))) (if (and (boundp (quote byte-compile-current-file)) byte-compile-current-file) (setq args* (use-package-plist-cons args* :preface (cons (quote eval-when-compile) (append (mapcar (function ...) (plist-get args* :defines)) (list (list ... ... ... ...))))))) (let ((body (macroexp-progn (use-package-process-keywords name-symbol args*)))) (if use-package-debug (display-buffer (save-current-buffer (let ((buf ...)) (save-current-buffer (set-buffer buf) (delete-region ... ...) (emacs-lisp-mode) (insert ...)) buf)))) body))
  (if (member :disabled args) nil (let* ((name-symbol (if (stringp name) (intern name) name)) (args0 (use-package-plist-maybe-put (use-package-normalize-plist name-symbol args) :config (quote (t)))) (args* (use-package-sort-keywords (if use-package-always-ensure (use-package-plist-maybe-put args0 :ensure use-package-always-ensure) args0)))) (if (and (boundp (quote byte-compile-current-file)) byte-compile-current-file) (setq args* (use-package-plist-cons args* :preface (cons (quote eval-when-compile) (append (mapcar ... ...) (list ...)))))) (let ((body (macroexp-progn (use-package-process-keywords name-symbol args*)))) (if use-package-debug (display-buffer (save-current-buffer (let (...) (save-current-buffer ... ... ... ...) buf)))) body)))
  (lambda (name &rest args) "Declare an Emacs package by specifying a group of configuration options.\n\nFor full documentation, please see the README file that came with\nthis file.  Usage:\n\n  (use-package package-name\n     [:keyword [option]]...)\n\n:init          Code to run before PACKAGE-NAME has been loaded.\n:config        Code to run after PACKAGE-NAME has been loaded.  Note that if\n               loading is deferred for any reason, this code does not execute\n               until the lazy load has occurred.\n:preface       Code to be run before everything except `:disabled'; this can\n               be used to define functions for use in `:if', or that should be\n               seen by the byte-compiler.\n\n:mode          Form to be added to `auto-mode-alist'.\n:interpreter   Form to be added to `interpreter-mode-alist'.\n\n:commands      Define autoloads for commands that will be defined by the\n               package.  This is useful if the package is being lazily loaded,\n               and you wish to conditionally call functions in your `:init'\n               block that are defined in the package.\n\n:bind          Bind keys, and define autoloads for the bound commands.\n:bind*         Bind keys, and define autoloads for the bound commands,\n               *overriding all minor mode bindings*.\n:bind-keymap   Bind a key prefix to an auto-loaded keymap defined in the\n               package.  This is like `:bind', but for keymaps.\n:bind-keymap*  Like `:bind-keymap', but overrides all minor mode bindings\n\n:defer         Defer loading of a package -- this is implied when using\n               `:commands', `:bind', `:bind*', `:mode' or `:interpreter'.\n               This can be an integer, to force loading after N seconds of\n               idle time, if the package has not already been loaded.\n:demand        Prevent deferred loading in all cases.\n\n:if EXPR       Initialize and load only if EXPR evaluates to a non-nil value.\n:disabled      The package is ignored completely if this keyword is present.\n:defines       Declare certain variables to silence the byte-compiler.\n:functions     Declare certain functions to silence the byte-compiler.\n:load-path     Add to the `load-path' before attempting to load the package.\n:diminish      Support for diminish.el (if installed).\n:ensure        Loads the package using package.el if necessary.\n:pin           Pin the package to an archive." (if (member :disabled args) nil (let* ((name-symbol (if (stringp name) (intern name) name)) (args0 (use-package-plist-maybe-put (use-package-normalize-plist name-symbol args) :config (quote (t)))) (args* (use-package-sort-keywords (if use-package-always-ensure (use-package-plist-maybe-put args0 :ensure use-package-always-ensure) args0)))) (if (and (boundp (quote byte-compile-current-file)) byte-compile-current-file) (setq args* (use-package-plist-cons args* :preface (cons (quote eval-when-compile) (append ... ...))))) (let ((body (macroexp-progn (use-package-process-keywords name-symbol args*)))) (if use-package-debug (display-buffer (save-current-buffer (let ... ... buf)))) body))))(de-ansi :load-path "elisp/de-ansi" :commands (de-ansify) :init (progn (bind-to-modi-map "d" de-ansify)))
  (use-package de-ansi :load-path "elisp/de-ansi" :commands (de-ansify) :init (progn (bind-to-modi-map "d" de-ansify)))
  eval-buffer(#<buffer  *load*-146766> nil "/home/kmodi/.emacs.d/setup-files/setup-de-ansi.el" nil t)  ; Reading at buffer position 174
  load-with-code-conversion("/home/kmodi/.emacs.d/setup-files/setup-de-ansi.el" "/home/kmodi/.emacs.d/setup-files/setup-de-ansi.el" nil t)
  #<subr require>(setup-de-ansi nil nil)
  ad-Advice-require(#<subr require> setup-de-ansi)
  apply(ad-Advice-require #<subr require> setup-de-ansi)
  require(setup-de-ansi)
  eval-buffer(#<buffer  *load*> nil "/home/kmodi/.emacs.d/init.el" nil t)  ; Reading at buffer position 5489
  load-with-code-conversion("/home/kmodi/.emacs.d/init.el" "/home/kmodi/.emacs.d/init.el" t t)
  load("/home/kmodi/.emacs.d/init" t t)
  #[0 "\205\262
brontitall commented 9 years ago

Could someone who relies on :ensure comment on whether its logic should happen at expansion time or at evaluation time?

I don't have enough experience to be sure, but personally I think I'd like to see it at evaluation time.

This might solve my issue of :ensure not being controlled by :disabled. Now, if only :disabled could take a conditional ... :)

jwiegley commented 9 years ago

:disabled + a condition is :if or :unless.

:disabled is special because it causes the macro expansion to be entirely omitted from the byte-compiler output.

brontitall commented 9 years ago

:disabled + a condition is :if or :unless.

I don't want to sound like a broken record on this, but this is not the case, since :if and :unless do not control :ensure, while :disabled does.

Perhaps if :ensure is moved to evaluation time this will change.

brontitall commented 9 years ago

Just an example to show what I mean:

ELISP> (emacs-version)
"GNU Emacs 23.1.1 (x86_64-redhat-linux-gnu)\n of 2013-05-07 on x86-022.build.eng.bos.redhat.com"
ELISP> (use-package helm :disabled t :ensure t)
nil
ELISP> (use-package helm :if nil :ensure t)
*** Eval error ***  Package `emacs-24' is unavailable
brontitall commented 9 years ago

Actually, now that I've updated my use-package on that system, my example is moot. There is no macroexp-progn on emacs 23, so at this point, use-package is off the table for those systems.

Better example:

ELISP> (emacs-version)
"GNU Emacs 24.3.1 (x86_64-redhat-linux-gnu)\n of 2014-01-25 on x86-024.build.eng.bos.redhat.com"
ELISP> (use-package paradox :disabled t :ensure t)
nil
ELISP> (use-package paradox :unless t :ensure t)
*** Eval error ***  Package `emacs-24.4' is unavailable
jwiegley commented 9 years ago

Right, I should have clarified: If you need a conditional, you must use :if and it's semantics will be different from :disabled. :disabled happens before everything: before keyword normalization, before handling of any keywords, everything. You can have a totally invalid use-package declaration, but if it's been disabled, it doesn't matter. This protects you from having to constantly maintain unused declarations as use-package develops over time. You only pay to migrate declarations you will actually use. So, :disabled takes effect at expansion time, full priority.

:if has a meaning at evaluation time only, and after :load-path, :pin, :ensure, and :preface. So it has a very different semantics from :disabled.

I think the ultimate answer may be to move :ensure to evaluation time. However, this will also mean that nothing gets ensured at compilation time, which may also be the wrong answer.

thomasf commented 9 years ago

It need to be at compile time for performance.. I have probably successfully stopped requiring 'package at load time as well which saved me just about 150ms startup time. I stopped using 'package for setting up my load path along time ago since package-initialize does too much stuff.

What I have now is based on @jwiegley's load-path.el from some years ago: https://github.com/thomasf/dotfiles-thomasf-emacs/blob/master/emacs.d/load-path.el#L144

It's quite messy right now but it get's the job done.

I just recently added the byte compilation of load-path.el/the load-path variable and not requiring 'package at load time, If this works well I will also look into letting 'package initialize the load-path variable at byte compilation time since It won't matter as much If I include the result in the resulting .elc.

I'm down to about 1s emacs start time now,

thomasf commented 9 years ago

I vote to remove use-package-always-ensure instead of again heading towards a needlessly complicated use-package. I'm totally fine with specifying my :ensure's manually.

jwiegley commented 9 years ago

Anyone else? I'm all for removing features. I would even like to move :ensure and :pin into their own addon package, maintained by someone who uses those features.

kaushalmodi commented 9 years ago

I am fine with that. But still would you consider https://github.com/jwiegley/use-package/issues/190#issuecomment-85205066 to be a bug? Should :ensure t try to find a package from package manager even if I specify :load-path?

npostavs commented 9 years ago

I'm totally fine with specifying my :ensure's manually.

@kaushalmodi's situation can be solved with :ensure nil, which is also manual specification, just in the other direction.

kaushalmodi commented 9 years ago

@npostavs Yes, but I am thinking of using this solution I found on github to emulate the use-package-always-ensure option. But for that to work, :ensure t has to be ineffective if the user is using :load-path.

Here is the reasoning:

Example:

(use-package de-ansi
  :load-path "elisp/de-ansi"
thomasf commented 9 years ago

@kaushalmodi I think the problem is that we don't want to move :ensure to load time or :load-path to compile time.. I haven't really read the new use-package code so the correctness of my input might vary. I guess it could make sense if :load-path were evaluated at compile time, if it's like that now you can disregard my comment altogether.

kaushalmodi commented 9 years ago

@thomasf The actual load path entered by the user doesn't need to be checked for validity. If the user used :load-path keyword then :ensure should be set to nil even if user set it to t.

That should be possible at compile time, correct? Do you guys think that's a fair assumption?

thomasf commented 9 years ago

@kaushalmodi Yeah, that should work. It it's still slightly more complicated than I would like when keywords starts affecting each other in non obvious ways,.

jwiegley commented 9 years ago

:load-path is already handled at both compile-time and load-time, it just doesn't happen before processing of :ensure.

kaushalmodi commented 9 years ago

@jwiegley Do you think it will be a fair modification to process :load-path before :ensure?

jwiegley commented 9 years ago

In the new architecture, it's as easy as moving symbols in a list, and verifying that correct semantics are preserved by way of manual testing.

jwiegley commented 9 years ago

But we still have the question of exactly what :ensure's behavior should be for every argument type that it accepts. I'm still not clear on that.

thomasf commented 9 years ago

arg t is (package-install package).. Symbol argument is (package-install symbol-name). And add nil to disable..

jwiegley commented 9 years ago

And nil?

kaushalmodi commented 9 years ago

nil would mean that the package-install call is not made at all.

jwiegley commented 9 years ago

Ah, so nil should be equivalent to not having :ensure there at all.

ghost commented 9 years ago

Same problem for me, I'm not a lisp expert but it looks like the use-package macro add a `-' at the end of the package name. That said, I think the :ensure and :pin should be in another package. maybe a package+.el ;)

jwiegley commented 7 years ago

Is this solved now?

kaushalmodi commented 7 years ago

To summarize

This works

(setq use-package-always-ensure t)
(use-package my-local-pkg
  :ensure nil
  :load-path "my-local-pkg")

But this does not:

(setq use-package-always-ensure t)
(use-package my-local-pkg
  :load-path "my-local-pkg")

The second snippet above still tries to install my-local-pkg from the package manager (and fails with debug backtrace) even though the :load-path is specified.

Would it be possible to effectively do ":ensure nil" if user has provided the ":load-path"?

waymondo commented 7 years ago

@kaushalmodi you could accomplish what you are seeking by customizing the new use-package-defaults option. Try this:

(eval-and-compile
  (defun local-package-load-path (name)
    (concat user-emacs-directory (symbol-name name))))

(setf (alist-get :load-path use-package-defaults)
      '((list (local-package-load-path name))
        (file-directory-p (local-package-load-path name))))

(setf (alist-get :ensure use-package-defaults)
      '(t (not (file-directory-p (local-package-load-path name)))))

This assumes any local package that exists in a folder of the same name at the root level of your emacs is added to your load-path and used, otherwise it will fall back to :ensure t. In your example (use-package my-local-pkg) would use the local package, or install it if it didn't exist.

jwiegley commented 7 years ago

What do you all think should be the default behavior?

kaushalmodi commented 7 years ago

@waymondo Thanks for the code snippet, I will understand and try it out when I get to a computer.

@jwiegley I would of course want the default behavior to not auto-install package if load-path is explicitly mentioned. :)

My reasoning is that the user would specify the load-path only if they don't want to use the GNU Elpa/Melpa/etc package versions, or if the package doesn't exist on external repositories. That indirectly means that an attempt to fetch the package from outside should not be made.

To pose this scenario from a different angle, when would one want to specify :load-path, but want to still auto-install the package from outside?

jacksgt commented 7 years ago

I spent a week trying to figure out why use-package wanted to download my local packages from the archives, even though I had explicitly set the :load-path. Until I found this thread.

Please make the behavior suggested above (no auto-installation of packages with explicit load-path) the default.

Alexander-Shukaev commented 7 years ago

Please, fix it as has already been suggested by multiple users. It's simply odd to attempt to install (the most-likely non-existent) packages from online archives when :load-path has already been explicitly specified. If somebody really wants to also perform :ensure on top of :load-path, then let them explicitly say :ensure t to achieve that.

kaushalmodi commented 6 years ago

@jwiegley Can you please consider making the behavior change as discussed above? I and many other folks believe that the right thing to do is to auto-set :ensure to nil internally when :load-path is specified.

Relevant recent comments so that you don't need to read the whole thread again:

jwiegley commented 6 years ago

Only 2.5 years to close!

kaushalmodi commented 6 years ago

Alright! Time to refactor my use-package organization! Thank you :)

jacksgt commented 6 years ago

@jwiegley Please don't forget to tag a relase, so MELPA Stable gets an update, too :-)

yyadavalli commented 6 years ago

I am getting warnings when I remove :ensure nil

The use-pacakge definitions are similar to

(use-package novel-mode
   :load-path "lisp/novel-mode"
   :init (setq novel-feedback t)
   :config
   (add-hook 'novel-mode-post-start-hook '(λ () (linum-mode -1)))
   (add-hook 'novel-mode-post-stop-hook '(λ () (linum-mode +1))))

I think it is still trying to fetch the pacakges. use-pacakge version is use-package-20171129.2308 from melpa, it is

Error (use-package): Failed to install evil-evilified-state: Package
‘evil-evilified-state-’ is unavailable
Error (use-package): Failed to install novel-mode: Package ‘novel-mode-’ is
unavailable
Error (use-package): Failed to install evil-lispy-state: Package
‘evil-lispy-state-’ is unavailable
Error (use-package): Failed to install org-ref-ivy: Package ‘org-ref-ivy-’ is
unavailable
Error (use-package): Failed to install agda2-mode: Package ‘agda2-mode-’ is
unavailable
yyadavalli commented 6 years ago

Here is the stacktrace with --debug-init

Debugger entered--Lisp error: (error "Package ‘evil-evilified-state-’ is unavailable")
   signal(error ("Package ‘evil-evilified-state-’ is unavailable"))
   error("Package `%s-%s' is unavailable" evil-evilified-state "")
   package-compute-transaction(nil ((evil-evilified-state)))
   package-install(evil-evilified-state)
   use-package-ensure-elpa(evil-evilified-state t nil :ensure)
   eval-buffer(#<buffer  *load*> nil "/home/yyadavalli/.emacs.d/init.el" nil t)  ; Reading at buffer position 3907
   load-with-code-conversion("/home/yyadavalli/.emacs.d/init.el" "/home/yyadavalli/.emacs.d/init.el" t t)
   load("/home/yyadavalli/.emacs.d/init" t t)
   #[0 "^H\205\266^@ \306=\203^Q^@\307^H\310Q\202?^@ \311=\204^^^@\307^H\312Q\202?^@\313\307\314\315#\203*^@\316\202?^@\31$
   command-line()
   normal-top-level()                                                                                                                                                                                                                                  |                                             
kaushalmodi commented 6 years ago

@jwiegley I cloned this repo to get the latest commit and byte compiled it. Sorry to say, but I am still seeing the same issue.

Here's an all-inclusive test case that you can right-away C-x C-e (assuming that use-package is in load-path):

(progn
  (defun use-package-test-190 (str)
    "Test setup for https://github.com/jwiegley/use-package/issues/190
STR is a string input."
    ;; Create ~/.emacs.d/axv-jks-vgv (I really hope you don't actually have a dir
    ;; named thusly).
    (let* ((test-pkg-name "axv-jks-vgv")
           (test-pkg-dir (let ((dir (concat user-emacs-directory
                                            test-pkg-name "/"))) ; must end with /
                           (make-directory dir :parents)
                           dir)))
      ;; Create axv-jks-vgv.el in that dir and make it print "Hello from
      ;; axv-jks-vgv" when loaded.
      (with-temp-buffer
        (insert (format "(message \"\n** [%s] Hello from %s **\n\")" str test-pkg-name))
        (insert (concat "(provide '" test-pkg-name ")"))
        (write-file (expand-file-name (concat test-pkg-name ".el") test-pkg-dir)))

      ;; Use use-package to load axv-jks-vgv.
      (eval `(use-package ,(intern test-pkg-name)
               :load-path ,test-pkg-dir))))

  ;; 1. This will work
  (let ((use-package-always-ensure nil))
    (use-package-test-190 "works"))
  ;; This prints:
  ;;
  ;;   ** [works] Hello from axv-jks-vgv **
  ;;

  ;; 2. This will fail
  ;; It fails because it is still trying to download that package even when :load-path is specified.
  (let ((use-package-always-ensure t))
    (use-package-test-190 "fails"))
  ;; Backtrace:

  ;; Debugger entered--Lisp error: (error "Package ‘axv-jks-vgv-’ is unavailable")
  ;;   signal(error ("Package ‘axv-jks-vgv-’ is unavailable"))
  ;;   error("Package `%s-%s' is unavailable" axv-jks-vgv "")
  ;;   #f(compiled-function (packages requirements &optional seen) "Return a list of packages to be installed, including PACKAGES.\nPACKAGES should be a list of `package-desc'.\n\nREQUIREMENTS should be a list of additional requirements; each\nelement in this list should have the form (PACKAGE VERSION-LIST),\nwhere PACKAGE is a package name and VERSION-LIST is the required\nversion of that package.\n\nThis function recursively computes the requirements of the\npackages in REQUIREMENTS, and returns a list of all the packages\nthat must be installed.  Packages that are already installed are\nnot included in this list.\n\nSEEN is used internally to detect infinite recursion." #<bytecode 0x7cd1f5>)(nil ((axv-jks-vgv)))
  ;;   apply(#f(compiled-function (packages requirements &optional seen) "Return a list of packages to be installed, including PACKAGES.\nPACKAGES should be a list of `package-desc'.\n\nREQUIREMENTS should be a list of additional requirements; each\nelement in this list should have the form (PACKAGE VERSION-LIST),\nwhere PACKAGE is a package name and VERSION-LIST is the required\nversion of that package.\n\nThis function recursively computes the requirements of the\npackages in REQUIREMENTS, and returns a list of all the packages\nthat must be installed.  Packages that are already installed are\nnot included in this list.\n\nSEEN is used internally to detect infinite recursion." #<bytecode 0x7cd1f5>) (nil ((axv-jks-vgv))))
  ;;   package-compute-transaction(nil ((axv-jks-vgv)))
  ;;   package-install(axv-jks-vgv)
  ;;   use-package-ensure-elpa(axv-jks-vgv t nil :ensure)
  ;;   (progn (use-package-ensure-elpa 'axv-jks-vgv 't 'nil :ensure) (eval-and-compile (add-to-list 'load-path "/home/kmodi/.emacs.d/axv-jks-vgv/")) (if (not (require 'axv-jks-vgv nil 't)) (ignore (message (format "Cannot load %s" 'axv-jks-vgv)))))
  ;;   (use-package axv-jks-vgv :load-path "/home/kmodi/.emacs.d/axv-jks-vgv/")
  ;;   eval((use-package axv-jks-vgv :load-path "/home/kmodi/.emacs.d/axv-jks-vgv/"))
  ;;   (let* ((test-pkg-name "axv-jks-vgv") (test-pkg-dir (let ((dir (concat user-emacs-directory test-pkg-name "/"))) (make-directory dir :parents) dir))) (let ((temp-buffer (generate-new-buffer " *temp*"))) (save-current-buffer (set-buffer temp-buffer) (unwind-protect (progn (insert (format "(message \"\n** [%s] Hello from %s **\n\")" str test-pkg-name)) (insert (concat "(provide '" test-pkg-name ")")) (write-file (expand-file-name (concat test-pkg-name ".el") test-pkg-dir))) (and (buffer-name temp-buffer) (kill-buffer temp-buffer))))) (eval (list 'use-package (intern test-pkg-name) ':load-path test-pkg-dir)))
  ;;   use-package-test-190("fails")
  ;;   (let ((use-package-always-ensure t)) (use-package-test-190 "fails"))
  ;;   (progn (defalias 'use-package-test-190 (function (lambda (str) "Test setup for https://github.com/jwiegley/use-package/issues/190\nSTR is a string input." (let* ((test-pkg-name "axv-jks-vgv") (test-pkg-dir (let ((dir (concat user-emacs-directory test-pkg-name "/"))) (make-directory dir :parents) dir))) (let ((temp-buffer (generate-new-buffer " *temp*"))) (save-current-buffer (set-buffer temp-buffer) (unwind-protect (progn (insert (format "(message \"\n** [%s] Hello from %s **\n\")" str test-pkg-name)) (insert (concat "(provide '" test-pkg-name ")")) (write-file (expand-file-name (concat test-pkg-name ".el") test-pkg-dir))) (and (buffer-name temp-buffer) (kill-buffer temp-buffer))))) (eval (list 'use-package (intern test-pkg-name) ':load-path test-pkg-dir)))))) (let ((use-package-always-ensure nil)) (use-package-test-190 "works")) (let ((use-package-always-ensure t)) (use-package-test-190 "fails")))
  ;;   eval((progn (defalias 'use-package-test-190 (function (lambda (str) "Test setup for https://github.com/jwiegley/use-package/issues/190\nSTR is a string input." (let* ((test-pkg-name "axv-jks-vgv") (test-pkg-dir (let ((dir (concat user-emacs-directory test-pkg-name "/"))) (make-directory dir :parents) dir))) (let ((temp-buffer (generate-new-buffer " *temp*"))) (save-current-buffer (set-buffer temp-buffer) (unwind-protect (progn (insert (format "(message \"\n** [%s] Hello from %s **\n\")" str test-pkg-name)) (insert (concat "(provide '" test-pkg-name ")")) (write-file (expand-file-name (concat test-pkg-name ".el") test-pkg-dir))) (and (buffer-name temp-buffer) (kill-buffer temp-buffer))))) (eval (list 'use-package (intern test-pkg-name) ':load-path test-pkg-dir)))))) (let ((use-package-always-ensure nil)) (use-package-test-190 "works")) (let ((use-package-always-ensure t)) (use-package-test-190 "fails"))) nil)
  ;;   elisp--eval-last-sexp(nil)
  ;;   #f(compiled-function (eval-last-sexp-arg-internal) "Evaluate sexp before point; print value in the echo area.\nInteractively, with a non `-' prefix argument, print output into\ncurrent buffer.\n\nNormally, this function truncates long output according to the\nvalue of the variables `eval-expression-print-length' and\n`eval-expression-print-level'.  With a prefix argument of zero,\nhowever, there is no such truncation.  Such a prefix argument\nalso causes integers to be printed in several additional formats\n(octal, hexadecimal, and character when the prefix argument is\n-1 or the integer is `eval-expression-print-maximum-character' or\nless).\n\nIf `eval-expression-debug-on-error' is non-nil, which is the default,\nthis command arranges for all errors to enter the debugger." (interactive "P") #<bytecode 0x2ce611>)(nil)
  ;;   apply(#f(compiled-function (eval-last-sexp-arg-internal) "Evaluate sexp before point; print value in the echo area.\nInteractively, with a non `-' prefix argument, print output into\ncurrent buffer.\n\nNormally, this function truncates long output according to the\nvalue of the variables `eval-expression-print-length' and\n`eval-expression-print-level'.  With a prefix argument of zero,\nhowever, there is no such truncation.  Such a prefix argument\nalso causes integers to be printed in several additional formats\n(octal, hexadecimal, and character when the prefix argument is\n-1 or the integer is `eval-expression-print-maximum-character' or\nless).\n\nIf `eval-expression-debug-on-error' is non-nil, which is the default,\nthis command arranges for all errors to enter the debugger." (interactive "P") #<bytecode 0x2ce611>) nil)
  ;;   eval-last-sexp(nil)
  ;;   funcall-interactively(eval-last-sexp nil)
  ;;   call-interactively(eval-last-sexp nil nil)
  ;;   command-execute(eval-last-sexp)
  )
jwiegley commented 6 years ago

Thanks Kaushal, I'll take a look at it this weekend if I don't get to it before then.

jwiegley commented 6 years ago

@kaushalmodi Please try now.

kaushalmodi commented 6 years ago

@jwiegley Yay! Now I can start reorganizing my config :)

It works.. also I realized that in my test case above, if everything went well, the second form won't load the same package again :)

So, after commenting the first "works" let form and changing the test-pkg-name to something else, I was able to get

** [fails] Hello from axv-jks-vdgv **

.. well, which actually doesn't fail any more :D

jwiegley commented 6 years ago

@kaushalmodi I don't use package.el or :ensure; is there some way to add tests for these things in use-package-tests.el that is independent from whether I do or don't use it in my own config?