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

fix(shim): Manipulating shims with UTF8 encoding #4791

Merged
merged 6 commits into from
Mar 10, 2022
Merged

fix(shim): Manipulating shims with UTF8 encoding #4791

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
022e79a
fix(shim): Manipulating shims with UTF8 encoding
chawyehsu Mar 9, 2022
0e06d2a
fix(tests): Tests fix
chawyehsu Mar 9, 2022
0a0b9c0
fix(tests): Tests fix
chawyehsu Mar 9, 2022
cbbc6be
Add NoNewLine option
chawyehsu Mar 9, 2022
7a996b9
Merge branch 'develop' into fix/unicode-fix
niheaven Mar 10, 2022
77f2f4c
Update CHANGELOG.md
chawyehsu Mar 10, 2022
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
43 changes: 24 additions & 19 deletions lib/core.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -479,8 +479,8 @@ function Invoke-ExternalCommand {
return $false
}
if ($LogPath -and ($FilePath -notmatch '(^|\W)msiexec($|\W)')) {
Out-File -FilePath $LogPath -Encoding Default -Append -InputObject $Process.StandardOutput.ReadToEnd()
Out-File -FilePath $LogPath -Encoding Default -Append -InputObject $Process.StandardError.ReadToEnd()
Out-UTF8File -FilePath $LogPath -Append -InputObject $Process.StandardOutput.ReadToEnd()
Out-UTF8File -FilePath $LogPath -Append -InputObject $Process.StandardError.ReadToEnd()
Copy link
Member Author

@chawyehsu chawyehsu Mar 10, 2022 •
edited
Loading

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.

}
$Process.WaitForExit()
if ($Process.ExitCode -ne 0) {
Expand Down Expand Up @@ -625,24 +625,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-File "$shim.shim" -Encoding ASCII
Write-Output "path = `"$resolved_path`"" | Out-UTF8File "$shim.shim"
if ($arg) {
Write-Output "args = $arg" | Out-File "$shim.shim" -Encoding ASCII -Append
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-File "$shim.cmd" -Encoding ASCII
) -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-File $shim -Encoding ASCII -NoNewline
) -join "`n" | Out-UTF8File $shim
} elseif ($path -match '\.ps1$') {
# if $path points to another drive resolve-path prepends .\ which could break shims
warn_on_overwrite "$shim.ps1" $path
Expand All @@ -661,7 +661,7 @@ function shim($path, $global, $name, $arg) {
"exit `$LASTEXITCODE"
)
}
$ps1text -join "`r`n" | Out-File "$shim.ps1" -Encoding ASCII
$ps1text -join "`r`n" | Out-UTF8File "$shim.ps1"

