nix-community / home-manager

Manage a user environment using Nix [maintainer=@rycee]
https://nix-community.github.io/home-manager/
MIT License
6.95k stars 1.8k forks source link

bug: `programs.git.extraConfig` doesn't handle order-dependent keys properly #4439

Open lilyball opened 1 year ago

lilyball commented 1 year ago

Are you following the right branch?

Is there an existing issue for this?

Issue description

Git credentials are sensitive to the order of keys across sections. In particular, it collects all credential.helper and credential.<url>.helper values in the order specified by config (where <url> is matched against the remote), and also specifying the empty string causes it to forget all entries up to that point.

In addition, the nixpkgs git package on darwin will include a credentia.helper=osxkeychain entry in the system config.

Given this, with programs.gh.gitCredentialHelper.enable = true, I end up with the following abbreviated config:

# from system config
[credential]
helper = "osxkeychain"
# from user config
[credential."https://github.com"]
helper = "/path/to/gh auth git-credential"

And for github remotes git will therefore try git-credential-osxkeychain prior to gh auth git-credential. But I want to flip this order and prioritize gh auth git-credential. The intended user config looks like

[credential]
helper = ""
[credential."https://github.com"]
helper = "/path/to/gh auth git-credential"
[credential]
helper = "osxkeychain"

This is currently impossible to specify with home-manager. Workarounds include specifying the empty string and osxkeychain separately for each credential host, or adding extra text directly to xdg.configFile."git/config".text.

I don't know what the correct solution here is, given that the desired config requires having two sepearate [credential] sections. Credential support could potentially be moved into a separate programs.git.credentials option that supports this sort of ordering (though this would likely produce confusing results when putting credentials into extraConfig as well). Or maybe something could be done by making extraConfig into a submodule (with a freeform type) that makes credential an option with special handling, although I don't know precisely what to do here.

There's also potential ordering issues between the url subsections. For example, if I want to have credential."https://github.com".helper and then specialize one repo with credential."https://github.com/foo/bar.git".helper, I'd want the specialized entry to occur in the config first, but right now it will occur second (due to the default lexicographical sorting of attributes).

Maintainer CC

@rycee

System information

- system: `"aarch64-darwin"`
 - host os: `Darwin 22.6.0, macOS 13.5.1`
 - multi-user?: `yes`
 - sandbox: `relaxed`
 - version: `nix-env (Nix) 2.17.0`
 - channels(lily): `""`
 - channels(root): `""`
 - nixpkgs: `/etc/nix/inputs/nixpkgs`
lilyball commented 1 year ago

This issue may affect other config values that support url-specific overrides too, but I haven't checked.

stale[bot] commented 10 months ago

Thank you for your contribution! I marked this issue as stale due to inactivity. Please be considerate of people watching this issue and receiving notifications before commenting 'I have this issue too'. We welcome additional information that will help resolve this issue. Please read the relevant sections below before commenting.

If you are the original author of the issue

* If this is resolved, please consider closing it so that the maintainers know not to focus on this. * If this might still be an issue, but you are not interested in promoting its resolution, please consider closing it while encouraging others to take over and reopen an issue if they care enough. * If you know how to solve the issue, please consider submitting a Pull Request that addresses this issue.

If you are not the original author of the issue

* If you are also experiencing this issue, please add details of your situation to help with the debugging process. * If you know how to solve the issue, please consider submitting a Pull Request that addresses this issue.

Memorandum on closing issues

Don't be afraid to manually close an issue, even if it holds valuable information. Closed issues stay in the system for people to search, read, cross-reference, or even reopen – nothing is lost! Closing obsolete issues is an important way to help maintainers focus their time and effort.

azban commented 8 months ago

this also is an issue for includeIf blocks as, if you want to use includeIf to override some default configuration

here's an example

  home.file.".config/git/<work>".text = ''
    [user]
            email = "<work_email>"
  '';
  programs.git = {
    enable = true;
    userName = "<username>";
    userEmail = "<personal_email>";
    extraConfig = {
      "includeIf \"gitdir:~/Work/\"" = { path = "~/.config/git/<work>"; };
    };
  };

which emits

[includeIf "gitdir:~/Work/"]
    path = "~/.config/git/work""

[user]
    email = "<personal_email>"
    name = "<username>"
lilyball commented 8 months ago

@azban In your particular case you can use the built-in includes support, as those go at the end of the file.

programs.git = {
  enable = true;
  userName = "«username»";
  userEmail = "«personal_email»";
  includes = [{
    condition = "gitdir:~/Work/";
    contents.user.email = "«work_email»";
  }];
};

Now that I've written this out, I suppose using non-conditional includes would be a possible partial workaround for my issue, as I could use multiple such files to give myself precise control over the ordering of these sections. I say partial as this requires me to write all the credential helpers myself, but other modules like programs.gh set credentials too and so I can't necessarily achieve the control I want. If it's just gh I can still hack it together, but if there's a second other module that sets credentials for me then it won't work.

stale[bot] commented 4 months ago

Thank you for your contribution! I marked this issue as stale due to inactivity. Please be considerate of people watching this issue and receiving notifications before commenting 'I have this issue too'. We welcome additional information that will help resolve this issue. Please read the relevant sections below before commenting.

If you are the original author of the issue

* If this is resolved, please consider closing it so that the maintainers know not to focus on this. * If this might still be an issue, but you are not interested in promoting its resolution, please consider closing it while encouraging others to take over and reopen an issue if they care enough. * If you know how to solve the issue, please consider submitting a Pull Request that addresses this issue.

If you are not the original author of the issue

* If you are also experiencing this issue, please add details of your situation to help with the debugging process. * If you know how to solve the issue, please consider submitting a Pull Request that addresses this issue.

Memorandum on closing issues

Don't be afraid to manually close an issue, even if it holds valuable information. Closed issues stay in the system for people to search, read, cross-reference, or even reopen – nothing is lost! Closing obsolete issues is an important way to help maintainers focus their time and effort.