spacemacs icon indicating copy to clipboard operation
spacemacs copied to clipboard

org-roam-setup cause race condition with user config file

Open emacs18 opened this issue 3 years ago • 27 comments

Following is part of org-roam setup within spacemacs:

(defun org/init-org-roam ()
  (use-package org-roam
    :defer t
    :hook (after-init . org-roam-setup)
    :init
    ...))

org-roam-setup is an obsolete function as indicated by these lines from org-roam:

(define-obsolete-function-alias
  'org-roam-setup
  'org-roam-db-autosync-enable "org-roam 2.0")

The damage done by adding org-roam-setup is that automatic database update mode is enabled. This cause org-roam-db-sync function to be called which will then start to update sqlite database of all my org files. The problem is that org-roam-db-sync is called before my init file is loaded! Thus org-roam-directory has not yet been setup properly. org-roam-db-sync thus uses incorrect directories and happily updates the sqlite db.

After emacs comes up if I now execute org-roam-db-sync manually, then it thinks that all files are out of date, because org-roam-directory has now changed! Thus sqlite db is updated for all my files again. At this point if I restart emacs, then the cycle repeats all over again where sqlite db for all my org files are updated again. This can add up to 8 minute delay in my emacs start up time! For some unknown reason sqlite db update for some files take very long time like two minutes for some files.

What I would like to ask is that we not enable automatic update of org-roam database and let the user choose when this is enabled, i.e., do not add org-roam-setup to after-init-hook.

emacs18 avatar Sep 06 '22 17:09 emacs18

should we use

org-roam-db-autosync-mode instead?

this seems to be indicated in

https://www.orgroam.com/manual.html#Setting-up-Org_002droam

lebensterben avatar Sep 06 '22 18:09 lebensterben

org-roam-db-autosync-mode should be used instead of obsolete org-roam-setup if one were to enable this minor mode.

However this should not be used until after user configuration is done, i.e., dotspacemacs/user-config function has been executed.

emacs18 avatar Sep 06 '22 20:09 emacs18

I do not use org-roam (except for debugging..). Can you show me what user might need to config in Spacemacs/user-config?

lebensterben avatar Sep 06 '22 20:09 lebensterben

(org-roam-db-autosync-mode) enables the minor mode which updates sqlite db of org files as they are modified. This is mentioned in the link that you mentioned above.

emacs18 avatar Sep 06 '22 20:09 emacs18

one solution is to tell users to call org-roam-db-autosync-mode AFTER any user configuration.

another solution is to tell users to add all user configurations in

(spacemacs|use-package-add-hook org-roam
  :pre-config
  ;; user configuration
  )

and we call org-roam-db-autosync-mode inside the regular :config.

What do you think?

lebensterben avatar Sep 06 '22 21:09 lebensterben

My preference would be to simply tell to call the function in question to enable the minor mode. This is the documented method in org-roam manual.

I think the alternative seems much more complicated. I don't know what spacemacs|use-package-add-hook does. Even if I were to figure out what it does once, I'm sure I'll forget it soon. So I would rather not deal with things like this.

emacs18 avatar Sep 06 '22 21:09 emacs18

So, should we not use post-config for org-roam at all?

It looks like post-config is never being called.

breadncup avatar Sep 08 '22 17:09 breadncup

@breadncup

I've no idea why you bring up post-config... It's not used.

lebensterben avatar Sep 08 '22 17:09 lebensterben

(spacemacs|use-package-add-hook org-roam
  :post-config
  ;; user configuration
  )

I used the above format, and it works well to me before this change.

breadncup avatar Sep 08 '22 17:09 breadncup

@breadncup

what do you have in :post-config

lebensterben avatar Sep 08 '22 17:09 lebensterben

I have a private layer for my purpose and use this:

(defun bnc_personal/pre-init-org-roam ()
  "Pre-initialize org-roam for personal configuration."
  (spacemacs|use-package-add-hook org-roam
    :post-init
    (progn
      (setq org-roam-directory (expand-file-name "~/roamfiles/")
            org-roam-db-location (concat bnc:dir:base "org-roam.db")
            )
      )
    :post-config
    ;; Journal Template
    (let (
          (mylist
           `(
             ("d" "default" entry "* %?"
              :target (file+head "%<%Y-%m-%d>.org.gpg" "#+setupfile: ../personal/personal_org_setup.org\n#+title: %<%Y-%m-%d>")
              :jump-to-captured t
              )
             ))
          )
      (setq org-roam-dailies-capture-templates mylist)
      )
    ))

And, it was working before this change, and now org-roam-dailies-capture-templates is not even shown.

Excluding this change, the above works fine.

breadncup avatar Sep 08 '22 17:09 breadncup

Just adding org-roam-db-autosync-mode in your :post-config should work,

lebensterben avatar Sep 08 '22 18:09 lebensterben

It doesn't seem to work.

(defun bnc_personal/pre-init-org-roam ()
  "Pre-initialize org-roam for personal configuration."
  (spacemacs|use-package-add-hook org-roam
    :post-init
    (progn
      (setq org-roam-directory (expand-file-name "~/roamfiles/")
            org-roam-db-location (concat bnc:dir:base "org-roam.db")
            )
      )
    :post-config
    (org-roam-db-autosync-mode)
    ;; Journal Template
    (let* (
           (mylist
            `(
              ("d" "default" entry "* %?"
               :target (file+head "%<%Y-%m-%d>.org.gpg" "#+setupfile: ../personal/personal_org_setup.org\n#+title: %<%Y-%m-%d>")
               :jump-to-captured t
               )
              ))
           )
      (setq org-roam-dailies-capture-templates mylist)
      )
    ))

It seems that post-config section is not running at all. I put a test code like (setq mydummy t) but mydummy is not seen at all. Without this change, I'm able to see the test variable of mydummy is seen.

breadncup avatar Sep 08 '22 18:09 breadncup

C-h v use-package--org-roam--post-config-hook

Is it empty?

lebensterben avatar Sep 08 '22 18:09 lebensterben

you may also try to move it from :post-config to :post-init.

lebensterben avatar Sep 08 '22 18:09 lebensterben

@lebensterben

Thanks,

Moving (org-roam-db-autosync-mode) to :post-init, then it works now.

breadncup avatar Sep 08 '22 20:09 breadncup

This is what I have in my init file which is loaded by dotspacemacs/user-config:

(use-package org-roam
  :init
  (setq org-roam-directory ...)
  :config
  (org-roam-db-autosync-enable)
  )

This seems to work for me. Isn't this much simpler?

emacs18 avatar Sep 09 '22 04:09 emacs18

My init file is setup to use vanilla code that is not spacemacs specific, because my init file may be loaded by spacemacs, doom, scimax, etc.

Since this more general (and simpler) code seems to work fine, I don't see why I should resort to spacemacs specific code which is also more complicated.

emacs18 avatar Sep 09 '22 04:09 emacs18

@emacs18

before your PR, org-roam syncs the database in the after-init hook.


after your PR, with your new configuration, org-roam syncs the database after it's loaded. (because :config is evaluated after the package is loaded)

This works for you because spacemacs/user-config is evaluated after layers, but before packages are loaded.

I've no issue with adopting this. If you want you can make a PR.

lebensterben avatar Sep 09 '22 04:09 lebensterben

Are there any updates to this? What's the proper way to call org-roam-db-autosync-mode? Especially if, like most users, I don't have a private layer.

Is there a way to do it when loading the layer, like

   dotspacemacs-configuration-layers
   '(
     (org :variables
          org-enable-roam-support t
          :post-config
          org-roam-db-autosync-mode)
    )

or is this not valid syntax?

Otherwise, do I need to use this type of block?

(spacemacs|use-package-add-hook org-roam
  :post-config
  ;; user configuration
  )

Should it be :post-config or :post-init or something else? Also, where does this entire block go? Does it go in the standard dotspacemacs/user-config where the rest of my config is, or somewhere else?

albertfgu avatar Aug 29 '23 11:08 albertfgu

I've tried using

(spacemacs|use-package-add-hook org-roam
  :post-config
  ;; user configuration
  )

with both :post-config and :post-init, placing it in either the dotspacemacs/user-config or dotspacemacs/user-init functions.

I think it's supposed to be recommended that org-roam-db-autosync-mode is turned on, so having it be part of the org-roam layer by default or making it easier to configure would be great.

albertfgu avatar Aug 29 '23 12:08 albertfgu

It has been too long so I don't remember the details. I just looked up my current setup which calls org-roam-db-autosync-mode within dotspacemacs/user-config function.

emacs18 avatar Aug 29 '23 17:08 emacs18

So you're no longer loading a separate init file that has use-package?

How do you call org-roam-db-autosync-mode within dotspacemacs/user-config? I tried just literally adding

(org-roam-db-autosync-mode)

as a line inside the function, but that broke a lot of stuff.

albertfgu avatar Aug 29 '23 19:08 albertfgu

