Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

git: optionally build gnome-keyring and libsecret credential helpers #43616

Merged
merged 1 commit into from Jul 22, 2018
Merged

git: optionally build gnome-keyring and libsecret credential helpers #43616

merged 1 commit into from Jul 22, 2018

Conversation

benley
Copy link
Member

@benley benley commented Jul 16, 2018

Some very minor cleanup of the build/install phase too.

Motivation for this change

Encrypted storage for git secret credentials.

There is no change to the default git derivation.

This adds ~200mb to the transitive closure of gitFull because of the new dependencies on libgnome-keyring and libsecret.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Fits CONTRIBUTING.md.

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: git

Partial log (click to expand)

All tests successful.
Files=802, Tests=18597, 199 wallclock secs (85.12 usr 17.62 sys + 2748.74 cusr 7085.20 csys = 9936.68 CPU)
Result: PASS
make clean-except-prove-cache
make[2]: Entering directory '/build/git-2.17.1/t'
rm -f -r 'trash directory'.* 'test-results'
rm -f -r valgrind/bin
make[2]: Leaving directory '/build/git-2.17.1/t'
make[1]: Leaving directory '/build/git-2.17.1/t'
/nix/store/q7mvbajifj0imyvmvk38ij53bq3lkc4i-git-2.17.1

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: git

Partial log (click to expand)

All tests successful.
Files=802, Tests=18597, 1700 wallclock secs (11.08 usr  2.23 sys + 670.09 cusr 855.41 csys = 1538.81 CPU)
Result: PASS
make clean-except-prove-cache
make[2]: Entering directory '/build/git-2.17.1/t'
rm -f -r 'trash directory'.* 'test-results'
rm -f -r valgrind/bin
make[2]: Leaving directory '/build/git-2.17.1/t'
make[1]: Leaving directory '/build/git-2.17.1/t'
/nix/store/yp4yarn6kwr4m79kvlk7f83ic2zrj3z3-git-2.17.1

jtojnar
jtojnar previously requested changes Jul 16, 2018
View reviewed changes
pkgs/applications/version-management/git-and-tools/git/default.nix Outdated
++ stdenv.lib.optionals stdenv.isDarwin [ darwin.Security ];
++ stdenv.lib.optionals stdenv.isDarwin [ darwin.Security ]
++ stdenv.lib.optionals withLibsecret [ pkgconfig glib libsecret ]
++ stdenv.lib.optionals withGnomeKeyring [ pkgconfig glib libgnome-keyring ];
Copy link
Contributor

Choose a reason for hiding this comment

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

libgnome-keyring is deprecated in favour of libsecret, we do not want to use it.

Copy link
Member Author

Choose a reason for hiding this comment

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

aha, good to know. I was having trouble figuring out which one of those was deprecated in favor of the other; I'll drop the gnome-keyring version.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

@benley benley force-pushed the benley/git-credential-helpers branch from c981f88 to f6e4103 Compare July 17, 2018 19:27
@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: gitFull

Partial log (click to expand)

All tests successful.
Files=816, Tests=19219, 218 wallclock secs ( 5.11 usr  0.95 sys + 287.14 cusr 277.36 csys = 570.56 CPU)
Result: PASS
make clean-except-prove-cache
make[2]: Entering directory '/build/git-2.18.0/t'
rm -f -r 'trash directory'.* 'test-results'
rm -f -r valgrind/bin
make[2]: Leaving directory '/build/git-2.18.0/t'
make[1]: Leaving directory '/build/git-2.18.0/t'
/nix/store/402f8v55m63n0qnxj58m4imbvc0z5xqs-git-2.18.0

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: gitFull

Partial log (click to expand)

All tests successful.
Files=816, Tests=19219, 277 wallclock secs (90.65 usr 17.67 sys + 3055.00 cusr 7830.16 csys = 10993.48 CPU)
Result: PASS
make clean-except-prove-cache
make[2]: Entering directory '/build/git-2.18.0/t'
rm -f -r 'trash directory'.* 'test-results'
rm -f -r valgrind/bin
make[2]: Leaving directory '/build/git-2.18.0/t'
make[1]: Leaving directory '/build/git-2.18.0/t'
/nix/store/bvj35zx9jmi9wwxcl3ckl4xb8kr9kpw1-git-2.18.0

@benley
Copy link
Member Author

benley commented Jul 20, 2018

@jtojnar take another look?

@jtojnar
Copy link
Contributor

jtojnar commented Jul 20, 2018 •
edited

When I install it using nix-env, I see the following:

warning: skipping dangling symlink '/nix/store/1kj7d2ch4kg3ym0yy5kz02ilnb2dn1pd-user-environment/bin/lgit-credential-libsecret'

@benley
gitFull: build libsecret credential helper
e161b00 
...except on Darwin, where it won't be useful.
@benley benley force-pushed the benley/git-credential-helpers branch from f6e4103 to e161b00 Compare July 20, 2018 15:51
@benley
Copy link
Member Author

benley commented Jul 20, 2018

When I install it using nix-env, I see the following:
warning: skipping dangling symlink '/nix/store/1kj7d2ch4kg3ym0yy5kz02ilnb2dn1pd-user-environment/bin/lgit-credential-libsecret'

Grah, that's because I made a dumb typo. Fixed, sorry.

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: gitFull

Partial log (click to expand)

All tests successful.
Files=816, Tests=19219, 277 wallclock secs (90.56 usr 17.60 sys + 3071.82 cusr 7853.25 csys = 11033.23 CPU)
Result: PASS
make clean-except-prove-cache
make[2]: Entering directory '/build/git-2.18.0/t'
rm -f -r 'trash directory'.* 'test-results'
rm -f -r valgrind/bin
make[2]: Leaving directory '/build/git-2.18.0/t'
make[1]: Leaving directory '/build/git-2.18.0/t'
/nix/store/0kjzqvh05ll90iq61gmxlc3prvny845c-git-2.18.0

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: gitFull

Partial log (click to expand)

All tests successful.
Files=816, Tests=19219, 1870 wallclock secs (11.41 usr  2.40 sys + 765.68 cusr 927.53 csys = 1707.02 CPU)
Result: PASS
make clean-except-prove-cache
make[2]: Entering directory '/build/git-2.18.0/t'
rm -f -r 'trash directory'.* 'test-results'
rm -f -r valgrind/bin
make[2]: Leaving directory '/build/git-2.18.0/t'
make[1]: Leaving directory '/build/git-2.18.0/t'
/nix/store/5is7rb9cki1yjcarribcjr0vbmql9k70-git-2.18.0

@benley
Copy link
Member Author

benley commented Jul 22, 2018

thanks for reviewing!

@benley benley merged commit 815ae70 into NixOS:master Jul 22, 2018
@benley benley deleted the benley/git-credential-helpers branch July 22, 2018 19:20
@jtojnar
Copy link
Contributor

jtojnar commented Jul 24, 2018 •
edited

Are you able to use it? I have the helper set

$ git config --system --get credential.helper
libsecret

but git send-email still uses $SSH_ASKPASS for prompt.

So does

echo "protocol=https
host=example.com
path=foo.git
" | git credential fill

and neither action seems to add the key to secret agent.

Edit: Hmm, it is only added to keyring after git credential approve. Not sure why the ugly $SSH_ASKPASS propmt it is used though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthewbauer matthewbauer matthewbauer approved these changes

@jtojnar jtojnar jtojnar left review comments

Assignees
No one assigned
Labels
10.rebuild-darwin: 501+ 10.rebuild-linux: 501+
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@benley @GrahamcOfBorg @jtojnar @matthewbauer