use direnv-builtin DIRENV_WATCHES to list watched files

[kingarrrt]

nix_direnv_watch_file is replaced with a function that wraps watch_file and issues a deprecation warning.

See #408 (which this fixes) for discussion.

[bbenne10]

I will not personally merge this in the current state of affairs. I mentioned this on #408 as well, but to reiterate: @zimbatm has explicitly requested we not do this. If their opinion changes, I'll be fine to merge - until then, I respect them and their software too much to go against their wishes on this.

EDIT: Wait...I reacted too quickly. This doesn't wrap watch_file, but rather parses the "storage mechanism" that direnv uses. I'm leaving the original up for some context, but I have to give this one some thought. I don't care for the underlying direnv implementation of DIRENV_WATCHES as it is rather complex and hard to parse - this should be possible in a simpler manner.

[kingarrrt]

Somewhat contradictory... You respect them and their software, too much, but don't care for its implementation?

In any case, it is what it is, and I doubt they're gonna change it. For reference, the implementation is at https://github.com/direnv/direnv/blob/master/gzenv/gzenv.go.

Objectively, there's nothing very difficult about gzip and base64url - it's bog standard. You already depend on coreutils (for base64), this adds gzip, which is ubiquitous. It could be done without jq, but this would be more complex, requiring sed or awk.

The implementation here is a simple bash pipeline. It cannot be simpler. An equivalent Python one-liner is below, if you prefer, but this of course requires a Python interpreter, which I thought a bit heavy.

python -c "import base64, json, os, zlib; print('\n'.join([p['Path'] for p in json.loads(zlib.decompress(base64.urlsafe_b64decode(os.environ['DIRENV_WATCHES'])).decode()) if p['Exists'] and not p['Path'].startswith(os.environ['XDG_DATA_HOME'] + '/direnv/allow')]))"

(I should have commented in the patch. The $XDG_DATA_HOME/direnv/allow file is filtered from the list as not relevant.)

In any case, as covered in #408, nix_direnv_watch_file is a serious problem in that it introduces a hard dependency on this project.

You advertise as:

A faster, persistent implementation of direnv's use_nix and use_flake, to replace the built-in one.

This is (currently) incorrect.

I appreciate your software and want to use it, but cannot as it stands. Do you agree that there is a problem?

[bbenne10]

Somewhat contradictory... You respect them and their software, too much, but don't care for its implementation?

I very often respect the net result of a developer's code, but disagree on implementation details.

In any case, it is what it is, and I doubt they're gonna change it. For reference, the implementation is at https://github.com/direnv/direnv/blob/master/gzenv/gzenv.go.

I've read it, thanks though.

Objectively, there's nothing very difficult about gzip and base64url - it's bog standard. You already depend on coreutils (for base64), this adds gzip, which is ubiquitous. It could be done without jq, but this would be more complex, requiring sed or awk.

"Difficult" != "Complex"
This is a simple implementation, but an exposed list of files, separated by the record-separator character would be even easier and nearly as robust.

In any case, as covered in #408, nix_direnv_watch_file is a serious problem in that it introduces a hard dependency on this project.

You advertise as:

A faster, persistent implementation of direnv's use_nix and use_flake, to replace the built-in one.

This is (currently) incorrect.

No - it does not add a hard dependency. #408 details how to work around the exposed interface inconsistencies. Alias wf to nix-direnv-watch-file or watch-file depending on the builtin has function's return value. Inconvenient? Yes - certainly. Not insurmountable when you have the entirety of bash at your fingertips.

I appreciate your software and want to use it, but cannot as it stands. Do you agree that there is a problem?

As I made clear in my responses on #408 - I personally think that trying to match the names and interface exposed by the standard direnv implementation of use nix and use flake to be a bit of a mistake, but hindsight is 20/20 and we are where we are. Saying you "cannot use it" is a bit of a misdirect though - it is 100% possible to use nix-direnv in a scenario where you want to support direnv standard use_{nix,flake} also - you just have to do some papering over the interface differences.

Please do not take my initial reaction as indicative of some overall apathy in fixing this or some dislike of the solution - you did what you had to do to fix the problem in this software. My frustration stemmed from the fact that it appeared initially that you'd ignored my statements in #408 and opened a PR fixing the problem in the exact way I had said I didn't want to do it. This seems to do the right thing in light of the underlying implementation of DIRENV_WATCHES. I merely want to ruminate a bit on the implementation complexity vs gains in UX before I commit to merging this though.

