Files: Allow `target="."`, and add symlinkJoin to the documentation

[niacdoial]

Description

• Allow the use the home directory as a target for recursively-linked directories
• Add symlinkJoin to the documentation for home.file

Checklist

• [X] Change is backwards compatible.

• [ ] Code formatted with ./format. -> wont fix just yet, changes are too big to be in a simple pull request (both files.nix and file-type.nix are on the ignore list of ./format)

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

• [ ] Test cases updated/added. See example. -> arguably not needed for this change -> would need more discussion anyway, as the previous behavior tripped a "this should have been caught earlier" check, which cannot be used in tests

• [X] Commit messages are formatted like

  {component}: {description}
  
  {long description}
  

[niacdoial]

There's just a hickup which might pause this pull request:

I have a weird unrelated issue which might require this to be paused. Using the builtin pkgs.symlinkJoin within my home.nix file fails (the symlinkJoin-based derivation appears (and its "symlink-joined-dir.drv" file too), but never gets built.) However, copying the code for symlinkJoin from nixpkgs's trivial-builders.nix into home.nix and using that copy of the function fixes this?

I'll paste the exact code (and version info, etc) if you want, in a later comment.

This pull request might be the "better solution" when compared to my earlier #3141.

[axgfn]

I was about to submit a PR with nearly the same fix, except that instead of making a special case out of target="."; I just tested that both the source and target were directories (so lndir will work):

          # If the target already exists then we have a collision. Note, this
          # should not happen due to the assertion found in the 'files' module.
          # We therefore simply log the conflict and otherwise ignore it, mainly
          # to make the `files-target-config` test work as expected.
          if [[ -e "$realOut/$relTarget" && ]]; then
            if [[ $recursive && -d "$realOut/$relTarget" && -d "$source" ]]; then
              : skip
            else
              echo "File conflict for file '$relTarget'" >&2
              return
            fi
          fi

I think this is probably better, because I would also expect recursively-linked directories to work elsewhere, too. For example, this works with my solution but not with yours:

xdg.dataFile.hello.text = "world\n";
home.file.cowsay = {
  target = ".local";
  source = "${pkgs.cowsay}";
  recursive = true;
};

[niacdoial]

Sorry, but there are… a few issues with the changes you are suggesting (save for the typo. well spotted.) Though, I created a previous pull request that does what you are interested in, at #3141.

First, the script you modified (the bash script that checks for collisions) is filling the derivation (/nix/store/HASH-home-manager-files/, go look at where $realOut is defined there), so it doesn't even see collisions with directories that only exist in the target /home/USER/. Basically, the example of "what wouldn't work with my patch" would actually work. The reason why my original patch here is needed is because there would have been a collision with the created-but-empty /nix/store/HASH-home-manager-files/ directory.

Second, this wouldn't actually allow multiple recursively-linked directories to have the same destination, because there are checks in the nix code that runs before that bash script.

Lastly, but most importantly, the check you removed introduces a huge footgun: here's an example with some overdramatic names for emphasis:

home.file.myfiles = {
  target = ".config";
  source = /data/user/my-really-important-and-really-delicate-config-files-that-are-worth-gold;
  recursive = false;
};
home.file.expconfig = {
  target = ".config/sway";
  source = "${pkgs.some-half-assed-experimental-config-generator-that-is-currently-incomplete}";
  recursive = true;
};

[edit: a few changes in phrasing, because wow I sounded rude. also, added a small extra explanation]

[stale[bot]]

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.

[niacdoial]

…so. Basically, looking at this again, I just realize how condescending I sounded to you, even after the edit.

Anyway, if someone looks at this and can make decisions about the project, I'm not sure whether this pull or #3141 is the most appropriate for home-manager. so I guess until there's a decision on that, this will stay a stale pull.

[niacdoial]

if someone stumbles upon this issue and it's not dealt with, here's a workaround that works on nixos unstable (and only unstable as of 2023-02-18):

  home.file = builtins.listToAttrs (
    let link_me =
     (readDir pkgs.inhome-data.outDir);
    in
    builtins.map ( filename: {
      name = filename;
      value = {
        source = pkgs.inhome-data + "/" + filename;
        target = ( "./" +  filename);
        recursive = (link_me."${filename}" == "directory");
      };
    })
    (builtins.attrNames link_me)
  );

On NixOs stable, (readDir pkgs.inhome-data.outDir); will not work because builtins.readDir is not defined. instead, you need to generate that manually, with the value looking like (for example) {".config" = "directory"; "bin" = "directory"; ".profile" = "symlink"; ".bashrc" = "symlink";}

[axgfn]

@niacdoial No problem, don't worry about it.

Is there anything blocking this from being merged? I rely pretty heavily on this feature in my own config, and it would be nice to be able to remove the giant workaround from my dotfiles that I need without this upstream.

[niacdoial]

For both this pull request and the one I refer to, what blocks the merge is that it has not been reviewed yet? I guess the main question is which of the two should be merged (they are kinda mutually exclusive, since they are different workarounds for the same problem). I don't think I can do anything to speed things up on my end.

edit: actually yes: I changed the first message to make the pull look less of a WIP

[stale[bot]]

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.