Skip to content

Commit e4c9bf2

Browse files
marcelsafinCopilot
andcommitted
fix(scripts): harden cross-platform parity
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 716c221 commit e4c9bf2

5 files changed

Lines changed: 137 additions & 36 deletions

File tree

scripts/powershell/create-new-feature.ps1

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ $hasNumber = $PSBoundParameters.ContainsKey('Number') -and $Number -ne ''
147147

148148
# Warn if -Number and -Timestamp are both specified.
149149
if ($Timestamp -and $hasNumber) {
150-
Write-Warning "[specify] Warning: -Number is ignored when -Timestamp is used"
150+
[Console]::Error.WriteLine("[specify] Warning: -Number is ignored when -Timestamp is used")
151151
$Number = ''
152152
}
153153

@@ -239,8 +239,12 @@ if (-not $DryRun) {
239239
$env:SPECIFY_FEATURE = $branchName
240240
$env:SPECIFY_FEATURE_DIRECTORY = $featureDir
241241

242-
[Console]::Error.WriteLine("# To persist: export SPECIFY_FEATURE=$branchName")
243-
[Console]::Error.WriteLine("# export SPECIFY_FEATURE_DIRECTORY=$featureDir")
242+
$quotedBranchName = "'" + $branchName.Replace("'", "''") + "'"
243+
$quotedFeatureDir = "'" + $featureDir.Replace("'", "''") + "'"
244+
$featureAssignment = '$env:SPECIFY_FEATURE = ' + $quotedBranchName
245+
$directoryAssignment = '$env:SPECIFY_FEATURE_DIRECTORY = ' + $quotedFeatureDir
246+
[Console]::Error.WriteLine("# To persist: $featureAssignment")
247+
[Console]::Error.WriteLine("# $directoryAssignment")
244248
}
245249

