-
Notifications
You must be signed in to change notification settings - Fork 402
Conversation
@ungb Would you mind giving this PR a spin on Windows and Linux? On each OS we should test that a commit can successfully be signed with the default install of GPG and:
|
I'm guessing that I broke our support for gpg 2.0.x when I fixed it for 2.1.x 😅 Silent failures are no good, though. gpg1 is unlikely to work either now that I think about it. I suspect what we're going to need to make this really robust is something that lets us automatically exercise the scripts against a variety of gpg versions. I'll see what I can whip up. |
@ajsb85 are you testing this branch? Also, which version of GPG is git configured to use? |
Sorry for getting to you late, I have both gpg 1 and gpg 2 setup for windows for repro. If you need help getting this working, This tutorial was amazingly helpful. https://jamesmckay.net/2016/02/signing-git-commits-with-gpg-on-windows/ Here's the results, doesn't seem like it works :/ I think you need to quote the string since on windows spaces are weird... See stack trace below. Also this is from master, I haven't tested this pr. GPG Version: 1.1.4 Result
GPG Version: 2.0.3 Result
|
Ah, nuts. Thanks for the data, @ungb... I'll likely revisit this next week. |
dd1cb79
to
665427d
Compare
This is taking me a while, and is likely to take me a bit longer. I've updated the PR description with a checklist as a status update. |
The good news is that gpg4win ships with a graphical pinentry application that works fine:
🎉 |
... And, on further inspection, gpg4win doesn't even ship with the curses pinentry. I'm just going to punt on Atom pinentry support on Windows. |
@ungb I found a solution for this error:
Open up On line 22 of the file, change this:
To this:
Notice the quotation marks. This tells the system that there are spaces in the path. Hope this helps! |
@SirFaizdat D'oh! Good catch. I've opened a PR at #966 since I've backburnered this larger gpg overhaul for a little while. |
A heads up because of the mentioning gpg2 here. https://www.gpg4win.org/ has now released v3.0.0 @FaizaanD yep just tripped on that bug myself., thx. Though it actually is already done in that file, so eh, +1 to that bug. |
Ah, thanks for the heads-up. From the changelog it looks like gpg4win 3.0.0 bundles gpg 2.2.1. I'll make sure I include that in the test matrix. |
Youre welcome, though I see after so many months of testing so many bugs were left behind :/ |
@smashwilson hows this coming along? When you have any win 64 builds to test give me a shout if you need a tester. |
@the-j0k3r Well, I'm back from (a) getting atom/watcher shipped and (b) paternity leave, and this is high on our list so I should be getting back to it pretty soon. I've been rethinking this a bit, too, and:
|
I'm closing this is favor of an issue that I just wrote up that captures my current thinking on this over in #1373. There's a bunch of work here that I'll likely be able to rescue in some other form, so I'll leave the branch around. |
I don't get the native GPG graphical Pinentry when i try to commit from atom. I've ran |
Providing passwords to gpg non-interactively via
--passphrase-fd
is fragile and somewhat counter to gpg's architecture. As I understand it, gpg2 expects secret key material to only be handled by thegpg-agent
process:This only works if the agent was configured with
--allow-loopback-pinentry
when it was started and, in my version of gpg at least, if--pinentry-mode loopback
is provided on the gpg command line, which has the side-effect of preventing user-configured pinentry programs from being attempted at all. At least it looks like aren't the only ones to run into this 😉This pull request replaces the
--passphrase-fd
approach with agpg-wrapper
helper that:--homedir
and specifyinggpg-pinentry.sh
as a pinentry program.--homedir
so that it will connect to our isolated agent.gpg-pinentry.sh
adapts the pinentry Assuan protocol togit-askpass-atom.sh
, which connects to the GitPromptServer socket and requests a password just like it does for SSH connections.Compatibility
The trickiest part here is ensuring that we have reasonably compatibility across both gpg versions and platforms. gpg uses a "home directory" to isolate its keychain, configuration, and so on, and it really doesn't like it if you share a home directory among different gpg versions.
To help with this a bit, I've created a test matrix repository that automates the testing of our driver scripts across gpg versions on macOS and Linux with isolated build prefixes and home directories. Calling gpg and calling our scripts are a bit tricky so that's a work in progress as well. To keep the combinatorial explosion in check somewhat I've limited it to the latest patch release of each minor version.
Here are the platforms I'll be testing on personally, and the current status of each on this branch:
Key:
Remaining Work
gpg-wrapper.sh
andgpg-pinentry.sh
to.js
scripts executed viash
shell that we can use, but it turns out that just about anything that wasn't a shell builtin (like, say,mkdir
) was being picked up from my personal git installation and not playing nicely with the Dugite-bundled shell operations at all.gpg-wrapper.sh
.gpg-pinentry.sh
.--passphrase-fd
method. I'd like to avoid an explicit version switch if possible. Right now I'm thinking that I can fall back to--passphrase-fd
if nogpg-agent
binary is found.useGitPromptServer
calls.gpg-pinentry.js
correctly on Windows.github.gitDiagnostics
are enabled. Debugging gpg operations is a royal pain becausegit
eats anything the gpg binary writes to stdout or stderr and only reports success or failure. I'm going to shunt script output to a file in the GitPromptServer temporary directory and dump it to the terminal output if present after a git operation.Fixes #814.