fix(shim): Manipulating shims with UTF8 encoding #4791
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -482,8 +482,8 @@ function Invoke-ExternalCommand { | |
| return $false | ||
| } | ||
| if ($LogPath -and ($FilePath -notmatch '(^|\W)msiexec($|\W)')) { | ||
| Out-UTF8File -FilePath $LogPath -Append -InputObject $Process.StandardOutput.ReadToEnd() | ||
| Out-UTF8File -FilePath $LogPath -Append -InputObject $Process.StandardError.ReadToEnd() | ||
| } | ||
| $Process.WaitForExit() | ||
| if ($Process.ExitCode -ne 0) { | ||
|
|
@@ -628,24 +628,24 @@ function shim($path, $global, $name, $arg) { | |
| # for programs with no awareness of any shell | ||
| warn_on_overwrite "$shim.shim" $path | ||
| Copy-Item (get_shim_path) "$shim.exe" -Force | ||
| Write-Output "path = `"$resolved_path`"" | Out-UTF8File "$shim.shim" | ||
| if ($arg) { | ||
| Write-Output "args = $arg" | Out-UTF8File "$shim.shim" -Append | ||
| } | ||
| } elseif ($path -match '\.(bat|cmd)$') { | ||
| # shim .bat, .cmd so they can be used by programs with no awareness of PSH | ||
| warn_on_overwrite "$shim.cmd" $path | ||
| @( | ||
| "@rem $resolved_path", | ||
| "@`"$resolved_path`" $arg %*" | ||
| ) -join "`r`n" | Out-UTF8File "$shim.cmd" | ||
|
|
||
| warn_on_overwrite $shim $path | ||
| @( | ||
| "#!/bin/sh", | ||
| "# $resolved_path", | ||
| "MSYS2_ARG_CONV_EXCL=/C cmd.exe /C `"$resolved_path`" $arg `"$@`"" | ||
| ) -join "`n" | Out-UTF8File $shim -NoNewLine | ||
| } elseif ($path -match '\.ps1$') { | ||
| # if $path points to another drive resolve-path prepends .\ which could break shims | ||
| warn_on_overwrite "$shim.ps1" $path | ||
|
|
@@ -664,7 +664,7 @@ function shim($path, $global, $name, $arg) { | |
| "exit `$LASTEXITCODE" | ||
| ) | ||
| } | ||
| $ps1text -join "`r`n" | Out-UTF8File "$shim.ps1" | ||
|
|
||
| # make ps1 accessible from cmd.exe | ||
| warn_on_overwrite "$shim.cmd" $path | ||
|
|
@@ -685,7 +685,7 @@ function shim($path, $global, $name, $arg) { | |
| ") else (", | ||
| " powershell -noprofile -ex unrestricted -file `"$resolved_path`" $arg %args%", | ||
| ")" | ||
| ) -join "`r`n" | Out-UTF8File "$shim.cmd" | ||
|
There was a problem hiding this comment. If the PATH contains special characters, the user might need to enable support for them, like this: lptstr/winfetch#132 (comment) There was a problem hiding this comment. That will not work on Windows 7. There was a problem hiding this comment. Does Windows 7 lack that setting or does it not work on 7 even after enabling that setting? Idk anything about 7. There was a problem hiding this comment. This setting was introduced on Windows 10, so only Windows 10 (build 17134 or above explicitly) can use this setting to turn on the UTF-8 as ASCII feature. Otherwise, users need to use trick like This is a well-known issue, and There was a problem hiding this comment. This is Win7/CMD bug, not on PowerShell, right? So it's okay and ready to be merged IMO. There was a problem hiding this comment.
It depends. I'm here just saying it's a breaking change, we have no idea of how many Windows 7 users in our community, I tend to push it forword though. The PR is ready ofc. There was a problem hiding this comment. There are certainly some Win 7 users, I mean is it only causes bug on CMD and run well on PowerShell? If so, then it's okay. |
||
|
|
||
| warn_on_overwrite $shim $path | ||
| @( | ||
|
|
@@ -696,33 +696,33 @@ function shim($path, $global, $name, $arg) { | |
| "else", | ||
| " powershell.exe -noprofile -ex unrestricted -file `"$resolved_path`" $arg $@", | ||
| "fi" | ||
| ) -join "`n" | Out-UTF8File $shim -NoNewLine | ||
| } elseif ($path -match '\.jar$') { | ||
| warn_on_overwrite "$shim.cmd" $path | ||
| @( | ||
| "@rem $resolved_path", | ||
| "@java -jar `"$resolved_path`" $arg %*" | ||
| ) -join "`r`n" | Out-UTF8File "$shim.cmd" | ||
|
|
||
| warn_on_overwrite $shim $path | ||
| @( | ||
| "#!/bin/sh", | ||
| "# $resolved_path", | ||
| "java.exe -jar `"$resolved_path`" $arg `"$@`"" | ||
| ) -join "`n" | Out-UTF8File $shim -NoNewLine | ||
| } elseif ($path -match '\.py$') { | ||
| warn_on_overwrite "$shim.cmd" $path | ||
| @( | ||
| "@rem $resolved_path", | ||
| "@python `"$resolved_path`" $arg %*" | ||
| ) -join "`r`n" | Out-UTF8File "$shim.cmd" | ||
|
|
||
| warn_on_overwrite $shim $path | ||
| @( | ||
| "#!/bin/sh", | ||
| "# $resolved_path", | ||
| "python.exe `"$resolved_path`" $arg `"$@`"" | ||
| ) -join "`n" | Out-UTF8File $shim -NoNewLine | ||
| } else { | ||
| warn_on_overwrite "$shim.cmd" $path | ||
| # find path to Git's bash so that batch scripts can run bash scripts | ||
|
|
@@ -733,14 +733,14 @@ function shim($path, $global, $name, $arg) { | |
| @( | ||
| "@rem $resolved_path", | ||
| "@`"$(Join-Path (Join-Path $gitdir.FullName 'bin') 'bash.exe')`" `"$resolved_path`" $arg %*" | ||
| ) -join "`r`n" | Out-UTF8File "$shim.cmd" | ||
|
|
||
| warn_on_overwrite $shim $path | ||
| @( | ||
| "#!/bin/sh", | ||
| "# $resolved_path", | ||
| "`"$resolved_path`" $arg `"$@`"" | ||
| ) -join "`n" | Out-UTF8File $shim -NoNewLine | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -1104,14 +1104,25 @@ function Out-UTF8File { | |
| [Parameter(Mandatory = $True, Position = 0)] | ||
| [Alias("Path")] | ||
| [String] $FilePath, | ||
| [Switch] $Append, | ||
| [Switch] $NoNewLine, | ||
| [Parameter(ValueFromPipeline = $True)] | ||
| [PSObject] $InputObject | ||
| ) | ||
| process { | ||
| if ($Append) { | ||
| [System.IO.File]::AppendAllText($FilePath, $InputObject) | ||
| } else { | ||
| if (!$NoNewLine) { | ||
| # Ref: https://stackoverflow.com/questions/5596982 | ||
| # Performance Note: `WriteAllLines` throttles memory usage while | ||
| # `WriteAllText` needs to keep the complete string in memory. | ||
| [System.IO.File]::WriteAllLines($FilePath, $InputObject) | ||
| } else { | ||
| # However `WriteAllText` does not add ending newline. | ||
| [System.IO.File]::WriteAllText($FilePath, $InputObject) | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
It seems that tarball decompression function breaks after this change on decompression logging. The decompression function failed to get the nested tarball filename from the log file. And I found the relevant commits are from #4582
cd6d31d#diff-8408a53252c7afe378fec3971ffb95781f58079a5e50bf901bcd7e5fd632093eR113
@niheaven Reason of getting the nested tarball filename from log file, instead of using a trimmed one by the outer archive filename? According to https://regex101.com/r/dlvXi2/1, the regex replacement does not work as expected.