# make ps1 accessible from cmd.exe
warn_on_overwrite "$shim.cmd" $path
Expand All @@ -682,7 +682,7 @@ function shim($path, $global, $name, $arg) {
") else (",
" powershell -noprofile -ex unrestricted -file `"$resolved_path`" $arg %args%",
")"
) -join "`r`n" | Out-File "$shim.cmd" -Encoding ASCII
) -join "`r`n" | Out-UTF8File "$shim.cmd"
Copy link
Member Author

Choose a reason for hiding this comment

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

This breaks on CMD,
image

Copy link
Member

Choose a reason for hiding this comment

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

If the PATH contains special characters, the user might need to enable support for them, like this: lptstr/winfetch#132 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

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

That will not work on Windows 7.

Copy link
Member

Choose a reason for hiding this comment

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

Does this matter?

Copy link
Member

@rashil2000 rashil2000 Mar 10, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Does Windows 7 lack that setting or does it not work on 7 even after enabling that setting? Idk anything about 7.

Copy link
Member Author

Choose a reason for hiding this comment

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

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 chcp 65001 to work with UTF-8. https://en.wikipedia.org/wiki/Unicode_in_Microsoft_Windows

This is a well-known issue, and chcp 65001 has been talked about widely as a workaround to enable UTF-8 support for CMD/PowerShell(old versions), though this trick has bad user experiences.

Copy link
Member

@rashil2000 rashil2000 Mar 10, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I don't know how many Win 7 users are out there, but this feature will allow us to close #2573 and #2230 as well

Copy link
Member

Choose a reason for hiding this comment

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

This is Win7/CMD bug, not on PowerShell, right? So it's okay and ready to be merged IMO.

Copy link
Member Author

@chawyehsu chawyehsu Mar 10, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Does this matter?

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.

Copy link
Member

Choose a reason for hiding this comment

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

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
@(
Expand All @@ -693,33 +693,33 @@ function shim($path, $global, $name, $arg) {
"else",
" powershell.exe -noprofile -ex unrestricted -file `"$resolved_path`" $arg $@",
"fi"
) -join "`n" | Out-File $shim -Encoding ASCII -NoNewline
) -join "`n" | Out-UTF8File $shim
} elseif ($path -match '\.jar$') {
warn_on_overwrite "$shim.cmd" $path
@(
"@rem $resolved_path",
"@java -jar `"$resolved_path`" $arg %*"
) -join "`r`n" | Out-File "$shim.cmd" -Encoding ASCII
) -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-File $shim -Encoding ASCII -NoNewline
) -join "`n" | Out-UTF8File $shim
} elseif ($path -match '\.py$') {
warn_on_overwrite "$shim.cmd" $path
@(
"@rem $resolved_path",
"@python `"$resolved_path`" $arg %*"
) -join "`r`n" | Out-File "$shim.cmd" -Encoding ASCII
) -join "`r`n" | Out-UTF8File "$shim.cmd"

warn_on_overwrite $shim $path
@(
"#!/bin/sh",
"# $resolved_path",
"python.exe `"$resolved_path`" $arg `"$@`""
) -join "`n" | Out-File $shim -Encoding ASCII -NoNewline
) -join "`n" | Out-UTF8File $shim
} else {
warn_on_overwrite "$shim.cmd" $path
# find path to Git's bash so that batch scripts can run bash scripts
Expand All @@ -730,14 +730,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-File "$shim.cmd" -Encoding ASCII
) -join "`r`n" | Out-UTF8File "$shim.cmd"

warn_on_overwrite $shim $path
@(
"#!/bin/sh",
"# $resolved_path",
"`"$resolved_path`" $arg `"$@`""
) -join "`n" | Out-File $shim -Encoding ASCII -NoNewline
) -join "`n" | Out-UTF8File $shim
}
}

Expand Down Expand Up @@ -1101,14 +1101,19 @@ function Out-UTF8File {
[Parameter(Mandatory = $True, Position = 0)]
[Alias("Path")]
[String] $FilePath,
[Switch] $Append,
[Parameter(ValueFromPipeline = $True)]
[PSObject] $InputObject
)
process {
# 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)
if ($Append) {
[System.IO.File]::AppendAllText($FilePath, $InputObject)
} else {
# 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)
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion libexec/scoop-alias.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ function add_alias($name, $command) {
# Summary: $description
$command
"@
$script | Out-File "$shimdir\$alias_file.ps1" -Encoding ASCII
$script | Out-UTF8File "$shimdir\$alias_file.ps1"

# add alias to config
$aliases | Add-Member -MemberType NoteProperty -Name $name -Value $alias_file
Expand Down
10 changes: 5 additions & 5 deletions test/Scoop-Alias.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
reset_aliases

Describe 'add_alias' -Tag 'Scoop' {
Mock shimdir { 'TestDrive:\shim' }
Mock shimdir { "$env:TEMP\shims" }
Mock set_config { }
Mock get_config { @{} }

$shimdir = shimdir
mkdir $shimdir
New-Item -ItemType Directory -Path $shimdir -ErrorAction Ignore

Context "alias doesn't exist" {
It 'creates a new alias' {
Expand All @@ -23,7 +23,7 @@ Describe 'add_alias' -Tag 'Scoop' {
Context 'alias exists' {
It 'does not change existing alias' {
$alias_file = "$shimdir\scoop-rm.ps1"
New-Item $alias_file -type file
New-Item $alias_file -Type File -Force
$alias_file | Should -Exist

add_alias 'rm' 'test'
Expand All @@ -33,12 +33,12 @@ Describe 'add_alias' -Tag 'Scoop' {
}

Describe 'rm_alias' -Tag 'Scoop' {
Mock shimdir { 'TestDrive:\shim' }
Mock shimdir { "$env:TEMP\shims" }
Mock set_config { }
Mock get_config { @{} }

$shimdir = shimdir
mkdir $shimdir
New-Item -ItemType Directory -Path $shimdir -ErrorAction Ignore

Context 'alias exists' {
It 'removes an existing alias' {
Expand Down