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

contib: add direnv implementation #4

Merged
merged 10 commits into from
Jul 11, 2023

Conversation

blaggacao
Copy link
Contributor

@blaggacao blaggacao commented Apr 8, 2023

Without PRJ_ID:

$ direnv reload # initial
[ ... ]
direnv: Add to .git/info/exclude: /.config
direnv: Add to .git/info/exclude: /.cache
direnv: Add to .git/info/exclude: /.local/share
direnv: Add to .git/info/exclude: /.local/state
direnv: Add to .git/info/exclude: /.run
direnv: PRJ_ROOT:        /home/blaggacao/src/github.com/paisano-nix/tui
direnv: PRJ_ID:          none
direnv: PRJ_CONFIG_HOME: .config
direnv: PRJ_RUNTIME_DIR: .run
direnv: PRJ_CACHE_HOME:  .cache
direnv: PRJ_DATA_HOME:   .local/share
direnv: PRJ_STATE_HOME:  .local/state
direnv: PRJ_PATH:        .local/bin
[ ... ]
$ direnv reload # subsequent
[ ... ]
direnv: PRJ_ROOT:        /home/blaggacao/src/github.com/paisano-nix/tui
direnv: PRJ_ID:          none
direnv: PRJ_CONFIG_HOME: .config
direnv: PRJ_RUNTIME_DIR: .run
direnv: PRJ_CACHE_HOME:  .cache
direnv: PRJ_DATA_HOME:   .local/share
direnv: PRJ_STATE_HOME:  .local/state
direnv: PRJ_PATH:        .local/bin
[ ... ]

With PRJ_ID:

uuidgen > .config/prj_id
$ direnv reload # initial
[ ... ]
direnv: Add to .git/info/exclude: /.config
direnv: Add to .git/info/exclude: /.run
direnv: PRJ_ROOT:        /home/blaggacao/src/github.com/paisano-nix/tui
direnv: PRJ_ID:          44850561-5378-499b-9e95-8549b4bc8596
direnv: PRJ_CONFIG_HOME: .config
direnv: PRJ_RUNTIME_DIR: .run
direnv: PRJ_CACHE_HOME:  /home/blaggacao/.cache/prj/44850561-5378-499b-9e95-8549b4bc8596
direnv: PRJ_DATA_HOME:   /home/blaggacao/.local/share/prj/44850561-5378-499b-9e95-8549b4bc8596
direnv: PRJ_STATE_HOME:  /home/blaggacao/.local/state/prj/44850561-5378-499b-9e95-8549b4bc8596
direnv: PRJ_PATH:        /home/blaggacao/.local/bin/prj/44850561-5378-499b-9e95-8549b4bc8596
[ ... ]
$ direnv reload # subsequent
[ ... ]
direnv: PRJ_ROOT:        /home/blaggacao/src/github.com/paisano-nix/tui
direnv: PRJ_ID:          44850561-5378-499b-9e95-8549b4bc8596
direnv: PRJ_CONFIG_HOME: .config
direnv: PRJ_RUNTIME_DIR: .run
direnv: PRJ_CACHE_HOME:  /home/blaggacao/.cache/prj/44850561-5378-499b-9e95-8549b4bc8596
direnv: PRJ_DATA_HOME:   /home/blaggacao/.local/share/prj/44850561-5378-499b-9e95-8549b4bc8596
direnv: PRJ_STATE_HOME:  /home/blaggacao/.local/state/prj/44850561-5378-499b-9e95-8549b4bc8596
direnv: PRJ_PATH:        /home/blaggacao/.local/bin/prj/44850561-5378-499b-9e95-8549b4bc8596
[ ... ]

Silence, please ...

$ export DIRENV_PRJ_SILENCE=true;
$ direnv reload # initial
[ ... ]
direnv: PRJ_CONFIG_HOME: .config
direnv: PRJ_RUNTIME_DIR: .run
# more if PRJ_ID is not set
[ ... ]
$ direnv reload # subsequent
[ ... ]

