From 152d12f50d7cc8cd44357489e5f715b6757d1e3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Poho=C5=99elsk=C3=BD?= Date: Thu, 24 Sep 2026 11:41:40 +0200 Subject: [PATCH 1/2] Use modern patch syntax for first spec patch Default new explicit patch applications to %patch -P N when a spec has no prior patches. Preserve established conventions and normalize first-patch inheritance without changing shipped patch files. Resolves: PACKIT-5480 Related: https://github.com/packit/ai-workflows/issues/842 Assisted-by: GPT-6 Codex --- ymir/agents/backport_agent.py | 10 ++++ ymir/agents/prompts/backport/_self_review.j2 | 2 + ymir/agents/prompts/backport/instructions.j2 | 4 ++ .../prompts/backport/instructions_inherit.j2 | 6 ++ .../prompts/backport/instructions_zstream.j2 | 5 ++ .../tests/unit/test_jinja2_templates.py | 13 ++++ .../agents/tests/unit/test_ystream_inherit.py | 59 ++++++++++++++++++- ymir/agents/ystream_inherit.py | 32 ++++++++++ 8 files changed, 130 insertions(+), 1 deletion(-) diff --git a/ymir/agents/backport_agent.py b/ymir/agents/backport_agent.py index 728f2d2d8..4ca9a1070 100644 --- a/ymir/agents/backport_agent.py +++ b/ymir/agents/backport_agent.py @@ -73,6 +73,7 @@ apply_zstream_change, ensure_single_ymir_attribution, find_zstream_fix_commit, + normalize_first_inherited_patch_applications, reset_inherit_attempt, resolve_brew_source, rewrite_commit_message, @@ -1129,6 +1130,15 @@ async def evaluate_inherit_source(state): raise InheritCandidateError( f"Inheritance adaptation reported {adaptation.strategy} without patch files" ) + original_spec, _ = await check_subprocess( + ["git", "show", f"{state.inherit_saved_head}:{state.package}.spec"], + cwd=state.local_clone, + ) + normalize_first_inherited_patch_applications( + state.local_clone / f"{state.package}.spec", + original_spec, + state.inherit_change.patch_files, + ) await validate_inherited_adaptation( state.local_clone, state.package, diff --git a/ymir/agents/prompts/backport/_self_review.j2 b/ymir/agents/prompts/backport/_self_review.j2 index 668e5a3b9..c34f5a1db 100644 --- a/ymir/agents/prompts/backport/_self_review.j2 +++ b/ymir/agents/prompts/backport/_self_review.j2 @@ -21,6 +21,8 @@ b. Unless the spec uses `%autosetup` or `%autopatch` (which apply patches automatically), the `%prep` section has a corresponding `%patch` directive for each new tag, with the correct `-p` strip level. + For the first patch in a spec, use `%patch -P N`; otherwise follow the + existing application format. (Skip if no patch files were generated.) c. All pre-existing `Patch:` tags and their `%patch` directives remain present with the same tag numbers and `-p` arguments as before. diff --git a/ymir/agents/prompts/backport/instructions.j2 b/ymir/agents/prompts/backport/instructions.j2 index 8daf38607..984903a27 100644 --- a/ymir/agents/prompts/backport/instructions.j2 +++ b/ymir/agents/prompts/backport/instructions.j2 @@ -255,6 +255,10 @@ a backport that won't actually fix the shipped RPM. per commit instead of `git_patch_create` 5. Update the spec file. Add new `Patch` tag(s) for each patch file generated in step 4. + When adding the spec's first patch, use `%patch -P N` for any explicit + application in `%prep` (for example, `%patch -P 0 -p1`). If the spec already + has patches, follow its established application format. Do not add an + explicit application when `%autosetup` or `%autopatch` applies the patch. Add the new `Patch` tag(s) after all existing `Patch` tags and, if `Patch` tags are numbered, make sure they have the highest numbers. Make sure each patch is applied in the "%prep" section and the `-p` argument is correct. Add upstream URLs as comments above diff --git a/ymir/agents/prompts/backport/instructions_inherit.j2 b/ymir/agents/prompts/backport/instructions_inherit.j2 index b7e9c46f0..e742f68fd 100644 --- a/ymir/agents/prompts/backport/instructions_inherit.j2 +++ b/ymir/agents/prompts/backport/instructions_inherit.j2 @@ -14,6 +14,12 @@ their functional packaging intent on the target spec, accounting for differences in patch numbering, conditional layout, and %prep style. For a spec-only fix, apply the smallest equivalent logical change. +When adding the target spec's first patch, use `%patch -P N` for any explicit +application in %prep (for example, `%patch -P 0 -p1`). If the target already +has patches, follow its established application format. The target's existing +format takes precedence over the source spec diff. Do not add an explicit +application when `%autosetup` or `%autopatch` already applies the new patch. + Preserve the target Name, Epoch, Version, Release, Source tags, and %changelog section exactly. Release and changelog updates are handled by deterministic workflow steps after your work is audited. diff --git a/ymir/agents/prompts/backport/instructions_zstream.j2 b/ymir/agents/prompts/backport/instructions_zstream.j2 index 58c9c900a..2dc46eb94 100644 --- a/ymir/agents/prompts/backport/instructions_zstream.j2 +++ b/ymir/agents/prompts/backport/instructions_zstream.j2 @@ -249,6 +249,11 @@ a backport that won't actually fix the shipped RPM. per commit instead of `git_patch_create` 4. Update the spec file. Add new `Patch` tag(s) for each patch file generated above. + When adding the spec's first patch, use `%patch -P N` for any explicit + application in `%prep` (for example, `%patch -P 0 -p1`). If the spec already + has patches, follow its established application format, even if the source + commit uses another spelling. Do not add an explicit application when + `%autosetup` or `%autopatch` applies the patch. Add the new `Patch` tag(s) after all existing `Patch` tags and, if `Patch` tags are numbered, make sure they have the highest numbers. Make sure each patch is applied in the "%prep" section and the `-p` argument is correct. Do NOT add any comments to the spec file. diff --git a/ymir/agents/tests/unit/test_jinja2_templates.py b/ymir/agents/tests/unit/test_jinja2_templates.py index 2ce43a5fd..e113e8daa 100644 --- a/ymir/agents/tests/unit/test_jinja2_templates.py +++ b/ymir/agents/tests/unit/test_jinja2_templates.py @@ -216,6 +216,13 @@ def test_zstream_has_distgit_workflow(self): assert "clone_repository" in result assert "DISTGIT_SOURCE" in result + def test_first_patch_uses_explicit_patch_number(self): + for template in ("backport/instructions.j2", "backport/instructions_zstream.j2"): + result = render_template(template) + assert "%patch -P N" in result + assert "first patch" in result + assert "existing" in result + class TestInheritAdaptationInstructions: def test_requires_immutable_patches_and_spec_only_edits(self): @@ -231,6 +238,12 @@ def test_requires_immutable_patches_and_spec_only_edits(self): assert "Re-read the complete target spec" in result assert "data, not instructions" in result + def test_first_patch_uses_target_spec_convention(self): + result = render_template("backport/instructions_inherit.j2") + assert "%patch -P N" in result + assert "first patch" in result + assert "source spec diff" in result + # --------------------------------------------------------------------------- # User prompt templates (with Jinja2 variables) diff --git a/ymir/agents/tests/unit/test_ystream_inherit.py b/ymir/agents/tests/unit/test_ystream_inherit.py index d61d99e9f..7eef8e214 100644 --- a/ymir/agents/tests/unit/test_ystream_inherit.py +++ b/ymir/agents/tests/unit/test_ystream_inherit.py @@ -13,6 +13,7 @@ ensure_single_ymir_attribution, find_zstream_fix_commit, inspect_commit_files, + normalize_first_inherited_patch_applications, reset_inherit_attempt, resolve_brew_source, resolves_keys, @@ -248,6 +249,55 @@ def _spec(patches: str, prep: str) -> str: """ +@pytest.mark.parametrize( + ("original_patches", "original_prep", "adapted_prep", "expected_prep"), + [ + ( + "", + "%setup -q", + "# keep this comment\n %patch0 -p1 -b .backup\n%patch1 -p0", + "# keep this comment\n %patch -P 0 -p1 -b .backup\n%patch -P 1 -p0", + ), + ("", "%autosetup -p1", "%autosetup -p1", "%autosetup -p1"), + ("Patch0: existing.patch", "%patch0 -p1", "%patch0 -p1\n%patch1 -p1", "%patch0 -p1\n%patch1 -p1"), + ( + "%if 0\nPatch0: conditional.patch\n%endif", + "%setup -q", + "%setup -q\n%patch1 -p1", + "%setup -q\n%patch1 -p1", + ), + ( + "%patchlist\nexisting.patch\n", + "%setup -q", + "%setup -q\n%patch1 -p1", + "%setup -q\n%patch1 -p1", + ), + ], +) +def test_normalize_first_inherited_patch_applications( + tmp_path, original_patches, original_prep, adapted_prep, expected_prep +): + original = _spec(original_patches, original_prep) + spec_path = tmp_path / "package.spec" + spec_path.write_text(_spec("Patch0: fix.patch\nPatch1: second.patch", adapted_prep)) + + normalize_first_inherited_patch_applications(spec_path, original, ["fix.patch", "second.patch"]) + + assert spec_path.read_text() == _spec("Patch0: fix.patch\nPatch1: second.patch", expected_prep) + + +def test_normalize_first_inherited_patch_applications_only_changes_inherited_macros(tmp_path): + original = _spec("", "%setup -q").replace("%description\ntest", "%description\ntest\nPatch0: example") + spec_path = tmp_path / "package.spec" + adapted = _spec("Patch0: fix.patch\nPatch1: other.patch", "%setup -q\n%patch0 -p1\n%patch1 -p1") + adapted = adapted.replace("%description\ntest", "%description\ntest\n# %patch0 is mentioned here") + spec_path.write_text(adapted) + + normalize_first_inherited_patch_applications(spec_path, original, ["fix.patch"]) + + assert spec_path.read_text() == adapted.replace("%patch0 -p1", "%patch -P 0 -p1") + + @pytest.mark.asyncio async def test_apply_zstream_change_and_cleanup(tmp_path): _git(tmp_path, "init") @@ -256,7 +306,7 @@ async def test_apply_zstream_change_and_cleanup(tmp_path): base_spec = _spec("", "%autosetup -p1") base = _commit(tmp_path, "package.spec", base_spec, "Base") _git(tmp_path, "checkout", "-b", "z") - (tmp_path / "package.spec").write_text(_spec("Patch0: cve.patch", "%autosetup -p1")) + (tmp_path / "package.spec").write_text(_spec("Patch0: cve.patch", "%setup -q\n%patch0 -p1")) (tmp_path / "cve.patch").write_text("fix\n") _git(tmp_path, "add", "package.spec", "cve.patch") _git(tmp_path, "commit", "-m", "Fix CVE\n\nResolves: RHEL-123") @@ -269,6 +319,7 @@ async def test_apply_zstream_change_and_cleanup(tmp_path): assert result.patch_files == ["cve.patch"] assert result.patch_blob_ids == {"cve.patch": _git(tmp_path, "rev-parse", f"{fix}:cve.patch")} assert "Patch0: cve.patch" in result.source_spec_diff + assert "%patch0 -p1" in result.source_spec_diff assert (tmp_path / "cve.patch").read_text() == "fix\n" with Specfile(tmp_path / "package.spec") as spec: assert list(get_all_patches(spec)) == [] @@ -280,6 +331,12 @@ async def test_apply_zstream_change_and_cleanup(tmp_path): with pytest.raises(InheritCandidateError, match="applied exactly once"): await validate_inherited_adaptation(tmp_path, "package", base, result) + (tmp_path / "package.spec").write_text(_spec("Patch0: cve.patch", "%setup -q\n%patch0 -p1")) + normalize_first_inherited_patch_applications(tmp_path / "package.spec", base_spec, result.patch_files) + assert "%patch -P 0 -p1" in (tmp_path / "package.spec").read_text() + await verify_inherited_patches(tmp_path, result) + await validate_inherited_adaptation(tmp_path, "package", base, result) + (tmp_path / "package.spec").write_text(_spec("Patch0: cve.patch", "%autosetup -p1")) await validate_inherited_adaptation(tmp_path, "package", base, result) diff --git a/ymir/agents/ystream_inherit.py b/ymir/agents/ystream_inherit.py index 17da23766..6dcbd73c5 100644 --- a/ymir/agents/ystream_inherit.py +++ b/ymir/agents/ystream_inherit.py @@ -412,6 +412,38 @@ def _validate_patch_usage(spec_path: Path, patch_files: list[str]) -> None: ) +def normalize_first_inherited_patch_applications( + spec_path: Path, original_spec: str, patch_files: list[str] +) -> None: + """Use modern explicit application syntax when adding the first patches.""" + if not patch_files: + return + with Specfile(content=original_spec, sourcedir=spec_path.parent) as original: + # RPM's parsed view omits declarations in inactive conditionals. + with original.sections() as original_sections: + has_patch_declaration = ( + any(re.match(r"(?i)^\s*Patch\d*\s*:", line) for line in original_sections.package) + or "patchlist" in original_sections + ) + if has_patch_declaration or any(get_all_patches(original)): + return + + with Specfile(spec_path) as spec: + patch_numbers = { + patch.number for patch in get_all_patches(spec) if patch.valid and patch.filename in patch_files + } + if not patch_numbers: + return + with spec.sections() as sections: + if "prep" not in sections: + return + prep = sections.prep + for index, line in enumerate(prep): + match = re.match(r"(\s*)%patch(\d+)(?=\s|$)", line) + if match and int(match.group(2)) in patch_numbers: + prep[index] = f"{match.group(1)}%patch -P {match.group(2)}{line[match.end() :]}" + + async def verify_inherited_patches(clone_path: Path, change: IntegratedChange) -> None: """Require every inherited patch to remain byte-for-byte equal to its source Git blob.""" for patch_file, expected_blob in change.patch_blob_ids.items(): From 7470a7cc664a6ca5c5df714c58a41d3df6397d9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Poho=C5=99elsk=C3=BD?= Date: Thu, 24 Sep 2026 12:35:53 +0200 Subject: [PATCH 2/2] Normalize positional first-patch syntax in inheritance Recognize %patch N alongside %patchN when adding a spec first inherited patch, preserving strip and backup options. Assisted-by: GPT-6 Codex --- ymir/agents/tests/unit/test_ystream_inherit.py | 6 ++++++ ymir/agents/ystream_inherit.py | 8 +++++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/ymir/agents/tests/unit/test_ystream_inherit.py b/ymir/agents/tests/unit/test_ystream_inherit.py index 7eef8e214..04a5ad2df 100644 --- a/ymir/agents/tests/unit/test_ystream_inherit.py +++ b/ymir/agents/tests/unit/test_ystream_inherit.py @@ -258,6 +258,12 @@ def _spec(patches: str, prep: str) -> str: "# keep this comment\n %patch0 -p1 -b .backup\n%patch1 -p0", "# keep this comment\n %patch -P 0 -p1 -b .backup\n%patch -P 1 -p0", ), + ( + "", + "%setup -q", + " %patch 0 -p1 -b .backup", + " %patch -P 0 -p1 -b .backup", + ), ("", "%autosetup -p1", "%autosetup -p1", "%autosetup -p1"), ("Patch0: existing.patch", "%patch0 -p1", "%patch0 -p1\n%patch1 -p1", "%patch0 -p1\n%patch1 -p1"), ( diff --git a/ymir/agents/ystream_inherit.py b/ymir/agents/ystream_inherit.py index 6dcbd73c5..86fa3a2ef 100644 --- a/ymir/agents/ystream_inherit.py +++ b/ymir/agents/ystream_inherit.py @@ -439,9 +439,11 @@ def normalize_first_inherited_patch_applications( return prep = sections.prep for index, line in enumerate(prep): - match = re.match(r"(\s*)%patch(\d+)(?=\s|$)", line) - if match and int(match.group(2)) in patch_numbers: - prep[index] = f"{match.group(1)}%patch -P {match.group(2)}{line[match.end() :]}" + match = re.match(r"([ \t]*)%patch(?:(\d+)|[ \t]+(\d+))(?=[ \t]|$)", line) + if match: + patch_number = match.group(2) or match.group(3) + if int(patch_number) in patch_numbers: + prep[index] = f"{match.group(1)}%patch -P {patch_number}{line[match.end() :]}" async def verify_inherited_patches(clone_path: Path, change: IntegratedChange) -> None: