Skip to content

git: move credential helpers to exec path#346556

Closed
joshheinrichs-shopify wants to merge 1 commit intoNixOS:stagingfrom
joshheinrichs-shopify:git-creds-on-exec-path
Closed

git: move credential helpers to exec path#346556
joshheinrichs-shopify wants to merge 1 commit intoNixOS:stagingfrom
joshheinrichs-shopify:git-creds-on-exec-path

Conversation

@joshheinrichs-shopify
Copy link
Contributor

If git-credential-osxkeychain isn't on your PATH, git will fail to find it. Moving it to git's exec path1 allows it to be located more reliably. In other distributions such as Fedora2 and Homebrew3, credential helpers are not added to the user's PATH unless they are supplied by another package (e.g. git-credential-gcloud).

If there are concerns that people may be relying on referencing these binaries directly we can add symlinks in bin for backwards compatibility, but that's unlikely and easy to address.

This unfortunately affects a lot of packages so I haven't ran nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD" to completion, but since this only affects credential helpers it's unlikely to impact other packages.

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/2.46.0#Documentation/git.txt---exec-pathltpathgt

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

  3. 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 5, 2024 02:17
@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 5, 2024
@joshheinrichs-shopify
Copy link
Contributor Author

I first encountered this just running git fetch with a profile that wasn't on my PATH, but I think this would also affect things like git maintenance which sets up a job in launchd, and likely won't have PATH configured by default. This would likely affect maintenance tasks running via systemd too if folks were relying on git-credential-netrc.

If git-credential-osxkeychain isn't on your PATH, git will fail to find
it. Moving it to git's exec path[1] allows it to be located more
reliably. In other distributions such as Fedora[2] and Homebrew[3],
credential helpers are not added to the user's PATH unless they are
supplied by another package (e.g. git-credential-gcloud).

If there are concerns that people may be relying on referencing these
binaries directly we can add symlinks in bin for backwards
compatibility, but that's unlikely and easy to address.

[1]: https://git-scm.com/docs/git/2.46.0#Documentation/git.txt---exec-pathltpathgt
[2]: https://src.fedoraproject.org/rpms/git/blob/rawhide/f/git.spec#_651
[3]: https://github.com/Homebrew/homebrew-core/blob/12120ff79ce009489b958d8271af686d74403859/Formula/g/git.rb#L112
@K900
Copy link
Contributor

K900 commented Oct 9, 2024

Mass ping, please resubmit.

@NixOS NixOS locked and limited conversation to collaborators Oct 9, 2024
@K900 K900 closed this Oct 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

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.

2 participants