Skip to content

Conversation

@Buttars
Copy link

@Buttars Buttars commented Sep 20, 2025

Description

If a user has a trailing / in their user.users.<user>.home value with the zsh enabled the module fails to properly check the equality of dotDir and the home path.

The libs.nix module is stripping the trailing / from the dotDirAbs and dotDirRel and not for homeDir.

You can see the how the values are evaluated by running this script:

nix eval --json --impure --expr '
let
  flake   = builtins.getFlake ("path:" + toString ./.);
  libPkgs = if flake.inputs ? nixpkgs then flake.inputs.nixpkgs.lib else import <nixpkgs/lib> {};
  user    = builtins.getEnv "USER";
  host    = builtins.getEnv "HOSTNAME";

  hmCfg =
    if flake ? darwinConfigurations then
      let
        dcs = flake.darwinConfigurations;
        machineKey =
          if host != "" && builtins.hasAttr host dcs
          then host
          else builtins.head (builtins.attrNames dcs);
        hmUsers = dcs.${machineKey}.config.home-manager.users;
      in
        if builtins.hasAttr user hmUsers
        then hmUsers.${user}
        else throw "User ${user} not found in home-manager.users for ${machineKey}"
    else
      let
        hcs = flake.homeConfigurations;
        keys = builtins.attrNames hcs;
        matches = builtins.filter (k: (hcs.${k}.config.home.username or "") == user) keys;
        key = if matches != [] then builtins.head matches else builtins.head keys;
      in
        hcs.${key}.config;

  hmSrc = flake.inputs.home-manager.outPath;
  vals  = import (hmSrc + "/modules/programs/zsh/lib.nix") { config = hmCfg; lib = libPkgs; };
in { inherit (vals) homeDir dotDirAbs dotDirRel; }
' | jq

@lzpreslav actually mentioned this on the PR a few months back.
#6089 (comment)

Relevant LOC:
https://github.com/nix-community/home-manager/blob/master/modules/programs/zsh/lib.nix#L6
https://github.com/nix-community/home-manager/blob/master/modules/programs/zsh/lib.nix#L69
https://github.com/nix-community/home-manager/blob/master/modules/programs/zsh/lib.nix#L99-L100
https://github.com/nix-community/home-manager/blob/master/modules/programs/zsh/default.nix#L415

Checklist

  • Change is backwards compatible.

  • Code formatted with nix fmt or
    nix-shell -p treefmt nixfmt deadnix keep-sorted --run treefmt.

  • Code tested through nix run .#tests -- test-all or
    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.

@Buttars
Copy link
Author

Buttars commented Sep 20, 2025

If someone could guide me on writing tests for this I would appreciate it. I only see tests testing the output of the module and not any unit tests.

A trailing slash in user.users.<user>.home broke zshenv setup in Home
Manager. The generated ~/.zshenv ended up sourcing itself, causing an
infinite loop at shell startup.
@Buttars
Copy link
Author

Buttars commented Sep 22, 2025

@khaneliman

@stale
Copy link

stale bot commented Dec 21, 2025

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 Dec 21, 2025
in
rec {
homeDir = config.home.homeDirectory;
homeDir = cleanPathStr config.home.homeDirectory;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you possibly add a test case for this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nvm just realized I missed this PR a while ago and you had a comment about not knowing how to.

Copy link
Author

@Buttars Buttars Dec 22, 2025

Choose a reason for hiding this comment

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

Thanks for getting back to me on this. I'd happily add test cases if you could point me to an example or documentation. Let me know if you'd still like to get this merged. 😁

I tried looking for other examples and documentation but couldn't figure it out.

Copy link
Collaborator

@khaneliman khaneliman Dec 22, 2025

Choose a reason for hiding this comment

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

I was playing around with tests and found some other cases to solve for. Just gonna push to a new branch and can you verify it works for you.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@Buttars can you test #8378 it should work

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.

2 participants