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

nixos: use user service for activation #2548

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

pasqui23
Copy link
Contributor

@pasqui23 pasqui23 commented Dec 11, 2021

Description

Use user units to activate home configurations. This has several benefits:

  1. Less units are run on startup, especially if more than one user is configured
  2. Home manager can access $XDG_RUNTIME_DIR, including calling dbus units
  3. Commands depending on X or Wayland are similarly aviable

ssh is special cased so that its config file still is linked on boot to allow remote login.
If other components needs to be similarly special cased I could rework this PR to include a few dedicated options.

EDIT(@ncfavier): fixes #3415

Checklist

  • Change is backwards compatible.

  • Code formatted with ./format.

  • Code tested through nix-shell --pure tests -A run.all.

  • Test cases updated/added. See example.

  • Commit messages are formatted like

    {component}: {description}
    
    {long description}
    

    See CONTRIBUTING for more information and recent commit messages for examples.

  • If this PR adds a new module

    • Added myself as module maintainer. See example.

    • Added myself and the module files to .github/CODEOWNERS.

@pasqui23 pasqui23 marked this pull request as ready for review December 11, 2021 15:05
@pasqui23 pasqui23 requested a review from rycee as a code owner December 11, 2021 15:05
@pasqui23 pasqui23 force-pushed the nixos-late-start branch 3 times, most recently from 25ab14f to d1c291b Compare December 11, 2021 15:46
@pasqui23
Copy link
Contributor Author

I also suspect that this will fix https://gitlab.com/rycee/sd-switch/-/issues/2

nixos/default.nix Outdated Show resolved Hide resolved
@pasqui23 pasqui23 mentioned this pull request Dec 15, 2021
7 tasks
@pasqui23 pasqui23 force-pushed the nixos-late-start branch 4 times, most recently from 44313f9 to c92001a Compare December 25, 2021 17:49
@ncfavier ncfavier mentioned this pull request May 20, 2022
5 tasks
@ncfavier
Copy link
Member

ncfavier commented Jun 7, 2022

Hi, I've rebased this PR against master following #2971: ncfavier@d28c365. Feel free to force-pull from my fork:

git fetch https://github.com/ncfavier/home-manager nixos-late-start
git reset --hard FETCH_HEAD

