Spec URL: https://raw.githubusercontent.com/underground-software/RPi.GPIO2/packaging2/packaging/python-RPi-GPIO2.spec SRPM URL: https://kojipkgs.fedoraproject.org//work/tasks/1552/49801552/python-RPi-GPIO2-0.3.0-1.fc34.src.rpm Description: This library implements a compatibility layer between RPi.GPIO syntax and libgpiod semantics, allowing a fedora user on the Raspberry Pi platform to use the popular RPi.GPIO API, the original implementation of which depends on features provided by a non-mainline kernel. Fedora Account System Username: theyoyojo Koji Build: https://koji.fedoraproject.org/koji/taskinfo?taskID=49801551 This is my first Fedora package submission and I need sponsorship. I am the primary author and upstream maintainer for this project and I am also currently employed by Red Hat. I can be reached internally at jsavitz
Just some comments: - The package should be named "python-rpi-gpio2" not "python-RPi-GPIO2" - There is a mismatch between the releases (upstream is 0.3.0a3, package is 0.3.0 but 0.3.0a3 is used). - The source tarball contains tests. Run them in %check. - %check is for running tests and not maintenance. - Ship the examples as part of %doc or as a subpackage. The same applies for the documentation. - %{python3_sitelib}/RPi/ should be renamed. Looks like that there could be a naming clash if python-rpi-gpio is installed as well.
Source0: https://github.com/underground-software/RPi.GPIO2/archive/v0.3.0a3.tar.gz → Source0: https://github.com/underground-software/RPi.GPIO2/archive/v%{version}a3/%{name}-%{version}a3.tar.gz - Version: 0.3.0 Release: 1%{?dist} → Version: 0.3.0 Release: 1.a3%{?dist} - This is not needed for a noarch package: # This package is pure python code so debuginfo is useless %global debug_package %{nil} Just move: BuildArch: noarch to the main package. - Do this at the end of install: rm -rf %{buildroot}%{python3_sitelib}/examples rm -rf %{buildroot}%{python3_sitelib}/tests - As far as I understand, it needs to run on RPi to do the tests. - Don't mix tabs and spaces - Won't there be an import clash? One is in sitelib, the other in sitearch, but if I import the package, which one will be loaded? Shouln't you change the name of derectories to GPIO2?
(In reply to Fabian Affolter from comment #1) (In reply to Robert-André Mauchin 🐧 from comment #2) > - The package should be named "python-rpi-gpio2" not "python-RPi-GPIO2" Package renamed. > - There is a mismatch between the releases (upstream is 0.3.0a3, package is > 0.3.0 but 0.3.0a3 is used). > > - > > Version: 0.3.0 > Release: 1%{?dist} > > → > > Version: 0.3.0 > Release: 1.a3%{?dist} I added the alpha version number to the Release > - The source tarball contains tests. Run them in %check. > > - As far as I understand, it needs to run on RPi to do the tests. Yes, the tests only run on the actual Raspberry Pi device since they rely on the data structures provided by libgpiod to represent the actual pins. I was considering instrumenting the library to use a fake gpiod module that simulates the pin state in software, but I don't much of a purpose to that other than getting the tests to run on hardware where the library has no use case. I opened an issue for it on the GitHub repo a while ago to track this behavior: https://github.com/underground-software/RPi.GPIO2/issues/16 > - %check is for running tests and not maintenance. > > - Do this at the end of install: > > rm -rf %{buildroot}%{python3_sitelib}/examples > rm -rf %{buildroot}%{python3_sitelib}/tests Got it. Moved the maintenance steps to the end of install. > - This is not needed for a noarch package: > > # This package is pure python code so debuginfo is useless > %global debug_package %{nil} > > Just move: > > BuildArch: noarch > > to the main package. Done. > - Ship the examples as part of %doc or as a subpackage. The same applies for > the documentation. I included the examples as a -doc subpackage, but I have updated the other documentation files since the v0.3.0a3 release. I plan to make another upstream release soon, so I could add those files as an update to this package, or I could do the upstream release first. The documentation is also available at http://rpi.gpio2.underground.software/ > Source0: > https://github.com/underground-software/RPi.GPIO2/archive/v0.3.0a3.tar.gz > > → > > Source0: > https://github.com/underground-software/RPi.GPIO2/archive/v%{version}a3/ > %{name}-%{version}a3.tar.gz > Changed it to the following to reflect the current repo structure, but I could modify the release if that is necessary: Source0: https://github.com/underground-software/%{pypi_name}/archive/v%{version} a3.tar.gz > - Don't mix tabs and spaces I'm not sure where I made this mistake. Did you spot this in the spec? The source files were linted with flake8 but I certainly may have missed something. > - Won't there be an import clash? One is in sitelib, the other in sitearch, > but if I import the package, which one will be loaded? Shouln't you change > the name of derectories to GPIO2? > - %{python3_sitelib}/RPi/ should be renamed. Looks like that there could be > a naming clash if python-rpi-gpio is installed as well. Yes, there is a naming clash and this is intentional. python-rpi-gpio is completely broken and does not work on the latest releases of Fedora as it relies on non-mainline kernel functionality or access to /dev/mem, disallowed in Fedora. When one attempts to use python-rpi-gpio, e.g. by entering `>>> import RPi.GPIO` at the python REPL, the following error is shown: Traceback (most recent call last): File "./callback.py", line 2, in <module> import RPi.GPIO as GPIO File "/usr/lib64/python3.7/site-packages/RPi/GPIO/__init__.py", line 23, in <module> from RPi._GPIO import * RuntimeError: This module can only be run on a Raspberry Pi! I propose that python-rpi-gpio be dropped from Fedora as it is broken on the Raspberry Pi, its only use case. This was my original motivation to develop this library, to provide a working transparent drop-in replacement for the RPi.GPIO API to allow users to make use of the RPi.GPIO API as it is widely used in tutorials and higher-level libraries (e.g. gpiozero), and enable more functionality for users of the Raspberry Pi who wish to run Fedora as their OS. Updated spec: https://raw.githubusercontent.com/underground-software/RPi.GPIO2/packaging2/packaging/python-RPi-GPIO2.spec Updated koji build: https://koji.fedoraproject.org/koji/taskinfo?taskID=50280565
>> - Don't mix tabs and spaces > I'm not sure where I made this mistake. Did you spot this in the spec? The source files were linted with flake8 but I certainly may have missed something. Yes in the SPEC, not the source code. There's a tab line 2: %global pypi_name RPi.GPIO2 > I propose that python-rpi-gpio be dropped from Fedora as it is broken on the Raspberry Pi, its only use case. Please discuss this with the current maintainers of RPi.GPIO before any action. If they accept, and if the functionality are the same, add an Obsoletes/Provides for the old package. - please rename you SPEC python-rpi-gpio2 and also rename this bug as well. - fix the changelog entry to reflect the header: * Wed Aug 19 2020 Joel Savitz <joelsavitz> - 0.3.0-1.a3 - Please include the name in your archive: Source0: https://github.com/underground-software/%{pypi_name}/archive/v%{version}a3/%{pypi_name}-%{version}a3.tar.gz I won't approve the package until the name clash is resolved with the maintainers of the v1. Ark them to comment in this bug if they're okay with it.
(In reply to Robert-André Mauchin 🐧 from comment #4) > >> - Don't mix tabs and spaces > > I'm not sure where I made this mistake. Did you spot this in the spec? The source files were linted with flake8 but I certainly may have missed something. > > > Yes in the SPEC, not the source code. There's a tab line 2: > > %global pypi_name RPi.GPIO2 Fixed. > > > - please rename you SPEC python-rpi-gpio2 and also rename this bug as well. Done. > > - fix the changelog entry to reflect the header: > > * Wed Aug 19 2020 Joel Savitz <joelsavitz> - 0.3.0-1.a3 Done. > > - Please include the name in your archive: > > Source0: > https://github.com/underground-software/%{pypi_name}/archive/v%{version}a3/ > %{pypi_name}-%{version}a3.tar.gz > Done. > > > I propose that python-rpi-gpio be dropped from Fedora as it is broken on the Raspberry Pi, its only use case. > > > Please discuss this with the current maintainers of RPi.GPIO before any > action. If they accept, and if the functionality are the same, add an > Obsoletes/Provides for the old package. > > > I won't approve the package until the name clash is resolved with the > maintainers of the v1. Ark them to comment in this bug if they're okay with > it. Will needinfo the maintainers. Kushal and Fabian, what do you think of my proposal to replace python-rpi-gpio with python-rpi-gpio2? Updated spec: https://raw.githubusercontent.com/underground-software/RPi.GPIO2/packaging2/packaging/python-rpi-gpio2.spec Updated koji build: https://koji.fedoraproject.org/koji/taskinfo?taskID=50339420
(In reply to Joel Savitz from comment #5) > (In reply to Robert-André Mauchin 🐧 from comment #4) > > > > I won't approve the package until the name clash is resolved with the > > maintainers of the v1. Ark them to comment in this bug if they're okay with > > it. > Will needinfo the maintainers. Kushal and Fabian, what do you think of my > proposal to replace python-rpi-gpio with python-rpi-gpio2? I fine with dropping python-rpi-gpio but ultimately it's Kushal's call.
(In reply to Robert-André Mauchin 🐧 from comment #4) > I won't approve the package until the name clash is resolved with the > maintainers of the v1. Ark them to comment in this bug if they're okay with > it. I asked Kushal if he'd be OK with dropping the v1 via email in early September and he replied with his approval, but I have not been able to get ahold of him since. Is there anything else I can do to move this process forward?
Any news on that? This package is very interesting.
What is it waiting for now? I made my own package and spec for it, used on my COPR [1]. Since upstream contains version RPi.GPIO2 in the name, I think it should be separate package anyway. According to comment #5, it was renamed and does not have to wait for original maintainer's approval. It just have to Conflicts: with it, because it provides python module of the same name. According to my tests, this is more useful than the old one, because it works on Fedora kernel. Can we move to formal review, Robert? 1. https://copr.fedorainfracloud.org/coprs/pemensik/raspberry-pi/
(In reply to Petr Menšík from comment #9) > What is it waiting for now? I made my own package and spec for it, used on > my COPR [1]. > > Since upstream contains version RPi.GPIO2 in the name, I think it should be > separate package anyway. According to comment #5, it was renamed and does > not have to wait for original maintainer's approval. It just have to > Conflicts: with it, because it provides python module of the same name. > According to my tests, this is more useful than the old one, because it > works on Fedora kernel. > > Can we move to formal review, Robert? > > 1. https://copr.fedorainfracloud.org/coprs/pemensik/raspberry-pi/ Hello Petr, thanks for doing a rebuild and helping push this along. I called it RPi.GPIO2 to distinguish it from the original implementation. At this point I'd be fine with whatever naming needs to be done in order to get this into Fedora. I had briefly been in touch with Peter Robinson about this package and he suggested trying to get this functionality integrated with Ben Croston's original RPi.GPIO, but I have had no luck getting ahold of him via email. The advantage of obsoleting the broken RPi.GPIO package would be that the replacement would be fully transparent, but this package, with the '2', would still come up via a `dnf search RPi.GPIO`. However, due to the fact that the fedora RPi.GPIO package has been quietly broken for years at this point, I don't think many people were using it in the first place. Fedora was somewhat painful to use on the rpi3 in my experience, however usage on the rpi4 is much better and I think there is much more potential for Fedora on these newer models. Recently I tested this library on my rpi4B and besides a few needed tweaks to the tests it worked very well. Anyway, I have been emailing Kushal every now and then since I saw comment 4, but I have not heard from him and I am not sure how to move this forward.
I was out due to many family health issues and also full COVID and near death experience. Anyway, I retired the package from both f35 and rawhide branch.
I am co-maintainer of the RPi.GPIO2 package. Here is an updated build for f34 and rawhide for review. Robert, is this package ready for formal review? f34: https://koji.fedoraproject.org/koji/taskinfo?taskID=78366949 rawhide: https://koji.fedoraproject.org/koji/taskinfo?taskID=78368438
(In reply to mguma from comment #12) > I am co-maintainer of the RPi.GPIO2 package. Here is an updated build for > f34 and rawhide for review. > Robert, is this package ready for formal review? > > f34: > > https://koji.fedoraproject.org/koji/taskinfo?taskID=78366949 > > rawhide: > > https://koji.fedoraproject.org/koji/taskinfo?taskID=78368438 Please include a link to the spec and srpm so `fedora-review` works properly. Spec URL: ... SRPM URL: ...
Here is the spec and SRPM urls Spec URL: https://github.com/underground-software/RPi.GPIO2/blob/packaging3/packaging/python-rpi-gpio2.spec SRPM URL: https://github.com/underground-software/RPi.GPIO2/blob/packaging3/packaging/python-rpi-gpio2-0.3.0-1.a3.el8.src.rpm
I am resetting the assignee, as eclipseo seems to be unavailable. This way, this package will appear under the "New Tickets" section of the review tracker and someone else can take it on.
(In reply to mguma from comment #14) > Here is the spec and SRPM urls > > Spec URL: > https://github.com/underground-software/RPi.GPIO2/blob/packaging3/packaging/ > python-rpi-gpio2.spec > > SRPM URL: > https://github.com/underground-software/RPi.GPIO2/blob/packaging3/packaging/ > python-rpi-gpio2-0.3.0-1.a3.el8.src.rpm Please link to the raw files, not the Github HTML pages. Otherwise, fedora-review won't work properly. Spec URL: https://github.com/underground-software/RPi.GPIO2/raw/packaging3/packaging/python-rpi-gpio2.spec SRPM URL: https://github.com/underground-software/RPi.GPIO2/raw/packaging3/packaging/python-rpi-gpio2-0.3.0-1.a3.el8.src.rpm
Here are a couple comments: > mkdir -p %{buildroot}%_pkgdocdir > cp -r %{buildroot}%{python3_sitelib}/examples %{buildroot}%_pkgdocdir Why don't you just add the relative path with the `%doc` directive in the `%files` list? You can change `%_pkgdocdir/examples` to `%doc examples` and remove those two lines. > Version: 0.3.0 > Release: 1.a3%{?dist} Why don't you just set the version to `0.3.0.a3`? > %{?python_provide:%python_provide python3-%{pypi_name}} This line is not necessary. > Provides: python3-RPi.GPIO This line and the other virtual provides should provide a specific version. You should change `Provides: python3-RPi.GPIO` to `Provides: python3-RPi.GPIO = %{version}-%{release}`. While the srpm is named correctly, the python3 and docs subpackages are still misnamed. For the docs subpackage, you can shorten `-n python-%{pypi_name}-doc` to `doc` in the `%package`, `%description`, and `%files` directives. I would also add `%global pkgname rpi-gpio2` and replace `%{pypi_name}` with `%{pkgname}` where appropriate. I would recommend adding `Recommends: python-%{pkgname}-doc` to the `python3-%{pkgname}` subpackage, but that's up to you.
Also, who is planning to submit the package? fedscm-admin will not work if the review bug creator is different than the package submitter.
(In reply to Maxwell G from comment #18) > Also, who is planning to submit the package? fedscm-admin will not work if > the review bug creator is different than the package submitter. Joel should be able to get it set up initially and then add co-maintainers.
I have made the necessary changes to address the comments above. Here is the Spec and SRPM urls Spec URL: https://raw.githubusercontent.com/underground-software/RPi.GPIO2/packaging3/packaging/python-rpi-gpio2.spec SRPM URL: https://github.com/underground-software/RPi.GPIO2/blob/packaging3/python-rpi-gpio2-0.3.0a3-1.el8.src.rpm?raw=true
Robert, are you still interested in reviewing this? It's been a while since you've commented.
I've fixed your URLs to make fedora-review work properly. Spec URL: https://github.com/underground-software/RPi.GPIO2/raw/packaging3/packaging/python-rpi-gpio2.spec SRPM URL: https://github.com/underground-software/RPi.GPIO2/raw/packaging3/python-rpi-gpio2-0.3.0a3-1.el8.src.rpm
I'll take over this review from here. You will still need to find a sponsor. I have a couple more notes, but then, we should be good to go! Thank you for your patience. Notes ===== - > URL: https://pypi.org/project/RPi.GPIO2/ > Source0: https://github.com/underground-software/%{pypi_name}/archive/v%{version}/%{pypi_name}-%{version}.tar.gz I would change this to: URL: https://github.com/underground-software/%{pkgname} Source0: %{url}/archive/v%{version}/%{pkgname}-%{version}.tar.gz - You should fix the shebangs in the examples/ directory to all use #!/usr/bin/python3. Currently, there is a mix of different shebangs in use. ``` $ rg /bin examples examples/pwm3.py:#!/bin/python3 examples/random_usage.py:#!/bin/python3 examples/pwm2.py:#!/bin/python3 examples/callback.py:#!/usr/bin/python3 examples/button_test.py:#!/bin/env python3 examples/pwm4.py:#!/bin/python3 examples/callback2.py:#!/bin/python3 examples/output_examples.py:#!/bin/python3 examples/try_import.py:#!/bin/python3 examples/morse.py:#!/bin/env python3 examples/flash18.py:#!/bin/python3 examples/pwm1.py:#!/bin/python3 examples/input_examples.py:#!/bin/python3 examples/pwm_usage.py:#!/bin/python3 examples/event_test.py:#!/bin/python3 ``` - You should add ``` %check %py3_check_import RPi ``` as a basic smoke test and add a comment explaining why you can't run the actual tests. - The python3 subpackage is still improperly named. It should be named `python3-rpi-gpio2`. Please see the Python Naming Guidelines[1] for more info. When I suggested to use a separate `pkgname` variable, I meant to keep `pypi_name` for referring to the upstream name (RPi.GPIO2) and add `pkgname` for referring to the package name (rpi-gpio2). Right now, the `Recommends: python-%{pkgname}-doc` line evaluates to `Recommends: python-RPi.GPIO2-doc` when it should be `Recommends: python-rpi-gpio2-doc`. Adding back `%pypi_name` and changing `%pkgname` to `rpi-gpio2` will fix this isue. [1]: https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_naming - The source package's `Provides` line should also be fully versioned. Provides: python-rpi-gpio = %{version}-%{release} - Please note that you cannot push this package to `f34`, because the original package has not been retired on that branch; packages cannot be retired from stable releases. Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated ===== 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. [x]: License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "Unknown or generated", "*No copyright* GNU General Public License v3.0 or later", "*No copyright* GNU General Public License, Version 3". 36 files have unknown license. Detailed output of licensecheck in /home/gotmax/Sync/git- repos/packaging/fedora_rpms/review.repos/1871171-python-rpi- gpio2/licensecheck.txt [x]: License file installed when any subpackage combination is installed. [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. [x]: 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]: Package is not known to require an ExcludeArch tag. [x]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 10240 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]: 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]: 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]: 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]: Packages must not store files under /srv, /opt or /usr/local Python: [ ]: Python eggs must not download any dependencies during the build process. [x]: 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). [x]: Fully versioned dependency in subpackages if applicable. Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in python3-RPi.GPIO2 [?]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [-]: Sources are verified with gpgverify first in %prep if upstream publishes signatures. Note: gpgverify is not used. [-]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [x]: Package should compile and build into binary rpms on all supported architectures. [!]: %check is present and all tests pass. See my note above. [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: There are rpmlint messages (see attachment). [x]: Spec file according to URL is the same as in SRPM. Rpmlint ------- Cannot parse rpmlint output: Rpmlint (installed packages) ---------------------------- Cannot parse rpmlint output: Source checksums ---------------- https://github.com/underground-software/RPi.GPIO2/archive/v0.3.0a3/RPi.GPIO2-0.3.0a3.tar.gz : CHECKSUM(SHA256) this package : 018d400f662eeba78952a078392b599d47848662122d546b4ce01c9841e3b8e3 CHECKSUM(SHA256) upstream package : 018d400f662eeba78952a078392b599d47848662122d546b4ce01c9841e3b8e3 Requires -------- python3-RPi.GPIO2 (rpmlib, GLIBC filtered): python(abi) python3-libgpiod python-rpi-gpio2-doc (rpmlib, GLIBC filtered): Provides -------- python3-RPi.GPIO2: python-RPi.GPIO2 python3-RPi.GPIO python3-RPi.GPIO2 python3.10-RPi.GPIO2 python3.10dist(rpi-gpio2) python3.10dist(rpi.gpio2) python3dist(rpi-gpio2) python3dist(rpi.gpio2) python-rpi-gpio2-doc: python-rpi-gpio2-doc Generated by fedora-review 0.7.6 (b083f91) last change: 2020-11-10 Command line :/usr/bin/fedora-review -b 1871171 Buildroot used: fedora-rawhide-x86_64 Active plugins: Generic, Shell-api, Python Disabled plugins: SugarActivity, Perl, Java, Ocaml, R, C/C++, fonts, Haskell, PHP Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH
I can sponsor.
I have made changes to fix the package naming and shebangs in the examples files. Here is a the SPEC and SRPM urls SPEC : https://raw.githubusercontent.com/underground-software/RPi.GPIO2/packaging3/packaging/python-rpi-gpio2.spec SRPM : https://github.com/underground-software/RPi.GPIO2/blob/packaging3/python-rpi-gpio2-0.3.0a3-1.fc35.src.rpm
I've fixed your URLs to make fedora-review work properly. Spec URL: https://github.com/underground-software/RPi.GPIO2/raw/packaging3/packaging/python-rpi-gpio2.spec SRPM URL: https://github.com/underground-software/RPi.GPIO2/raw/packaging3/python-rpi-gpio2-0.3.0a3-1.fc35.src.rpm The spec url was fine, but the SRPM url links to an HTML page instead of the raw SRPM.
Thank you for your patience and for applying my changes, Mguma! > Name: python-rpi-gpio2 I would change this to `python-%{pkgname}`. It looks like you fixed the shebang issue in this commit[1] on the git branch that you're using to host this spec file, but not in the actual package; the source archive is unchanged. You would have to release a new version or add a patch containing this change to the specfile/srpm for it to actually be reflected in the package. This is the only remaining issue. Once this package is approved, Joel will have to import the package, as he is the owner of this ticket. Then, he can you as a co-maintainer. Of course, Matthew will have to sponsor you both into the `packager` group before this can happen. [1]: https://github.com/underground-software/RPi.GPIO2/commit/1b160bc74b4d5d8ae7748102ecfde8d0582494db
After a long delay due to the gpio subsystem breaking in Fedora [0], we have released a new version of the package. I believe we have addressed all the issues. Spec URL: https://raw.githubusercontent.com/underground-software/RPi.GPIO2/packaging-0.4.0-1/packaging/python-rpi-gpio2.spec SRPM URL: https://raw.githubusercontent.com/underground-software/RPi.GPIO2/packaging-0.4.0-1/python-rpi-gpio2-0.4.0-1.fc36.src.rpm [0] https://bugzilla.redhat.com/show_bug.cgi?id=2060490 Please let me know if you would like any more changes.
I'm looking at this with fresh eyes and more packaging experience, so apologizes that I'm pointing things out that I didn't before: Consider using following the new Python Packaging Guidelines[1] and the new pyproject macros. Particularly, look at the example specfile[2]. I use this for all of my packages, except if I'm planning to branch them for epel7 and/or epel8. Note that the old macros and the old Python Packaging Guidelines[3] are still valid. [1]: https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/ [2]: https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_empty_spec_file [3]: https://docs.fedoraproject.org/en-US/packaging-guidelines/Python_201x/ > rm -rf %{buildroot}%{python3_sitelib}/tests > rm -rf %{buildroot}%{python3_sitelib}/examples This indicates a problem with your setup.py/setup.cfg configuration. You should fix this upstream so it doesn't include these directories as `packages`, but I won't block the review on it. > # python-RPi-GPIO.spec This seems unnecessary to me, but meh. > %global pkgname rpi-gpio2 > %global pypi_name RPi.GPIO2 I do not like having all of these *name macros. They make the specfile harder to read. I'd suggest using the the the actual values in the appropriate places, instead. ``` %check %py3_check_import RPi # the tests rely on the presence of the actual physical GPIO pins on the system for now and though we may develop emulation functionality to run the tests on any system in the future we think the software is ready to be packaged as-is and we will just update it when the better tests are done ``` Please put the %check block below %install. > Recommends: python-%{pkgname}-doc I'd recommend against this. Having the main package pull in the doc subpackage kind of negates the point of having that in the first place. > License: GPLv3+ Fedora now has new licensing guidelines and uses SPDX license identifiers. This should be `GPL-3.0-or-later.
I will run this through the fedora-review tool after you've applied those changes. I am now a packager sponsor, so I should be able to sponsor you myself.
Ok, I just made all of your recommended changes (besides the issues with setup.*).
Please provide a link to the new SRPM and specfile.
(In reply to Maxwell G from comment #32) > Please provide a link to the new SRPM and specfile. Ah, it looks like the links are the same and you updated it in Git: https://github.com/underground-software/RPi.GPIO2/commit/fc7ba311c2e870015f5187b1cba78ce0784fd37c https://github.com/underground-software/RPi.GPIO2/commit/57e408a39a8956472294217e0a4c190b7df3a4ad You don't have to copy and paste the same link each time, but it would be useful if you at least told me each time that it's the same.
(In reply to Maxwell G from comment #29) > I'm looking at this with fresh eyes and more packaging experience, so > apologizes that I'm pointing things out that I didn't before: > > Consider using following the new Python Packaging Guidelines[1] and the new > pyproject macros. Particularly, look at the example specfile[2]. I use this > for all of my packages, except if I'm planning to branch them for epel7 > and/or epel8. Note that the old macros and the old Python Packaging > Guidelines[3] are still valid. > > [1]: https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/ > [2]: > https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/ > #_empty_spec_file > [3]: https://docs.fedoraproject.org/en-US/packaging-guidelines/Python_201x/ This was optional, so it's fine that you didn't implement it. I still recommend doing so, however. > > # python-RPi-GPIO.spec > > This seems unnecessary to me, but meh. > > > %global pkgname rpi-gpio2 > > %global pypi_name RPi.GPIO2 > > I do not like having all of these *name macros. They make the specfile > harder to read. I'd suggest using the the the actual values in the > appropriate places, instead. Done. However, it seems the SRPM is named python-rpi-gpio2, while the python3 subpackage is named python3-RPi.GPIO2. It should also be named python3-rpi-gpio2. > > ``` > %check > %py3_check_import RPi > > > # the tests rely on the presence of the actual physical GPIO pins on the > system for now and though we may develop emulation functionality to run the > tests on any system in the future we think the software is ready to be > packaged as-is and we will just update it when the better tests are done > > ``` > > Please put the %check block below %install. Fixed. Thank you. > > > Recommends: python-%{pkgname}-doc > > I'd recommend against this. Having the main package pull in the doc > subpackage kind of negates the point of having that in the first place. Fixed. Thank you. > > > License: GPLv3+ > > Fedora now has new licensing guidelines and uses SPDX license identifiers. > This should be `GPL-3.0-or-later. Fixed. Thank you.
Okay, I ran this through fedora-review. In addition to the comment about naming above, it uncovered a couple more small issues. Thank you for sticking with this! Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated ==== Reviewer Notes ==== > %package doc > Summary: Examples for python-rpi-gpio2 > %description doc > A set of examples for python-rpi-gpio2 I would recommend adding a newline or two before %description to make the spec file more readable The package obsoleting does not appear correct to me. Here is how I'd approach it: > Obsoletes: python-rpi-gpio = %{version}-%{release} > Provides: python-rpi-gpio These shouldn't be necessary. There is no binary package named python-rpi-gpio2. > Obsoletes: python3-RPi.GPIO <= 0.7.1 The constraint should be "< 0.7.0-7". See https://docs.fedoraproject.org/en-US/packaging-guidelines/#renaming-or-replacing-existing-packages. > Provides: python3-RPi.GPIO = %{version}-%{release} I would make this python3-RPI.GPIO = 1:%{version}-%{release}. Normally, we try to avoid epochs, but this is necessary, as the latest version of rpi-gpio2 sorts before the last version of rpi-gpio. ===== MUST items ===== Generic: [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [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: "Unknown or generated", "*No copyright* GNU General Public License v3.0 or later". 40 files have unknown license. Detailed output of licensecheck in /tmp/tmp.aolsHa4fGS/python-rpi- gpio2/licensecheck.txt [x]: License file installed when any subpackage combination is installed. [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). [!]: Package is named according to the Package Naming Guidelines. See my previous comment. [?]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [x]: 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]: Package is not known to require an ExcludeArch tag. [x]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 10240 bytes in 1 files. [x]: Package complies to the Packaging Guidelines [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]: 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]: 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]: 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]: 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: [x]: Reviewer should test that the package builds in mock. [-]: 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). [-]: Fully versioned dependency in subpackages if applicable. Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in python3-RPi.GPIO2 [?]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [-]: 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. Noarch package. [x]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [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: There are rpmlint messages (see attachment). Rpmlint ------- Cannot parse rpmlint output: Rpmlint (installed packages) ---------------------------- ============================ rpmlint session starts ============================ rpmlint: 2.2.0 configuration: /usr/lib/python3.11/site-packages/rpmlint/configdefaults.toml /etc/xdg/rpmlint/fedora.toml /etc/xdg/rpmlint/licenses.toml /etc/xdg/rpmlint/scoring.toml /etc/xdg/rpmlint/users-groups.toml /etc/xdg/rpmlint/warn-on-functions.toml checks: 32, packages: 2 python3-RPi.GPIO2.noarch: W: self-obsoletion python3-RPi.GPIO <= 0.7.1 obsoletes python3-RPi.GPIO = 0.4.0-1.fc38 See the note above. python-rpi-gpio2-doc.noarch: W: invalid-license GPL-3.0-or-later python3-RPi.GPIO2.noarch: W: invalid-license GPL-3.0-or-later This is okay. rpmlint is not yet aware of the new licensing guidelines. 2 packages and 0 specfiles checked; 0 errors, 3 warnings, 0 badness; has taken 0.0 s Source checksums ---------------- https://github.com/underground-software/RPi.GPIO2/archive/v0.4.0/RPi.GPIO2-0.4.0.tar.gz : CHECKSUM(SHA256) this package : fe2f7ff0ce98a814b885be973be7976fe84e27fe15f69e7ef799e9ac4d8a5b06 CHECKSUM(SHA256) upstream package : fe2f7ff0ce98a814b885be973be7976fe84e27fe15f69e7ef799e9ac4d8a5b06 Requires -------- python3-RPi.GPIO2 (rpmlib, GLIBC filtered): python(abi) python3-libgpiod python-rpi-gpio2-doc (rpmlib, GLIBC filtered): Provides -------- python3-RPi.GPIO2: python-RPi.GPIO2 python3-RPi.GPIO python3-RPi.GPIO2 python3.11-RPi.GPIO2 python3.11dist(rpi-gpio2) python3dist(rpi-gpio2) python-rpi-gpio2-doc: python-rpi-gpio2-doc Generated by fedora-review 0.9.0 (6761b6c) last change: 2022-08-23 Command line :/usr/bin/fedora-review -prn python-rpi-gpio2 Buildroot used: fedora-rawhide-x86_64 Active plugins: Generic, Shell-api, Python Disabled plugins: Ocaml, C/C++, Perl, R, fonts, PHP, Haskell, Java, SugarActivity Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH
Ok I updated the specfile. Links to specfile and SRPM should be the same as before.
(In reply to Maxwell G from comment #35) > Okay, I ran this through fedora-review. In addition to the comment about > naming above, it uncovered a couple more small issues. You still need to fix this. From comment 34: > However, it seems the SRPM is named python-rpi-gpio2, while the python3 subpackage is named python3-RPi.GPIO2. It should also be named python3-rpi-gpio2. > ==== Reviewer Notes ==== > > %package doc > > Summary: Examples for python-rpi-gpio2 > > %description doc > > A set of examples for python-rpi-gpio2 > > I would recommend adding a newline or two before %description to make the > spec file more readable Done, thanks! > > The package obsoleting does not appear correct to me. Here is how I'd > approach it: > > > Obsoletes: python-rpi-gpio = %{version}-%{release} > > Provides: python-rpi-gpio > > These shouldn't be necessary. There is no binary package named > python-rpi-gpio2. Done. > > > Obsoletes: python3-RPi.GPIO <= 0.7.1 > > The constraint should be "< 0.7.0-7". See > https://docs.fedoraproject.org/en-US/packaging-guidelines/#renaming-or- > replacing-existing-packages. > > > Provides: python3-RPi.GPIO = %{version}-%{release} > > I would make this python3-RPI.GPIO = 1:%{version}-%{release}. > > Normally, we try to avoid epochs, but this is necessary, as the latest > version of rpi-gpio2 sorts before the last version of rpi-gpio. Done. Thanks Nitpick that you can choose to implement or not: ``` %description doc A set of examples for python-rpi-gpio2 ``` I'd include the main description here as well. Something like this would work: ``` %description doc %{_description} A set of examples for python-rpi-gpio2 ``` Besides the aforementioned naming issue and the %description nitpick, this looks good to me. Please make sure to fix those issues and send a self-introduction[1] to the devel mailing list [1]: https://docs.fedoraproject.org/en-US/package-maintainers/Joining_the_Package_Maintainers/#introduce_yourself
Ok, I believe I have fixed the remaining problems. I put the new spec and SRPM in the same place. I will send a self-introduction soon.
Thank you for incorporating all of my feedback and being patient. I have approved your package and will sponsor you into the packager group. Look out for an email from me about how to import your package into Fedora. Welcome! Please line wrap the "# The tests rely on the presence of the actual physical GPIO pins on the system for now..." comment on import, as well.
(In reply to Maxwell G from comment #39) > Thank you for incorporating all of my feedback and being patient. I have > approved your package and will sponsor you into the packager group. Look out > for an email from me about how to import your package into Fedora. Welcome! > > Please line wrap the "# The tests rely on the presence of the actual > physical GPIO pins on the system for now..." comment on import, as well. Hi, I don't think I received an email about importing the package. What would the subject of the email be? And would it be from the email listed for your bugzilla account?
Hi, I wanted to follow up on this. Could you please resend the email? Thanks again for the review/sponsorship.
I'm so sorry! I'm usually pretty on top of my Bugzilla inbox, but it seems I've missed your follow up. I will resend it.
(In reply to Maxwell G from comment #42) > I'm so sorry! I'm usually pretty on top of my Bugzilla inbox, but it seems > I've missed your follow up. I will resend it. (I have a meeting now, so I'll do it after.)
I sent the email "Fedora packager Status" from gotmax to joelsavitz on September 8th. Gmail likes to send email from smaller email servers to spam :(. I've just resent it. Let me know if I should instead send it your RH email.
Ah I see. I just got the email. I checked spam and searched for emails containing the subject "Fedora packager Status" but I only see your latest. Strange. Anyway, thank you for the quick response. I'll take a look at your instructions.
(fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/python-rpi-gpio2
FEDORA-2022-d8761f282e has been submitted as an update to Fedora 38. https://bodhi.fedoraproject.org/updates/FEDORA-2022-d8761f282e
FEDORA-2022-d8761f282e has been pushed to the Fedora 38 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-2022-0b86e5ee22 has been submitted as an update to Fedora 37. https://bodhi.fedoraproject.org/updates/FEDORA-2022-0b86e5ee22
FEDORA-2022-48f71dcb21 has been submitted as an update to Fedora 35. https://bodhi.fedoraproject.org/updates/FEDORA-2022-48f71dcb21
FEDORA-2022-0b86e5ee22 has been pushed to the Fedora 37 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2022-0b86e5ee22 \*` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2022-0b86e5ee22 See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2022-cca6d008ac has been pushed to the Fedora 36 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2022-cca6d008ac \*` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2022-cca6d008ac See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2022-48f71dcb21 has been pushed to the Fedora 35 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2022-48f71dcb21 \*` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2022-48f71dcb21 See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2022-cca6d008ac has been pushed to the Fedora 36 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-2022-48f71dcb21 has been pushed to the Fedora 35 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-2022-0b86e5ee22 has been pushed to the Fedora 37 stable repository. If problem still persists, please make note of it in this bug report.