mirror of
https://github.com/github/spec-kit.git
synced 2026-07-03 12:28:06 +08:00
fix: PS 5.1 compat — replace non-ASCII chars in shipped PowerShell scripts (#2709)
* Initial plan * fix: replace non-ASCII chars in PS1 files, add encoding regression tests, fix ANSI stripping in tests, update docs --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -35,7 +35,7 @@ Replace the script to add project-specific Git initialization steps:
|
|||||||
## Output
|
## Output
|
||||||
|
|
||||||
On success:
|
On success:
|
||||||
- `✓ Git repository initialized`
|
- `[OK] Git repository initialized`
|
||||||
|
|
||||||
## Graceful Degradation
|
## Graceful Degradation
|
||||||
|
|
||||||
|
|||||||
@@ -115,7 +115,7 @@ if (Test-Path $configFile) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
# No config file — auto-commit disabled by default
|
# No config file -- auto-commit disabled by default
|
||||||
exit 0
|
exit 0
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
#!/usr/bin/env pwsh
|
#!/usr/bin/env pwsh
|
||||||
# Git-specific common functions for the git extension.
|
# Git-specific common functions for the git extension.
|
||||||
# Extracted from scripts/powershell/common.ps1 — contains only git-specific
|
# Extracted from scripts/powershell/common.ps1 -- contains only git-specific
|
||||||
# branch validation and detection logic.
|
# branch validation and detection logic.
|
||||||
|
|
||||||
function Test-HasGit {
|
function Test-HasGit {
|
||||||
|
|||||||
@@ -1,7 +1,7 @@
|
|||||||
#!/usr/bin/env pwsh
|
#!/usr/bin/env pwsh
|
||||||
# Git extension: initialize-repo.ps1
|
# Git extension: initialize-repo.ps1
|
||||||
# Initialize a Git repository with an initial commit.
|
# Initialize a Git repository with an initial commit.
|
||||||
# Customizable — replace this script to add .gitignore templates,
|
# Customizable -- replace this script to add .gitignore templates,
|
||||||
# default branch config, git-flow, LFS, signing, etc.
|
# default branch config, git-flow, LFS, signing, etc.
|
||||||
$ErrorActionPreference = 'Stop'
|
$ErrorActionPreference = 'Stop'
|
||||||
|
|
||||||
@@ -66,4 +66,4 @@ try {
|
|||||||
exit 1
|
exit 1
|
||||||
}
|
}
|
||||||
|
|
||||||
Write-Host "✓ Git repository initialized"
|
Write-Host "[OK] Git repository initialized"
|
||||||
|
|||||||
@@ -336,10 +336,10 @@ function Get-FeaturePathsEnv {
|
|||||||
function Test-FileExists {
|
function Test-FileExists {
|
||||||
param([string]$Path, [string]$Description)
|
param([string]$Path, [string]$Description)
|
||||||
if (Test-Path -Path $Path -PathType Leaf) {
|
if (Test-Path -Path $Path -PathType Leaf) {
|
||||||
Write-Output " ✓ $Description"
|
Write-Output " [OK] $Description"
|
||||||
return $true
|
return $true
|
||||||
} else {
|
} else {
|
||||||
Write-Output " ✗ $Description"
|
Write-Output " [FAIL] $Description"
|
||||||
return $false
|
return $false
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -347,10 +347,10 @@ function Test-FileExists {
|
|||||||
function Test-DirHasFiles {
|
function Test-DirHasFiles {
|
||||||
param([string]$Path, [string]$Description)
|
param([string]$Path, [string]$Description)
|
||||||
if ((Test-Path -Path $Path -PathType Container) -and (Get-ChildItem -Path $Path -ErrorAction SilentlyContinue | Where-Object { -not $_.PSIsContainer } | Select-Object -First 1)) {
|
if ((Test-Path -Path $Path -PathType Container) -and (Get-ChildItem -Path $Path -ErrorAction SilentlyContinue | Where-Object { -not $_.PSIsContainer } | Select-Object -First 1)) {
|
||||||
Write-Output " ✓ $Description"
|
Write-Output " [OK] $Description"
|
||||||
return $true
|
return $true
|
||||||
} else {
|
} else {
|
||||||
Write-Output " ✗ $Description"
|
Write-Output " [FAIL] $Description"
|
||||||
return $false
|
return $false
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -591,7 +591,7 @@ except Exception:
|
|||||||
|
|
||||||
if ($layerPaths.Count -eq 0) { return $null }
|
if ($layerPaths.Count -eq 0) { return $null }
|
||||||
|
|
||||||
# If the top (highest-priority) layer is replace, it wins entirely —
|
# If the top (highest-priority) layer is replace, it wins entirely --
|
||||||
# lower layers are irrelevant regardless of their strategies.
|
# lower layers are irrelevant regardless of their strategies.
|
||||||
if ($layerStrategies[0] -eq 'replace') {
|
if ($layerStrategies[0] -eq 'replace') {
|
||||||
return (Get-Content $layerPaths[0] -Raw)
|
return (Get-Content $layerPaths[0] -Raw)
|
||||||
|
|||||||
@@ -312,7 +312,7 @@ if (-not $DryRun) {
|
|||||||
if ($AllowExistingBranch) {
|
if ($AllowExistingBranch) {
|
||||||
# If we're already on the branch, continue without another checkout.
|
# If we're already on the branch, continue without another checkout.
|
||||||
if ($currentBranch -eq $branchName) {
|
if ($currentBranch -eq $branchName) {
|
||||||
# Already on the target branch — nothing to do
|
# Already on the target branch -- nothing to do
|
||||||
} else {
|
} else {
|
||||||
# Otherwise switch to the existing branch instead of failing.
|
# Otherwise switch to the existing branch instead of failing.
|
||||||
$switchBranchError = git checkout -q $branchName 2>&1 | Out-String
|
$switchBranchError = git checkout -q $branchName 2>&1 | Out-String
|
||||||
|
|||||||
@@ -335,10 +335,11 @@ class TestInitIntegrationFlag:
|
|||||||
_install_shared_infra(project, "sh", force=False)
|
_install_shared_infra(project, "sh", force=False)
|
||||||
|
|
||||||
captured = capsys.readouterr()
|
captured = capsys.readouterr()
|
||||||
assert "already exist and were not updated" in captured.out
|
plain = strip_ansi(captured.out)
|
||||||
assert "specify init --here --force" in captured.out
|
assert "already exist and were not updated" in plain
|
||||||
|
assert "specify init --here --force" in plain
|
||||||
# Rich may wrap long lines; normalize whitespace for the second command
|
# Rich may wrap long lines; normalize whitespace for the second command
|
||||||
normalized = " ".join(captured.out.split())
|
normalized = " ".join(plain.split())
|
||||||
assert "specify integration upgrade --force" in normalized
|
assert "specify integration upgrade --force" in normalized
|
||||||
|
|
||||||
def test_shared_infra_warns_when_manifest_cannot_be_loaded(self, tmp_path, capsys):
|
def test_shared_infra_warns_when_manifest_cannot_be_loaded(self, tmp_path, capsys):
|
||||||
|
|||||||
@@ -7,6 +7,7 @@ import pytest
|
|||||||
from typer.testing import CliRunner
|
from typer.testing import CliRunner
|
||||||
|
|
||||||
from specify_cli import app
|
from specify_cli import app
|
||||||
|
from tests.conftest import strip_ansi
|
||||||
|
|
||||||
|
|
||||||
runner = CliRunner()
|
runner = CliRunner()
|
||||||
@@ -49,7 +50,8 @@ def _write_invalid_manifest(project, key):
|
|||||||
|
|
||||||
|
|
||||||
def _integration_list_row_cells(output: str, key: str) -> list[str]:
|
def _integration_list_row_cells(output: str, key: str) -> list[str]:
|
||||||
row = next(line for line in output.splitlines() if line.startswith(f"│ {key}"))
|
plain = strip_ansi(output)
|
||||||
|
row = next(line for line in plain.splitlines() if line.startswith(f"│ {key}"))
|
||||||
return [cell.strip() for cell in row.split("│")[1:-1]]
|
return [cell.strip() for cell in row.split("│")[1:-1]]
|
||||||
|
|
||||||
|
|
||||||
@@ -160,8 +162,9 @@ class TestIntegrationInstall:
|
|||||||
finally:
|
finally:
|
||||||
os.chdir(old_cwd)
|
os.chdir(old_cwd)
|
||||||
assert result.exit_code == 0
|
assert result.exit_code == 0
|
||||||
assert "already installed" in result.output
|
plain = strip_ansi(result.output)
|
||||||
normalized = " ".join(result.output.split())
|
assert "already installed" in plain
|
||||||
|
normalized = " ".join(plain.split())
|
||||||
assert "specify integration upgrade copilot" in normalized
|
assert "specify integration upgrade copilot" in normalized
|
||||||
assert "already the default integration" in normalized
|
assert "already the default integration" in normalized
|
||||||
assert "No files were changed" in normalized
|
assert "No files were changed" in normalized
|
||||||
@@ -197,9 +200,10 @@ class TestIntegrationInstall:
|
|||||||
finally:
|
finally:
|
||||||
os.chdir(old_cwd)
|
os.chdir(old_cwd)
|
||||||
assert result.exit_code != 0
|
assert result.exit_code != 0
|
||||||
assert "Installed integrations: copilot" in result.output
|
plain = strip_ansi(result.output)
|
||||||
assert "Default integration: copilot" in result.output
|
assert "Installed integrations: copilot" in plain
|
||||||
normalized = " ".join(result.output.split())
|
assert "Default integration: copilot" in plain
|
||||||
|
normalized = " ".join(plain.split())
|
||||||
assert "To replace the default integration" in normalized
|
assert "To replace the default integration" in normalized
|
||||||
assert "specify integration switch claude" in normalized
|
assert "specify integration switch claude" in normalized
|
||||||
assert "To install 'claude' alongside" in normalized
|
assert "To install 'claude' alongside" in normalized
|
||||||
@@ -309,9 +313,10 @@ class TestIntegrationInstall:
|
|||||||
finally:
|
finally:
|
||||||
os.chdir(old_cwd)
|
os.chdir(old_cwd)
|
||||||
assert result.exit_code != 0
|
assert result.exit_code != 0
|
||||||
assert "Installed integrations: copilot" in result.output
|
plain = strip_ansi(result.output)
|
||||||
assert "multi-install safe" in result.output
|
assert "Installed integrations: copilot" in plain
|
||||||
normalized = " ".join(result.output.split())
|
assert "multi-install safe" in plain
|
||||||
|
normalized = " ".join(plain.split())
|
||||||
assert "To replace the default integration" in normalized
|
assert "To replace the default integration" in normalized
|
||||||
assert "specify integration switch claude" in normalized
|
assert "specify integration switch claude" in normalized
|
||||||
assert "To install 'claude' alongside" in normalized
|
assert "To install 'claude' alongside" in normalized
|
||||||
|
|||||||
54
tests/test_ps1_encoding.py
Normal file
54
tests/test_ps1_encoding.py
Normal file
@@ -0,0 +1,54 @@
|
|||||||
|
"""Regression tests for PowerShell 5.1 compatibility (GitHub issue #2680).
|
||||||
|
|
||||||
|
PowerShell 5.1 (built-in on Windows) defaults to the system's legacy encoding
|
||||||
|
when reading .ps1 files. Non-ASCII characters in UTF-8-encoded scripts cause
|
||||||
|
parse errors because multi-byte sequences are misinterpreted as individual bytes.
|
||||||
|
|
||||||
|
These tests ensure that all shipped .ps1 files remain ASCII-only so they work
|
||||||
|
on both PowerShell 5.1 and 7+.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
REPO_ROOT = Path(__file__).resolve().parent.parent
|
||||||
|
|
||||||
|
# All directories that contain shipped PowerShell scripts.
|
||||||
|
_PS1_DIRS = [
|
||||||
|
REPO_ROOT / "scripts" / "powershell",
|
||||||
|
REPO_ROOT / "extensions" / "git" / "scripts" / "powershell",
|
||||||
|
]
|
||||||
|
|
||||||
|
|
||||||
|
def _collect_ps1_files():
|
||||||
|
"""Yield all .ps1 files under the known script directories."""
|
||||||
|
for d in _PS1_DIRS:
|
||||||
|
if d.is_dir():
|
||||||
|
yield from sorted(d.rglob("*.ps1"))
|
||||||
|
|
||||||
|
|
||||||
|
_PS1_FILES = list(_collect_ps1_files())
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("ps1_file", _PS1_FILES, ids=lambda p: str(p.relative_to(REPO_ROOT)))
|
||||||
|
def test_ps1_file_is_ascii_only(ps1_file: Path):
|
||||||
|
"""Every .ps1 file must contain only ASCII characters (PS 5.1 compat)."""
|
||||||
|
content = ps1_file.read_bytes()
|
||||||
|
non_ascii = [
|
||||||
|
(i + 1, byte)
|
||||||
|
for i, byte in enumerate(content)
|
||||||
|
if byte > 127
|
||||||
|
]
|
||||||
|
assert not non_ascii, (
|
||||||
|
f"{ps1_file.relative_to(REPO_ROOT)} contains non-ASCII bytes "
|
||||||
|
f"(PowerShell 5.1 incompatible): "
|
||||||
|
f"first at byte offset {non_ascii[0][0]} (0x{non_ascii[0][1]:02x})"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_ps1_files_discovered():
|
||||||
|
"""Sanity check: at least the known script files are found."""
|
||||||
|
names = {p.name for p in _PS1_FILES}
|
||||||
|
assert "common.ps1" in names
|
||||||
|
assert "initialize-repo.ps1" in names
|
||||||
Reference in New Issue
Block a user