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

direnv: allow configuring extra files to watch from devenv.nix #1186

Closed
wants to merge 9 commits into from

Conversation

sandydoo
Copy link
Member

@sandydoo sandydoo commented May 3, 2024

Proof of concept. Based on ideas from #1180.

direnvrc Outdated Show resolved Hide resolved
@soundofspace
Copy link
Contributor

Question on the side: will _nix_argsum_suffix "${files_to_watch[@]}" not get expensive depending on the amount of files and size they are?

direnvrc Outdated
# Watch extra files configured in devenv.nix
if [[ -f "$extra_files_watchlist" ]]; then
for file in $(cat "$extra_files_watchlist"); do
files_to_watch+=("$file")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would probably just call nix_direnv_watch_file here to prevent files_to_watch getting bigger resulting in heavier checksum and amount of profiles getting bigger. But not sure if this is really a problem.

Copy link
Member Author

@sandydoo sandydoo May 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, maybe. So the idea is to cache the evaluated shell and skip re-evaluation next time. The question is whether we want the watched files to form part of the cache?

For example, if you have a requirements.txt and change the version of a package, would you like that to be a new cache key? Or overwrite the existing one? I think you'd just want the latter 🤔

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure, would it also have separate nix caches? Switching between branches with different versions is not that uncommon and if nix cache could be reused it would be almost instantaneous, which would be nice. On the other hand this use case is not that super common that it needs to be a big concern. I'd say separate profiles if nix cache can be reused, if not just overwrite the existing cache?

Did some research/testing about shasum overhead, and as long as the amount of files and size of these files doesn't get completely out of control, it should have almost no impact. It would even be possible to skip checking files last edit date completely when shasumming everything (but leaving that on is probably nicer).

direnvrc Outdated Show resolved Hide resolved
@soundofspace
Copy link
Contributor

@sandydoo Thanks for taking the time to implement this


# Watch any extra files configured via `config.direnv.watchFiles` for changes
# This picks up new files and reloads the shell on initial run
# TODO: deduplicate already watched file paths?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could easily be done with assosiative arrays but that needs bash 4. Seeing as macos only ships with version 3, that is probably a no go.

I wonder if it would makes sense to move some of this logic to the rust core where you would have much more freedom and power to do what you want. But keeping everything in simple bash has the benefit that the barrier to enter/debug is quite a bit lower.

@domenkozar
Copy link
Member

#1468

@domenkozar domenkozar closed this Sep 30, 2024
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.

3 participants