(MODULES-7580) Fix PowerShell task for puppet version on 32bit #309
Conversation
glennsarti
force-pushed
the
modules-7580-powershell
branch
from
c6708e4 to
65cf3f5
Compare
|
Tested manually on Windows 10 32bit, 64bit and 2008R2 64bit. |
|
CLA signed by all contributors. |
tasks/version_powershell.ps1
Outdated
| if ($loc -ne $null) { | ||
| Write-Output "{`"version`":`"$(type $loc)`",`"source`":`"$($loc.replace('\', '/'))`"}" | ||
| if ($null -ne $loc) { | ||
| Write-Output "{`"version`":`"$(Get-Content -Path $loc -ErrorAction SilentlyContinue)`",`"source`":`"$($loc.replace('\', '/'))`"}" |
There was a problem hiding this comment.
-ErrorAction SilentlyContinue will result in an empty version if reading the file fails for some reason (permissions?). I'm not sure that's what we want... if the file is present but we can't read it, we may want to fail the task.
There was a problem hiding this comment.
Fair enough. Can just read it first before write-Output
|
|
||
| $reg = Get-ItemProperty -Path $rootPath -ErrorAction SilentlyContinue | ||
| if ($null -ne $reg) { | ||
| if ($null -ne $reg.RememberedInstallDir64) { |
There was a problem hiding this comment.
I think we only want to set $loc if the path exists, can we also Test-Path? If the registry entry is there but the file isn't, I interpret that as the package is not installed (but registry cleanup didn't succeed for some reason).
There was a problem hiding this comment.
We could add that in Line 12
if ( ($null -ne $loc) -and (Test-Path -Path $loc) ) {There was a problem hiding this comment.
Are you going to follow up on these changes, or would you like me to take over the PR?
There was a problem hiding this comment.
Feel free to take over. Just busy with other tasks first.
donoghuc
force-pushed
the
modules-7580-powershell
branch
from
65cf3f5 to
1504c52
Compare
|
CLA signed by all contributors. |
|
Ready for a rebase. |
donoghuc
force-pushed
the
modules-7580-powershell
branch
from
1504c52 to
693f85a
Compare
Previously the powershell based version task was failing on 32bit windows operating systems due to passing in a null path to Test-Path. This commit modifies the detection to be far more tolerant of errors and instead detects if the registry entries are null instead of using Test-Path. This commit also cleans up the code as per PS Script Analyzer rules; * Remove use of aliases * Prefer `$null -eq ...` instead of `... -eq $null`
donoghuc
force-pushed
the
modules-7580-powershell
branch
from
693f85a to
984dadd
Compare
Previously the powershell based version task was failing on 32bit windows
operating systems due to passing in a null path to Test-Path. This commit
modifies the detection to be far more tolerant of errors and instead detects if
the registry entries are null instead of using Test-Path.
This commit also cleans up the code as per PS Script Analyzer rules;
$null -eq ...instead of... -eq $null