Spec URL: https://raw.githubusercontent.com/knight-of-ni/specfiles/master/python-ouimeaux.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/kni/ouimeaux-fedora/fedora-rawhide-x86_64/01403469-python-ouimeaux/python-ouimeaux-0.8.2-1.git.6b6984b.fc33.src.rpm Fedora Account System Username: kni Description: Open source control for Belkin WeMo devices - Supports WeMo Switch, Light Switch, Insight Switch and Motion - Command-line tool to discover and control devices in your environment - REST API to obtain information and perform actions on devices - Simple responsive Web app provides device control on mobile - Python API to interact with device at a low level RPMLint output: > $ rpmlint /var/lib/mock/fedora-32-x86_64/result/*.rpm > python3-ouimeaux.noarch: W: no-manual-page-for-binary wemo > 2 packages and 0 specfiles checked; 0 errors, 1 warnings. > There is no man page available for the wemo binary, but it does have decent commandline help: > $ wemo -h > usage: wemo [-h] [-b BIND] [-d] [-e] [-v] [-t TIMEOUT] {status,switch,maker,light,list,server} ... > > positional arguments: > {status,switch,maker,light,list,server} > status Print status of WeMo devices > switch Turn a WeMo Switch on or off > maker Get sensor or switch state of a Maker or Turn on or off > light Turn a WeMo LED light on or off > list List all devices found in the environment > server Run the API server and web app > optional arguments: > -h, --help show this help message and exit > -b BIND, --bind BIND ip:port to which to bind the response server. Default is localhost:54321 > -d, --debug Enable debug logging > -e, --exact-match Disable fuzzy matching for device names > -v, --human-readable Print statuses as human-readable words > -t TIMEOUT, --timeout TIMEOUT > Time in seconds to allow for discovery Scope: - The intended scope of this review is for recent Fedora only - EPEL builds are not planned at this time due to missing dependencies - Unless someone can make a compelling argument for python2, only a python3 package is planned
UPDATE: I moved the examples folder out of the build and into the docs folder The wemo binary requires udp port 54321 to be open in the firewall. A firewalld config file and readme instructions were added to document this. UPDATE SPECFILE URL: https://raw.githubusercontent.com/knight-of-ni/specfiles/master/python-ouimeaux.spec UPDATED SRPM URL: https://download.copr.fedorainfracloud.org/results/kni/ouimeaux-fedora/fedora-rawhide-x86_64/01407798-python-ouimeaux/python-ouimeaux-0.8.2-2.git.6b6984b.fc33.src.rpm Note that this does introduce new errors running rpmlint: > $ rpmlint /var/lib/mock/fedora-32-x86_64/result/*.rpm > python3-ouimeaux.noarch: W: no-manual-page-for-binary wemo > python-ouimeaux.src:76: E: hardcoded-library-path in %{_prefix}/lib/firewalld/services > python-ouimeaux.src:77: E: hardcoded-library-path in %{_prefix}/lib/firewalld/services/ > python-ouimeaux.src:91: E: hardcoded-library-path in %{_prefix}/lib/firewalld/services/%{srcname}.xml > 2 packages and 0 specfiles checked; 3 errors, 1 warnings. However, these library paths have been implemented in the manner documented in the following link and in the firewalld specfile. https://fedoraproject.org/wiki/PackagingDrafts/ScriptletSnippets/Firewalld
UPDATE: Created the fw_services macro, so now rpmlint complains a little less: > $ rpmlint /var/lib/mock/fedora-32-x86_64/result/*.rpm > python3-ouimeaux.noarch: W: no-manual-page-for-binary wemo > python-ouimeaux.src:9: E: hardcoded-library-path in %{_prefix}/lib/firewalld/services > 2 packages and 0 specfiles checked; 1 errors, 1 warnings. UPDATED SPECFILE URL: https://raw.githubusercontent.com/knight-of-ni/specfiles/master/python-ouimeaux.spec UPDATED SRPM URL: https://download.copr.fedorainfracloud.org/results/kni/ouimeaux-fedora/fedora-rawhide-x86_64/01407906-python-ouimeaux/python-ouimeaux-0.8.2-3.git.6b6984b.fc33.src.rpm
UPDATE: The author updated his Releases page to reflect the version found in the source code. For details see: https://github.com/iancmcc/ouimeaux/issues/200 I have updated (simplified) the specfile to reflect this. RPMLINT: >$ rpmlint /var/lib/mock/fedora-32-x86_64/result/*.rpm >python3-ouimeaux.noarch: W: no-manual-page-for-binary wemo >python-ouimeaux.src:5: E: hardcoded-library-path in %{_prefix}/lib/firewalld/services >2 packages and 0 specfiles checked; 1 errors, 1 warnings. UPDATED SPECFILE URL: https://raw.githubusercontent.com/knight-of-ni/specfiles/master/python-ouimeaux.spec UPDATED SRPM URL: https://download.copr.fedorainfracloud.org/results/kni/ouimeaux-fedora/fedora-rawhide-x86_64/01465491-python-ouimeaux/python-ouimeaux-0.8.2-4.fc33.src.rpm
I will take this review. Can you review bug 1847117 in return? It should also be a simple python review.
on it, thanks!
Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated Issues: ======= - The Requires on lines 18 and 19 of the spec file should be inside the python3 subpackage instead. This is reason for the "Directories without known owners" error below. - The package bundles glyphicons-halflings-fonts. Please replace the bundled font files with a reference to the font in the Fedora package. This package contains eot, svg, ttf, and woff versions of the font, but you only need the ttf version. See: https://docs.fedoraproject.org/en-US/packaging-guidelines/FontsPolicy/#_web_fonts - The package bundles pysignals (https://pypi.org/project/pysignals/) in ouimeaux/pysignals. Preferably that package would be unbundled and submitted separately for review. If that is not possible for some reason, please supply the reason in a comment in the spec file above "Provides: bundled(python3-ouimeaux)". - These files, which are included in the binary package, have license ASL 2.0: ouimeaux/server/static/css/bootstrap-responsive.css ouimeaux/server/static/js/bootstrap.js ouimeaux/server/static/lib/bootstrap/bootstrap.js And this file, also included in the binary package, has license MIT: ouimeaux/server/static/lib/jquery/jquery.js So I think the combined license should be "BSD and ASL 2.0 and MIT" (unless you unbundle jquery, as noted below). There should be a comment above the License field in the spec file explaining this. - That bundled version of jquery is quite old. If it is really needed, please consider unbundling it in favor of the js-jquery1 package (which has version 1.12.4, versus the bundled version of 1.8.3). - In Rawhide, with python 3.9, attempting to run wemo fails: Traceback (most recent call last): File "/usr/bin/wemo", line 11, in <module> load_entry_point('ouimeaux==0.8.2', 'console_scripts', 'wemo')() File "/usr/lib/python3.9/site-packages/pkg_resources/__init__.py", line 490, in load_entry_point return get_distribution(dist).load_entry_point(group, name) File "/usr/lib/python3.9/site-packages/pkg_resources/__init__.py", line 2853, in load_entry_point return ep.load() File "/usr/lib/python3.9/site-packages/pkg_resources/__init__.py", line 2453, in load return self.resolve() File "/usr/lib/python3.9/site-packages/pkg_resources/__init__.py", line 2459, in resolve module = __import__(self.module_name, fromlist=['__name__'], level=0) File "/usr/lib/python3.9/site-packages/ouimeaux/cli.py", line 6, in <module> from .environment import Environment File "/usr/lib/python3.9/site-packages/ouimeaux/environment.py", line 7, in <module> from ouimeaux.device import DeviceUnreachable File "/usr/lib/python3.9/site-packages/ouimeaux/device/__init__.py", line 4, in <module> from .api.service import Service File "/usr/lib/python3.9/site-packages/ouimeaux/device/api/service.py", line 2, in <module> from xml.etree import cElementTree as et ImportError: cannot import name 'cElementTree' from 'xml.etree' (/usr/lib64/python3.9/xml/etree/__init__.py) As noted in https://docs.python.org/3/library/xml.etree.elementtree.html, cElementTree was deprecated in python 3.3, and has been removed entirely in python 3.9. ElementTree should be used instead. ===== MUST items ===== Generic: [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [!]: License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "Unknown or generated", "BSD 3-clause "New" or "Revised" License", "*No copyright* GNU General Public License", "Apache License 2.0", "Expat License". 94 files have unknown license. [!]: Package must own all directories that it creates. Note: Directories without known owners: /usr/lib/firewalld/services, /usr/lib/firewalld [!]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Package contains desktop file if it is a GUI application. [-]: Development files must be in a -devel package [x]: Package uses nothing in %doc for runtime. [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [!]: 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 30720 bytes in 7 files. [x]: Package complies to the Packaging Guidelines [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [x]: Package requires other packages for directories it uses. [x]: Package 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: [!]: Avoid bundling fonts in non-fonts packages. Note: Package contains font files [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: Final provides and requires are sane (see attachments). [?]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [-]: 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. [x]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [x]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: Sources can be downloaded from URI in Source: tag [x]: SourceX is a working URL. [x]: Spec use %global instead of %define unless justified. ===== EXTRA items ===== Generic: [x]: Rpmlint is run on all installed packages. Note: No rpmlint messages. [x]: Spec file according to URL is the same as in SRPM. Rpmlint ------- Checking: python3-ouimeaux-0.8.2-4.fc33.noarch.rpm python-ouimeaux-0.8.2-4.fc33.src.rpm python3-ouimeaux.noarch: W: no-manual-page-for-binary wemo python-ouimeaux.src:5: E: hardcoded-library-path in %{_prefix}/lib/firewalld/services 2 packages and 0 specfiles checked; 1 errors, 1 warnings. Rpmlint (installed packages) ---------------------------- python3-ouimeaux.noarch: W: no-manual-page-for-binary wemo 1 packages and 0 specfiles checked; 0 errors, 1 warnings. Source checksums ---------------- https://github.com/iancmcc/ouimeaux/archive/0.8.2.tar.gz#/python-ouimeaux-0.8.2.tar.gz : CHECKSUM(SHA256) this package : 1cc8a930face70b78b253f91495cc04c8bc70f27d9b8730ddd4f00804870b002 CHECKSUM(SHA256) upstream package : 1cc8a930face70b78b253f91495cc04c8bc70f27d9b8730ddd4f00804870b002 Requires -------- python3-ouimeaux (rpmlib, GLIBC filtered): /usr/bin/python3 python(abi) python3.9dist(future) python3.9dist(gevent) python3.9dist(pyyaml) python3.9dist(requests) python3.9dist(setuptools) python3.9dist(six) Provides -------- python3-ouimeaux: python-ouimeaux python3-ouimeaux python3.9-ouimeaux python3.9dist(ouimeaux) python3dist(ouimeaux) Generated by fedora-review 0.7.5 (5fa5b7e) last change: 2020-02-16 Command line :/usr/bin/fedora-review -b 1839242 -m fedora-rawhide-x86_64 Buildroot used: fedora-rawhide-x86_64 Active plugins: Generic, Shell-api, Python Disabled plugins: R, Ocaml, C/C++, Ruby, SugarActivity, Perl, Java, Haskell, PHP, fonts Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH
Hmmmm.... I see my work is cut out for me. I must apologize. I've been using ouimeaux on several machines for years now with success. I totally missed the additional libraries that were embedded into the project. I'll get on these action items one at a time.
No need to apologize. Catching this sort of thing is what reviews are for. Let me know when you are ready to proceed. If you decide to unbundle pysignals, let me know and I will review that too.
Yes, I am well into making all the changes, including unbundling jquery, the fonts, and pysignals. pysignals lives on pypi, so I anticipate that to be an easy review: https://pypi.org/project/pysignals/ I fixed the runtime crash pn python 3.9. Patch submitted upstream: https://github.com/iancmcc/ouimeaux/pull/204 I don't expect to get everything quite done today (juggling between this and $dayjob), but I will post my progress later this evening.
COMPLETED TASKS: - patch for python 3.9 compatibility - move runtime requirements into subpackage - unbundle glyphicons-halflings - bootstrap java files are ASL 2.0 license - unbundle jquery TO-DO: - unbundle pysignals - create new package review for pysignals - add pysignals as a runtime requirement to python3-ouimeaux UPDATE SPECFILE URL: https://raw.githubusercontent.com/knight-of-ni/specfiles/master/python-ouimeaux.spec UPDATED SRPM URL: https://download.copr.fedorainfracloud.org/results/kni/ouimeaux-fedora/fedora-rawhide-x86_64/01469092-python-ouimeaux/python-ouimeaux-0.8.2-5.fc33.src.rpm NOTE: Calling "wemo list" runs w/o error on fedora 33, but it doesn't find the wemo device on my network. Works fine on f32. I will need to investigate this further.
python-pysignals package review is up: https://bugzilla.redhat.com/show_bug.cgi?id=1848049
All action items have been completed. UPDATE SPECFILE URL: https://raw.githubusercontent.com/knight-of-ni/specfiles/master/python-ouimeaux.spec UPDATED SRPM URL: https://download.copr.fedorainfracloud.org/results/kni/ouimeaux-fedora/fedora-rawhide-x86_64/01470047-python-ouimeaux/python-ouimeaux-0.8.2-6.fc33.src.rpm NOTE: I will add a build override for recently approved pysignals package as soon as I am able.
And the fun never stops... I just discovered a custom subclass, called StateChange, which was added to the bundled pysignals, but is not part of the upstream pysignals project. I've opened an issue with the author here to decide how to proceed: https://github.com/iancmcc/ouimeaux/issues/205
This package is ready for another round of review. The custom statechange subclass has been moved out of dispatch.py and into signal.py. PR, along with request for feedback, has been sent upstream: https://github.com/iancmcc/ouimeaux/pull/206 UPDATE SPECFILE URL: https://raw.githubusercontent.com/knight-of-ni/specfiles/master/python-ouimeaux.spec UPDATED SRPM URL: https://download.copr.fedorainfracloud.org/results/kni/ouimeaux-fedora/fedora-rawhide-x86_64/01485140-python-ouimeaux/python-ouimeaux-0.8.2-7.fc33.src.rpm
It looks good. You did a lot of work to get this in shape. This package is APPROVED.
Thank you. It was a fun challenge.
(fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/python-ouimeaux
FEDORA-2020-d1145e9bb9 has been submitted as an update to Fedora 32. https://bodhi.fedoraproject.org/updates/FEDORA-2020-d1145e9bb9
FEDORA-2020-d1145e9bb9 has been pushed to the Fedora 32 testing repository. In short time you'll be able to install the update with the following command: `sudo dnf install --enablerepo=updates-testing --advisory=FEDORA-2020-d1145e9bb9 \*` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2020-d1145e9bb9 See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2020-54d98b5b1f has been submitted as an update to Fedora 32. https://bodhi.fedoraproject.org/updates/FEDORA-2020-54d98b5b1f
FEDORA-2020-32c26ec183 has been submitted as an update to Fedora 32. https://bodhi.fedoraproject.org/updates/FEDORA-2020-32c26ec183
FEDORA-2020-32c26ec183 has been pushed to the Fedora 32 testing repository. In short time you'll be able to install the update with the following command: `sudo dnf upgrade --enablerepo=updates-testing --advisory=FEDORA-2020-32c26ec183` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2020-32c26ec183 See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2020-32c26ec183 has been pushed to the Fedora 32 stable repository. If problem still persists, please make note of it in this bug report.