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

Allow multiple installation of home-manager to co-exist on the same system #1156

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

Conversation

kalbasit
Copy link
Collaborator

Description

I currently have home-manager installed as a user to manage my own system, and I'm working to include home-manager as a submodule of our repository at work so we could install systemd units. This pull request changes some hardcoded settings to options so we can customize the activation script to not disrupt the other installation. All options are marked as internal to avoid documenting them because I have not updated any of the shell scripts (besides the activation) to refer to the options instead of the hardcoded values as I do not see the point of having multiple installations both managed by the user via home-manager command.

Checklist

  • 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.

@kalbasit kalbasit requested a review from rycee as a code owner April 15, 2020 20:39
@kalbasit
Copy link
Collaborator Author

@rycee the tests work locally for me, do you happen to know why they are failing here?

@rycee
Copy link
Member

rycee commented Apr 15, 2020

It was due to some change w.r.t tmux plugins in Nixpkgs. I've resolved it in master so hopefully it should build ok if you rebase.

@kalbasit kalbasit force-pushed the allow-setting-gcroot-link-name branch from 010c785 to a5143c4 Compare April 15, 2020 21:37
@stale
Copy link

stale bot commented Apr 28, 2021

Thank you for your contribution! I marked this pull request as stale due to inactivity. If this remains inactive for another 7 days, I will close this PR. 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 issue

  • 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 Apr 28, 2021
@stale stale bot closed this May 5, 2021
@terlar
Copy link
Contributor

terlar commented May 6, 2021

I guess this was ready and potentially useful?

@rycee
Copy link
Member

rycee commented May 6, 2021

Yeah, I've been meaning to revisit this.

@rycee rycee reopened this May 6, 2021
@stale stale bot removed the status: stale label May 6, 2021
@rycee rycee added the pinned Prevent marking as stale label May 6, 2021
@dguibert
Copy link
Contributor

dguibert commented Jan 4, 2022

Hi,
I've merged this in dguibert@50de2b9 but straightforward resolved conflicts (and/but also on top of some of my local changes).

Work seamlessly to support 2 profiles, one for x86_64 and one for aarch64 on a same shared $HOME, with:

          ({ ... }: {
            home.sessionVariablesFileName = "hm-x86_64-session-vars.sh";
            home.sessionVariablesGuardVar = "__HM_X86_64_SESS_VARS_SOURCED";
            home.pathName = "home-manager-x86_64_path";
            home.gcLinkName = "current-home-x86_64";
            home.generationLinkNamePrefix = "home-manager-x86_64";
          })

or

          ({ ... }: {
            home.sessionVariablesFileName = "hm-aarch64-session-vars.sh";
            home.sessionVariablesGuardVar = "__HM_AARCH64_SESS_VARS_SOURCED";
            home.pathName = "home-manager-aarch64_path";
            home.gcLinkName = "current-home-aarch64";
            home.generationLinkNamePrefix = "home-manager-aarch64";
          })

It solves #2542.

@kalbasit
Copy link
Collaborator Author

kalbasit commented Jan 5, 2022

@dguibert if you've resolved merge conflicts, do you mind sharing a branch with me I can merge here to update this PR? Thanks!

@dguibert
Copy link
Contributor

dguibert commented Jan 6, 2022

@dguibert if you've resolved merge conflicts, do you mind sharing a branch with me I can merge here to update this PR? Thanks!

sure, here it is: https://github.com/dguibert/home-manager/tree/dg/merge-1156

@dguibert
Copy link
Contributor

dguibert commented Mar 18, 2022

Hi @kalbasit,
Could you please get the merge and update this PR.

@kalbasit
Copy link
Collaborator Author

kalbasit commented Apr 2, 2022

@dguibert thank you, that helped a lot. I went ahead and merged your branch, and merged the current master as well.

@kalbasit
Copy link
Collaborator Author

Yeah, I've been meaning to revisit this.

@rycee, what's needed to get this considered and merged?

@dguibert
Copy link
Contributor

dguibert commented Jan 11, 2024

Hi @rycee,

I'm still using it and need to resolve conflicts (from time to time):
dguibert@047ff6d

I've also extend it with

How can I help for this to be merged?

@kalbasit
Copy link
Collaborator Author

fyi this same approach for nix-darwin was rejected upstream. @rycee if you want to consider merging this, then I'll update it by merging some of @dguibert's work but I won't do that unless you want to merge it (being cautious about my time).

@dguibert
Copy link
Contributor

Hi

I'll update it by merging some of @dguibert's work

I've updated this branch https://github.com/dguibert/home-manager/commits/allow-setting-gcroot-link-name

  • by merging master
  • by cherry-picking some of my work related to this PR

Hope to see this considered merged.

@bew
Copy link

bew commented Jan 31, 2024

Hello!

From a usability point of view I think it would be much nicer to have a single variable like a home-manager profile name, that is then used in all these points that are specific to an 'instance' of home-manager.

