Skip to content

Conversation

@joshheinrichs-shopify
Copy link
Contributor

@joshheinrichs-shopify joshheinrichs-shopify commented Oct 9, 2024

Currently if the credential helpers packaged with git aren't on your PATH, git will fail to locate them. This most notably affects git maintanence1 which generates systemd units / launchd agents that invoke git directly without setting PATH. Home Manager is also able to generate maintanence units for systemd and does not set PATH2, meaning credential helpers will not work without additional configuration.

Moving the credential helpers to git's exec path3 allows git to locate them regardless of how PATH is set, and matches the packaging of Fedora4 and Homebrew5. Typically credential helpers are not added to PATH unless they are provided by another package separate from git (e.g. git-credential-gcloud).

I opted to create symlinks at the old bin paths in case some users were relying on invoking them directly. In practice I doubt this is done much. If we want to slim down git's bin dir I think the symlinks could be removed, and in doing so we'd better match the packaging done in other distributions.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

Footnotes

  1. https://git-scm.com/docs/git-maintenance/2.47.1

  2. https://github.com/nix-community/home-manager/blob/b6fd653ef8fbeccfd4958650757e91767a65506d/modules/programs/git.nix#L619

  3. https://git-scm.com/docs/git/2.47.1#Documentation/git.txt---exec-pathltpathgt

  4. https://src.fedoraproject.org/rpms/git/blob/rawhide/f/git.spec#_651

  5. https://github.com/Homebrew/homebrew-core/blob/12120ff79ce009489b958d8271af686d74403859/Formula/g/git.rb#L112

@ofborg ofborg bot requested review from globin, kashw2, primeos and wmertens October 9, 2024 17:28
@ofborg ofborg bot added 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. labels Oct 9, 2024
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Feb 15, 2025
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Mar 31, 2025
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Apr 7, 2025
Currently if the credential helpers packaged with git aren't on your
PATH, git will fail to locate them. This most notably affects git
maintanence[1] which generates systemd units / launchd agents that
invoke git directly without setting PATH. Home Manager is also able to
generate maintanence units for systemd and does not set PATH[2], meaning
credential helpers will not work without additional configuration.

Moving the credential helpers to git's exec path[3] allows git to locate
them regardless of how PATH is set, and matches the packaging of
Fedora[4] and Homebrew[5]. Typically credential helpers are not added to
PATH unless they are provided by another package separate from git (e.g.
git-credential-gcloud).

I opted to create symlinks at the old bin paths in case some users were
relying on invoking them directly. In practice I doubt this is done
much. If we want to slim down git's bin dir I think the symlinks could
be removed, and in doing so we'd better match the packaging done in
other distributions.

[1]: https://git-scm.com/docs/git-maintenance/2.47.1
[2]: https://github.com/nix-community/home-manager/blob/b6fd653ef8fbeccfd4958650757e91767a65506d/modules/programs/git.nix#L619
[3]: https://git-scm.com/docs/git/2.47.1#Documentation/git.txt---exec-pathltpathgt
[4]: https://src.fedoraproject.org/rpms/git/blob/rawhide/f/git.spec#_651
[5]: https://github.com/Homebrew/homebrew-core/blob/12120ff79ce009489b958d8271af686d74403859/Formula/g/git.rb#L112
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Apr 8, 2025
joshheinrichs-shopify added a commit to joshheinrichs-shopify/home-manager that referenced this pull request Apr 21, 2025
This doesn't yet work for folks using git-credential-osxkeychain as it
isn't placed on git's exec path[1]. I mostly attempted to mimic the
agents that git maintanence generates for itself. The settings differ
slightly because git configures calandar intervals incorrectly[2], and
git started injecting some additional config settings recently[3] which
would be good to mimic in another PR.

[1] NixOS/nixpkgs#347515
[2] https://lore.kernel.org/git/20250421054633.231069-1-joshiheinrichs@gmail.com
[3] git/git@4f55519
joshheinrichs-shopify added a commit to joshheinrichs-shopify/home-manager that referenced this pull request Apr 21, 2025
This doesn't yet work for folks using git-credential-osxkeychain as it
isn't placed on git's exec path[1]. I mostly attempted to mimic the
agents that git maintanence generates for itself. The settings differ
slightly because git configures calandar intervals incorrectly[2], and
git started injecting some additional config settings recently[3] which
would be good to mimic in another PR.

[1] NixOS/nixpkgs#347515
[2] https://lore.kernel.org/git/20250421054633.231069-1-joshiheinrichs@gmail.com
[3] git/git@4f55519
joshheinrichs-shopify added a commit to joshheinrichs-shopify/home-manager that referenced this pull request Apr 21, 2025
This doesn't yet work for folks using git-credential-osxkeychain as it
isn't placed on git's exec path[1]. I mostly attempted to mimic the
agents that git maintanence generates for itself. The settings differ
slightly because git configures calendar intervals incorrectly[2], and
git started injecting some additional config settings recently[3] which
would be good to mimic in another PR.

[1] NixOS/nixpkgs#347515
[2] https://lore.kernel.org/git/20250421054633.231069-1-joshiheinrichs@gmail.com
[3] git/git@4f55519
joshheinrichs-shopify added a commit to joshheinrichs-shopify/home-manager that referenced this pull request Apr 21, 2025
This doesn't yet work for folks using git-credential-osxkeychain as it
isn't placed on git's exec path[1]. I mostly attempted to mimic the
agents that git maintenance generates for itself. The settings differ
slightly because git configures calendar intervals incorrectly[2], and
git started injecting some additional config settings recently[3] which
would be good to mimic in another PR.

[1] NixOS/nixpkgs#347515
[2] https://lore.kernel.org/git/20250421054633.231069-1-joshiheinrichs@gmail.com
[3] git/git@4f55519
Copy link
Contributor

@khaneliman khaneliman left a comment

Choose a reason for hiding this comment

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

LGTM

joshheinrichs-shopify added a commit to joshheinrichs-shopify/home-manager that referenced this pull request Apr 21, 2025
This doesn't yet work for folks using git-credential-osxkeychain as it
isn't placed on git's exec path[1]. I mostly attempted to mimic the
agents that git maintenance generates for itself. The settings differ
slightly because git configures calendar intervals incorrectly[2], and
git started injecting some additional config settings recently[3] which
would be good to mimic in another PR.

[1] NixOS/nixpkgs#347515
[2] https://lore.kernel.org/git/20250421054633.231069-1-joshiheinrichs@gmail.com
[3] git/git@4f55519
@khaneliman
Copy link
Contributor

Can merge as-is with the backwards compat, if someone has the appetite for removing the old binary location links and/or deprecating them. They can do it in a follow up PR.

@khaneliman khaneliman merged commit b47b3a9 into NixOS:staging Apr 21, 2025
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants