-
Notifications
You must be signed in to change notification settings - Fork 402
Exec GPG wrapper script from temp directory #1110
Conversation
@@ -25,25 +26,41 @@ export default class GitPromptServer { | |||
// TODO: [mkt] Windows?? yes. | |||
this.promptForInput = promptForInput; | |||
const windows = process.platform === 'win32'; | |||
this.tmpFolderPath = await getTempDir('github-'); | |||
this.tmpFolderPath = await getTempDir({ | |||
dir: windows ? os.tmpdir() : '/tmp', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this hard-coded outside of Windows instead of just using os.tmpdir? Seems it would ignore the environment https://github.com/nodejs/node-v0.x-archive/blob/master/lib/os.js#L47
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, yeah, there's a long story here. The short version is that we create a Unix domain socket as a subdirectory of this, and on macOS and Linux, domain socket paths must be less than 104 or 108 bytes long. On macOS, os.tmpdir()
returns a path beneath /private/
that eats up enough of the available space that (sometimes!) the domain socket creation will fail. So, we use os.tmpdir()
on Windows and hard-code it to the nice, short /tmp
on other OSes.
Note that this PR didn't actually introduce this, it just lost and restored the call by accident while I was doing git checkout -p
from my other branch. The windows
ternary is there on master:
github/lib/git-prompt-server.js
Lines 29 to 33 in 7835bc4
this.tmpFolderPath = await getTempDir({ | |
dir: windows ? os.tmpdir() : '/tmp', | |
prefix: 'github-', | |
symlinkOk: true, | |
}); |
Also, this is absolutely a hack, and I'd love to find a better way to ensure that the socket can be created successfully.
/cc @BinaryMuse who explained this to me the first time I saw this code 😉, in case she has anything to add
Copy and execute the GPG wrapper script from the git-operation temporary directory rather than directly from the package source, so that it will work correctly when bundled in the Atom
.asar
file. This is mostly extracted from #846 so I'll have some nice, satisfying merge conflicts once I get back to that PR.Fixes #1109.