👉 This way we don't need to know that all these variables are related and should change together.

We could say that now the default profile name is main or default, and have that name appear in all relevant positions.
This would be a notable breaking change, but should be nicely future-proof.

And if backward compatibility is really something you want to keep, we could always default the profile name to null/false, and handle this.

WDYT?

@dguibert
Copy link
Contributor

WDYT?

I think it can be added on top of this by introducing a module option, like you mention, profile-name:

if it's null, fallback to default behavior, allowing to override each variable individually.
otherwise use the profile-name to set the variable as suggested in #1156 (comment) and implemented in https://github.com/dguibert/dotfiles/blob/master/admin/nixops/modules/home-manager/dguibert/custom-profile.nix

@mwoodpatrick
Copy link

Where are we on this, it seems like a useful addition?

@kalbasit
Copy link
Collaborator Author

I haven't had a reason to use multiple profiles myself for a long while and so I haven't been maintaining this PR. I think a top-level profileName makes sense to me as long as it does not change current behavior or break existing installation. If anyone's willing to do that work, I'd be happy to test it and incorporate that into this PR. @rycee does that work for you? We should probably consolidate our effort and get this merged in as it's been lingering for quite some time now.

@dguibert
Copy link
Contributor

Thx, hope we could polish this to get merged.

I've rebased the branch and introduce an option name-profile.suffix to suffix files/directory/profile paths.

@kalbasit, it can be found https://github.com/dguibert/home-manager/commits/allow-setting-gcroot-link-name-rebased

@github-actions github-actions bot added the shell label Dec 16, 2024
@kalbasit kalbasit force-pushed the allow-setting-gcroot-link-name branch from d5f660e to ccb8231 Compare December 16, 2024 16:06
@kalbasit
Copy link
Collaborator Author

Thx, hope we could polish this to get merged.

I've rebased the branch and introduce an option name-profile.suffix to suffix files/directory/profile paths.

@kalbasit, it can be found dguibert/home-manager@allow-setting-gcroot-link-name-rebased (commits)

When I try to merge your branch, it conflicts with mine

Unmerged paths:
  (use "git add <file>..." to mark resolution)
        both modified:   modules/home-environment.nix
        both modified:   modules/lib-bash/activation-init.sh
        both modified:   modules/programs/bash.nix
        both modified:   modules/programs/fish.nix
        both modified:   modules/targets/generic-linux.nix
        both modified:   tests/modules/home-environment/default.nix
        both added:      tests/modules/home-environment/gc-link-name-custom.nix
        both added:      tests/modules/home-environment/gc-link-name-default.nix
        both added:      tests/modules/home-environment/generation-link-name-prefix-custom.nix
        both added:      tests/modules/home-environment/generation-link-name-prefix-default.nix
        both added:      tests/modules/home-environment/home-path-drv-name-custom.nix
        both added:      tests/modules/home-environment/home-path-drv-name-default.nix

@dguibert
Copy link
Contributor

dguibert commented Dec 16, 2024

Thx, hope we could polish this to get merged.
I've rebased the branch and introduce an option name-profile.suffix to suffix files/directory/profile paths.
@kalbasit, it can be found dguibert/home-manager@allow-setting-gcroot-link-name-rebased (commits)

When I try to merge your branch, it conflicts with mine

That's because my branch is a rebased of yours on top of master.
If you want can try to cherry-pick my changes on top of your branch and merge the master branch to resolve conflicts.

@kalbasit kalbasit force-pushed the allow-setting-gcroot-link-name branch from ccb8231 to 6f9ac2d Compare December 16, 2024 16:29
@kalbasit
Copy link
Collaborator Author

@dguibert done. Do you mind testing? If it works, please ensure your branch is based back off of this one so any more changes on your end I can easily bring to my side.

kalbasit and others added 7 commits December 16, 2024 10:19
By changing the GC link name, one could have multiple installations of
home-manager to co-exist without interfering with each other.
…-path derivation

By changing the name of the derivation, one could have multiple
installations of home-manager to co-exist without interfering with each
other.
By changing the name of the session vars, one could have multiple
installations of home-manager to co-exist without interfering with each
other.
…ssVars

By changing the variable name, one could have multiple installations of
home-manager to co-exist without interfering with each other.
@kalbasit kalbasit force-pushed the allow-setting-gcroot-link-name branch from 6f9ac2d to db8db79 Compare December 16, 2024 18:26
@kalbasit
Copy link
Collaborator Author

@dguibert do you mind taking a look at the failing test?

@dguibert
Copy link
Contributor

@kalbasit
Copy link
Collaborator Author

@dguibert perfect, thank you. Tests worked locally, hopefully it'll work on the CI as well.

Can someone else test it before we hand it off to @rycee to consider merging it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pinned Prevent marking as stale shell
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants