Bug 2307912 - Review Request: python-pyliblo3 - Python bindings for the liblo Open Sound Control (OSC) library
Summary: Review Request: python-pyliblo3 - Python bindings for the liblo Open Sound Co...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Cristian Le
QA Contact: Fedora Extras Quality Assurance
URL: https://github.com/gesellkammer/pyliblo3
Whiteboard:
Depends On:
Blocks: 2307541 2307546 2307547 2307552
TreeView+ depends on / blocked
 
Reported: 2024-08-26 14:15 UTC by MartinKG
Modified: 2024-10-11 09:42 UTC (History)
4 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2024-10-11 09:42:01 UTC
Type: ---
Embargoed:
fedora: fedora-review+


Attachments (Terms of Use)
The .spec file difference from Copr build 8064481 to 8069100 (3.15 KB, patch)
2024-09-25 11:25 UTC, Fedora Review Service
no flags Details | Diff

Description MartinKG 2024-08-26 14:15:49 UTC
Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/pyliblo3.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/pyliblo3-0.16.2-0.1.git91d1781.fc40.src.rpm

Description: pyliblo is a Python wrapper for the liblo OSC library.
It supports almost the complete functionality of liblo,
allowing you to send and receive OSC messages using a nice and simple
Python API.

This is a Python3 fork of the original bindings for liblo

Fedora Account System Username: martinkg

%changelog
* Sun Aug 25 2024 Martin Gansser <martinkg> - 0.16.2-0.1.git91d1781
- initial build

Comment 1 Adam Williamson 2024-08-26 15:16:38 UTC
I don't like the uncommented, opaquely-named patch: "fix_liblo3.patch". What does it fix? What is its upstream status? All patches should have a comment line that provides a brief explanation of what they're for, a comment line linking to a bug/issue report, and either a comment line linking to an upstream PR or a comment line explaining why there isn't one.

As this is a new package, unless it's intended to also be built for EPEL 8, it might be nice to follow the modern Python guidelines using the pyproject macros, if they work for this project: https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/

Comment 2 MartinKG 2024-08-27 11:46:09 UTC
package update

Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/pyliblo3.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/pyliblo3-0.16.2-0.2.git91d1781.fc40.src.rpm

%changelog
* Tue Aug 27 2024 Martin Gansser <martinkg> - 0.16.2-0.2.git91d1781
- remove Cython generated files
- use macro %%pyproject_wheel
- use macro %%py3_shebang_fix

Comment 3 Fedora Review Service 2024-08-27 15:30:33 UTC
There seems to be some problem with the following file.
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/pyliblo3-0.16.2-0.2.git91d1781.fc40.src.rpm
Fetching it results in a 404 Not Found error.
Please make sure the URL is correct and publicly available.


---
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.

Comment 4 Cristian Le 2024-08-28 11:47:34 UTC
- With `%pyproject_buildrequires` please remove all of the other `BuildRequires: python3-` (other than `python3-devel`). Also `-r` flag has no effect [1]
- Please put a note of why the commit is used there, or consider using the `pypi_source` instead `0.16.2` verison there was released on 2024/07/31. Should also contact upstream to encourage to tag the releases accordingly
- Why the `chmod -x` on text files?
- You should use `%{py3_test_envvars}` before the `%python3 setup.py test` [2]
- Instead of manually populating `%{python3_sitearch}/pyliblo3/` use `-f %{pyproject_files}` and `%pyproject_save_files pyliblo3`. You would probably need an `%exclude` if you want `pyliblo3-tools` to be separate
- Don't you want `Provides: pyliblo` in this case in order to have an upgrade path?
- If you go with `pypi_source` consider also having `rpm-autospec` macros
- You do not have a `BuildArch: noarch` for the top-level package (probably would also not need for subpackages as well afterwards?)

[1]: https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#pyproject_buildrequires
[2]: https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_other_test_runners

Comment 5 Adam Williamson 2024-08-28 14:47:09 UTC
I think not having "Provides: pyliblo" is probably correct, because it doesn't provide the Python library called 'liblo'. It provides a library called 'liblo3'. It's not, at least as stands, a drop-in replacement. Consumers have to adjust their imports, and can update the dependency at the same time.

Comment 6 MartinKG 2024-08-28 14:51:20 UTC
package update

