@coderabbitai[bot] commented on this pull request.
**Actionable comments posted: 3**
<details>
<summary>🤖 Prompt for all review comments with AI agents</summary>
```
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/man/rpm-spec.5.scd`:
- Line 974: Correct the suffix reference in the documentation sentence to use
“.specpart” consistently with the parser contract in parseSpec.cc, replacing the
incorrect “.specparts” form while preserving the surrounding compatibility
guidance.
- Around line 966-971: Revise the guidance in the _.specpart_ documentation to
scope the warning about beginning-of-line spec directives to directives present
in the build scriptlet source. Explicitly clarify that generated .specpart
contents may intentionally contain such directives, preserving Example 3’s use
of %package, %description, and %files.
- Line 973: In the documentation for $RPM_SPECPARTS_DIR at line 973, replace the
phrase "at the start of the build" with the specific build stage name
"mkbuilddir" to accurately document when the variable is created and becomes
available, making the availability guarantee precise rather than generic.
```
</details>
<details>
<summary>🪄 Autofix (Beta)</summary>
Fix all unresolved CodeRabbit comments on this PR:
- [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a
commit to this branch (recommended)
- [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a
new PR with the fixes
</details>
---
<details>
<summary>ℹ️ Review info</summary>
<details>
<summary>⚙️ Run configuration</summary>
**Configuration used**: defaults
**Review profile**: CHILL
**Plan**: Pro Plus
**Run ID**: `61d178b3-7be1-47e5-8222-7b1fe500f908`
</details>
<details>
<summary>📥 Commits</summary>
Reviewing files that changed from the base of the PR and between
c1fe256483b4802af27c2fe67a31443ac4045bd4 and
16fe52d33dc41a4248c7a55fa15ac449de68e759.
</details>
<details>
<summary>📒 Files selected for processing (2)</summary>
* `docs/man/rpm-spec.5.scd`
* `docs/manual/dynamic_specs.md`
</details>
</details>
<!-- This is an auto-generated comment by CodeRabbit for review status -->
> +Generally, _.specpart_ files should be generated by separate executables
> invoked
+from build scriptlets rather than by the scriptlets directly. This is to ensure
+that the spec syntax written to such files is not interpreted prematurely (that
+is, when the spec file is read) as that could have an unintended side effect on
+the build process. Spec directives starting at the beginning of the line should
+thus be avoided entirely as they would be interpreted right away.
_🎯 Functional Correctness_ | _🟡 Minor_ | _⚡ Quick win_
**Scope the directive warning to the build scriptlet source.**
The generated `.specpart` must contain directives at the beginning of lines.
Example 3 relies on this for `%package`, `%description`, and `%files`. Clarify
that the warning applies to the build scriptlet source, not to generated file
contents.
<details>
<summary>Proposed wording</summary>
```diff
- Spec directives starting at the beginning of the line should
- thus be avoided entirely as they would be interpreted right away.
+ Spec directives starting at the beginning of the line should be
+ avoided in the build scriptlet source, as they would be interpreted
+ when the spec is read.
```
</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately
> replaces the highlighted code, contains no missing lines, and has no issues
> with indentation. Thoroughly test & benchmark the code to ensure it meets the
> requirements.
```suggestion
Generally, _.specpart_ files should be generated by separate executables invoked
from build scriptlets rather than by the scriptlets directly. This is to ensure
that the spec syntax written to such files is not interpreted prematurely (that
is, when the spec file is read) as that could have an unintended side effect on
the build process. Spec directives starting at the beginning of the line should
be
avoided in the build scriptlet source, as they would be interpreted
when the spec is read.
```
</details>
<!-- suggestion_end -->
<details>
<summary>🤖 Prompt for AI Agents</summary>
```
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/man/rpm-spec.5.scd` around lines 966 - 971, Revise the guidance in the
_.specpart_ documentation to scope the warning about beginning-of-line spec
directives to directives present in the build scriptlet source. Explicitly
clarify that generated .specpart contents may intentionally contain such
directives, preserving Example 3’s use of %package, %description, and %files.
```
</details>
<!-- fingerprinting:phantom:poseidon:luna -->
<!-- cr-indicator-types:potential_issue -->
<!-- cr-comment:v1:40d3fa87983d4213050ba9a8 -->
<!-- This is an auto-generated comment by CodeRabbit -->
> +Spec parts cannot include preamble tags or directives that are needed for, or
+influence, the build process. This includes all build scriptlets, sources and
+patches, *BuildRequires*, *BuildArch* (except for *noarch* sub-packages),
+*BuildSystem*, *ExcludeArch*, *ExclusiveArch*, *ExcludeOS* and *ExclusiveOS*.
+These will terminate the build with an error if encountered in a spec part.
+Declaring macros meant to be used in build scriptlets is not an error, however
+they will not influence the build for obvious reasons.
+
+Generally, _.specpart_ files should be generated by separate executables
invoked
+from build scriptlets rather than by the scriptlets directly. This is to ensure
+that the spec syntax written to such files is not interpreted prematurely (that
+is, when the spec file is read) as that could have an unintended side effect on
+the build process. Spec directives starting at the beginning of the line should
+thus be avoided entirely as they would be interpreted right away.
+
+*$RPM_SPECPARTS_DIR* is created automatically at the start of the build, and
_🎯 Functional Correctness_ | _🟡 Minor_ | _⚡ Quick win_
**Document the `mkbuilddir` creation stage.**
The PR objective states that `$RPM_SPECPARTS_DIR` is created during the
`mkbuilddir` stage. Replace “at the start of the build” with that stage name so
the availability guarantee is precise.
<details>
<summary>Proposed wording</summary>
```diff
-*$RPM_SPECPARTS_DIR* is created automatically at the start of the build, and
+*$RPM_SPECPARTS_DIR* is created automatically during the *mkbuilddir* stage,
and
```
</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately
> replaces the highlighted code, contains no missing lines, and has no issues
> with indentation. Thoroughly test & benchmark the code to ensure it meets the
> requirements.
```suggestion
*$RPM_SPECPARTS_DIR* is created automatically during the *mkbuilddir* stage, and
```
</details>
<!-- suggestion_end -->
<details>
<summary>🤖 Prompt for AI Agents</summary>
```
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/man/rpm-spec.5.scd` at line 973, In the documentation for
$RPM_SPECPARTS_DIR at line 973, replace the phrase "at the start of the build"
with the specific build stage name "mkbuilddir" to accurately document when the
variable is created and becomes available, making the availability guarantee
precise rather than generic.
```
</details>
<!-- fingerprinting:phantom:poseidon:luna -->
<!-- cr-indicator-types:potential_issue -->
<!-- cr-comment:v1:74430da3013c2fe073ee7a49 -->
<!-- This is an auto-generated comment by CodeRabbit -->
> +influence, the build process. This includes all build scriptlets, sources and
+patches, *BuildRequires*, *BuildArch* (except for *noarch* sub-packages),
+*BuildSystem*, *ExcludeArch*, *ExclusiveArch*, *ExcludeOS* and *ExclusiveOS*.
+These will terminate the build with an error if encountered in a spec part.
+Declaring macros meant to be used in build scriptlets is not an error, however
+they will not influence the build for obvious reasons.
+
+Generally, _.specpart_ files should be generated by separate executables
invoked
+from build scriptlets rather than by the scriptlets directly. This is to ensure
+that the spec syntax written to such files is not interpreted prematurely (that
+is, when the spec file is read) as that could have an unintended side effect on
+the build process. Spec directives starting at the beginning of the line should
+thus be avoided entirely as they would be interpreted right away.
+
+*$RPM_SPECPARTS_DIR* is created automatically at the start of the build, and
+_.specparts_ files are read in alphabetical order. For backwards compatibility
_🎯 Functional Correctness_ | _🟡 Minor_ | _⚡ Quick win_
**Use the `.specpart` suffix consistently.**
Line 974 uses `_.specparts_`, but the documented and implemented suffix is
`_.specpart_`. Correct this typo to avoid users creating files that RPM ignores.
<details>
<summary>Proposed fix</summary>
```diff
-_.specparts_ files are read in alphabetical order.
+_.specpart_ files are read in alphabetical order.
```
</details>
The parser contract in `build/parseSpec.cc` uses the `*.specpart` pattern.
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately
> replaces the highlighted code, contains no missing lines, and has no issues
> with indentation. Thoroughly test & benchmark the code to ensure it meets the
> requirements.
```suggestion
_.specpart_ files are read in alphabetical order. For backwards compatibility
```
</details>
<!-- suggestion_end -->
<details>
<summary>🤖 Prompt for AI Agents</summary>
```
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/man/rpm-spec.5.scd` at line 974, Correct the suffix reference in the
documentation sentence to use “.specpart” consistently with the parser contract
in parseSpec.cc, replacing the incorrect “.specparts” form while preserving the
surrounding compatibility guidance.
```
</details>
<!-- fingerprinting:phantom:poseidon:luna -->
<!-- cr-indicator-types:potential_issue -->
<!-- cr-comment:v1:27a578e24fb0c372e06a1f39 -->
<!-- This is an auto-generated comment by CodeRabbit -->
--
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/4299#pullrequestreview-4829461052
You are receiving this because you are subscribed to this thread.
Message ID: <rpm-software-management/rpm/pull/4299/review/[email protected]>_______________________________________________
Rpm-maint mailing list
[email protected]
https://lists.rpm.org/mailman/listinfo/rpm-maint