[kingarrrt]

Thank you for your considered reply.

Just to touch on the proposed workaround. It doesn't work for my (and others, @infinisil) use cases. "Hard dependency" was perhaps the wrong term, but I can't justify extra noise in .envrc purely to support a tool that I alone want to use.

I believe that matching the direnv interface is a good thing, and that it would be to this project's advantage if it was a pure drop-in over direnv, requiring no boilerplate.

[bbenne10]

I'd still like to see what @Mic92 has to say on the matter, but I think I'm okay with merging this. It does just enough to integrate with the direnv internals to alleviate the need for a wrapping function but not more than necessary. I haven't given thought to the exception you made for $XDG_DATA_HOME/direnv/allow and whether we need to add more exceptions though...

I have opened a discussion about what it would take to merge our version of use_nix and use_flake upstream on the direnv matrix channel (#direnv:numtide.com) and from what has been said so far, @zimbatm would prefer to divest of most (if not all) use and layout functions from direnv's core. This is interesting and I think I agree with that approach, as it democratizes the ability to get use or layout functions. (Python developers can work on use python and layout python; Ruby devs on the equivalent Ruby ones; Nix developers on the Nix ones; etc), but the question is how we'd LOAD the installed plugins. The discussion is ongoing, but I think that the takeaway so far is that once that were to happen - nix-direnv would have a bit of work to do to comply, but we would be the de facto standard for nix integration.

[kingarrrt]

I've pushed an updated version with a polyfill using nix-shell for jq. It is of course slow. Do you think it might be worth issuing a warning if it falls back to this?

I also changed the parsed watches to be NUL-separated which should be more robust.

[infinisil]

Is it really better to do this instead of just overriding watch_file? This feels very brittle to me

[Mic92]

Is it really better to do this instead of just overriding watch_file? This feels very brittle to me

We don't want to override more builtins. You can also see the complexity that comes from overriding just two builtins.

[Mic92]

@kingarrrt I also added the functionality to direnv itself: direnv/direnv#1198

Only caveat is debian and other slow moving targets.

[Mic92]

We can drop this code when we get direnv/direnv#1198

[Mic92]

Ok. After my refactoring this touches a lot less builtins in direnv. And if we get the PR merged it will be even less. Also now it no longer adds any new dependencies.

[Mic92]

I have opened a discussion about what it would take to merge our version of use_nix and use_flake upstream on the direnv matrix channel (#direnv:numtide.com) and from what has been said so far, @zimbatm would prefer to divest of most (if not all) use and layout functions from direnv's core. This is interesting and I think I agree with that approach, as it democratizes the ability to get use or layout functions. (Python developers can work on use python and layout python; Ruby devs on the equivalent Ruby ones; Nix developers on the Nix ones; etc), but the question is how we'd LOAD the installed plugins. The discussion is ongoing, but I think that the takeaway so far is that once that were to happen - nix-direnv would have a bit of work to do to comply, but we would be the de facto standard for nix integration.

I am aware of these plans, but to me direnv's stdlib is also one of the biggest strengths. If someone has to check 5 different places to get high-quality plugins for each ecosystem (the nodejs community would probably end up with 3 for the same thing) than this is worse user experience. I trust zimbatm to be more thoughtful when it comes to backwards compatibility compared to many other project maintainers.

[bbenne10]

I am aware of these plans, but to me direnv's stdlib is also one of the biggest strengths. If someone has to check 5 different places to get high-quality plugins for each ecosystem (the nodejs community would probably end up with 3 for the same thing) than this is worse user experience. I trust zimbatm to be more thoughtful when it comes to backwards compatibility compared to many other project maintainers.

IMHO if the nodejs community shoots themselves in the foot with this change, I'm not sure I think it is direnv's problem in the first place. There are things I think we would need to implement (naming conventions, a plugin repository with a basic search feature or some other way to find plugins that've been "blessed") and we'd need to find maintainers for the separate implementations (or maybe only for the "sufficiently complex" ones), but I think that the separation of implementation out into smaller chunks might help improve the contributor base to each specialized bit. It's certainly more managerial work to make everything work together, but it makes the technical bits more approachable imo.