I have tried to change a few things:
- contacting Upstream to encourage them to mark the versions accordingly
- using `%{py3_test_envars}` before `%python3 setup.py test` does not work
- I have removed the tools subpkg.
- BuildArch: noarch` for the top-level package, does not work either, then the message “Arch dependent binaries in noarch package” appears
- I didn't use `pypi_source` because I don't know how it works.

Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/pyliblo3.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/pyliblo3-0.16.2-0.3.fc40.src.rpm


%changelog
## START: Generated by rpmautospec
* Wed Aug 28 2024 Martin Gansser <martinkg> - 0.16.2-0.3
- remove -r option from  macro pyproject_buildrequires
- use macro %%{pyproject_files} and %%pyproject_save_files pyliblo3}
- remove python3 requirements, because the macro %%pyproject_buildrequires is used

Comment 7 Cristian Le 2024-08-28 15:00:09 UTC
(In reply to Adam Williamson from comment #5)
> I think not having "Provides: pyliblo" is probably correct, because it
> doesn't provide the Python library called 'liblo'. It provides a library
> called 'liblo3'. It's not, at least as stands, a drop-in replacement.
> Consumers have to adjust their imports, and can update the dependency at the
> same time.

Fair point I would revise it to removing both `Provides` and `Obsolutes`. Dnf should automatically remove any transient dependencies by itself.

(In reply to MartinKG from comment #6)
> - BuildArch: noarch` for the top-level package, does not work either, then the message “Arch dependent binaries in noarch package” appears

Aaah, sorry I've had pure-python packages too much on my mind lately. It is correct that the main package is archful, and the others are not.

> - using `%{py3_test_envars}` before `%python3 setup.py test` does not work

That is odd, could you post in the Fedora-python matrix channel what errors you get there?

> - I didn't use `pypi_source` because I don't know how it works.

The guidelines example package has an example usage, but basically
```
Source:        %{pypi_source pyliblo3}
```
It would also make release-monitoring.org happy ;)

Comment 8 Adam Williamson 2024-08-28 15:21:40 UTC
No, keeping Obsoletes is correct. It's not safe to rely on dnf removing transient dependencies (it does try, which is more than it used to, but it's not 100% reliable). By policy, retired packages are supposed to be obsoleted by something else, and this package is clearly the best candidate to Obsolete pyliblo.

Comment 9 MartinKG 2024-09-02 12:18:36 UTC
package update

Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/pyliblo3.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/pyliblo3-0.16.2-0.3.git91d1781.fc40.src.rpm

%changelog
## START: Generated by rpmautospec
* Mon Sep 02 2024 Martin Gansser <martinkg> - 0.16.2-0.4.git91d1781
- Use correct source tag address
- Use %%tox macro for testsuite
- add BR python3-tox-current-env

Comment 10 Cristian Le 2024-09-02 13:09:19 UTC
I did a bit more digging on this, and I think this is an issue with the `__init__.py` file. If I replace it with a more standard format like:

```python
from ._liblo import *
```

and it seems to work better when I run it in the mock environment. I think the culprit here is `from __future__ import absolute_import`, but it seemed to still have worked after installing.
I believe you would find the same issue if you ran `%pyproject_check_import`.

As for the tox approach, I am not certain that the tests actually ran appropriately, and it feels wrong to add a tox dependency for this.

Can you try 2 more things one after another?
- Configure `unittest` check to discover only within the `test` folder and with a more relevant file selection: 
```
%check
%{py3_test_envvars} %{python3} -P -m unittest discover -s ./test -p "*.py" 
```
(see python3 -m unittest discover --help for more details on why those flags)

- If that didn't work try to also fix the `pyliblo3/__init__.py` to have the minimal form as I've shown at the beginning.

Comment 11 MartinKG 2024-09-03 07:26:22 UTC
the file `pyliblo3/__init__.py` now only contains these three lines:

```python
 from ._liblo import *
```

$ rpmbuild -bs pyliblo3.spec
$ mock -r fedora-40-x86_64 ../SRPMS/pyliblo3-0.16.2-0.3.git91d1781.fc40.src.rpm


+ PYTHONDONTWRITEBYTECODE=1
+ PYTEST_ADDOPTS=' --ignore=/builddir/build/BUILD/pyliblo3-91d17815b911ccc2c1d1408412e7885c32f2d460/.pyproject-builddir'
+ PYTEST_XDIST_AUTO_NUM_WORKERS=2
+ /usr/bin/python3 -P -m unittest discover -s ./test -p '*.py'
E
======================================================================
ERROR: unit (unittest.loader._FailedTest.unit)
----------------------------------------------------------------------
ImportError: Failed to import test module: unit
Traceback (most recent call last):
  File "/usr/lib64/python3.12/unittest/loader.py", line 396, in _find_test_path
    module = self._get_module_from_name(name)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib64/python3.12/unittest/loader.py", line 339, in _get_module_from_name
    __import__(name)
  File "/builddir/build/BUILD/pyliblo3-91d17815b911ccc2c1d1408412e7885c32f2d460/test/unit.py", line 246, in <module>
    class DecoratorTestCase(unittest.TestCase):
  File "/builddir/build/BUILD/pyliblo3-91d17815b911ccc2c1d1408412e7885c32f2d460/test/unit.py", line 247, in DecoratorTestCase
    class TestServer(liblo.Server):
                     ^^^^^^^^^^^^
