nh icon indicating copy to clipboard operation
nh copied to clipboard

handle permissions issues for `--update` arg

Open NotAShelf opened this issue 9 months ago • 14 comments

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 avatar May 01 '24 09:05 NotAShelf

Might address #79

NotAShelf avatar May 01 '24 09:05 NotAShelf

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

viperML avatar May 01 '24 10:05 viperML

to handle the error ourselves :)

NotAShelf avatar May 01 '24 10:05 NotAShelf

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

viperML avatar May 01 '24 10:05 viperML

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

viperML avatar May 01 '24 10:05 viperML

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 avatar May 01 '24 10:05 NotAShelf

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.

NotAShelf avatar May 01 '24 10:05 NotAShelf

Does nh attempt to read flake.lock before the rebuild? Looks like permission denied error is given before we can reach the check.

NotAShelf avatar Jul 28 '24 11:07 NotAShelf

In home.rs, it is read some lines before to check for the HM output

viperML avatar Jul 28 '24 13:07 viperML

I'm currently testing with nh os switch - should home.rs still be referenced in this case?

NotAShelf avatar Jul 28 '24 14:07 NotAShelf

no

viperML avatar Jul 28 '24 14:07 viperML

Then I have no idea why we are not getting the custom error message.

NotAShelf avatar Jul 28 '24 14:07 NotAShelf

it's so over

viperML avatar Jul 28 '24 17:07 viperML

nhover

NotAShelf avatar Jul 28 '24 18:07 NotAShelf

@NotAShelf should I close this PR?

viperML avatar Sep 05 '24 08:09 viperML

If you are uninterested in figuring out why the behaviour occurs, feel free.

NotAShelf avatar Sep 05 '24 11:09 NotAShelf