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 permissions issues for --update arg #101

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

NotAShelf
Copy link
Contributor

Tries to open flake.lock with both read and write permissions (which should, in theory, be enough to avoid failures due to flake.lock being owned by root.

If it fails (e.g., due to insufficient permissions or the file not existing), it bails with the error message and a short warning.

@NotAShelf
Copy link
Contributor Author

Might address #79

@viperML
Copy link
Owner

viperML commented May 1, 2024

I don't know the purpose of this. Nix already complains with flake.lock: Permission denied

@NotAShelf
Copy link
Contributor Author

to handle the error ourselves :)

@viperML
Copy link
Owner

viperML commented May 1, 2024

another option is wrapping the update command with .wrap_err https://docs.rs/eyre/latest/eyre/trait.WrapErr.html#tymethod.wrap_err

@viperML
Copy link
Owner

viperML commented May 1, 2024

But thinking about it, an error in the update command might be caused by other things, so it sounds reasonable to pre-check for the flake.lock permission error. Although I would add a message saying that nh doesn't support updating flakes owned by root

@NotAShelf
Copy link
Contributor Author

Error message is a bit better now.

Side note, I was unable to test this because even when my flake.lock is owned by root, it can still update on my system. So, testing on a sane system would be appreciated.

@NotAShelf
Copy link
Contributor Author

Side note, the file check should probably go into util.rs as a function since it's the exact same in both home-manager and nixos command interfaces, but I don't have the the time to get on that.

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.

None yet

2 participants