@blaggacao blaggacao marked this pull request as ready for review April 8, 2023 20:01
contrib/direnv Outdated
find_prj_config() (
local old_pwd
while [[ $old_pwd != $PWD ]]; do
if [[ -d .config ]]; then
Copy link
Member

Choose a reason for hiding this comment

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

Something I just thought about: this will find the $HOME/.config folder if the repo doesn't hold one. Maybe the existence of the prj_id should be mandated, it would make that more explicit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this can be too ambiguous. And I already sort of thought about this problem, but then chose not to address it on the first iteration. But it needs to be addressed before merge.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed in 44d2b5f.

@zimbatm is that imlementation good enough for our purpose here?

contrib/direnv Outdated Show resolved Hide resolved
contrib/direnv Outdated
Comment on lines 23 to 24
: "${XDG_STATE_HOME:=${HOME}/.local/state}"
: "${XDG_RUNTIME_DIR:=${HOME}/.local/state}"
Copy link
Member

Choose a reason for hiding this comment

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

On my system, XDG_RUNTIME_DIR is pointing to /run/user/1000

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is a "deviaton" from the XDG convention.
This is mainly useful for dev services, which can be configured to use it and the user will be able to find it in the same place.

For example mariadb by default can't be killed with CTRL+D so having the pid file at hand may end up being handy for some extra whichcraft. Or another thing could be project-local service sockets....

contrib/direnv Outdated Show resolved Hide resolved
contrib/direnv Show resolved Hide resolved
contrib/direnv Outdated Show resolved Hide resolved
contrib/direnv Outdated Show resolved Hide resolved
contrib/direnv Show resolved Hide resolved
contrib/direnv Outdated
Comment on lines 109 to 111
export PRJ_DATA_HOME
export PRJ_STATE_HOME
export PRJ_RUNTIME_DIR
Copy link
Member

Choose a reason for hiding this comment

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

What is the difference between PRJ_DATA_HOME, PRJ_STATE_HOME and PRJ_RUNTIME_DIR?

Copy link
Contributor Author

@blaggacao blaggacao Apr 10, 2023

Choose a reason for hiding this comment

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

According to XDG, DATA is for potentially sharable data that is useful independent of the context of the concrete user environment. One example that comes to my mind would be blockchain state. A way to think about it would be a hypothetical user that works on mac and linux interchangeably but shares projects on a NFS. DATA should not hold arch-specific data.

STATE, however, could be a stateful application file (these are rare these days), that don't immediately make sense to share, either because they are arch-specific or their lifetime is bound to the host machine. A better use in most cases would be /tmp, but still, maybe an application stores last startup configuration to resume on. The Paisano TUI/CLI is recording the last executed action there in order to facilitate manual debugging but also because the invocation sequence requires a file proxy at some point. Another state would be the non-atomic bits of a transaction before it becomes settled and moves to DATA.

Copy link
Contributor Author

@blaggacao blaggacao Apr 10, 2023

Choose a reason for hiding this comment

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

PRJ_RUNTIME_DIR is usually just pids and sockets.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed PRJ_STATE_HOME in 7dfd31d.

But I'm keeping PRJ_RUNTIME_DIR for deveshell services use case ("find their pids easily").

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zimbatm would that be ok (keep PRJ_RUNTIME_DIR)? Would you like me to expand more on the use case?

Copy link

@cscutcher cscutcher left a comment

Choose a reason for hiding this comment

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

Stopping by mostly after discovering (I think) some issues with how this code behaves in an environment with linked git worktrees (i.e. created via git worktree add).

Hope it's OK if I made some other related suggestions while I was in the area.

As always, thanks for making the effort and sharing the code!

contrib/direnv Outdated Show resolved Hide resolved
contrib/direnv Outdated Show resolved Hide resolved
contrib/direnv Outdated Show resolved Hide resolved
contrib/direnv Show resolved Hide resolved
contrib/direnv Outdated Show resolved Hide resolved
@zimbatm
Copy link
Member

zimbatm commented Jun 25, 2023

@blaggacao happy to merge this once you tell me it's ready

@blaggacao
Copy link
Contributor Author

blaggacao commented Jun 26, 2023

@zimbatm I'll bump downstream useages to the tip of this branch and keep running it for a while in real life. It looks like everything is addressed, now, so if that remains true I'll ping you in a couple of weeks. Thank you!

Copy link

@cscutcher cscutcher left a comment

Choose a reason for hiding this comment

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

Hopefully, this doesn't cause more problems. I only just noticed the review request and wanted to make sure the lack of change to find_prj_root_with_git was intentional.

contrib/direnv Outdated Show resolved Hide resolved
contrib/direnv Outdated Show resolved Hide resolved
contrib/direnv Show resolved Hide resolved
contrib/direnv Outdated Show resolved Hide resolved
contrib/direnv Show resolved Hide resolved
@blaggacao
Copy link
Contributor Author

@zimbatm 9b0ffcd has already propagated for a while ot some real world environments via this facility.

@cscutcher Has been an excellent reviewer. I greatly appreciate your engagement. It would not have been possible to polish this off so fast without your involvement.

All this to say: This is reasonably ready for merge. Should something arise, a fix it shall be.

@zimbatm zimbatm merged commit 61f25b0 into numtide:main Jul 11, 2023
@blaggacao blaggacao deleted the contrib-add-direnv-lib branch July 11, 2023 10:06
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