Bug 2136778 - Review Request: python-setupmeta - Simplify your setup.py
Summary: Review Request: python-setupmeta - Simplify your setup.py
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Sandro
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard: Trivial
Depends On:
Blocks: fedora-neuro, NeuroFedora
TreeView+ depends on / blocked
 
Reported: 2022-10-21 10:07 UTC by Ankur Sinha (FranciscoD)
Modified: 2022-11-07 13:20 UTC (History)
4 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2022-11-07 13:20:35 UTC
Type: ---
Embargoed:
gui1ty: fedora-review+


Attachments (Terms of Use)

Description Ankur Sinha (FranciscoD) 2022-10-21 10:07:04 UTC
Spec URL: https://ankursinha.fedorapeople.org/python-setupmeta/python-setupmeta.spec
SRPM URL: https://ankursinha.fedorapeople.org/python-setupmeta/python-setupmeta-3.3.2-2.fc38.src.rpm

Description:
Writing a setup.py typically involves lots of boilerplate and copy-pasting from
project to project.

This package aims to simplify that and bring some DRY principle to python
packaging.

Fedora Account System Username: ankursinha

Comment 1 Ankur Sinha (FranciscoD) 2022-10-21 10:24:12 UTC
Scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=93271431

Comment 2 Sandro 2022-10-29 17:09:16 UTC
I'm taking this.

Comment 3 Sandro 2022-10-29 19:36:28 UTC
Just one issue:

[!]: License field in the package spec file matches the actual license.
=> Please add LICENSE file to package using %license

Once fixed the package is good to go --> APPROVED

Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed


===== MUST items =====

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[!]: License field in the package spec file matches the actual license.     
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[x]: Package is not known to require an ExcludeArch tag.
[ ]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 20480 bytes in 1 files.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package must not depend on deprecated() packages.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

Python:
[x]: Python eggs must not download any dependencies during the build
     process.
[x]: Package meets the Packaging Guidelines::Python
[x]: Package contains BR: python2-devel or python3-devel
[x]: Packages MUST NOT have dependencies (either build-time or runtime) on
     packages named with the unversioned python- prefix unless no properly
     versioned package exists. Dependencies on Python packages instead MUST
     use names beginning with python2- or python3- as appropriate.
