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

Fixes issues with openssh-win32 agent. #295

Closed
wants to merge 7 commits into from
Closed

Fixes issues with openssh-win32 agent. #295

Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
931d299
Fix issues with openssh-win32 in vague way
nitin88 Jun 29, 2016
3abada0
Small fix
nitin88 Jun 29, 2016
37ba80d
Update GitUtils.ps1
nitin88 Jun 29, 2016
edd12c0
Make posh-git comply with win32-openssh
nitin88 Jun 30, 2016
893ec41
Cleanup and polishing
nitin88 Jul 1, 2016
769c26f
Fixed Removing env param
Aug 12, 2016
e82cc14
Downmerged from master
Aug 22, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 67 additions & 24 deletions GitUtils.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,20 +222,20 @@ function Get-AliasPattern($exe) {
"($($aliases -join '|'))"
}

function setenv($key, $value) {
[void][Environment]::SetEnvironmentVariable($key, $value, [EnvironmentVariableTarget]::Process)
Set-TempEnv $key $value
function Set-Env($Key, $Value) {
Set-Item Env:$Key -Value $Value -ErrorAction SilentlyContinue
Set-TempEnv -Key $Key -Value $Value
}

function Get-TempEnv($key) {
$path = Join-Path ($Env:TEMP) ".ssh\$key.env"
if (Test-Path $path) {
$value = Get-Content $path
[void][Environment]::SetEnvironmentVariable($key, $value, [EnvironmentVariableTarget]::Process)
Set-Item Env:$key -Value $value -ErrorAction SilentlyContinue
Copy link
Owner

Choose a reason for hiding this comment

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

Why switch from [Environment]::SetEnvironmentVariable(...) to Set-Item? I don't remember why I used SetEnvironmentVariable() in the first place, but I'm curious what the difference is from a OpenSsh-Win32 perspective?

}
}

function Set-TempEnv($key, $value) {
function Set-TempEnv($Key, $Value) {
$path = Join-Path ($Env:TEMP) ".ssh\$key.env"
if ($value -eq $null) {
if (Test-Path $path) {
Expand All @@ -256,14 +256,13 @@ function Get-SshAgent() {
} else {
$agentPid = $Env:SSH_AGENT_PID
if ($agentPid) {
$sshAgentProcess = Get-Process | Where-Object { $_.Id -eq $agentPid -and $_.Name -eq 'ssh-agent' }
if ($null -ne $sshAgentProcess) {
$pid = Get-Process -Id $agentPid -ErrorAction SilentlyContinue
if ($pid) {
return $agentPid
} else {
setenv 'SSH_AGENT_PID', $null
setenv 'SSH_AUTH_SOCK', $null
}
}
Set-Env 'SSH_AGENT_PID', $null
Set-Env 'SSH_AUTH_SOCK', $null
}

return 0
Expand All @@ -282,15 +281,34 @@ function Find-Pageant() {

# Attempt to guess $program's location. For ssh-agent/ssh-add.
function Find-Ssh($program = 'ssh-agent') {
$sshLocation = Get-Command $program -TotalCount 1 -ErrorAction SilentlyContinue
if($sshLocation){
return $sshLocation
}

Write-Verbose "$program not in path. Trying to guess location."
$gitItem = Get-Command git -Erroraction SilentlyContinue | Get-Item
if ($gitItem -eq $null) { Write-Warning 'git not in path'; return }

$sshLocation = join-path $gitItem.directory.parent.fullname bin/$program
if (get-command $sshLocation -Erroraction SilentlyContinue) { return $sshLocation }
if (get-command $sshLocation -Erroraction SilentlyContinue) {
return $sshLocation
}

$sshLocation = join-path $gitItem.directory.parent.fullname usr/bin/$program
if (get-command $sshLocation -Erroraction SilentlyContinue) { return $sshLocation }
if (get-command $sshLocation -Erroraction SilentlyContinue) {
return $sshLocation
}
}

function Find-Service($service = 'ssh-agent') {
# Find if wind32-openssh available and use it
return Get-Service -Name $service -ErrorAction SilentlyContinue
Copy link
Collaborator

Choose a reason for hiding this comment

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

All references to Get-Service and Start-Service need to be behind a "Windows-only" firewall.

}

function Find-SericePID($service = 'ssh-agent') {
Copy link
Owner

Choose a reason for hiding this comment

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

Typo: Serice

# Find if wind32-openssh available and use it
return (get-wmiobject win32_service | where { $_.name -eq $service}).processID
Copy link
Collaborator

Choose a reason for hiding this comment

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

(Get-Service $Service | Get-Process).Id will be much faster than this.

}

# Loosely based on bash script from http://help.github.com/ssh-key-passphrases/
Expand All @@ -312,13 +330,19 @@ function Start-SshAgent([switch]$Quiet) {
if (!$pageant) { Write-Warning "Could not find Pageant."; return }
Start-Process -NoNewWindow $pageant
} else {
$sshAgent = Get-Command ssh-agent -TotalCount 1 -ErrorAction SilentlyContinue
$sshAgent = if ($sshAgent) {$sshAgent} else {Find-Ssh('ssh-agent')}
if (!$sshAgent) { Write-Warning 'Could not find ssh-agent'; return }
$sshService = Find-Service('ssh-agent')
if($sshService) {
Start-Service ssh-agent -ErrorAction SilentlyContinue
$pid = Find-SericePID('ssh-agent')
Copy link
Collaborator

Choose a reason for hiding this comment

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

You've already got the service as $sshService so could avoid calling Find-ServicePID with ($sshService | Get-Process).Id

Set-Env -Key 'SSH_AGENT_PID' -Value $pid
} else {
$sshAgent = Find-Ssh('ssh-agent')
if (!$sshAgent) { Write-Warning 'Could not find ssh-agent'; return }

& $sshAgent | foreach {
if($_ -match '(?<key>[^=]+)=(?<value>[^;]+);') {
setenv $Matches['key'] $Matches['value']
& $sshAgent | foreach {
if($_ -match '(?<key>[^=]+)=(?<value>[^;]+);') {
Set-Env -Key $Matches['key'] -Value $Matches['value']
}
}
}
}
Expand All @@ -339,7 +363,6 @@ function Add-SshKey() {
if (!$pageant) { Write-Warning 'Could not find Pageant'; return }

if ($args.Count -eq 0) {
$keystring = ""
$keyPath = Join-Path $Env:HOME ".ssh"
$keys = Get-ChildItem $keyPath/"*.ppk" -ErrorAction SilentlyContinue | Select -ExpandProperty FullName
& $pageant $keys
Expand All @@ -349,10 +372,21 @@ function Add-SshKey() {
}
}
} else {
$sshAdd = Get-Command ssh-add -TotalCount 1 -ErrorAction SilentlyContinue
$sshAdd = if ($sshAdd) {$sshAdd} else {Find-Ssh('ssh-add')}
$agentPid = (Get-Item Env:SSH_AGENT_PID -ErrorAction SilentlyContinue).Value
$sshAdd = '';
if($agentPid){
$agentPath = (Get-Process -Id $agentPid -FileVersionInfo -ErrorAction SilentlyContinue).Filename
if($agentPath){
$sshPath = Split-Path ($agentPath) -Parent
}
if($sshPath){
$sshAdd = Get-Command (Join-Path $sshPath 'ssh-add')
}
}
if(!$sshAdd){
$sshAdd = Find-Ssh('ssh-add')
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This feels overcomplicated- if we've got a running ssh-agent, Get-Command ssh-add or Find-Ssh('ssh-add') should return the matching ssh-add so just calling ssh-add should work. If not, maybe the logic to find the correct binary for the agent should be in Find-Ssh?

Copy link
Author

Choose a reason for hiding this comment

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

If you look carefully, the new logic always gets the ssh-add correctly [with openssh or without]. Just to not to break anyone in the worst case scenario, kept the old logic, we should cleanup in next release or so if nothing is breaking 👍

if (!$sshAdd) { Write-Warning 'Could not find ssh-add'; return }

if ($args.Count -eq 0) {
& $sshAdd
} else {
Expand All @@ -363,6 +397,15 @@ function Add-SshKey() {
}
}

function Get-SshAdd()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Doesn't appear to be used?

{
$sshPath = Split-Path -Parent $Global:GitSshSettings.DefaultSshPath -ErrorAction SilentlyContinue
$sshAdd = Get-Command $sshPath/ssh-add -TotalCount 1 -ErrorAction SilentlyContinue
if($sshAdd){
return $sshAdd;
}
}

# Stop a running SSH agent
function Stop-SshAgent() {
[int]$agentPid = Get-SshAgent
Expand All @@ -373,8 +416,8 @@ function Stop-SshAgent() {
Stop-Process $agentPid
}

setenv 'SSH_AGENT_PID', $null
setenv 'SSH_AUTH_SOCK', $null
Set-Env -Key 'SSH_AGENT_PID' -Value $null
Set-Env -Key 'SSH_AUTH_SOCK' -Value $null
}
}

Expand Down