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

home-manager: adds configurable symlink/bindfs option #99

Merged
merged 1 commit into from
Aug 27, 2022

Conversation

Misterio77
Copy link
Contributor

@Misterio77 Misterio77 commented Jul 14, 2022

Hello, all! A huge thanks to the maintainers :), I've been using impermanence for a while and really love it.


As mentioned in #42, bindfs causes some issues with a few specific programs (steam, for example). I'm aware that we have options for using the NixOS module to avoid bindfs, but that may not be preferred (or possible) for some.

This PR allows for setting a specific method for each individual directory. They can (atm) be either "bindfs" (the default if not specified) or "symlink".

I've reused the code that links files, but now it runs on directories that have method = "symlink" as well.

I made sure that changing between the two methods works reliably. For that end, I've added:

  • a cleanEmptyLinkTargets activation script that handles changing from bindfs to symlink. If a link's intended location has something mounted, it is unmounted (earlier than usual) and its mount point is deleted (safely with rm -d).
  • a mount | grep condition on the usual unmount phase, to avoid systemd trying to unmount a symlink (causes error messagens and a few seconds of hanging, although it ultimately works).

Oh, and by the way, git got a little confused with the repeated unmount code and messed up the diff. The only change I made to mkUnmount was adding the mount | grep check.

As we have three places with the same unmount code now, I've factored it out to a function. Diff looks fine now.

Hope this is useful, and please let me know if there's anything to be improved!


Fixes #42

Copy link
Collaborator

@talyz talyz left a comment

Choose a reason for hiding this comment

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

Diff looks good and it works as expected for me. Thanks a lot for doing this! Just one tiny thing: could you rename path to directory to match the NixOS module?

@Misterio77
Copy link
Contributor Author

Misterio77 commented Aug 21, 2022

No problem at all! Thank you for this awesome tool :)

could you rename path to directory

Done!

@Misterio77 Misterio77 requested review from talyz and removed request for lovesegfault August 21, 2022 21:22
@Misterio77
Copy link
Contributor Author

Oops removed the review request by mistake, sorry

@talyz
Copy link
Collaborator

talyz commented Aug 22, 2022

It still needs to be updated in the readme. Also, please squash the update into the previous commit.

@Misterio77
Copy link
Contributor Author

All done :)

@talyz talyz merged commit 2237ad2 into nix-community:master Aug 27, 2022
@talyz
Copy link
Collaborator

talyz commented Aug 27, 2022

Great work! Thank you!

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.

Why a bindfs rather than a symlink?
2 participants