AttributeError: module 'pyliblo3' has no attribute 'Server'

----------------------------------------------------------------------
Ran 1 test in 0.000s
FAILED (errors=1)
RPM build errors:

I just can't get it to work.
maybe you can send me your pyliblo3.src.rpm file.

Comment 12 Cristian Le 2024-09-03 07:58:10 UTC
Thanks Martin. My knowledge of direct cython interface is very limited since I usually interface with it via CMake, but the following seems to work 

```
%prep
%autosetup -p1 -n pyliblo3-%{commit0}
# Remove pregenerated Cython C sources and build it again
rm -rf pyliblo3/_liblo.c

%generate_buildrequires
%pyproject_buildrequires

%build
cython -I pyliblo3 pyliblo3/_liblo.pyx
%pyproject_wheel

%install
%py3_shebang_fix .
%pyproject_install
%pyproject_save_files pyliblo3

mkdir -p %{buildroot}%{_mandir}/man1
cp scripts/dump_osc.1 %{buildroot}%{_mandir}/man1/
cp scripts/send_osc.1 %{buildroot}%{_mandir}/man1/

%check
%{py3_test_envvars} %{python3} -P -m unittest discover -s ./test -p '*.py'
```

The issue was that you were compiling the `.pyd` instead of the `.pyx` (don't ask me the difference between those, I think it's like header/source).

I've also moved the parts a bit because the cython compilation is technically a build step. `%py3_shebang_fix` could be anywhere, but I think it's common to be in `%install` or `%build`

Comment 13 MartinKG 2024-09-03 09:51:43 UTC
package update

Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/pyliblo3.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/pyliblo3-0.16.2-0.4.git91d1781.fc40.src.rpm

%changelog
## START: Generated by rpmautospec
* Tue Sep 03 2024 Martin Gansser <martinkg> - 0.16.2-0.4.git91d1781
- Use correct source tag address
- Recompile `.pyx` file
- Fix permission of NEWS README.md and COPYING

$ rpmlint pyliblo3.spec /home/martin/rpmbuild/SRPMS/pyliblo3-0.16.2-0.4.git91d1781.fc40.src.rpm /home/martin/rpmbuild/RPMS/x86_64/pyliblo3-debuginfo-0.16.2-0.4.git91d1781.fc40.x86_64.rpm /home/martin/rpmbuild/RPMS/x86_64/pyliblo3-0.16.2-0.4.git91d1781.fc40.x86_64.rpm /home/martin/rpmbuild/RPMS/noarch/pyliblo3-doc-0.16.2-0.4.git91d1781.fc40.noarch.rpm /home/martin/rpmbuild/RPMS/x86_64/pyliblo3-debugsource-0.16.2-0.4.git91d1781.fc40.x86_64.rpm
========================================================= rpmlint session starts ==================================
rpmlint: 2.5.0
configuration:
    /usr/lib/python3.12/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: 32, packages: 6

pyliblo3.x86_64: W: no-manual-page-for-binary dump_osc.py
pyliblo3.x86_64: W: no-manual-page-for-binary send_osc.py

Comment 14 Cristian Le 2024-09-03 11:51:37 UTC
There seems to be a build error on rawhide, could you debug that one?
```
  gcc -fno-strict-overflow -Wsign-compare -DDYNAMIC_ANNOTATIONS_ENABLED=1 -DNDEBUG -fcf-protection -fexceptions -fcf-protection -fexceptions -fcf-protection -fexceptions -O3 -O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -march=x86-64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -mtls-dialect=gnu2 -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -fPIC -Ipyliblo3 -I/usr/include -I/usr/local/include -I/usr/include/python3.13 -c pyliblo3/_liblo.c -o build/temp.linux-x86_64-cpython-313/pyliblo3/_liblo.o -fno-strict-aliasing -Werror-implicit-function-declaration -Wfatal-errors
  pyliblo3/_liblo.c: In function ‘__pyx_f_8pyliblo3_6_liblo__msg_callback’:
  pyliblo3/_liblo.c:8980:92: error: passing argument 1 of ‘lo_blob_dataptr’ from incompatible pointer type [-Wincompatible-pointer-types]
   8980 |       __pyx_t_7 = __Pyx_PyBytes_FromCString(((unsigned char *)lo_blob_dataptr((__pyx_v_argv[__pyx_v_i])))); if (unlikely(!__pyx_t_7)) __PYX_ERR(0, 274, __pyx_L1_error)
        |                                                                               ~~~~~~~~~~~~~^~~~~~~~~~~~
        |                                                                                            |
        |                                                                                            lo_arg *
  pyliblo3/_liblo.c:1322:78: note: in definition of macro ‘__Pyx_PyBytes_FromCString’
   1322 | #define __Pyx_PyBytes_FromCString(s)   __Pyx_PyBytes_FromString((const char*)s)
        |                                                                              ^
  compilation terminated due to -Wfatal-errors.
  error: command '/usr/bin/gcc' failed with exit code 1
  error: subprocess-exited-with-error
```

