Skip to content
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

Allow watching of arbitrary files for nix flake updates #148

Merged
merged 1 commit into from
Feb 26, 2022

Conversation

polygon
Copy link
Contributor

@polygon polygon commented Feb 21, 2022

For more complex flake projects that branch out into other nix-files, it is currently not possible to trigger a rebuild of the environment when one of these files changes. Even if they are added to the direnv watches using watch_file, use_flake will check only some pre-determined files and otherwise will re-use the cached environment resulting in an out-of-sync environment.

This is a first suggestion on how an approach to watching multiple files might look and will likely require some re-work. Essentially, instead of using watch_file <file>, you now write nix_watches+=(<file1> <file2>). The functionality was changed such that all the files in that array are now checked and whenever one of them changed, it will correctly trigger rebuilding the environment.

This would also properly fix #51

@Mic92
Copy link
Member

Mic92 commented Feb 22, 2022

cc @bbenne10

@bbenne10
Copy link
Contributor

@Mic92: I suppose the question is whether we want a difference between invalidation of nix cache independent of the direnv cache. This would work fine for my use case and is a lot simpler than the PR I've opened since my approach uses functions rather than a variable. Simplicity, clarity, and aligning the nix cache and the direnv cache are all good things and so I am all for this approach over the one taken in #145.

The only trouble that I see with this is that it doesn't seem to touch use_nix, which (imo) it should. That should be trivial.

(As an aside: What do we think of maybe overriding watch_file? Doing so would allow us to unify the nix-direnv "watches" with the direnv watches (by simply dispatching to the original function) and would provide new users a discoverable and expected entry point that's already documented and utilized by the upstream. I don't want to cause more churn, but that seems like a pretty good solution to me?)

@polygon polygon force-pushed the allow_watching_arbitrary_files branch 2 times, most recently from eee1d8f to 3f5b5cc Compare February 22, 2022 22:21
@polygon
Copy link
Contributor Author

polygon commented Feb 22, 2022

@bbenne10: Thanks for the feedback. Sorry, I guess we solved a similar problem at the same time. I haven't seen your PR before opening this one. In any case, If needed, I think the function approach would still be compatible with this. We could call this custom function if it exists, and then set need_update if it returns 1. However, I don't have a use-case myself beyond watching files.

Indeed, I was going for a minimum viable version to show at first. I've added support for use_nix now. I also like the idea of overriding watch_file and implemented this. I've had to add a check if the function was already renamed because for some reason, this file is sourced twice for me.

@bbenne10
Copy link
Contributor

bbenne10 commented Feb 23, 2022

No need to be sorry for solving the same problem but better. I believe that your use-case is the same as mine overall and that overriding watch_file is a clever solution. If use_flake or use_nixis called, we unify the concepts of the two unique caches. If not, nothing unexpected happens (presuming no one else reads nix_watches). Sounds like a win to me :)

I'm hoping to get feedback from @dvzubarev on one or both of these PRs soon, but I understand that life happens and we may need to move forward without that.

I may suggest also updating the README to describe the functionality

@bbenne10
Copy link
Contributor

Thinking on it - if we're overriding watch_file we may also need to override watch_dir to keep everything consistent, no?

direnvrc Outdated Show resolved Hide resolved
@polygon polygon force-pushed the allow_watching_arbitrary_files branch from 5473036 to 9f84b5a Compare February 23, 2022 22:37
@polygon
Copy link
Contributor Author

polygon commented Feb 23, 2022

Function is called nix_direnv_watch_file now to not override any existing functionality. Open for other name suggestions. If everyone is fine with this name, it should be ready now.

@bbenne10
Copy link
Contributor

I think that some documentation is in order for this, but otherwise I'm happy to sign off on it.

@polygon polygon force-pushed the allow_watching_arbitrary_files branch 3 times, most recently from 36d8f36 to 9ccda52 Compare February 24, 2022 21:04
@polygon
Copy link
Contributor Author

polygon commented Feb 24, 2022

I've replaced the Manually re-triggering evaluation chapter in the readme. Please have a look.

@bbenne10
Copy link
Contributor

Lgtm

direnvrc Outdated Show resolved Hide resolved
Used to watch files that, when changed, trigger an update of the
cached nix environment.
@polygon polygon force-pushed the allow_watching_arbitrary_files branch from 9ccda52 to 1241f11 Compare February 25, 2022 20:09
@Mic92 Mic92 merged commit 3638354 into nix-community:master Feb 26, 2022
@Mic92
Copy link
Member

Mic92 commented Feb 26, 2022

Thanks!

PetarKirov added a commit to PetarKirov/nix-direnv that referenced this pull request Mar 22, 2022
* Update hashes
  * Hash obtained via:
    ```sh
    direnv fetchurl "https://raw.githubusercontent.com/nix-community/nix-direnv/master/direnvrc" \
      | grep -m1 -o 'sha256-.*'
    ```
* Bump tag to 1.6.1 in:
  * README.md
  * direnvrc
  * template/.envrc

This release includes the following changes:

* Allow watching of arbitrary files for nix flake updates ([nix-community#148][1])

[1]: nix-community#148
PetarKirov added a commit to PetarKirov/nix-direnv that referenced this pull request Mar 22, 2022
* Update hashes
  * Hash obtained via:
    ```sh
    direnv fetchurl 'https://raw.githubusercontent.com/nix-community/nix-direnv/master/direnvrc' \
      | grep -m1 -o 'sha256-.*'
    ```
* Bump tag to 1.6.1 in:
  * README.md
  * direnvrc
  * template/.envrc

This release includes the following changes:

* Allow watching of arbitrary files for nix flake updates ([nix-community#148][1])

[1]: nix-community#148
@PetarKirov PetarKirov mentioned this pull request Mar 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

watch_file doesn't invalidate the cache
4 participants