My init code is about 7,000 thousand lines in a file which is loaded within dotspacemacs/user-config. I don't have any explicit use-config, load, or require of org-roam. Also I use straight.el rather than package.el. I don't know whether this could change init code loading order.

emacs18 avatar Sep 01 '23 04:09 emacs18

I just tested a vanilla spacemacs with the following change to enable org layer and org-roam. Also (org-roam-db-autosync-mode) is called in dotspacemacs/user-config. This worked fine in that emacs started up without error and org-roam-db-autosync-mode was t.

diff --git a/core/templates/.spacemacs.template b/core/templates/.spacemacs.template
index 8abd9c0d9..00a240225 100644
--- a/core/templates/.spacemacs.template
+++ b/core/templates/.spacemacs.template
@@ -46,7 +46,10 @@ This function should only modify configuration layer settings."
      ;; lsp
      ;; markdown
      multiple-cursors
-     ;; org
+     (org :variables
+          org-directory dotspacemacs-directory
+          org-roam-directory dotspacemacs-directory
+          org-enable-roam-support t)
      ;; (shell :variables
      ;;        shell-default-height 30
      ;;        shell-default-position 'bottom)
@@ -568,6 +571,7 @@ This function is called at the very end of Spacemacs startup, after layer
 configuration.
 Put your configuration code here, except for variables that should be set
 before packages are loaded."
+  (org-roam-db-autosync-mode)
 )
 

In order to prevent emacs from loading any file outside of the spacemacs directory, I first applied the following patch which also used core/template/.spacemacs.template file as the init file.

commit 74d361e6727e4d3cd1a574536595447f2dbfc5a1 (spacemacsdir)
Author: Richard Kim <[email protected]>
Date:   Sat Aug 26 16:28:08 2023 -0700

    set SPACEMACSDIR to use .spacemacs.template

diff --git a/.spacemacs.d/init.el b/.spacemacs.d/init.el
new file mode 120000
index 000000000..2813af572
--- /dev/null
+++ b/.spacemacs.d/init.el
@@ -0,0 +1 @@
+../core/templates/.spacemacs.template
\ No newline at end of file
diff --git a/early-init.el b/early-init.el
index 218328397..598bca7a0 100644
--- a/early-init.el
+++ b/early-init.el
@@ -35,6 +35,8 @@
 ;; needed nor loaded on those versions.
 (setq package-enable-at-startup nil)
 
+(setenv "SPACEMACSDIR" (expand-file-name ".spacemacs.d" (file-name-directory load-file-name)))
+
 (load (concat (file-name-directory load-file-name)
               "core/core-early-funcs")
       nil (not init-file-debug))

emacs18 avatar Sep 02 '23 13:09 emacs18

Thanks a lot for this. It works now, but only if I include these lines that you had:

(org :variables
         org-directory dotspacemacs-directory
         org-roam-directory dotspacemacs-directory
         org-enable-roam-support t)

Previously I was setting org-roam-directory using a setq inside dotspacemacs/user-config, which worked fine, but broke once (org-roam-db-autosync-mode) was called. I don't completely understand the internals of Spacemacs so I'm not really sure why it broke, but it makes sense.

I'm also curious how you found this solution. The Org layer documentation doesn't seem to mention the org-roam-directory variable. I don't think I could have figured this out by myself without a lot of pain.

albertfgu avatar Sep 04 '23 12:09 albertfgu

spacemacs does not document every package. Instead it refers to the documentation for each package. For example org layer documentation simply refers to org-roam package documentation. org-roam documentation at https://www.orgroam.com/manual.html#Getting-Started says to set org-roam-directory very early on.

emacs18 avatar Sep 04 '23 21:09 emacs18