Bring stratis-cli.spec into sync with Fedora#556
Bring stratis-cli.spec into sync with Fedora#556mulkieran merged 1 commit intostratis-storage:masterfrom
Conversation
WalkthroughReplaces setuptools-based RPM packaging with a pyproject-driven flow: adjusts BuildRequires, adds %generate_buildrequires and %pyproject_buildrequires, switches build/install to %pyproject_wheel/%pyproject_install, enables pyproject file tracking and import checks, and replaces explicit %files/license/site-packages entries. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Packager
participant rpmbuild
participant Spec
participant Pyproject as PyprojectMacros
participant System
Packager->>rpmbuild: start build
rpmbuild->>Spec: %prep
Spec->>Pyproject: %generate_buildrequires / %pyproject_buildrequires
Pyproject-->>rpmbuild: provide buildrequires
rpmbuild->>Spec: %build
Spec->>Pyproject: %pyproject_wheel
Pyproject-->>rpmbuild: build wheel
rpmbuild->>Spec: %install
Spec->>Pyproject: %pyproject_install
Spec->>Pyproject: %pyproject_save_files -l stratis_cli
rpmbuild->>Spec: %check
Spec->>Pyproject: +%check / +%pyproject_check_import
rpmbuild->>Spec: %files
Spec->>Pyproject: %files -f %{pyproject_files}
Note over Pyproject,Spec: Files and metadata sourced from pyproject
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
mockbuild_test/stratis-cli.spec (3)
46-47: Install/save-files look correct; consider two small packaging tweaks
- -l with %pyproject_save_files is great to enforce license presence; ensure the buildroot has pyproject-rpm-macros ≥ 1.11 everywhere you build (that’s when -l landed). If older builders are in scope, gate this or require a minimum macros version. (lists.fedorahosted.org)
- Use shell-completion directory macros rather than hardcoded paths (required by Fedora guidelines). Suggested change:
-%{__install} -Dpm0644 -t %{buildroot}%{_datadir}/bash-completion/completions \ +%{__install} -Dpm0644 -t %{buildroot}%{bash_completions_dir} \ shell-completion/bash/stratis -%{__install} -Dpm0644 -t %{buildroot}%{_datadir}/zsh/site-functions \ +%{__install} -Dpm0644 -t %{buildroot}%{zsh_completions_dir} \ shell-completion/zsh/_stratis -%{__install} -Dpm0644 -t %{buildroot}%{_datadir}/fish/vendor_completions.d \ +%{__install} -Dpm0644 -t %{buildroot}%{fish_completions_dir} \ shell-completion/fish/stratis.fishRefs: Fedora shell completions macros guidance. (docs.fedoraproject.org)
Also applies to: 50-55
62-76: Files section is solid; consider not owning parent completion directoriesUsing -f %{pyproject_files} is correct; README stays listed (it’s not auto-included), and the entry-point binary remains explicit. Optionally, avoid owning parent completion directories that are typically provided by other packages; list only the files. This reduces co-ownership churn. (docs.stg.fedoraproject.org, docs.fedoraproject.org)
%files -f %{pyproject_files} %doc README.rst %{_bindir}/stratis %{_mandir}/man8/stratis.8* %if !0%{?rhel} -%dir %{_datadir}/bash-completion -%dir %{_datadir}/bash-completion/completions -%{_datadir}/bash-completion/completions/stratis -%dir %{_datadir}/zsh -%dir %{_datadir}/zsh/site-functions -%{_datadir}/zsh/site-functions/_stratis -%dir %{_datadir}/fish -%dir %{_datadir}/fish/vendor_completions.d -%{_datadir}/fish/vendor_completions.d/stratis.fish +%{bash_completions_dir}/stratis +%{zsh_completions_dir}/_stratis +%{fish_completions_dir}/stratis.fish %endif
42-42: Optional: declarative BuildSystem to trim boilerplate (Fedora ≥ 41/RPM ≥ 4.20)If you target newer Fedora builders, consider BuildSystem: pyproject to auto-generate the standard sections; you can still append your a2x manpage step. Purely optional. (git.almalinux.org)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
mockbuild_test/stratis-cli.spec(2 hunks)
🔇 Additional comments (3)
mockbuild_test/stratis-cli.spec (3)
38-40: Good switch to generated BuildRequires; defaults are saneUsing %generate_buildrequires with %pyproject_buildrequires is the supported path. Note: runtime deps are included by default; the -r flag is not needed anymore. Keeping that default helps your %pyproject_check_import later. (docs.fedoraproject.org)
42-42: Modern build path: %pyproject_wheelMigration from %py3_build to %pyproject_wheel aligns with current guidance and upcoming deprecation of setup.py macros. (fedoraproject.org)
59-60: Import smoke test is properly wired%pyproject_check_import after %pyproject_save_files is exactly the intended usage. No flags needed unless you hit known false positives. (git.almalinux.org)
Signed-off-by: mulhern <amulhern@redhat.com>
mulkieran
force-pushed
the
update-stratis-cli-spec
branch
from
d584b33 to
b87492f
Compare
1 participant
Related stratis-storage/project#811