Other than that one, I think it's in good enough shape, just need to be able to run fedora-review.

And make sure when you do the import you fix the spec file to have a clean spec file (before rpmautospec adds the header and footer parts like the "START: Set by rpmautospec").

Comment 15 Fedora Review Service 2024-09-03 14:43:11 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/7974553
(failed)

Build log:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2307912-pyliblo3/fedora-rawhide-x86_64/07974553-pyliblo3/builder-live.log.gz

Please make sure the package builds successfully at least for Fedora Rawhide.

- If the build failed for unrelated reasons (e.g. temporary network
  unavailability), please ignore it.
- If the build failed because of missing BuildRequires, please make sure they
  are listed in the "Depends On" field


---
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.

Comment 16 MartinKG 2024-09-04 08:05:45 UTC
(In reply to Cristian Le from comment #14)
> There seems to be a build error on rawhide, could you debug that one?
> ```

Unfortunately, I cannot fix the compilation error and need support.

Comment 17 MartinKG 2024-09-04 13:15:19 UTC
package update

Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/pyliblo3.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/pyliblo3-0.16.2-0.5.git91d1781.fc40.src.rpm

%changelog
## START: Generated by rpmautospec
* Wed Sep 04 2024 Martin Gansser <martinkg> - 0.16.2-0.5.git91d1781
- Add patch `type-erase lo_blob_dataptr input` for _liblo.pyx

Comment 18 Cristian Le 2024-09-04 15:07:47 UTC
LGTM, don't forget to fix the spec file when importing.

Package Approved.

Just a few minor points:
- `doc` is not built and it points to the old `pyliblo` documentation and sites. Don't know what the intention of upstream is to update those documentations or not. Since this is primarily used as a library for other dependencies, you could probably ignore the doc packaging. Otherwise add the dependency on the main package so that the license file is included.
- add the `Obsoletes` for the `piliblo` as Adam suggested

---


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

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



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

C/C++:
[-]: Development (unversioned) .so files in -devel subpackage, if present.
     Note: Unversioned so-files in private %_libdir subdirectory (see
     attachment). Verify they are not in ld path.

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.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "GNU Lesser General Public License, Version 2.1", "Unknown or
     generated", "*No copyright* GNU Lesser General Public License", "GNU
     Lesser General Public License v2.1 or later", "GNU General Public
     License v2.0 or later". 29 files have unknown license. Detailed output
     of licensecheck in
     /home/lecris/FedoraRPMS/2307912-pyliblo3/licensecheck.txt
[?]: License file installed when any subpackage combination is installed.
[x]: Package must own all directories that it creates.
     Note: Directories without known owners: /usr/lib64/python3.13/site-
     packages, /usr/lib64/python3.13
[x]: %build honors applicable compiler flags or justifies otherwise.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[-]: Development files must be in a -devel package
[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.
[!]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.
[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]: If (and only 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 is included in %license.
[x]: The License field must be a valid SPDX expression.
[x]: Package requires other packages for directories it uses.
[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]: Dist tag is present.
[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]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 6275 bytes in 2 files.
[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.
[-]: A package which is used by another package via an egg interface should
     provide egg info.
[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:
[-]: 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.
[-]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: Patches link to upstream bugs/comments/lists or are otherwise
     justified.
[-]: Sources are verified with gpgverify first in %prep if upstream
     publishes signatures.
     Note: gpgverify is not used.
[?]: Package should compile and build into binary rpms on all supported
     architectures.
[x]: %check is present and all tests pass.
[?]: Packages should try to preserve timestamps of original installed
     files.
[x]: Spec use %global instead of %define unless justified.
     Note: %define requiring justification: %define autorelease(e:s:pb:n)
     %{?-p:0.}%{lua:
[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]: Fully versioned dependency in subpackages if applicable.
[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.

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

Generic:
[x]: Rpmlint is run on debuginfo package(s).
     Note: No rpmlint messages.
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Large data in /usr/share should live in a noarch subpackage if package
     is arched.
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: pyliblo3-0.16.2-0.5.git91d1781.fc42.x86_64.rpm
          pyliblo3-doc-0.16.2-0.5.git91d1781.fc42.noarch.rpm
          pyliblo3-debuginfo-0.16.2-0.5.git91d1781.fc42.x86_64.rpm
          pyliblo3-debugsource-0.16.2-0.5.git91d1781.fc42.x86_64.rpm
          pyliblo3-0.16.2-0.5.git91d1781.fc42.src.rpm
================================================================= rpmlint session starts ================================================================
rpmlint: 2.5.0
configuration:
    /usr/lib/python3.12/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
rpmlintrc: [PosixPath('/tmp/tmp6r6y9ctz')]
checks: 32, packages: 5

pyliblo3.x86_64: W: no-manual-page-for-binary dump_osc.py
pyliblo3.x86_64: W: no-manual-page-for-binary send_osc.py
=========================== 5 packages and 0 specfiles checked; 0 errors, 2 warnings, 20 filtered, 0 badness; has taken 1.2 s ===========================




Rpmlint (debuginfo)
-------------------
Checking: pyliblo3-debuginfo-0.16.2-0.5.git91d1781.fc42.x86_64.rpm
================================================================= rpmlint session starts ================================================================
rpmlint: 2.5.0
configuration:
    /usr/lib/python3.12/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
rpmlintrc: [PosixPath('/tmp/tmpuwd9p0_z')]
checks: 32, packages: 1

============================ 1 packages and 0 specfiles checked; 0 errors, 0 warnings, 5 filtered, 0 badness; has taken 0.2 s ===========================





Rpmlint (installed packages)
----------------------------
============================ rpmlint session starts ============================
rpmlint: 2.5.0
configuration:
    /usr/lib/python3.13/site-packages/rpmlint/configdefaults.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: 32, packages: 4

pyliblo3.x86_64: W: no-manual-page-for-binary dump_osc.py
pyliblo3.x86_64: W: no-manual-page-for-binary send_osc.py
 4 packages and 0 specfiles checked; 0 errors, 2 warnings, 17 filtered, 0 badness; has taken 0.7 s 



Unversioned so-files
--------------------
pyliblo3: /usr/lib64/python3.13/site-packages/pyliblo3/_liblo.cpython-313-x86_64-linux-gnu.so

Source checksums
----------------
https://github.com/gesellkammer/pyliblo3/archive/91d17815b911ccc2c1d1408412e7885c32f2d460/pyliblo3-91d1781.tar.gz :
  CHECKSUM(SHA256) this package     : a17ef90dfc01cc298dadecbe7a2cedc4cadfaf78bc704260eb818099cb63366b
  CHECKSUM(SHA256) upstream package : a17ef90dfc01cc298dadecbe7a2cedc4cadfaf78bc704260eb818099cb63366b


Requires
--------
pyliblo3 (rpmlib, GLIBC filtered):
    /usr/bin/python3
    libc.so.6()(64bit)
    liblo.so.7()(64bit)
    python(abi)
    rtld(GNU_HASH)

pyliblo3-doc (rpmlib, GLIBC filtered):

pyliblo3-debuginfo (rpmlib, GLIBC filtered):

pyliblo3-debugsource (rpmlib, GLIBC filtered):



Provides
--------
pyliblo3:
    pyliblo3
    pyliblo3(x86-64)
    python3.13dist(pyliblo3)
    python3dist(pyliblo3)

pyliblo3-doc:
    pyliblo3-doc

pyliblo3-debuginfo:
    debuginfo(build-id)
    pyliblo3-debuginfo
    pyliblo3-debuginfo(x86-64)

pyliblo3-debugsource:
    pyliblo3-debugsource
    pyliblo3-debugsource(x86-64)



Generated by fedora-review 0.10.0 (e79b66b) last change: 2023-07-24
Command line :/usr/bin/fedora-review -b 2307912
Buildroot used: fedora-rawhide-x86_64
Active plugins: Python, Shell-api, Generic
Disabled plugins: R, Perl, PHP, fonts, Java, Ocaml, C/C++, SugarActivity, Haskell
Disabled flags: EXARCH, EPEL6, EPEL7, DISTTAG, BATCH

Comment 19 Fedora Admin user for bugzilla script actions 2024-09-05 08:01:58 UTC
The Pagure repository was created at https://src.fedoraproject.org/rpms/pyliblo3

Comment 20 Miro Hrončok 2024-09-05 12:41:22 UTC
Folks, I belive this package should be called python-pyliblo3 / python3-pyliblo3. Wait a bit before you import it.

Comment 21 Miro Hrončok 2024-09-05 12:51:06 UTC
> Obsoletes:      pyliblo <= 0.10.0

This does now work for multiple reasons.

 1. It needs to obsolete python3-pyliblo, not pyliblo. We obsolete the packages that get installed, not source package names.
 2. 0.10.0-30.fc40 is not less or equal to 0.10.0



> ...
> %pyproject_wheel
> 
> %install
> %py3_shebang_fix .           <---------- What is the purpose of this? Probably has no effect *after* %pyproject_wheel
> %pyproject_install
> ...


> # Main code is LGPL-2.1+
> License:        LGPL-2.1-or-later

This mixes 2 types of license identifiers.



> %license COPYING

Please use -l option for %pyproject_save_files instead.

Comment 22 MartinKG 2024-09-05 14:35:46 UTC
(In reply to Miro Hrončok from comment #21)
> > Obsoletes:      pyliblo <= 0.10.0
> 
> This does now work for multiple reasons.
> 
>  1. It needs to obsolete python3-pyliblo, not pyliblo. We obsolete the
> packages that get installed, not source package names.
>  2. 0.10.0-30.fc40 is not less or equal to 0.10.0
> 
> 
> 
> > ...
> > %pyproject_wheel
> > 
> > %install
> > %py3_shebang_fix .           <---------- What is the purpose of this? Probably has no effect *after* %pyproject_wheel
> > %pyproject_install
> > ...
> 
> 
> > # Main code is LGPL-2.1+
> > License:        LGPL-2.1-or-later
> 
> This mixes 2 types of license identifiers.
> 
> 
> 
> > %license COPYING
> 
> Please use -l option for %pyproject_save_files instead.

i changed this things in the spec-file, is this correct ?
https://martinkg.fedorapeople.org/Review/SPECS/pyliblo3.spec

Comment 23 Cristian Le 2024-09-05 15:23:18 UTC
> Folks, I belive this package should be called python-pyliblo3

Oh, I guess the original `pyliblo` was packaged before `python-*` were a thing? I was basing it on the original package's naming convention, sorry about that. What about the `doc` subpackage in this case? `python3-pyliblo3-doc`?

> Please use -l option for %pyproject_save_files instead.

Isn't this build-backend dependent? I guess setuptools and hatchling support it at the moment? Hopefully this doesn't need to be ported to EPEL.

> i changed this things in the spec-file, is this correct ?

`Provides: python3-pyliblo` should be removed, see Adam's comment.

Comment 24 Miro Hrončok 2024-09-05 16:20:30 UTC
> What about the `doc` subpackage in this case? `python3-pyliblo3-doc`?

Or python-pyliblo3-doc. Or python-pyliblo3-docs. There seem to be no rule about this. I usually just don't bother with such packages.


> Isn't this build-backend dependent? I guess setuptools and hatchling support it at the moment? Hopefully this doesn't need to be ported to EPEL.

The presence of the %license file is build-backedn dependent. But the -l asserts it worked.

BTW the spec file now has both -l and %ľicense COPYING which is redundant.

-------

Don't use <= in Obsoletes, it is almost always wrong. Use <, e.g.:

Obsoletes:      python3-pyliblo < 0.10.0-35



For Provides, if they are needed, use %py_provides. But they are not needed.


Also, rename the package, please.

Comment 25 MartinKG 2024-09-06 07:14:24 UTC
i renamed the package to python-pyliblo3 and changed the few things you mentioned in the spec-file, is this correct ?

https://martinkg.fedorapeople.org/Review/SPECS/python-pyliblo3.spec

Comment 26 Adam Williamson 2024-09-11 21:04:24 UTC
This LGTM now, but Miro, please take a look.

I'm not sure what we do about the repos. I guess we need https://src.fedoraproject.org/rpms/pyliblo3 removed, and then we would have to do another new repo request for the correct name.

Comment 27 Miro Hrončok 2024-09-11 21:19:28 UTC
https://martinkg.fedorapeople.org/Review/SPECS/python-pyliblo3.spec has been processed by rpmautospec. Do you have the original?



There must be a python3-pyliblo3 subpackage which is missing entirely.

See the example spec at https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_example_spec_file

Comment 28 Adam Williamson 2024-09-12 00:40:42 UTC
Filed https://pagure.io/releng/issue/12322 to decide what to do with the pyliblo3 repo.

Comment 29 MartinKG 2024-09-24 07:31:35 UTC
(In reply to Adam Williamson from comment #28)
> Filed https://pagure.io/releng/issue/12322 to decide what to do with the
> pyliblo3 repo.

Repository has been retired

Comment 30 MartinKG 2024-09-24 07:37:42 UTC
 (In reply to Miro Hrončok from comment #27)
> https://martinkg.fedorapeople.org/Review/SPECS/python-pyliblo3.spec has been
> processed by rpmautospec. Do you have the original?
> 
> 
> 
> There must be a python3-pyliblo3 subpackage which is missing entirely.
> 
> See the example spec at
> https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/
> #_example_spec_file

i tried to add a subpk, but this does'nt work.

https://martinkg.fedorapeople.org/ErrorReports/python-pyliblo3.spec


Provides: python-pyliblo3-doc = 0.16.2^20240801git91d1781-1.fc40
Requires(rpmlib): rpmlib(CompressedFileNames) <= 3.0.4-1 rpmlib(FileDigests) <= 4.6.0-1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1
Processing files: python3-pyliblo3+data-0.16.2^20240801git91d1781-1.fc40.x86_64

Error: The package name contains an extras name `data` that was not found in the metadata.
Check if the extras were removed from the project. If so, consider removing the subpackage and obsoleting it from another.

error: Dependency tokens must begin with alpha-numeric, '_' or '/': *** PYTHON_EXTRAS_NOT_FOUND_ERROR___SEE_STDERR ***

Error: The package name contains an extras name `data` that was not found in the metadata.
Check if the extras were removed from the project. If so, consider removing the subpackage and obsoleting it from another.

error: Dependency tokens must begin with alpha-numeric, '_' or '/': *** PYTHON_EXTRAS_NOT_FOUND_ERROR___SEE_STDERR ***
Provides: python-pyliblo3+data = 0.16.2^20240801git91d1781-1.fc40 python3-pyliblo3+data = 0.16.2^20240801git91d1781-1.fc40 python3-pyliblo3+data(x86-64) = 0.16.2^20240801git91d1781-1.fc40 python3.12-pyliblo3+data = 0.16.2^20240801git91d1781-1.fc40
Requires(rpmlib): rpmlib(CompressedFileNames) <= 3.0.4-1 rpmlib(FileDigests) <= 4.6.0-1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1

Comment 31 Cristian Le 2024-09-24 07:53:19 UTC
(In reply to MartinKG from comment #30)
> i tried to add a subpk, but this does'nt work.

The `Requires` of the `-doc` package should require `python3-pyliblo3` not `python-pyliblo3`.

There is no optional dependency for `[data]` in pyliblo3, so the `+data` package should be removed.

Comment 32 MartinKG 2024-09-24 08:22:39 UTC
(In reply to Cristian Le from comment #31)
> (In reply to MartinKG from comment #30)
> > i tried to add a subpk, but this does'nt work.
> 
> The `Requires` of the `-doc` package should require `python3-pyliblo3` not
> `python-pyliblo3`.
> 
> There is no optional dependency for `[data]` in pyliblo3, so the `+data`
> package should be removed.

and which extra name should i use for the subpkg ?
%pyproject_extras_subpkg -n python3-%{modname}

Comment 33 Cristian Le 2024-09-24 08:28:22 UTC
None at all, the project has no optional dependencies. I believe Miro was referring to adding `python3-pyliblo3` as a subpackage of the src package `python-pyliblo3` (i.e. in the current form you have with `%package -n python3-pyliblo3`), instead of using `python-pyliblo3` as the src and main python package.

Comment 34 MartinKG 2024-09-24 08:45:46 UTC
set the fedora-review flag to -


package update

Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/python-pyliblo3.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/python-pyliblo3-0.16.2%5e20240801git91d1781-2.fc40.src.rpm

%changelog
* Tue Sep 24 2024 Martin Gansser <martinkg> - 0.16.2^20240801git91d1781-2
- Add python3-pyliblo3 subpackage

Comment 35 Fedora Review Service 2024-09-24 08:51:57 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8064481
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2307912-python-pyliblo3/fedora-rawhide-x86_64/08064481-python-pyliblo3/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.

Comment 36 Cristian Le 2024-09-24 09:17:35 UTC
- Please remove the `-t` in the `%pyproject_buildrequires -t`. That is for using `tox` which this version doesn't use it anymore
- Any reason for reverting from `autospec` macros?

Other than that, it looks ok. There is a constant debate if macros like `modname` are necessary or not, just be consistent in the usage. I am on the camp of removing them until there are tools like `rust2rpm` that would make use of it.

Comment 37 Miro Hrončok 2024-09-24 10:30:49 UTC
> Obsoletes:      python3-pyliblo < 0.10.0-30

This needs to be moved in the python3-pyliblo3 subpackage, otherwise it does nothing (assuming you follow the other steps bellow).



> %package -n     python3-%{modname}
> Summary:        %{summary}

The %{summary} expands to the previous Summary in the specfile, hence "Documentation for python-pyliblo3". Either move the doc subpackage after the python3-pyliblo3 subpackage or avoid using %{summary} here.

> Recommends:     python3-pyliblo3

This is self-referencing Recommends. What is the idea behind it?




> %files -n python-pyliblo3 -f %{pyproject_files}

This needs to be python3-pyliblo3, not python-pyliblo3. Currently, the python3-pyliblo3 package is not generated because it has no %files section. Instead, the python-pyliblo3 package is generated which we don't want.

----

> There is a constant debate if macros like `modname` are necessary or not, just be consistent in the usage.

Exactly, the specfile has 3 uses of %{modname} and 16 uses of pyliblo3 (excluding the %changelog entries). Pick one, and stick with it. My opinion is that using %{modname} only makes the specfile harder to read and maintain.



---

Once again I'd like to point out the example Python specfile in https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_example_spec_file which should give you the general idea about the package structure.

Comment 38 MartinKG 2024-09-24 14:12:55 UTC
I only post the spec file with the changes:

Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/python-pyliblo3.spec

Comment 39 Miro Hrončok 2024-09-24 15:56:01 UTC
(In reply to Miro Hrončok from comment #37)
> > Obsoletes:      python3-pyliblo < 0.10.0-30
> 
> This needs to be moved in the python3-pyliblo3 subpackage, otherwise it does
> nothing (assuming you follow the other steps bellow).

Fixed.

> > %package -n     python3-%{modname}
> > Summary:        %{summary}
> 
> The %{summary} expands to the previous Summary in the specfile, hence
> "Documentation for python-pyliblo3". Either move the doc subpackage after
> the python3-pyliblo3 subpackage or avoid using %{summary} here.

Not fixed.

> > Recommends:     python3-pyliblo3
> 
> This is self-referencing Recommends. What is the idea behind it?

Removed, hence fixed.

> > %files -n python-pyliblo3 -f %{pyproject_files}
> 
> This needs to be python3-pyliblo3, not python-pyliblo3. Currently, the
> python3-pyliblo3 package is not generated because it has no %files section.
> Instead, the python-pyliblo3 package is generated which we don't want.

Fixed.

> > There is a constant debate if macros like `modname` are necessary or not, just be consistent in the usage.
> 
> Exactly, the specfile has 3 uses of %{modname} and 16 uses of pyliblo3
> (excluding the %changelog entries). Pick one, and stick with it. My opinion
> is that using %{modname} only makes the specfile harder to read and maintain.

Fixed.

Comment 40 MartinKG 2024-09-25 08:52:41 UTC
Is this correct now ?

Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/python-pyliblo3.spec

Comment 41 Miro Hrončok 2024-09-25 09:55:27 UTC
It seems so. Some things seem questionable, such as the requirement of pyhton3-pyliblo3 in the doc package, but that should not be a blocker.

Comment 42 MartinKG 2024-09-25 11:18:27 UTC
package update, need a review fo thefedora-review flag again ?

Spec URL: https://martinkg.fedorapeople.org/Review/SPECS/python-pyliblo3.spec
SRPM URL: https://martinkg.fedorapeople.org/Review/SRPMS/python-pyliblo3-0.16.2%5e20240801git91d1781-3.fc40.src.rpm

%changelog
* Wed Sep 25 2024 Martin Gansser <martinkg> - 0.16.2^20240801git91d1781-3
- remove pyhton3-pyliblo3 from the doc package

* Tue Sep 24 2024 Martin Gansser <martinkg> - 0.16.2^20240801git91d1781-2
- add python3-pyliblo3 subpackage
- move 'Obsoletes:' tag to python3-pyliblo3 subpackage
- move doc subpackage after the python3-pyliblo3 subpackage
- remove 'Recommends:' tag
- rename python-pyliblo3 to python3-pyliblo3
- remove macro %%{modname} for better readability

Comment 43 Fedora Review Service 2024-09-25 11:25:07 UTC
Created attachment 2048662 [details]
The .spec file difference from Copr build 8064481 to 8069100

Comment 44 Fedora Review Service 2024-09-25 11:25:09 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8069100
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2307912-python-pyliblo3/fedora-rawhide-x86_64/08069100-python-pyliblo3/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.

Comment 45 MartinKG 2024-09-27 08:50:21 UTC
can somebody approve the package if it is ok ?
Thanks Martin

Comment 46 Miro Hrončok 2024-09-27 08:54:11 UTC
Only the bug assignee is allowed to do that.

Comment 47 MartinKG 2024-09-27 09:39:00 UTC
Thanks for the process of review

Comment 48 Fedora Admin user for bugzilla script actions 2024-09-27 10:18:23 UTC
The Pagure repository was created at https://src.fedoraproject.org/rpms/python-pyliblo3


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