mirror of
https://github.com/github/spec-kit.git
synced 2026-07-03 12:28:06 +08:00
fix: suppress CRLF warnings in auto-commit.ps1 (#2258)
* fix: suppress CRLF warnings in auto-commit.ps1 (#2253) Replace 2> with 2>&1 redirection and assignment to properly suppress stderr output including CRLF warnings on Windows. Exit code logic preserved for change detection. Fixes #2253 * fix: use SilentlyContinue for CRLF stderr handling, add tests The 2>&1 approach still raises terminating errors under $ErrorActionPreference='Stop'. Instead, temporarily set SilentlyContinue around all native git calls that may emit CRLF warnings to stderr (rev-parse, diff, ls-files, add, commit). Adds 5 pytest tests (TestAutoCommitPowerShellCRLF) that set core.autocrlf=true with LF-ending files. On Windows runners this triggers actual CRLF warnings; on other platforms the tests pass trivially. Fixes #2253 * refactor: address Copilot review feedback - Use 'Continue' instead of 'SilentlyContinue' so error output is still captured in $out for diagnostics on real git failures. - Wrap all three EAP save/restore blocks in try/finally to guarantee restoration even on unexpected exceptions. - Fix CRLF test to commit a tracked LF file first, then modify it, so git diff --quiet HEAD actually inspects the tracked change and triggers the CRLF warning on Windows. * test: assert CRLF warning fires on Windows On Windows, probe git diff stderr before running the script to verify the test setup actually produces the expected CRLF warning. This makes the regression test deterministic on the Windows runner. On non-Windows the probe is skipped (warnings don't fire there). --------- Co-authored-by: Manfred Riem <15701806+mnriem@users.noreply.github.com>
This commit is contained in:
saram ali
@@ -36,10 +36,17 @@ if (-not (Get-Command git -ErrorAction SilentlyContinue)) {
|
||||
exit 0
|
||||
}
|
||||
|
||||
# Temporarily relax ErrorActionPreference so git stderr warnings
|
||||
# (e.g. CRLF notices on Windows) do not become terminating errors.
|
||||
$savedEAP = $ErrorActionPreference
|
||||
$ErrorActionPreference = 'Continue'
|
||||
try {
|
||||
git rev-parse --is-inside-work-tree 2>$null | Out-Null
|
||||
if ($LASTEXITCODE -ne 0) { throw "not a repo" }
|
||||
} catch {
|
||||
$isRepo = $LASTEXITCODE -eq 0
|
||||
} finally {
|
||||
$ErrorActionPreference = $savedEAP
|
||||
}
|
||||
if (-not $isRepo) {
|
||||
Write-Warning "[specify] Warning: Not a Git repository; skipped auto-commit"
|
||||
exit 0
|
||||
}
|
||||
@@ -117,9 +124,16 @@ if (-not $enabled) {
|
||||
}
|
||||
|
||||
# Check if there are changes to commit
|
||||
$diffHead = git diff --quiet HEAD 2>$null; $d1 = $LASTEXITCODE
|
||||
$diffCached = git diff --cached --quiet 2>$null; $d2 = $LASTEXITCODE
|
||||
$untracked = git ls-files --others --exclude-standard 2>$null
|
||||
# Relax ErrorActionPreference so CRLF warnings on stderr do not terminate.
|
||||
$savedEAP = $ErrorActionPreference
|
||||
$ErrorActionPreference = 'Continue'
|
||||
try {
|
||||
git diff --quiet HEAD 2>$null; $d1 = $LASTEXITCODE
|
||||
git diff --cached --quiet 2>$null; $d2 = $LASTEXITCODE
|
||||
$untracked = git ls-files --others --exclude-standard 2>$null
|
||||
} finally {
|
||||
$ErrorActionPreference = $savedEAP
|
||||
}
|
||||
|
||||
if ($d1 -eq 0 -and $d2 -eq 0 -and -not $untracked) {
|
||||
Write-Host "[specify] No changes to commit after $EventName" -ForegroundColor DarkGray
|
||||
@@ -136,6 +150,10 @@ if (-not $commitMsg) {
|
||||
}
|
||||
|
||||
# Stage and commit
|
||||
# Relax ErrorActionPreference so CRLF warnings on stderr do not terminate,
|
||||
# while still allowing redirected error output to be captured for diagnostics.
|
||||
$savedEAP = $ErrorActionPreference
|
||||
$ErrorActionPreference = 'Continue'
|
||||
try {
|
||||
$out = git add . 2>&1 | Out-String
|
||||
if ($LASTEXITCODE -ne 0) { throw "git add failed: $out" }
|
||||
@@ -144,6 +162,8 @@ try {
|
||||
} catch {
|
||||
Write-Warning "[specify] Error: $_"
|
||||
exit 1
|
||||
} finally {
|
||||
$ErrorActionPreference = $savedEAP
|
||||
}
|
||||
|
||||
Write-Host "[OK] Changes committed $phase $commandName"
|
||||
|
||||
@@ -14,6 +14,7 @@ import os
|
||||
import re
|
||||
import shutil
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
@@ -585,6 +586,156 @@ class TestAutoCommitPowerShell:
|
||||
assert "\u2713" not in result.stdout, "Must not use Unicode checkmark"
|
||||
|
||||
|
||||
# ── auto-commit.ps1 CRLF warning tests (issue #2253) ────────────────────────
|
||||
|
||||
|
||||
@pytest.mark.skipif(not HAS_PWSH, reason="pwsh not available")
|
||||
class TestAutoCommitPowerShellCRLF:
|
||||
"""Tests for CRLF warning handling in auto-commit.ps1 (issue #2253).
|
||||
|
||||
On Windows, git emits CRLF warnings to stderr when core.autocrlf=true
|
||||
and files use LF line endings. PowerShell's $ErrorActionPreference='Stop'
|
||||
converts stderr output into terminating errors, crashing the script.
|
||||
|
||||
These tests use core.autocrlf=true + explicit LF-ending files. On Windows
|
||||
the CRLF warnings fire and exercise the fix; on other platforms the tests
|
||||
still run (they just won't produce stderr warnings, so they pass trivially).
|
||||
"""
|
||||
|
||||
# -- positive tests (fix works) ----------------------------------------
|
||||
|
||||
def test_commit_succeeds_with_autocrlf(self, tmp_path: Path):
|
||||
"""auto-commit.ps1 creates a commit when core.autocrlf=true (CRLF
|
||||
warnings on stderr must not crash the script)."""
|
||||
project = _setup_project(tmp_path)
|
||||
_write_config(project, (
|
||||
"auto_commit:\n"
|
||||
" default: false\n"
|
||||
" after_specify:\n"
|
||||
" enabled: true\n"
|
||||
' message: "crlf commit"\n'
|
||||
))
|
||||
# Create and commit a tracked LF-ending file first so the script's
|
||||
# `git diff --quiet HEAD` checks inspect a tracked modification.
|
||||
tracked = project / "crlf-test.txt"
|
||||
tracked.write_bytes(b"line one\nline two\nline three\n")
|
||||
subprocess.run(["git", "add", "crlf-test.txt"], cwd=project, check=True)
|
||||
subprocess.run(
|
||||
["git", "commit", "-m", "seed tracked file"],
|
||||
cwd=project, check=True, env={**os.environ, **_GIT_ENV},
|
||||
)
|
||||
subprocess.run(
|
||||
["git", "config", "core.autocrlf", "true"],
|
||||
cwd=project, check=True,
|
||||
)
|
||||
# Modify the tracked file with explicit LF endings to trigger the
|
||||
# CRLF warning during diff/status checks on Windows.
|
||||
tracked.write_bytes(b"line one\nline two changed\nline three\n")
|
||||
|
||||
# On Windows, verify the test setup actually produces a CRLF warning.
|
||||
if sys.platform == "win32":
|
||||
probe = subprocess.run(
|
||||
["git", "diff", "--quiet", "HEAD"],
|
||||
cwd=project, capture_output=True, text=True,
|
||||
)
|
||||
assert "LF will be replaced by CRLF" in probe.stderr, (
|
||||
"Expected CRLF warning from git on Windows; test setup may be wrong"
|
||||
)
|
||||
|
||||
result = _run_pwsh("auto-commit.ps1", project, "after_specify")
|
||||
|
||||
assert result.returncode == 0, (
|
||||
f"Script crashed (likely CRLF stderr); stderr:\n{result.stderr}"
|
||||
)
|
||||
assert "[OK] Changes committed" in result.stdout
|
||||
|
||||
log = subprocess.run(
|
||||
["git", "log", "--oneline", "-1"],
|
||||
cwd=project, capture_output=True, text=True,
|
||||
)
|
||||
assert "crlf commit" in log.stdout
|
||||
|
||||
def test_custom_message_not_corrupted_by_crlf(self, tmp_path: Path):
|
||||
"""Commit message is the configured value, not a CRLF warning."""
|
||||
project = _setup_project(tmp_path)
|
||||
_write_config(project, (
|
||||
"auto_commit:\n"
|
||||
" default: false\n"
|
||||
" after_plan:\n"
|
||||
" enabled: true\n"
|
||||
' message: "[Project] Plan done"\n'
|
||||
))
|
||||
subprocess.run(
|
||||
["git", "config", "core.autocrlf", "true"],
|
||||
cwd=project, check=True,
|
||||
)
|
||||
(project / "plan.txt").write_bytes(b"plan\ncontent\n")
|
||||
|
||||
result = _run_pwsh("auto-commit.ps1", project, "after_plan")
|
||||
assert result.returncode == 0
|
||||
|
||||
log = subprocess.run(
|
||||
["git", "log", "--format=%s", "-1"],
|
||||
cwd=project, capture_output=True, text=True,
|
||||
)
|
||||
assert "[Project] Plan done" in log.stdout.strip()
|
||||
|
||||
def test_no_changes_still_skips_with_autocrlf(self, tmp_path: Path):
|
||||
"""Script correctly detects 'no changes' even with core.autocrlf=true."""
|
||||
project = _setup_project(tmp_path)
|
||||
_write_config(project, (
|
||||
"auto_commit:\n"
|
||||
" default: false\n"
|
||||
" after_specify:\n"
|
||||
" enabled: true\n"
|
||||
))
|
||||
subprocess.run(
|
||||
["git", "config", "core.autocrlf", "true"],
|
||||
cwd=project, check=True,
|
||||
)
|
||||
# Stage and commit everything so the working tree is clean.
|
||||
subprocess.run(["git", "add", "."], cwd=project, check=True,
|
||||
env={**os.environ, **_GIT_ENV})
|
||||
subprocess.run(["git", "commit", "-m", "setup", "-q"], cwd=project,
|
||||
check=True, env={**os.environ, **_GIT_ENV})
|
||||
|
||||
result = _run_pwsh("auto-commit.ps1", project, "after_specify")
|
||||
assert result.returncode == 0
|
||||
assert "[OK]" not in result.stdout, "Should not have committed anything"
|
||||
|
||||
# -- negative tests (real errors still surface) ------------------------
|
||||
|
||||
def test_not_a_repo_still_detected_with_autocrlf(self, tmp_path: Path):
|
||||
"""Script still exits gracefully when not in a git repo, even though
|
||||
ErrorActionPreference is relaxed around the rev-parse call."""
|
||||
project = _setup_project(tmp_path, git=False)
|
||||
_write_config(project, "auto_commit:\n default: true\n")
|
||||
|
||||
result = _run_pwsh("auto-commit.ps1", project, "after_specify")
|
||||
assert result.returncode == 0
|
||||
combined = result.stdout + result.stderr
|
||||
assert "not a git repository" in combined.lower() or "warning" in combined.lower()
|
||||
|
||||
def test_missing_config_still_exits_cleanly_with_autocrlf(self, tmp_path: Path):
|
||||
"""Script exits 0 when git-config.yml is absent (no over-suppression)."""
|
||||
project = _setup_project(tmp_path)
|
||||
subprocess.run(
|
||||
["git", "config", "core.autocrlf", "true"],
|
||||
cwd=project, check=True,
|
||||
)
|
||||
config = project / ".specify" / "extensions" / "git" / "git-config.yml"
|
||||
config.unlink(missing_ok=True)
|
||||
|
||||
result = _run_pwsh("auto-commit.ps1", project, "after_specify")
|
||||
assert result.returncode == 0
|
||||
# Should not have committed anything — config file missing means disabled.
|
||||
log = subprocess.run(
|
||||
["git", "log", "--oneline"],
|
||||
cwd=project, capture_output=True, text=True,
|
||||
)
|
||||
assert log.stdout.strip().count("\n") == 0 # only the seed commit
|
||||
|
||||
|
||||
# ── git-common.sh Tests ──────────────────────────────────────────────────────
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user