syl20bnr / spacemacs

A community-driven Emacs distribution - The best editor is neither Emacs nor Vim, it's Emacs *and* Vim!
http://spacemacs.org
GNU General Public License v3.0
23.67k stars 4.89k forks source link

many uses of (spacemacs|use-package-add-hook ... :post-config ...) broken #13545

Closed emacs18 closed 4 years ago

emacs18 commented 4 years ago

I would like to report that several spacemacs features are broken due to use-package issue 785 which has not been fixed in 9 months.

I use latest develop branch and latest emacs 26 and 27 on ubuntu 20.04.

With treemacs layer enabled, SPC 0 is supposed to be bound to treemacs-select-window command which starts treemacs if not started, then put the cursor on the left most treemacs window. However this does not happen. Instead I get

No window numbered 10

error message. This is not surprising since SPC 0 is not bound to the treemacs command. Instead it is bound to winum-select-window-0-or-10 command.

For now I applied following patch in my spacemacs code to work around this problem so that the intended code is executed after loading winum and treemacs packages.

diff --git a/layers/+filetree/treemacs/packages.el b/layers/+filetree/treemacs/packages.el
index f84ae8574..f1ba9072f 100644
--- a/layers/+filetree/treemacs/packages.el
+++ b/layers/+filetree/treemacs/packages.el
@@ -99,22 +99,20 @@
   (use-package treemacs-icons-dired
     :hook '(dired-mode . treemacs-icons-dired-mode)))

-(defun treemacs/pre-init-winum ()
-  (spacemacs|use-package-add-hook winum
-    :post-config
-    (progn
-      ;; `0', `M-0' and `C-x w 0' are bound to `winum-select-window-0-or-10'
-      (define-key winum-keymap
-        [remap winum-select-window-0-or-10] #'treemacs-select-window)
-      ;; replace the which-key name
-      (push '((nil . "winum-select-window-0-or-10") .
-              (nil . "treemacs-select-window"))
-            which-key-replacement-alist)
-      (with-eval-after-load 'treemacs
-        (dolist (n (number-sequence 1 5))
-          (add-to-list 'winum-ignored-buffers
-                       (format "%sFramebuffer-%s*"
-                               treemacs--buffer-name-prefix n)))))))
+(with-eval-after-load 'winum
+  ;; `0', `M-0' and `C-x w 0' are bound to `winum-select-window-0-or-10'
+  (define-key winum-keymap
+    [remap winum-select-window-0-or-10] #'treemacs-select-window)
+  ;; replace the which-key name
+  (push '((nil . "winum-select-window-0-or-10") .
+          (nil . "treemacs-select-window"))
+        which-key-replacement-alist))
+
+(with-eval-after-load 'treemacs
+  (dolist (n (number-sequence 1 5))
+    (add-to-list 'winum-ignored-buffers
+                 (format "%sFramebuffer-%s*"
+                         treemacs--buffer-name-prefix n))))

 (defun treemacs/init-treemacs-magit ()
   (use-package treemacs-magit

Another example of the same use-package related problem is that SPC l b does not work which is supposed to allow me to select a buffer within current persp-mode workspace. The problem is that the code to set couple of variables is never executed so that SPC l b does nothing, because it is bound to ignore command. Following change works around that problem.

diff --git a/layers/+completion/helm/packages.el b/layers/+completion/helm/packages.el
index 9a1cada62..c75e8b80d 100644
--- a/layers/+completion/helm/packages.el
+++ b/layers/+completion/helm/packages.el
@@ -453,12 +453,9 @@
   ;;  Restore popwin-mode after a Helm session finishes.
   (add-hook 'helm-cleanup-hook #'spacemacs//helm-restore-display))

-(defun helm/pre-init-persp-mode ()
-  (spacemacs|use-package-add-hook persp-mode
-    :post-config
-    (setq
-     spacemacs--persp-display-buffers-func 'spacemacs/persp-helm-mini
-     spacemacs--persp-display-perspectives-func 'spacemacs/helm-perspectives)))
+(setq
+ spacemacs--persp-display-buffers-func 'spacemacs/persp-helm-mini
+ spacemacs--persp-display-perspectives-func 'spacemacs/helm-perspectives)

 (defun helm/post-init-projectile ()
   (setq projectile-completion-system 'helm))
jjlee commented 4 years ago

Hi @emacs18 -- would you mind testing my patch for the use-package issue?

also @stianse also if you're a spacemacs user (and anybody else of course!)?

to test it:

  1. Revert the workarounds you have applied for the particular issues you ran into that were caused by the use-package bug
  2. Get patched use-package:
    git clone https://github.com/jjlee/use-package.git
    cd use-package
    git checkout call-hooks-even-if-no-config
  3. Configure spacemacs to use the patched version of use-package: SPC f e d and add to your dotspacemacs-additional-packages (I'm assuming here you cloned it under a directory ~/dev, please adjust to match where you put it):
    dotspacemacs-additional-packages
    '(
      (use-package :location "~/dev/use-package/")
      )
  4. Restart spacemacs: SPC q r
  5. See if it fixes the issues you had
  6. Use spacemacs for a few days and see if anything breaks
  7. Report back in a comment here

Thanks, really appreciate your help with this!

emacs18 commented 4 years ago

@jjlee thanks for your efforts. I tested your change, but the problem was not solved, so I looked into this. I discovered that the root cause of the problem was not in use-package. Rather it was in spacemacs itself! Even if I go back to using regular use-package without any changes, spacemacs works as expected if I simply add this line very early on during the initialization process:

(custom-set-variables '(use-package-inject-hooks t))

This variable is set to t by spacemacs during start-up, but not soon enough! So this looks like it is a bug in spacemacs as opposed to a bug in use-package.

emacs18 commented 4 years ago

During spacemacs initialization process, following call sequence is made

configuration-layer/load
  configuration-layer//load
    configuration-layer//declare-used-layer

where first function calls the second, and second calls the third. If use-package-inject-hooks is set to t prior to any of these three functions, then hook injection seems to work, i.e., SPC 0 brings up treemacs buffer as expected. If this variable is set to t after above functions, then SPC 0 results in an error message instead. Today spacemacs sets this variable to t within spacemacs-bootstrap/init-use-package which is called after above three functions are called.

It looks like we need to find a way to set use-package-inject-hooks sooner before configuration-layer//declare-used-layer is called.

jjlee commented 4 years ago

Thanks for testing. It sounds like you have found another issue (which I have not investigated). The use-package issue that you referenced (https://github.com/jwiegley/use-package/issues/785) is still a problem that affects user configuration (it's an issue I've run into multiple times in the past I believe). When you say you tested my patch: how long have you used it for, and have you encountered any problems resulting from it?

emacs18 commented 4 years ago

I used your patch for few minutes just enough to see if it made difference to the specific problem of my interest which is whether SPC 0 brought up treemacs window on left or not. When things work SPC 0 displays the treemacs window. When things don't work, then nothing comes up and only an error message is printed. Since setting use-package-inject-hooks variable to t early on in my own startup file resolved my problem, that is good enough solution for me although I think similar solution should be made within spacemacs files so that everyone benefits.

I''m willing to help make progress with jwiegley/use-package#785. If the problem replication procedure is simple enough for me to follow, then I can give it a try.

jjlee commented 4 years ago

Thanks! The testing needed from my point of view, is not to try to reproduce a problem or check that a problem is fixed, but just to apply the patch (https://github.com/syl20bnr/spacemacs/issues/13545#issuecomment-629619449), use emacs normally for at least a few days, and see if anything breaks.

emacs18 commented 4 years ago

@jjlee I've been using your one line patch for few days both at home (ubuntu 20.04) and at work (CentOS 6). I didn't observe any problems so far. I don't expect to observe any problems.

smile13241324 commented 4 years ago

@emacs18 do you experience the same issues with SPC 0 bindings when you use spacemacs without custom user-config code? If so please post a system info block as is generated by spacemacs/report-issues.

I am using treemacs with evil and I am not having any problems with SPC 0 not being bound.

In the mean time I had a more detailed look into the core bootstrap code. Basically loading the package configs are done in:

(defun configuration-layer//configure-packages (packages)
  "Configure all passed PACKAGES honoring the steps order."
  (spacemacs/init-progress-bar (length packages))
  (spacemacs-buffer/message "+ Configuring bootstrap packages...")
  (configuration-layer//configure-packages-2
   (configuration-layer/filter-objects
    packages (lambda (x)
               (let ((pkg (configuration-layer/get-package x)))
                 (eq 'bootstrap (oref pkg :step))))))
  (spacemacs-buffer/message "+ Configuring pre packages...")
  (configuration-layer//configure-packages-2
   (configuration-layer/filter-objects
    packages (lambda (x)
               (let ((pkg (configuration-layer/get-package x)))
                 (eq 'pre (oref pkg :step))))))
  (spacemacs-buffer/message "+ Configuring packages...")
  (configuration-layer//configure-packages-2
   (configuration-layer/filter-objects
    packages (lambda (x)
               (let ((pkg (configuration-layer/get-package x)))
                 (null (oref pkg :step)))))))

Here packages are loaded based on steps, which are bound for the core layers to define some kind of load order. When you check the code you will see that Spacemacs first loads bootstrap packages.

These you can find in spacemacs-bootstrap:

(setq spacemacs-bootstrap-packages
      '(
        ;; bootstrap packages,
        ;; `use-package' cannot be used for bootstrap packages configuration
        (async :step bootstrap)
        (bind-map :step bootstrap)
        (bind-key :step bootstrap)
        (diminish :step bootstrap)
        (evil :step bootstrap)
        (hydra :step bootstrap)
        (use-package :step bootstrap)
        (which-key :step bootstrap)
        ;; pre packages, initialized aftert the bootstrap packages
        ;; these packages can use use-package
        (dotenv-mode :step pre)
        (evil-evilified-state :location local :step pre :protected t)
        (pcre2el :step pre)
        (holy-mode :location local :step pre)
        (hybrid-mode :location (recipe :fetcher local) :step pre)
        (spacemacs-theme :location built-in)
        ))

Here use-package is loaded with priority which should also set use-package-inject-hooks to t. Only after this is loaded the other package setup functions are called. So theoretically it should already be set before any other package code would be run.

emacs18 commented 4 years ago

I provided more details at https://github.com/syl20bnr/spacemacs/issues/13649 In hind sight, my latest comments there should have been placed here instead. Sorry. As detailed there, the problem is that you need to set use-package-inject-hooks need to be set even before the bootstrap packages are loaded.

emacs18 commented 4 years ago

Description :octocat:

SPC 0 fails to bring up treemacs window.

Reproduction guide :beetle:

Observed behaviour: :eyes: :broken_heart: This error message is given:

winum-select-window-by-number: No window numbered 10

Expected behaviour: :heart: :smile: Treemacs buffer pop up on left.

System Info :computer:

emacs18 commented 4 years ago

I figured this out. The issue is the use-package was already activated before starting spacemacs initialization. So this I supposed could be considered my error since I think I am not supposed to activate use-package or do anything else before starting spacemacs.

A longer version of this is that I have unusual setup in that I start emacs in many different ways so that my startup process is quite different. Some of the reasons are:

For now the fix for me is not to actiavte use-package prior to starting spacemacs setup. This is easy enough to do. Sorry for the trouble.

emacs18 commented 4 years ago

As I commented at https://github.com/jwiegley/use-package/pull/840 this problem is back due to a use-package change made on Jun 29. As I commented above on May 16, a work around for now until use-package is fixed is to set use-package-inject-hooks much sooner to allow "hook injection" stuff to work.

jjlee commented 4 years ago

Back here you said that you thought that your personal emacs/spacemacs configuration had caused this issue (at least that's how I read it?) by means of causing use-package to be loaded earlier than spacemacs sets use-package-inject-hooks. Are you saying that that's not true after all?

I'm also not sure why you think that use-package change you referenced is relevant.

I think if you want this bug to get attention you should demonstrate it with a minimal spacemacs configuration that you can post here so that other people can reproduce the problem you see exactly.

emacs18 commented 4 years ago

In June, I modified my startup files so that spacemacs is initialized before use-package is activated. I double checked that yesterday.

The problem is trivial to demonstrate. Does SPC 0 bring up treemacs window or not? When it does not work treemacs window does not pop up and you get error message saying that window 10 does not exist or something like that.

jjlee commented 4 years ago

Does SPC 0 bring up treemacs window or not?

For me, yes. Have you tried with vanilla out-of-the-box spacemacs config?

emacs18 commented 4 years ago

I just re-verified that use-package is not loaded prior to spacemacs by printing the value of features variable at the start of spacemacs initialization. It did not contain use-package.

emacs18 commented 4 years ago

I use chemacs package and have setup emacs startup files for not only doom, but also for spacemacs with vanilla develop branch code. I thought I tested this problem with the vanilla develop branch code last night. I'll double check that again.

emacs18 commented 4 years ago

Well I just re-verified. Launch spacemacs and hit SPC 0. Rather than treemacs window popping up, I get this error message

No window numbered 10

This is exactly the problem reported initially. The problem goes away if I modify use-package to back out one problematic change from June 29.

Hopefully other spacemacs users will chime in and tell me whether SPC 0 works or not for them. If all say that it works, then I must have made a mistake somewhere.

emacs18 commented 4 years ago

@jjlee Can you share which version of use-package and spacemacs you are using?

emacs18 commented 4 years ago

Nevermind. You were right. It was my setup. My setup loaded ~/.spacemacs a bit sooner than default spacemacs would have. That seemed to have caused problems. To solve this I figured out how I can let spacemacs load ~/.spacemacs the usual way.

My apologies for the trouble. I hope that now my setup does not deviate from the usual spacemacs setup other than trivial ways.