SPEC: https://pbrobinson.fedorapeople.org/python-can.spec SRPM: https://pbrobinson.fedorapeople.org/python-can-1.5.2-1.fc25.src.rpm Description: The Controller Area Network is a bus standard designed to allow microcontrollers and devices to communicate with each other. It has priority based bus arbitration, reliable deterministic communication. It is used in cars, trucks, boats, wheelchairs and more. The can package provides controller area network support for Python developers; providing common abstractions to different hardware devices, and a suite of utilities for sending and receiving messages on a can bus. koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=15658619
The packaging tempalte from https://fedoraproject.org/wiki/Packaging:Python#Example_common_spec_file should be used. It's much better to use %pyX_build, etc. The description should be wrapped to <= 80 columns, this is even visible in the way that bugzilla formats the description above. Is python fully supported? Than the executables should use python3 not python2. You can replace the second and subsequent Summary texts with %{summary}, no need to repeat. Also, don't repeat the %description, it makes the spec file much harder to read than necessary: %global _description \ XXX \ YYY %description -n package-name %{_description} etc. In %files, please enumerate the items under %{pythonX_sitelib}/ explicitly. Something like %{python2_sitelib}/libname %{python2_sitelib}/libname-%{version}.egg-info
> You can replace the second and subsequent Summary texts with %{summary}, no > need to repeat. > > > Also, don't repeat the %description, it makes the spec file much harder to > read than necessary: > > %global _description \ > XXX \ > YYY > > %description -n package-name %{_description} > > etc. > > In %files, please enumerate the items under %{pythonX_sitelib}/ explicitly. > Something like > %{python2_sitelib}/libname > %{python2_sitelib}/libname-%{version}.egg-info All of the above is a matter of opinion and hence the choice of the maintainer.
SPEC: as above SRPM: https://pbrobinson.fedorapeople.org/python-can-1.5.2-2.fc25.src.rpm koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=16708332
> All of the above is a matter of opinion and hence the choice of the maintainer. Sure, this was just a suggestion. I do think it makes life easier (certainly for a reviewer, but I think also for the maintainer). The license is specified incorrectly: it's LGPLv3+ according to the LICENSE file. rpmlint: python2-can.noarch: E: script-without-shebang /usr/bin/j1939_logger.py python2-can.noarch: E: wrong-script-interpreter /usr/lib/python2.7/site-packages/can/util.py /usr/bin/env python3 python2-can.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/can/util.py 644 /usr/bin/env python3 python2-can.noarch: E: wrong-script-interpreter /usr/lib/python2.7/site-packages/can/broadcastmanager.py /usr/bin/env python3 python2-can.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/can/broadcastmanager.py 644 /usr/bin/env python3 At least the first should be fixed. Requires -------- python2-can (rpmlib, GLIBC filtered): /usr/bin/python3 <--------------- this looks wrong python(abi) Provides -------- python3-can: python-can <-------------------- this looks wrong python3-can python3.5dist(python-can) python3dist(python-can) Oh, it's a typo in python_provide for the py3 subpackage.
Fixed the typo in requires and the license (I think I got that from the PyPi site). I'll report the lint issues upstream. SPEC: same SRPM: https://pbrobinson.fedorapeople.org/python-can-1.5.2-3.fc25.src.rpm
License and Provides is fixed, but not Requires and the shebang lines. python2-can.noarch: E: wrong-script-interpreter /usr/lib/python2.7/site-packages/can/broadcastmanager.py /usr/bin/env python3 python2-can.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/can/broadcastmanager.py 644 /usr/bin/env python3 python2-can.noarch: E: script-without-shebang /usr/bin/j1939_logger.py python2-can.noarch: E: wrong-script-interpreter /usr/lib/python2.7/site-packages/can/util.py /usr/bin/env python3 python2-can.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/can/util.py 644 /usr/bin/env python3 Requires -------- python2-can (rpmlib, GLIBC filtered): /usr/bin/python3
(In reply to Zbigniew Jędrzejewski-Szmek from comment #6) > License and Provides is fixed, but not Requires and the shebang lines. The shebang I was going to report upstream and patch once upstream has agreement on it. > python2-can (rpmlib, GLIBC filtered): > /usr/bin/python3 No idea where that comes from unless one of the binaries are some how specifying it but they don't look to be. Will need to investigate later when I get time.
(In reply to Peter Robinson from comment #7) > (In reply to Zbigniew Jędrzejewski-Szmek from comment #6) > > License and Provides is fixed, but not Requires and the shebang lines. > > The shebang I was going to report upstream and patch once upstream has > agreement on it. There are two scripts there: can_logger.py with "#!/usr/bin/python2 -s", and j1939_logger.py without any header. I don't think there's anything to discuss here, the second one has to be the same as the first. > > python2-can (rpmlib, GLIBC filtered): > > /usr/bin/python3 > > No idea where that comes from unless one of the binaries are some how > specifying it but they don't look to be. Will need to investigate later when > I get time. When building on my F24 machine, the header is as quoted above. When building in rawhide mock, the header is "#!/usr/bin/python3 -s", and that generates the bad dependency in python2-can. Either something changed in rawhide, or it's missing some dependency that influences the header.
I need python-can for another package. It would be nice if we could go on with the review. BTW, the latest release 3.3.1 [1]. [1] https://github.com/hardbyte/python-can/releases
As of comment #8, there were outstanding dependency issues. I think it'd be fine to close this one as dead review, and simply open a new review. Feel free to assign me as reviewer.
... or if Peter wants to pick up the packaging, that'd be great of course too. Sorry, I shouldn't have discounted this possibility. tl;dr: I'm happy to finish either this review or a new one, whatever works.
I'll review and update, apologies I missed there was something awaiting me.
Thanks guys.
Updated for review and to latest 3.3.1 upstream release. Dropped the py2 sub-package too. SPEC: as above SRPM: https://pbrobinson.fedorapeople.org/python-can-3.3.1-1.fc30.src.rpm koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=36610367
/usr/lib/python3.7/site-packages/test/ → please either move this under /usr/lib/python3.7/site-packages/can/, or remove from the package. rpmlint says: python3-can.noarch: E: wrong-script-interpreter /usr/lib/python3.7/site-packages/test/data/__init__.py /usr/bin/env python python3-can.noarch: E: non-executable-script /usr/lib/python3.7/site-packages/test/data/__init__.py 644 /usr/bin/env python python3-can.noarch: E: wrong-script-interpreter /usr/lib/python3.7/site-packages/test/data/example_data.py /usr/bin/env python python3-can.noarch: E: non-executable-script /usr/lib/python3.7/site-packages/test/data/example_data.py 644 /usr/bin/env python ... but I don't think it matters much (no dependency is autogenerated). Probably not worth fixing. + latest version + builds and installs OK + package name is OK + follows standard python spec + P/R/BR look OK + license is acceptable (LGPLv3) + license is specified correctly Packages is approved. Please fix that one issue pointed out above.
(In reply to Zbigniew Jędrzejewski-Szmek from comment #15) > /usr/lib/python3.7/site-packages/test/ → please either move this under > /usr/lib/python3.7/site-packages/can/, > or remove from the package. Will remove. Thanks
(fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/python-can