Skip to content
This repository has been archived by the owner on Sep 23, 2021. It is now read-only.

Commit

Permalink
Run command idempotently when hostname cannot be resolved
Browse files Browse the repository at this point in the history
  • Loading branch information
jorangreef committed Sep 11, 2018
1 parent ad291f7 commit 8c1a302
Showing 1 changed file with 3 additions and 26 deletions.
29 changes: 3 additions & 26 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,33 +10,10 @@ var Node = {

function Attempt(instance, end) {
var platform = Node.process.platform;
if (platform === 'darwin') return Mac(instance, end);
if (platform === 'linux') return Linux(instance, end);
if (platform === 'win32') return Windows(instance, end);
// The -n (non-interactive) option prevents sudo from prompting the user for

This comment has been minimized.

Copy link
@bpasero

bpasero Dec 10, 2018

Contributor

@jorangreef can you explain what this change is for? I am evaluating if I should update the dependency in VSCode.

This comment has been minimized.

Copy link
@jorangreef

jorangreef Dec 10, 2018

Author Owner

This fixes a rare idempotency edge case where a command might have been run more than once, given a very specific OS environment setup.

The motivation for this is in the original PR: #76 (comment)

After discussion and various attempts, this seemed the most sure: #76 (comment)

There was no corresponding issue, but I should have referenced the original PR in my commit message.

This comment has been minimized.

Copy link
@jorangreef

jorangreef Dec 10, 2018

Author Owner

The code that was deleted here was only of benefit on Linux and macOS systems without TTY tickets enabled, and would have prevented prompting for a sudo session if one already existed.

However, most Linux and macOS (since Sierra) systems these days have TTY tickets enabled by default, so removing this code has no effect. On older systems, removing this code might mean that a UI prompt is shown where we could have avoided one.

This comment has been minimized.

Copy link
@bpasero

bpasero Dec 10, 2018

Contributor

@jorangreef got it, thank you

// a password. If a password is required, sudo will return an error and exit.
var command = [];
command.push('/usr/bin/sudo');
command.push('-n');
// Preserve user environment:
command.push('-E');
// Stop parsing command options:
command.push('--');
command.push(instance.command);
command = command.join(' ');
Node.child.exec(command, { maxBuffer: MAX_BUFFER },
function(error, stdout, stderr) {
if (/sudo: /i.test(stderr)) {
if (platform === 'linux') {
return Linux(instance, end);
} else if (platform === 'darwin') {
return Mac(instance, end);
} else {
end(new Error('Platform not yet supported.'));
}
} else {
end(error, stdout, stderr);
}
}
);
end(new Error('Platform not yet supported.'));
}

function EscapeDoubleQuotes(string) {
Expand Down

0 comments on commit 8c1a302

Please sign in to comment.