(I haven't reviewed the changes, only fixed the conflicts.)

@pasqui23 pasqui23 force-pushed the nixos-late-start branch 2 times, most recently from 20a1898 to 3c33445 Compare June 18, 2022 20:28
@pasqui23
Copy link
Contributor Author

@rycee why the test is failing with

error: The option `systemd.user' does not exist. Definition values:
       - In `/home/runner/work/home-manager/home-manager/nixos/default.nix':
           {
             _type = "if";
             condition = false;
             content = {
               services = {
           ...
(use '--show-trace' to show detailed location information)
Uh oh, the installation failed! Please create an issue at
   [ https://github.com/nix-community/home-manager/is](https://github.com/nix-community/home-manager/issues)
if the error seems to be the fault of Home Manager.

?

@pasqui23 pasqui23 changed the title nixos: use user service for activation WIP:nixos: use user service for activation Aug 12, 2022
@stale
Copy link

stale bot commented Nov 11, 2022

Thank you for your contribution! I marked this pull request as stale due to inactivity. Please read the relevant sections below before commenting.

If you are the original author of the PR

  • GitHub sometimes doesn't notify people who commented / reviewed a PR previously when you (force) push commits. If you have addressed the reviews you can officially ask for a review from those who commented to you or anyone else.
  • If it is unfinished but you plan to finish it, please mark it as a draft.
  • If you don't expect to work on it any time soon, please consider closing it with a short comment encouraging someone else to pick up your work.
  • To get things rolling again, rebase the PR against the target branch and address valid comments.

If you are not the original author of the PR

  • If you want to pick up the work on this PR, please create a new PR and indicate that it supercedes and closes this PR.

@stale stale bot added the status: stale label Nov 11, 2022
@pasqui23 pasqui23 force-pushed the nixos-late-start branch 2 times, most recently from 87a4452 to 0c4b136 Compare December 1, 2022 15:46
@pasqui23 pasqui23 changed the title WIP:nixos: use user service for activation nixos: use user service for activation Dec 1, 2022
@pasqui23
Copy link
Contributor Author

pasqui23 commented Dec 1, 2022

This compiles and works without any problems.
@rycee I am ready for review.

Copy link
Member

@ncfavier ncfavier left a comment

Choose a reason for hiding this comment

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

I think this should be hidden behind an option home-manager.useUserService or somesuch and default to false at first. That way we could merge sooner without worrying about breaking people's configurations.

nixos/default.nix Show resolved Hide resolved
nixos/default.nix Outdated Show resolved Hide resolved
@rycee
Copy link
Member

rycee commented Apr 3, 2023

Sorry for the delay. I haven't had a look at this yet but in general I think it is a good idea to use a user service for activation and it should be used in all installation methods, i.e., for standalone and darwin as well.

Useful, e.g., for when you want to activate a configuration on the next login or if you want to choose which configuration to activate in the desktop manager.

@pasqui23
Copy link
Contributor Author

pasqui23 commented Apr 3, 2023 via email

@stale
Copy link

stale bot commented Jul 2, 2023

Thank you for your contribution! I marked this pull request as stale due to inactivity. Please read the relevant sections below before commenting.

If you are the original author of the PR

  • GitHub sometimes doesn't notify people who commented / reviewed a PR previously when you (force) push commits. If you have addressed the reviews you can officially ask for a review from those who commented to you or anyone else.
  • If it is unfinished but you plan to finish it, please mark it as a draft.
  • If you don't expect to work on it any time soon, please consider closing it with a short comment encouraging someone else to pick up your work.
  • To get things rolling again, rebase the PR against the target branch and address valid comments.

If you are not the original author of the PR

  • If you want to pick up the work on this PR, please create a new PR and indicate that it supercedes and closes this PR.

@gkleen
Copy link

gkleen commented Aug 17, 2023

@rycee bump?

Copy link

stale bot commented Nov 17, 2023

Thank you for your contribution! I marked this pull request as stale due to inactivity. Please read the relevant sections below before commenting.

If you are the original author of the PR

  • GitHub sometimes doesn't notify people who commented / reviewed a PR previously when you (force) push commits. If you have addressed the reviews you can officially ask for a review from those who commented to you or anyone else.
  • If it is unfinished but you plan to finish it, please mark it as a draft.
  • If you don't expect to work on it any time soon, please consider closing it with a short comment encouraging someone else to pick up your work.
  • To get things rolling again, rebase the PR against the target branch and address valid comments.

If you are not the original author of the PR

  • If you want to pick up the work on this PR, please create a new PR and indicate that it supercedes and closes this PR.

@mikunimaru
Copy link

This is the best patch. With the introduction of this feature, errors no longer occur on systems that mount the home folder at login.

Copy link

stale bot commented Mar 14, 2024

Thank you for your contribution! I marked this pull request as stale due to inactivity. Please read the relevant sections below before commenting.

If you are the original author of the PR

  • GitHub sometimes doesn't notify people who commented / reviewed a PR previously when you (force) push commits. If you have addressed the reviews you can officially ask for a review from those who commented to you or anyone else.
  • If it is unfinished but you plan to finish it, please mark it as a draft.
  • If you don't expect to work on it any time soon, please consider closing it with a short comment encouraging someone else to pick up your work.
  • To get things rolling again, rebase the PR against the target branch and address valid comments.

If you are not the original author of the PR

  • If you want to pick up the work on this PR, please create a new PR and indicate that it supercedes and closes this PR.

@stale stale bot added the status: stale label Mar 14, 2024
@gkleen
Copy link

gkleen commented Mar 15, 2024

@rycee bump?

@stale stale bot removed the status: stale label Mar 15, 2024
@rycee
Copy link
Member

rycee commented Mar 15, 2024

Yeah, sorry about the long delay here. I'm very much in favor of doing this, and ultimately I would like for activation through systemd to be the normal procedure, even for standalone installations.

I've been slowly working towards this, for example, by experimenting in #4976.

That said, perhaps it's worth getting this in before that work is done.

@nyabinary
Copy link

Yeah, sorry about the long delay here. I'm very much in favor of doing this, and ultimately I would like for activation through systemd to be the normal procedure, even for standalone installations.

I've been slowly working towards this, for example, by experimenting in #4976.

That said, perhaps it's worth getting this in before that work is done.

Eventually, this can lead to upstreaming home manager (via a optional module) in nixpkgs right?

@mikunimaru
Copy link

My environment-specific issues were resolved with this patch.
I have been using this patch for several months and have not had any problems.
This patch has minimal impact on users other than those who have explicitly enabled it, so I would be happy if it was merged as soon as possible.

@gkleen
Copy link

gkleen commented Mar 23, 2024

My environment-specific issues were resolved with this patch. I have been using this patch for several months and have not had any problems. This patch has minimal impact on users other than those who have explicitly enabled it, so I would be happy if it was merged as soon as possible.

ditto

@mikunimaru
Copy link

I have already cherry-picked and used this best patch.
Is it possible to resolve the conflict for use with home manager on master branch?

pasqui23 and others added 2 commits April 8, 2024 15:03
systemd: remove unneeded XDG_RUNTIME_DIR workaround

nixos: documentation about home manager status

nixos: use enable for ssh_config units
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.

Activation as user service
7 participants