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

handle symlinks for hot-reloading (nix) #121

Merged
merged 1 commit into from
Jan 22, 2024
Merged

handle symlinks for hot-reloading (nix) #121

merged 1 commit into from
Jan 22, 2024

Conversation

sodiboo
Copy link
Contributor

@sodiboo sodiboo commented Jan 22, 2024

on nix, using home-manager, we typically have config files as symlinks to nix store. this breaks the current implementation, as it will not detect anything has changed. nix store does not store mtime (because the result should be identical no matter when you build it).

this PR adds correct handling of the watcher, not just for nix, but in general:

  • if it is a symlink and the mtime changes, but it still points to the same target file and that file hasn't updated mtime, then we can ignore this because the config file hasn't changed, only the symlink.

  • likewise, if the symlink path changes (from a previous symlink in a chain), but the ultimate target is unchanged, we can ignore this.

  • we cannot ignore if the symlinks end up pointing to a completely different canonical path, then it's a different file entirely and we reload it.

  • we cannot ignore if the target file's mtime is modified. the config changed and we reload it.

  • if the path changes and the mtime, then we need to reload it even more than before.

  • if neither the canonical path, nor the mtime of config changed, then the config didn't change. doesn't matter what the symlinks do, it still points to the same target.

@YaLTeR YaLTeR merged commit 18566e3 into YaLTeR:main Jan 22, 2024
5 checks passed
@YaLTeR
Copy link
Owner

YaLTeR commented Jan 22, 2024

Thanks, seems to work well.

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.

2 participants