Bug 2382327

Is it possible to follow the newer python packaging guidelines:
https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/

@benson_muite Originally, I wanted to avoid managing multiple branches in my dedicated gitlab repository, and I couldn't follow the newer python packaging guidelines to support older systems.

Now, I probably can revise this position.

(In reply to Benson Muite from comment #1)
> Is it possible to follow the newer python packaging guidelines:
> https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/

Note that the %py3_build/%py3_install macros were recently officially deprecated[1], which means you are not permitted to add a new dependency on them[2], so using the new guidelines is now a “MUST” for new packages.

[1] https://fedoraproject.org/wiki/Changes/DeprecateSetuppyMacros#Impacted_packages
[2] https://docs.fedoraproject.org/en-US/packaging-guidelines/deprecating-packages/#_consequences_of_a_package_being_deprecated

I finally found some time to work on this, and I've updated the spec file (and dropped support for RHEL8).

https://copr.fedorainfracloud.org/coprs/fberat/mass-prebuild/build/9377908/

Please use raw links to spec+srpm, not to HTML pages with them. That allows automatic checks via Fedora Review. Thanks.



Spec file sanity:


> %global pyname mass_prebuild

This macro is only used once. Consider replacing that usage with mass_prebuild verbatim.


> Release:        3%{?dist}

%autorelese is the preferred way in Fedora. https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/#_release_tag


> Summary:        A set of tools to massively pre-build reverse dependencies for a package

Consider saying this is for RPM packages.


> Patch:          drop-setuptools_scm.patch

This patch only includes an explanation within the patch itself. However, each patch reason should be explained in the specfile. https://docs.fedoraproject.org/en-US/packaging-guidelines/#_patch_guidelines


> Patch:          0001-docs-license-Update-GPL-to-latest-version.patch

Same here + please don't change licenses downstream.


> # The pyproject.toml file needs to be updated upstream to had more of these.
> BuildRequires:  python3-copr
> ...

Consider explaining what those are. Are those upstream-undeclared runtime dependencies? Or test dependencies? Or actual build dependencies?


> install -d %{buildroot}%{_sysconfdir}/bash_completion.d

Please use %{bash_completions_dir}. https://docs.fedoraproject.org/en-US/packaging-guidelines/ShellCompletions/

Note that this also has a different value (/usr/share-based rather than /etc-based), which is the correct location to install bash completions from RPM packages.



> %config(noreplace) %{_sysconfdir}/bash_completion.d/mpb*

If, for some reason, you actually need the bash completions in /etc, do you expect users to modify them themselves? The %config(noreplace) notation seems to be abused here.


> %license LICENSE

You use `%pyproject_save_files -l` hence manually listing a %license file should not be necessary.

@mhroncok I've updated the sources and the spec file, hopefully I didn't forget anything

Spec URL: https://gitlab.com/fberat/mass-prebuild-dist/-/blob/rawhide/mass-prebuild.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/fberat/mass-prebuild/fedora-rawhide-x86_64/09551786-mass-prebuild/mass-prebuild-1.6.1-1.fc44.src.rpm

Copr build:
https://copr.fedorainfracloud.org/coprs/build/9551800
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2382327-mass-prebuild/fedora-rawhide-x86_64/09551800-mass-prebuild/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

(In reply to Miro Hrončok from comment #5)
> Please use raw links to spec+srpm, not to HTML pages with them. That allows
> automatic checks via Fedora Review. Thanks.

spec file is still linked to a HTML page.

> > %global pyname mass_prebuild
> 
> This macro is only used once. Consider replacing that usage with
> mass_prebuild verbatim.


Fixed.


> > Release:        3%{?dist}
> 
> %autorelese is the preferred way in Fedora.
> https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/
> #_release_tag

Fixed.

> > Summary:        A set of tools to massively pre-build reverse dependencies for a package
> 
> Consider saying this is for RPM packages.

Fixed. I think it's "an RPM package", not "a RPM package", but I am not a native speaker.


> > Patch:          drop-setuptools_scm.patch
> 
> This patch only includes an explanation within the patch itself. However,
> each patch reason should be explained in the specfile.
> https://docs.fedoraproject.org/en-US/packaging-guidelines/#_patch_guidelines
> 
> 
> > Patch:          0001-docs-license-Update-GPL-to-latest-version.patch
> 
> Same here + please don't change licenses downstream.


Both patches removed.


> > # The pyproject.toml file needs to be updated upstream to had more of these.
> > BuildRequires:  python3-copr
> > ...
> 
> Consider explaining what those are. Are those upstream-undeclared runtime
> dependencies? Or test dependencies? Or actual build dependencies?

This was improved.

BuildRequires:  python3-pip  <- this one is redundant and generated by %pyproject_buildrequires (unless you need it for other reasons).

> > install -d %{buildroot}%{_sysconfdir}/bash_completion.d
> 
> Please use %{bash_completions_dir}.
> https://docs.fedoraproject.org/en-US/packaging-guidelines/ShellCompletions/
> 
> > %config(noreplace) %{_sysconfdir}/bash_completion.d/mpb*
> 
> If, for some reason, you actually need the bash completions in /etc, do you
> expect users to modify them themselves? The %config(noreplace) notation
> seems to be abused here.

Both fixed.


> > %license LICENSE
> 
> You use `%pyproject_save_files -l` hence manually listing a %license file
> should not be necessary.

Fixed.

----


Two notable problems caught by fedora-review-service:



     No known owner of /etc/mpb/repo.conf.d, /etc/mpb/copr.conf.d

This is correct, the %files should list %dir %{_sysconfdir}/mpb/*.conf.d as well, not only the content.




    [!]: Uses parallel make %{?_smp_mflags} macro.

The spec file uses plan `make` where it likely should invoke %make_build -- see https://docs.fedoraproject.org/en-US/packaging-guidelines/#_parallel_make

> Fixed. I think it's "an RPM package", not "a RPM package", but I am not a native speaker.

I agree with this. The article “a” is chosen when the first sound of the spoken acronym or initialism is a consonant, and “an” is chosen when it is a vowel. Since we say RPM by spelling it out, and the letter R is pronounced “arr,” “an” is correct.

- an RPM
- an XML file
- a PDF file
- a SQL database if you pronounce it “sequel”
- an SQL databse if you pronounce it “ess-queue-ell”

@mhroncok Thanks for your time, I've updated once more:

Spec URL: https://gitlab.com/fberat/mass-prebuild-dist/-/raw/rawhide/mass-prebuild.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/fberat/mass-prebuild/fedora-rawhide-x86_64/09589876-mass-prebuild/mass-prebuild-1.6.1-1.fc44.src.rpm

Created attachment 2107286 [details]
The .spec file difference from Copr build 9551800 to 9589896

Copr build:
https://copr.fedorainfracloud.org/coprs/build/9589896
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2382327-mass-prebuild/fedora-rawhide-x86_64/09589896-mass-prebuild/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

I believe this creates a duplicate file record:

  %config(noreplace) %{_sysconfdir}/mpb/*.conf.d/
  %config(noreplace) %{_sysconfdir}/mpb/*.conf.d/*

RPM build warnings:
    File listed twice: /etc/mpb/copr.conf.d/01_project.yaml
    File listed twice: /etc/mpb/copr.conf.d/02_packages.yaml
    File listed twice: /etc/mpb/repo.conf.d/centos-stream.yaml
    File listed twice: /etc/mpb/repo.conf.d/epel.yaml
    File listed twice: /etc/mpb/repo.conf.d/fedora.yaml


Perhaps use this instead?

  %dir %{_sysconfdir}/mpb/*.conf.d/
  %config(noreplace) %{_sysconfdir}/mpb/*.conf.d/*

@mhroncok The warning should now be fixed.

Package APPROVED.

The Pagure repository was created at https://src.fedoraproject.org/rpms/mass-prebuild

FEDORA-2025-04ccfbf2f0 (mass-prebuild-1.6.1-1.fc44) has been submitted as an update to Fedora 44.
https://bodhi.fedoraproject.org/updates/FEDORA-2025-04ccfbf2f0

FEDORA-2025-04ccfbf2f0 (mass-prebuild-1.6.1-1.fc44) has been pushed to the Fedora 44 stable repository.
If problem still persists, please make note of it in this bug report.