https://bugzilla.redhat.com/show_bug.cgi?id=1871171

Joel Savitz joelsavitz@gmail.com changed:

What |Removed |Added ---------------------------------------------------------------------------- Blocks| |177841 (FE-NEEDSPONSOR)

Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=177841 [Bug 177841] Tracker: Review requests from new Fedora packagers who need a sponsor

https://bugzilla.redhat.com/show_bug.cgi?id=1871171

Fabian Affolter mail@fabian-affolter.ch changed:

What |Removed |Added ---------------------------------------------------------------------------- CC| |mail@fabian-affolter.ch

--- Comment #1 from Fabian Affolter mail@fabian-affolter.ch --- 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.

https://bugzilla.redhat.com/show_bug.cgi?id=1871171

--- Comment #3 from Joel Savitz joelsavitz@gmail.com --- (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%%7Bversion%7Da3/ %{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/%%7Bpypi_name%7D/archive/v%%7Bversio... 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/... Updated koji build: https://koji.fedoraproject.org/koji/taskinfo?taskID=50280565

https://bugzilla.redhat.com/show_bug.cgi?id=1871171

Joel Savitz joelsavitz@gmail.com changed:

What |Removed |Added ---------------------------------------------------------------------------- Summary|Review Request: |Review Request: |python-RPi-GPIO2 - A |python-rpi-gpio2 - A |libgpiod compatibility |libgpiod compatibility |layer for the RPi.GPIO API |layer for the RPi.GPIO API

https://bugzilla.redhat.com/show_bug.cgi?id=1871171

Fabian Affolter mail@fabian-affolter.ch changed:

What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(mail@fabian-affol | |ter.ch) |

--- Comment #6 from Fabian Affolter mail@fabian-affolter.ch --- (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.

https://bugzilla.redhat.com/show_bug.cgi?id=1871171

--- Comment #8 from Alessio alciregi@posteo.net --- Any news on that? This package is very interesting.

https://bugzilla.redhat.com/show_bug.cgi?id=1871171

--- Comment #10 from Joel Savitz joelsavitz@gmail.com --- (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.

https://bugzilla.redhat.com/show_bug.cgi?id=1871171

mguma@redhat.com mguma@redhat.com changed:

What |Removed |Added ---------------------------------------------------------------------------- CC| |mguma@redhat.com Flags| |needinfo?(zebob.m@gmail.com | |)

--- Comment #12 from mguma@redhat.com mguma@redhat.com --- 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

https://bugzilla.redhat.com/show_bug.cgi?id=1871171

mguma@redhat.com mguma@redhat.com changed:

What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(zebob.m@gmail.com |needinfo?(jsavitz@redhat.co |) |m)

--- Comment #14 from mguma@redhat.com mguma@redhat.com --- Here is the spec and SRPM urls

Spec URL: https://github.com/underground-software/RPi.GPIO2/blob/packaging3/packaging/...

SRPM URL: https://github.com/underground-software/RPi.GPIO2/blob/packaging3/packaging/...

https://bugzilla.redhat.com/show_bug.cgi?id=1871171

Maxwell G gotmax@e.email changed:

What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |NEW Flags|fedora-review? | |needinfo?(zebob.m@gmail.com | |) |

--- Comment #15 from Maxwell G gotmax@e.email --- 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.

https://bugzilla.redhat.com/show_bug.cgi?id=1871171

Maxwell G gotmax@e.email changed:

What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(jsavitz@redhat.co | |m) | |needinfo?(mguma@redhat.com)

--- Comment #17 from Maxwell G gotmax@e.email --- 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.

https://bugzilla.redhat.com/show_bug.cgi?id=1871171

Matthew Miller mattdm@redhat.com changed:

What |Removed |Added ---------------------------------------------------------------------------- CC| |mattdm@redhat.com

--- Comment #19 from Matthew Miller mattdm@redhat.com --- (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.

https://bugzilla.redhat.com/show_bug.cgi?id=1871171

Matthew Miller mattdm@redhat.com changed:

What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(zebob.m@gmail.com | |)

--- Comment #21 from Matthew Miller mattdm@redhat.com --- Robert, are you still interested in reviewing this? It's been a while since you've commented.

https://bugzilla.redhat.com/show_bug.cgi?id=1871171

Maxwell G gotmax@e.email changed:

What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(zebob.m@gmail.com |fedora-review? |) | Assignee|zebob.m@gmail.com |gotmax@e.email Status|NEW |ASSIGNED

--- Comment #23 from Maxwell G gotmax@e.email --- 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/%%7Bpypi_name%7D/archive/v%%7Bversio...

I would change this to: URL: https://github.com/underground-software/%%7Bpkgname%7D 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... : 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

https://bugzilla.redhat.com/show_bug.cgi?id=1871171

--- Comment #25 from mguma@redhat.com mguma@redhat.com --- 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/...

SRPM : https://github.com/underground-software/RPi.GPIO2/blob/packaging3/python-rpi...

https://bugzilla.redhat.com/show_bug.cgi?id=1871171

--- Comment #27 from Maxwell G gotmax@e.email --- 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/1b160bc74b4d5d8ae77...

https://bugzilla.redhat.com/show_bug.cgi?id=1871171

--- Comment #29 from Maxwell G gotmax@e.email --- 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_spe... [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.

https://bugzilla.redhat.com/show_bug.cgi?id=1871171

--- Comment #31 from Joel Savitz jsavitz@redhat.com --- Ok, I just made all of your recommended changes (besides the issues with setup.*).

https://bugzilla.redhat.com/show_bug.cgi?id=1871171

--- Comment #33 from Maxwell G gotmax@e.email --- (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/fc7ba311c2e870015f5...

https://github.com/underground-software/RPi.GPIO2/commit/57e408a39a895647229...

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.

https://bugzilla.redhat.com/show_bug.cgi?id=1871171

--- Comment #35 from Maxwell G gotmax@e.email --- 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-repla....

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

https://bugzilla.redhat.com/show_bug.cgi?id=1871171

--- Comment #37 from Maxwell G gotmax@e.email --- (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...

https://bugzilla.redhat.com/show_bug.cgi?id=1871171

Maxwell G gotmax@e.email changed:

What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |POST Flags|fedora-review? |fedora-review+

--- Comment #39 from Maxwell G gotmax@e.email --- 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.