246250
if ($Json) {
@@ -258,7 +262,7 @@ if ($Json) {
258262
Write-Output "SPEC_FILE: $specFile"
259263
Write-Output "FEATURE_NUM: $featureNum"
260264
if (-not $DryRun) {
261-
Write-Output "# To persist in your shell: export SPECIFY_FEATURE=$branchName"
262-
Write-Output "# export SPECIFY_FEATURE_DIRECTORY=$featureDir"
265+
Write-Output "# To persist in your shell: $featureAssignment"
266+
Write-Output "# $directoryAssignment"
263267
}
264268
}

scripts/python/create_new_feature.py

Lines changed: 38 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,32 @@ def _json_line(payload: object) -> str:
3535
_MAX_FEATURE_NUMBER = 2**63 - 1
3636

3737

38+
def _int64_from_digits(value: str) -> int | None:
39+
normalized = value.lstrip("0") or "0"
40+
maximum = str(_MAX_FEATURE_NUMBER)
41+
if len(normalized) > len(maximum) or (
42+
len(normalized) == len(maximum) and normalized > maximum
43+
):
44+
return None
45+
return int(normalized, 10)
46+
47+
48+
def _persistence_assignments(
49+
branch_name: str, feature_dir: str, *, powershell: bool
50+
) -> tuple[str, str]:
51+
if powershell:
52+
quoted_branch = "'" + branch_name.replace("'", "''") + "'"
53+
quoted_dir = "'" + feature_dir.replace("'", "''") + "'"
54+
return (
55+
f"$env:SPECIFY_FEATURE = {quoted_branch}",
56+
f"$env:SPECIFY_FEATURE_DIRECTORY = {quoted_dir}",
57+
)
58+
return (
59+
f"export SPECIFY_FEATURE={shlex.quote(branch_name)}",
60+
f"export SPECIFY_FEATURE_DIRECTORY={shlex.quote(feature_dir)}",
61+
)
62+
63+
3864
def _usage(argv0: str) -> str:
3965
return (
4066
f"Usage: {argv0} [--json] [--dry-run] [--allow-existing-branch] "
@@ -172,8 +198,8 @@ def _get_highest_from_specs(specs_dir: Path) -> int:
172198
if re.match(r"^[0-9]{3,}-", name) and not re.match(
173199
r"^[0-9]{8}-[0-9]{6}-", name
174200
):
175-
number = int(re.match(r"^[0-9]+", name).group(), 10)
176-
if number <= _MAX_FEATURE_NUMBER:
201+
number = _int64_from_digits(re.match(r"^[0-9]+", name).group())
202+
if number is not None:
177203
highest = max(highest, number)
178204
return highest
179205

@@ -214,19 +240,14 @@ def main(argv: list[str] | None = None) -> int:
214240
file=sys.stderr,
215241
)
216242
return 1
217-
normalized_number = branch_number.lstrip("0") or "0"
218-
max_feature_number = str(_MAX_FEATURE_NUMBER)
219-
if len(normalized_number) > len(max_feature_number) or (
220-
len(normalized_number) == len(max_feature_number)
221-
and normalized_number > max_feature_number
222-
):
243+
number = _int64_from_digits(branch_number)
244+
if number is None:
223245
print(
224246
"Error: --number must be between 0 and "
225247
f"{_MAX_FEATURE_NUMBER}, got '{branch_number}'",
226248
file=sys.stderr,
227249
)
228250
return 1
229-
number = int(normalized_number, 10)
230251
else:
231252
number = _get_highest_from_specs(specs_dir) + 1
232253
if number > _MAX_FEATURE_NUMBER:
@@ -303,15 +324,13 @@ def main(argv: list[str] | None = None) -> int:
303324
persist_feature_json(repo_root, str(feature_dir))
304325

305326
# Inform the user how to set feature state in their own shell.
306-
print(
307-
f"# To persist: export SPECIFY_FEATURE={shlex.quote(branch_name)}",
308-
file=sys.stderr,
309-
)
310-
print(
311-
"# export "
312-
f"SPECIFY_FEATURE_DIRECTORY={shlex.quote(str(feature_dir))}",
313-
file=sys.stderr,
327+
feature_assignment, directory_assignment = _persistence_assignments(
328+
branch_name,
329+
str(feature_dir),
330+
powershell=sys.platform == "win32",
314331
)
332+
print(f"# To persist: {feature_assignment}", file=sys.stderr)
333+
print(f"# {directory_assignment}", file=sys.stderr)
315334

316335
if args.json_mode:
317336
payload: dict[str, object] = {
@@ -327,14 +346,8 @@ def main(argv: list[str] | None = None) -> int:
327346
print(f"SPEC_FILE: {spec_file}")
328347
print(f"FEATURE_NUM: {feature_num}")
329348
if not args.dry_run:
330-
print(
331-
"# To persist in your shell: export "
332-
f"SPECIFY_FEATURE={shlex.quote(branch_name)}"
333-
)
334-
print(
335-
"# export "
336-
f"SPECIFY_FEATURE_DIRECTORY={shlex.quote(str(feature_dir))}"
337-
)
349+
print(f"# To persist in your shell: {feature_assignment}")
350+
print(f"# {directory_assignment}")
338351
return 0
339352

340353

src/specify_cli/integrations/base.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -679,13 +679,21 @@ def process_template(
679679
script_commands: dict[str, str] = {}
680680
script_pattern = re.compile(r"^\s*([A-Za-z0-9_-]+):\s*(.+)$")
681681
# Find the scripts: block
682+
in_frontmatter = False
682683
in_scripts = False
683684
for line in content.splitlines():
684-
if line.strip() == "scripts:":
685+
if line == "---":
686+
if in_frontmatter:
687+
break
688+
in_frontmatter = True
689+
continue
690+
if not in_frontmatter:
691+
continue
692+
if line == "scripts:":
685693
in_scripts = True
686694
continue
687695
if in_scripts and line and not line[0].isspace():
688-
in_scripts = False
696+
break
689697
if in_scripts:
690698
m = script_pattern.match(line)
691699
if m:

tests/integrations/test_base.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -479,6 +479,24 @@ def test_sh_does_not_prefix_interpreter(self):
479479
assert ".specify/scripts/bash/check-prerequisites.sh --json" in result
480480
assert "python" not in result
481481

482+
def test_body_scripts_example_does_not_override_frontmatter(self):
483+
content = (
484+
"---\n"
485+
"scripts:\n"
486+
" sh: scripts/bash/real.sh --json\n"
487+
"---\n"
488+
"Run {SCRIPT} now.\n"
489+
"```yaml\n"
490+
"scripts:\n"
491+
" sh: examples/not-the-command.sh\n"
492+
"```\n"
493+
)
494+
495+
result = IntegrationBase.process_template(content, "agent", "sh")
496+
497+
assert ".specify/scripts/bash/real.sh --json" in result
498+
assert "examples/not-the-command.sh" in result
499+
482500
def test_py_quotes_interpreter_with_spaces(self, monkeypatch):
483501
# An interpreter path containing whitespace (e.g. Windows
484502
# ``Program Files``) must be quoted so it isn't split into args.

tests/test_create_new_feature_python_parity.py

Lines changed: 62 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
import pytest
99

10+
from scripts.python import create_new_feature
1011
from scripts.python.common import persist_feature_json
1112
from tests.conftest import requires_bash
1213
from tests.parity_helpers import (
@@ -136,7 +137,8 @@ def test_all_variants_timestamp_mode_match_shape(repo: Path) -> None:
136137

137138

138139
@requires_bash
139-
def test_python_timestamp_number_warning_matches_bash(repo: Path) -> None:
140+
@pytest.mark.skipif(not HAS_POWERSHELL, reason="no PowerShell available")
141+
def test_all_variants_timestamp_number_warning_matches(repo: Path) -> None:
140142
args = (
141143
"--json",
142144
"--dry-run",
@@ -148,12 +150,31 @@ def test_python_timestamp_number_warning_matches_bash(repo: Path) -> None:
148150
"x",
149151
)
150152
bash = run(bash_cmd(repo, SCRIPT, *args), repo)
153+
ps = run(
154+
ps_cmd(
155+
repo,
156+
SCRIPT,
157+
"-Json",
158+
"-DryRun",
159+
"-Timestamp",
160+
"-Number",
161+
"5",
162+
"-ShortName",
163+
"ua",
164+
"x",
165+
),
166+
repo,
167+
)
151168
py = run(py_cmd(repo, SCRIPT, *args), repo)
152169

153-
assert py.returncode == bash.returncode == 0
170+
assert bash.returncode == ps.returncode == py.returncode == 0
171+
assert json_stdout(ps)
154172
assert (
155173
py.stderr
156174
== bash.stderr
175+
== ps.stderr.replace("-Number", "--number").replace(
176+
"-Timestamp", "--timestamp"
177+
)
157178
== "[specify] Warning: --number is ignored when --timestamp is used\n"
158179
)
159180

@@ -569,6 +590,26 @@ def test_all_variants_ignore_out_of_range_existing_prefix(repo: Path) -> None:
569590
assert json_stdout(py)["FEATURE_NUM"] == "001"
570591

571592

593+
def test_python_ignores_unconvertibly_large_existing_prefix() -> None:
594+
class Entry:
595+
name = f"{'9' * 5000}-existing"
596+
597+
@staticmethod
598+
def is_dir() -> bool:
599+
return True
600+
601+
class SpecsDir:
602+
@staticmethod
603+
def is_dir() -> bool:
604+
return True
605+
606+
@staticmethod
607+
def iterdir() -> list[Entry]:
608+
return [Entry()]
609+
610+
assert create_new_feature._get_highest_from_specs(SpecsDir()) == 0
611+
612+
572613
@requires_bash
573614
@pytest.mark.skipif(not HAS_POWERSHELL, reason="no PowerShell available")
574615
def test_all_variants_text_mode_match(repo: Path) -> None:
@@ -603,14 +644,31 @@ def test_all_variants_non_dry_text_mode_match(tmp_path: Path) -> None:
603644
assert bash.returncode == ps.returncode == py.returncode == 0
604645
assert (
605646
normalize_repo_paths(bash.stdout, bash_repo)
606-
== normalize_repo_paths(ps.stdout, ps_repo)
607647
== normalize_repo_paths(py.stdout, py_repo)
608648
)
609649
assert (
610650
normalize_repo_paths(bash.stderr, bash_repo)
611-
== normalize_repo_paths(ps.stderr, ps_repo)
612651
== normalize_repo_paths(py.stderr, py_repo)
613652
)
653+
ps_stdout = normalize_repo_paths(ps.stdout, ps_repo)
654+
ps_stderr = normalize_repo_paths(ps.stderr, ps_repo)
655+
assert "$env:SPECIFY_FEATURE = '007-x'" in ps_stdout
656+
assert (
657+
"$env:SPECIFY_FEATURE_DIRECTORY = '<REPO>/specs/007-x'" in ps_stdout
658+
)
659+
assert "$env:SPECIFY_FEATURE = '007-x'" in ps_stderr
660+
assert (
661+
"$env:SPECIFY_FEATURE_DIRECTORY = '<REPO>/specs/007-x'" in ps_stderr
662+
)
663+
664+
665+
def test_python_powershell_persistence_assignments_escape_quotes() -> None:
666+
assert create_new_feature._persistence_assignments(
667+
"007-x", r"C:\repo\O'Brien", powershell=True
668+
) == (
669+
"$env:SPECIFY_FEATURE = '007-x'",
670+
"$env:SPECIFY_FEATURE_DIRECTORY = 'C:\\repo\\O''Brien'",
671+
)
614672

615673

616674
@requires_bash

0 commit comments

Comments
 (0)