[x]: Python packages must not contain %{pythonX_site(lib|arch)}/* in %files
[x]: Binary eggs must be removed in %prep

===== SHOULD items =====

Generic:
[x]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[?]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[x]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
     files.
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: No rpmlint messages.


Rpmlint
-------
No errors.


Rpmlint (installed packages)
----------------------------
============================ rpmlint session starts ============================
rpmlint: 2.4.0
configuration:
    /usr/lib/python3.11/site-packages/rpmlint/configdefaults.toml
    /etc/xdg/rpmlint/fedora-legacy-licenses.toml
    /etc/xdg/rpmlint/fedora-spdx-licenses.toml
    /etc/xdg/rpmlint/fedora.toml
    /etc/xdg/rpmlint/scoring.toml
    /etc/xdg/rpmlint/users-groups.toml
    /etc/xdg/rpmlint/warn-on-functions.toml
checks: 31, packages: 1

 1 packages and 0 specfiles checked; 0 errors, 0 warnings, 0 badness; has taken 0.0 s 



Source checksums
----------------
https://files.pythonhosted.org/packages/source/s/setupmeta/setupmeta-3.3.2.tar.gz :
  CHECKSUM(SHA256) this package     : 221463a64d2528ba558f14b087410e05a7ef0dab17d19004f124a262d6e007f5
  CHECKSUM(SHA256) upstream package : 221463a64d2528ba558f14b087410e05a7ef0dab17d19004f124a262d6e007f5


Requires
--------
python3-setupmeta (rpmlib, GLIBC filtered):
    python(abi)



Provides
--------
python3-setupmeta:
    python-setupmeta
    python3-setupmeta
    python3.11-setupmeta
    python3.11dist(setupmeta)
    python3dist(setupmeta)

Comment 4 Ankur Sinha (FranciscoD) 2022-10-31 09:44:28 UTC
Thanks for the quick review, Sandro

The license file is included by the pyproject macros:

rpm -ql --licensefiles -p ./results_python-setupmeta/3.3.2/2.fc38/python3-setupmeta-3.3.2-2.fc38.noarch.rpm
/usr/lib/python3.11/site-packages/setupmeta-3.3.2.dist-info/LICENSE


This, unfortunately, needs to be checked manually at the moment from what I know---fedora-review doesn't pick it up.

Requesting SCM now.

Comment 5 Sandro 2022-10-31 10:34:38 UTC
(In reply to Ankur Sinha (FranciscoD) from comment #4)

> The license file is included by the pyproject macros:
> 
> rpm -ql --licensefiles -p
> ./results_python-setupmeta/3.3.2/2.fc38/python3-setupmeta-3.3.2-2.fc38.
> noarch.rpm
> /usr/lib/python3.11/site-packages/setupmeta-3.3.2.dist-info/LICENSE

I thought the license files still need to go into /usr/share/licenses/%{name}. But looking at the Licensing Guide it's a bit vague regarding that requirement. Using %license, as suggested in the Licensing Guide, would put it there.

One advantage of having it in /usr/share/licenses is that it makes it transparent looking at files.dir from fedora-review.

Comment 6 Sandro 2022-10-31 10:41:09 UTC
I'm not sure if this applies here, but reading Miro's comment for his Root License Directory patch [1] seems to suggest, that you still need to use %license in the spec file:

%pyproject_save_files: Support License-Files installed into the *Root License Directory* from PEP 369

    Files still need to be marked as License-File to be considered %license,
    but if their path in METADATA is specified relative to dist-info/licenses,
    they are correctly recognised.

    This makes License-Files specified by hatchling 1.9.0+ marked as %license.

[1] https://src.fedoraproject.org/rpms/pyproject-rpm-macros/c/92ad52e5d4b941ebc70de84dbd53569dc5ea32b7?branch=rawhide

Comment 7 Miro Hrončok 2022-10-31 11:00:11 UTC
No, the packager does not need to repeat the %license thing if the license file is already part of --licensefiles.

There is no rule nor recommendation to have licenses in /usr/share/licenses/, the only rule is to make them with %license and %pyproject_save_files already does that, if upstream has the correct metadata.

See https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_example_spec_file and look up %license.


My comment for the Root License Directory patch does say that files marked as License-File in upstream metadata are marked as %license in the file list. It does not say that you should duplicate the %license entry at all.

Comment 8 Sandro 2022-10-31 11:19:56 UTC
Thanks for the clarification. I was going by the Licensing Guide, which says:

"If the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package must be included in %license"

That made me think the LICENSE file has to be explicitly included as %license LICENSE. If the marking by %pyproject_save_files is sufficient, I guess I can clean up my own spec files and remove the explicit %license LICENSE, which indeed duplicates the LICENSE file.

Comment 9 Miro Hrončok 2022-10-31 12:15:46 UTC
%pyproject_save_files marks it as %licens,e if the upstream metadata marks it as License-File.

Comment 10 Ben Beasley 2022-10-31 18:11:09 UTC
(In reply to Miro Hrončok from comment #7)
> No, the packager does not need to repeat the %license thing if the license
> file is already part of --licensefiles.
> 
> There is no rule nor recommendation to have licenses in
> /usr/share/licenses/, the only rule is to make them with %license and
> %pyproject_save_files already does that, if upstream has the correct
> metadata.
> 
> See
> https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/
> #_example_spec_file and look up %license.

This comes up a *lot*. I’d like to propose that we explicitly clarify this point in the Licensing Guidelines, but I haven’t yet figured out how to word it without making the relevant section too verbose and meandering.

Comment 11 Miro Hrončok 2022-10-31 21:27:29 UTC
(In reply to Ben Beasley from comment #10)
> (In reply to Miro Hrončok from comment #7)
> > No, the packager does not need to repeat the %license thing if the license
> > file is already part of --licensefiles.
> > 
> > There is no rule nor recommendation to have licenses in
> > /usr/share/licenses/, the only rule is to make them with %license and
> > %pyproject_save_files already does that, if upstream has the correct
> > metadata.
> > 
> > See
> > https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/
> > #_example_spec_file and look up %license.
> 
> This comes up a *lot*. I’d like to propose that we explicitly clarify this
> point in the Licensing Guidelines, but I haven’t yet figured out how to word
> it without making the relevant section too verbose and meandering.

https://pagure.io/packaging-committee/issue/1223


Note You need to log in before you can comment on or make changes to this bug.