-
-
Notifications
You must be signed in to change notification settings - Fork 35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Asynchronous direnv calls #6
Comments
Hi! Thanks for starting the conversation about this. Asynchronous updates are somewhat on my radar. You would think it would be an issue for me as an increasingly enthusiastic Nix user (I'm sure Guix is great too!), but the problem has largely been solved for me by lorri. I therefore wonder if the best solution overall would be to make something like that for Guix, because I imagine that Nix and Guix are the main two situations in which direnv evaluations might end up taking a long time. In terms of
Generally speaking, I'd like to start calling So I guess overall I'd like to better understand cases where people use Another related thing I need to do is tackle "reloading" a direnv by re-using the existing var values when re-invoking Any thoughts are appreciated: this is very much a 0.1.x release, and I plan to iterate on it as time allows and as I and other users get to know the pain points, like this issue. |
I think the optimal solution would be to make some kind of lorri for guix, but if we were to go down the asynchronous envrc route then I don't think the indeterminate state issue would be insurmountable. Even with Lorri you could access your project before the Lorri daemon has first initialized and be in a state that could be considered invalid. If you are using Envrc and you make some change to your All things considered I think my problem could be more cleanly solved from the Guix side. Regardless, It might be reasonable to develop this feature anyway since from the perspective of someone using direnv from a shell where you can just send a long running process to the background, having emacs lock up with no recourse could be jarring in comparison, even for a short duration. It might not be worth the effort or the extra complexity though. I'd be happy with the project either way. |
Yeah, agree. I'll definitely have a longer think about this. |
I tried hacking on emacs-direnv to support this feature in my own fork Mic92/emacs-direnv@f4f3dbb it sort of work but there are some bugs described here: https://discourse.nixos.org/t/emacs-direnv-help-needed-to-make-it-non-blocking/8595 |
@Mic92 Yeah, |
Yes. |
Sorry for chiming in, I was using this hack to avoid blocking on slow nix re-evaluations. (defun my-update-environment ()
(interactive)
(envrc-reload)
;; (my-restart-ycmd)
)
(defun my-run-lorri-watch-sentinel (process event)
(if (equal event "finished\n")
(my-update-environment)
(message "Process %s event %s" process event)))
(defun my-run-lorri-on-shell-nix-change ()
(interactive)
(when (projectile-project-p)
(let ((process-connection-type nil)) ; use a pipe
(start-file-process "lorri-watch"
"*lorri*"
"lorri" "watch" "--once")
(set-process-sentinel (get-process "lorri-watch") 'my-run-lorri-watch-sentinel))))
(defvar my-lorri-watch-files '("default.nix" "shell.nix"))
(add-hook 'nix-mode-hook
(defun enable-autoreload-for-nix-shell ()
(when (and (buffer-file-name)
(member (file-name-nondirectory (buffer-file-name))
my-lorri-watch-files))
(add-hook 'after-save-hook 'my-run-lorri-on-shell-nix-change t t))))
|
Thanks for sharing. This is a nice approach unfortunately I need a solution for flakes now as well and the author of lorri does not like flakes, so we won't see this beeing implemented soon. However I think using |
What does the rest of your configurations looks like? How do you disable |
(use-package envrc
:config
(envrc-global-mode))
Not sure what do you mean. Auto
Thank you for the tip! I replaced (defun my-update-environment ()
(interactive)
(envrc-reload)
(message "envrc was reloaded.")
;; (my-restart-ycmd)
)
(defun my-run-direnv-exec-watch-sentinel (process event)
(if (equal event "finished\n")
(my-update-environment)
(message "Process %s event %s" process event)))
(defun my-run-direnv-exec-on-shell-nix-change ()
(interactive)
(when (projectile-project-p)
(let ((process-connection-type nil)) ; use a pipe
(start-file-process "direnv-exec"
"*direnv-exec*"
"direnv" "exec" "." "true")
(set-process-sentinel (get-process "direnv-exec") 'my-run-direnv-exec-watch-sentinel))))
(defvar my-nix-project-watch-files '("default.nix" "shell.nix" "flake.nix"))
(add-hook 'nix-mode-hook
(defun enable-autoreload-for-nix-shell ()
(when (and (buffer-file-name)
(projectile-project-p)
(member (file-relative-name buffer-file-name (projectile-project-root))
my-nix-project-watch-files))
(add-hook 'after-save-hook 'my-run-direnv-exec-on-shell-nix-change t t)))) It has limitations that it won't update an environment if you edit a file that is imported from |
I think I mainly misunderstood what you did. I thought you would only load direnv on certain events asynchronously, but you only handle reloads this way. However is there maybe a project tile hook one could use to load direnv asynchronously on the first run? |
It seems that there are only hooks that trigger when you use |
would |
My fork is now asynchronous: https://github.com/Mic92/envrc/tree/async This is how to use it in doom-emacs:
If someone wants to upstream this feature, feel free to take my code. |
I'll have to try your fork out! About Lorri mentioned earlier in this thread... I haven't tried in some time, but I found Lorri to be very unreliable or at least not work as direnv does and had to abandon it. |
From what I read, the implementation would allow later minor modes / hooks to run that might expect the loaded environment to be available. The reason we are supposed to add the mode hook after other minor modes is so that they will not see the incomplete environment (because of LIFO hook ordering). While we don't want Emacs to block, I don't think loading the environment out of order is going to solve more problems than it causes for situations such as project provided language servers or environment settings for them. I'm not sure if it's possible to block just a single buffer. To implement it without any Emacs integration will probably mean stashing the hooks for a new buffer, replacing the buffer contents with some non-blocking loading indication, and then unstashing and restarting hooks after the environment is finished. How / how cleanly it can be done is the main issue on my mind. |
Well than it's going to be a long-term fork. I cannot have emacs blocking when I just want to look at a file. I rather restart my lsp if needed. |
Blocking isn't accurate. I think I have a better proposal, but first what I meant was to hijack the remainder of the mode switch and then run it after direnv finishes. The file would be visible and interactive, but with almost no minor modes active. We can do the same thing using two hooks instead of one and no hacking. The first hook on the major mode would put a function into the envrc hook. The second hook to load the minor mode would go off for both updates and asynchronous initialization. This solution depends on asynchronously loading the direnv while the buffer major mode is already finished. I think we can write a function or macro to create such a chain-loading hook function. Like other hooks, it would execute immediately if the direnv is set up or be called after direnv finishes. Having a hook would give some minor modes a chance to both initialize late and to re-initialize on any direnv update. Minor modes that are stateless, using getenv on every command, don't care about the environment changing out from under them. Only minor modes that derive state from the direnv and hold onto it in elisp need to be notified of environment updates. It's not super common, and by using a chain-hook generator, the user still writes the hook to begin on the major mode hook. We usually only want a language server for certain major modes. We need a direnv hook to actually load the minor mode for such a case. Still, it's wrong to load the minor mode on every direnv without looking at the major mode. The first hook handles the major mode decision while the second hook handles the late and re-initialization. So the solution is to use the major mode hook to set up a direnv hook to load minor modes that depend on direnv. |
I'm not sure if others might find it useful and it might not be appropriate for the implementations discussed but... maybe my idea could be useful. I'm a very happy detached.el user and my ideal envrc-mode would run the typical blocking call using (perhaps) This gives:
|
Anyone tried the ideas from the 2 last comments? |
Related to #6 and #53, some invocations of direnv can take a long time and block envrc.el. Managing all direnv invocations asynchronous would make everything less predictable and much more complex (either conceptually or in actual code terms), but nonetheless, sometimes we _really_ don't want to wait. With this commit, the user can hit C-g to halt invocation. This was possible before, but Emacs might nonetheless have tried the same thing again immediately. Now, envrc.el treats the interruption as a direnv failure and remembers it, so the user can proceed and reload the environment at their leisure.
Good news, I think... first a couple of comments. I think there's a plausible argument that direnv is going to block work even in a terminal when an I like that @Mic92's changes are pretty minimal, but it seems like the result will still be unpredictable use of outdated environments during mode startup and other times, and that doesn't seem a good default. Anyway, I'd noticed that I could usually hit |
Yeah breaking emacs (for example syntax highlighting) by hitting Ctrl-g was indeed an annoyance. So far I have not seen any downsides for the async variant for my personal usage. It's usually projects, where I just want to open a single file and where I don't even wait for |
|
(Just to be clear, with the new change, |
Hmmm, I use https://github.com/nix-community/nix-direnv (instead of Lorri) and in my regular terminal emulator + shell (Kitty + fish) that works great and the But when I open a I have Maybe this is unrelated to the OP's issue and it's got to do with |
@zeorin - unsure why you've seen that behaviour, sorry. It sounds unrelated so if it's still a problem for you, perhaps open another issue for discussion. |
Closing this as "not planned" because I intend to stick with the simpler and more predictable existing code now that interrupting with |
I didn't see a way to use (define-derived-mode foo-mode fundamental-mode "Foo"
"Major mode for doing nothing.")
(define-minor-mode bar-mode
"Minor mode to enable bar features."
:lighter " bar"
:global nil
(if bar-mode
(message "Bar mode enabled.")
(message "Bar mode disabled.")))
(add-hook 'foo-mode-hook #'bar-mode)
(defvar foo-ready nil)
(add-hook 'change-major-mode-after-body-hook
(lambda ()
(when (eq major-mode 'foo-mode)
(unless foo-ready (major-mode-suspend)))))
(defun foo-enable-and-complete-switch ()
"Disarm the hook and load the suspended-mode"
(interactive)
(setq foo-ready t)
(foo-mode)) After evaluating:
All that would need to happen is intercepting the mode switch and, if a direnv is detected, suspend the mode unless the cached state is fine and we can continue loading synchronously. After direnv finishes, re-use the normal Not sure how stable I think |
Hi all, I wrote a PR to add async to wbolster's direnv: wbolster/emacs-direnv#82. I didn't know there were competing emacs direnv plugins and I don't know what their differences are, but please feel free to use / adapt the code. I'm happy to release it under the GPL obviously--the other project is BSD 3-clause but the PR hasn't been merged yet. FWIW I've been using it since I wrote that PR and it's been seamless so far. Implementation note: I create a temp buffer to capture the output of the separate direnv process, with a deterministic name, which automatically acts as a lock to avoid concurrent direnv calls (something you want to avoid). Any concurrent direnv calls automatically enter a (non-blocking but noisy) 1-sec sleep retry loop until they are allowed to run. I see that you are not planning to support this first class on this package, @purcell, but I thought I'd share here anyway in case other people are interested. I don't know if the APIs of the two packages are compatible but feel free to have a look, and let me know. I would like to keep pursuing async direnv because I use a lot of direnv + nix, and Emacs is my primary IDE, meaning I regularly end up redownloading a project's latest shell context from within Emacs. Curious to hear others' thoughts & experiences! |
@hraban I think one difference between emacs-direnv and envrc-mode is that it avoids using global variables and instead uses buffer local variables... but it's been so long since I switched I can't remember. |
I've updated Mic92's envrc fork, pulling in latest changes and fixing some issues as well. Feel free to use it if you need this functionality: |
And since we're still on the topic: @czan posted some great feedback and analysis of my PR in wbolster/emacs-direnv#82 and long story short it's not looking like my approach was a good one. I will be trying out some of y'all's ideas here. |
Similar opinions expressed in that PR review to mine. I still feel like the simple, predictable approach of loading synchronously but allowing a It occurred to me that another fairly simple approach would be to allow users to configure an execution time limit, and then automatically abort any long Thereafter, you could manually recover by using |
I have hacked Envrc (gist) to call direnv asynchronously (with
make-process
and a sentinel) because as a Guix user using Envrc to establish a guix environment can block for quite a while as files are downloaded and binaries are built. I imagine the same problem happens when using direnv to establish a nix shell environment. The simplest solution is to manually establish the environment once to let the software be installed before using Envrc, but this doesn't feel great as it could be done automatically.Should I polish this feature so that it could be included, or do you think it doesn't fit with the project?
The text was updated successfully, but these errors were encountered: