Export-Image.ps1 compatibility with PowerShell 5.1#31
Conversation
|
|
There was a problem hiding this comment.
@NeuroThom I understand the frustration having hit this myself early on. However, installing PS 7 is simple using winget and otherwise only GitHub actions need to worry with this, and they all have PS7 pre-installed.
Still happy to consider this, but would be important to make sure all edge cases @PathogenDavid put in place are covered, so we are not inadvertently regressing other issues. I tried to review as best I could below.
| $bootstrapperArgs += "$workflowFile" | ||
|
|
||
| if (!$IsWindows) { | ||
| $isWindowsPlatform = if ($null -eq (Get-Variable 'IsWindows' -ErrorAction SilentlyContinue)) { $true } else { $IsWindows } |
There was a problem hiding this comment.
@NeuroThom why was it necessary to redefine this part of the implementation? It wasn't clear from the PR description.
| $ErrorActionPreference = 'Stop' | ||
| $PSNativeCommandUseErrorActionPreference = $true | ||
|
|
||
| function Get-RelativePath([string]$basePath, [string]$targetPath) { |
There was a problem hiding this comment.
Resolve-Path has a built-in replacement for this which does work on PS 5.0+ so we likely don't need to define a new function.
Resolve-Path -Relative abspath|
|
||
| Write-Verbose "$($bootstrapperPath) $($bootstrapperArgs)" | ||
| &$bootstrapperPath $bootstrapperArgs | ||
| if ($LASTEXITCODE -ne 0) { |
There was a problem hiding this comment.
What is the difference in error output with PSNativeCommandUseErrorActionPreference versus this error check? Do you still get the error stack trace printed by the Bonsai bootstrapper?
A good way to check would be to get a workflow which fails to export and check the output of one version versus the other.
| foreach ($workflowFile in Get-ChildItem -File -Recurse (Join-Path $workflowPath "*.bonsai")) { | ||
| $svgPath = Join-Path $workflowFile.DirectoryName "$($workflowFile.BaseName).svg" | ||
| $svgPathRelative = [IO.Path]::GetRelativePath($documentationRoot, $svgPath) | ||
| $svgPathRelative = Get-RelativePath $documentationRoot $svgPath |
There was a problem hiding this comment.
Looks like PS 5+ does not have -RelativeBasePath and always works relative to the current directory so we would need to push and pop location:
| $svgPathRelative = Get-RelativePath $documentationRoot $svgPath | |
| Push-Location $documentationRoot | |
| $svgPathRelative = Resolve-Path $svgPath -Relative | |
| Pop-Location |
This could probably even be done outside the loop since we are probably fine running the script from the context of $documentationRoot.
3 participants
Fix Export-Image.ps1 compatibility with Windows PowerShell 5.1
The script previously required users to install PS7 separately, discussed in #30. It's possible that documenting the powershell 7+ requirement is preferable, but I felt I had to fix this for a ucl-open project, so thought to update it so it work out of the box with the PowerShell 5.1 that ships with Windows and VS Code.
Changes:
Fixes #30