Description netjsonconfig is a python library that converts NetJSON DeviceConfiguration objects into real router configurations that can be installed on systems like OpenWRT, LEDE or OpenWisp Firmware. Could you please check if %{_bindir}/netjsonconfig on both python 2 and 3 sections, can cause any conflicts? Mock build works good, but I want to ask to be 100% sure. spec file: https://germano.fedorapeople.org/package_reviews/python-netjsonconfig/python-netjsonconfig.spec srpm file: https://germano.fedorapeople.org/package_reviews/python-netjsonconfig/python-netjsonconfig-0.5.1-1.fc24.src.rpm
Taking this review as I provided also so initial feedback regarding the tests.
If the binary has the same functionality (usually it does) under python2 and python3 it should only be packaged for the python3 subpackage. As it is currently the binary is pointing to usr/bin/python3, and by including it at both subpackages %files sections, as a result the python2 subpackage has a dependency on python3. If however you would like to package the binary for both python2 and python3 you can find guidance at the rpm porting guide [0] [0] http://python-rpm-porting.readthedocs.io/en/latest/tools.html#install
I edited the spec file to enable the binary for Python 3 only spec file: https://germano.fedorapeople.org/package_reviews/python-netjsonconfig/python-netjsonconfig.spec srpm file: https://germano.fedorapeople.org/package_reviews/python-netjsonconfig/python-netjsonconfig-0.5.1-1.fc24.src.rpm
Please remove the comment from the python2 subpackage, since it is explicitly stated in the guidelines about the binary situation, you don't have to mention it in the SPEC.
spec file: https://germano.fedorapeople.org/package_reviews/python-netjsonconfig/python-netjsonconfig.spec srpm file: https://germano.fedorapeople.org/package_reviews/python-netjsonconfig/python-netjsonconfig-0.5.1-1.fc24.src.rpm
python-setuptools BuildRequires should be replaced with python2-setuptools as the package provides that name from f24+ (unless you plan on building it for F23 as well, where the python2 subpackage doesn't not provide the python2 namespace). Same for python-nose where from F23+ the SPEC virtually provides the python2 name.
And my previous comment also applies for the runtime requirements, python-six, python-jinja2 (but not for python-jsonschema as that package doesn't use the python provide macro or an explicit provides tag)
I removed all BuildRequires required for tests, because: - python-coveralls has no builds for EPEL7; - since tests are disabled, we no longer need those BuildRequires spec file: https://germano.fedorapeople.org/package_reviews/python-netjsonconfig/new/python-netjsonconfig.spec srpm file: https://germano.fedorapeople.org/package_reviews/python-netjsonconfig/new/python-netjsonconfig-0.5.1-1.fc24.src.rpm
I have forgotten to write that the spec is made for EPEL7 & Fedora >= 24
When testing the python2 subpackage I stumbled upon some tracebacks The python2 subpackage should have a runtime requirement for python-ipaddress module because of: ImportError: No module named ipaddress Also there should be a runtime requirement for python-setuptools for epel7 due to: ImportError: No module named pkg_resources The python3_pkgversion macro can be used for the packages that actually use it on their own (like python-six), thus providing that name, but not every package uses it, so whenever you have it in you own SPEC you should check each dependency that they actually utilize this macro at their own SPEC's. Currently, while it can be built, it is not possible to install the python3 subpackage for epel7 as there is no python3-jinja2 and python3-jsonschema packages for epel7. I would suggest contacting either ttorling or orion, as they are quite active and aware as well, of the current situation in EPEL, so they might be able to build these dependencies or move some things, in order for your package to be accepted, although the package review would be stalled until this situation is resolved. Another possibility would be to not consider EPEL7 for the time being, and try to resolve the situation in the future, in order to make your package working for EPEL7, so for now the review/builds/branches will be only for Fedora. On another notice, it would be better (but not mandatory) to do some stylistic changes so the SPEC file is more readable. An empty line should exist between each subpackage and its respective description like in the example SPEC [0]. Also (this is more of a personal preference), the Requires as well as the BuildRequires for each subpackage can be placed between the summary and the python provide macro. [0] https://fedoraproject.org/wiki/Packaging:Python#Example_common_spec_file
===Missing python 2 dependencies=== I have filled a bugreport https://github.com/fedora-python/pyp2rpm/issues/88 since I have used pyp2rpm to fill the Requests. I will add the missing dependencies manually ===EPEL7 builds=== I have filled two bugreports (see Depends on) to ask Python 3 builds ===Stylistic changes=== Yes I agree with you As soon I get some feedbacks about EPEL7 Python 3 dependencies builds, I will decide if it is better to wait for them or temporarily exclude the EPEL7 build for python-netjsonconfig. Thank you
netjsonconfig 0.5.2 has been released spec file: https://germano.fedorapeople.org/package_reviews/python-netjsonconfig/python-netjsonconfig.spec srpm file: https://germano.fedorapeople.org/package_reviews/python-netjsonconfig/python-netjsonconfig-0.5.2-1.fc25.src.rpm I have runned fedora-review and I got two kind of errors: 1) various messages like python2-netjsonconfig.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/netjsonconfig/backends/openwisp/templates/install.sh 644 /bin/sh but they are fine and I inserted a comment into the spec file. The source of that comment is Miro Hrončok 2) python3-netjsonconfig.noarch: E: python-bytecode-wrong-magic-value /usr/lib/python3.6/site-packages/netjsonconfig/backends/openwrt/__pycache__/__init__.cpython-36.opt-1.pyc expected 3361 (3.6), found 3379 (unknown) is a rpmlint bug https://bugzilla.redhat.com/show_bug.cgi?id=1409376 Concerning past problems: ===Missing python 2 dependencies=== I have not found any error message while running fedora-review ===EPEL7 builds=== There are still missing dependencies. I will not ask an EPEL7 branch until those dependencies will be available into EPEL7 repository. But I do not want to remove the macros for EPEL7 branch from the spec file ===Stylistic changes=== Fixed
Regarding the python2 runtime requirements for ipaddress. In order to test it at a machine or chroot where you have installed the python2 rpm you can run python2 -c 'import netjsonconfig' and you will see the traceback about the missing module. fedora-review will not catch these issues. You should add 'Requires: python-ipaddress' for the python2 subpackage. rpmlint warns: python-netjsonconfig.src:77: W: macro-in-comment %{__python2} python-netjsonconfig.src:78: W: macro-in-comment %{__python3} the macros in comments should be in the form of: %%{__python2} Also a small typo at line 81, it should be 'templates'. After these issues are fixed, the package should be fine.
Also on line 82 it should be 'permissions'
https://germano.fedorapeople.org/package_reviews/python-netjsonconfig/python-netjsonconfig.spec https://germano.fedorapeople.org/package_reviews/python-netjsonconfig/python-netjsonconfig-0.5.3-1.fc25.src.rpm
Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed ===== 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. [x]: License file installed when any subpackage combination is installed. [x]: Package must own all directories that it creates. Note: Directories without known owners: /usr/lib/python3.6/site- packages, /usr/lib/python3.6 Note from reviewer: These directories are owned by python3 [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Package contains desktop file if it is a GUI application. [-]: Development files must be in a -devel package [x]: Package uses nothing in %doc for runtime. [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [x]: Package is not known to require an ExcludeArch tag. [-]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 20480 bytes in 2 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 does not own files or directories owned by other packages. [x]: All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [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 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]: 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 python2-netjsonconfig , python3-netjsonconfig Note from reviewer: package and its dependencies are noarch [x]: Package functions as described. [!]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [-]: 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. Note from reviewer: package downloads packages from pypi so tests are disabled [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 ------- Checking: python2-netjsonconfig-0.5.3-1.fc26.noarch.rpm python3-netjsonconfig-0.5.3-1.fc26.noarch.rpm python-netjsonconfig-0.5.3-1.fc26.src.rpm python2-netjsonconfig.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/netjsonconfig/backends/openwisp/templates/uninstall.sh 644 /bin/sh python2-netjsonconfig.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/netjsonconfig/backends/openwisp/templates/vpn_script_down.sh 644 /bin/sh python2-netjsonconfig.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/netjsonconfig/backends/openwisp/templates/tc_script.sh 644 /bin/sh /etc/rc.common python2-netjsonconfig.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/netjsonconfig/backends/openwisp/templates/install.sh 644 /bin/sh python2-netjsonconfig.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/netjsonconfig/backends/openwisp/templates/vpn_script_up.sh 644 /bin/sh python3-netjsonconfig.noarch: E: non-executable-script /usr/lib/python3.6/site-packages/netjsonconfig/backends/openwisp/templates/uninstall.sh 644 /bin/sh python3-netjsonconfig.noarch: E: non-executable-script /usr/lib/python3.6/site-packages/netjsonconfig/backends/openwisp/templates/tc_script.sh 644 /bin/sh /etc/rc.common python3-netjsonconfig.noarch: E: non-executable-script /usr/lib/python3.6/site-packages/netjsonconfig/backends/openwisp/templates/install.sh 644 /bin/sh python3-netjsonconfig.noarch: E: non-executable-script /usr/lib/python3.6/site-packages/netjsonconfig/backends/openwisp/templates/vpn_script_down.sh 644 /bin/sh python3-netjsonconfig.noarch: E: non-executable-script /usr/lib/python3.6/site-packages/netjsonconfig/backends/openwisp/templates/vpn_script_up.sh 644 /bin/sh python3-netjsonconfig.noarch: W: no-manual-page-for-binary netjsonconfig 3 packages and 0 specfiles checked; 10 errors, 1 warnings. Rpmlint (installed packages) ---------------------------- python2-netjsonconfig.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/netjsonconfig/backends/openwisp/templates/install.sh 644 /bin/sh python2-netjsonconfig.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/netjsonconfig/backends/openwisp/templates/tc_script.sh 644 /bin/sh /etc/rc.common python2-netjsonconfig.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/netjsonconfig/backends/openwisp/templates/uninstall.sh 644 /bin/sh python2-netjsonconfig.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/netjsonconfig/backends/openwisp/templates/vpn_script_down.sh 644 /bin/sh python2-netjsonconfig.noarch: E: non-executable-script /usr/lib/python2.7/site-packages/netjsonconfig/backends/openwisp/templates/vpn_script_up.sh 644 /bin/sh python3-netjsonconfig.noarch: E: non-executable-script /usr/lib/python3.6/site-packages/netjsonconfig/backends/openwisp/templates/install.sh 644 /bin/sh python3-netjsonconfig.noarch: E: non-executable-script /usr/lib/python3.6/site-packages/netjsonconfig/backends/openwisp/templates/tc_script.sh 644 /bin/sh /etc/rc.common python3-netjsonconfig.noarch: E: non-executable-script /usr/lib/python3.6/site-packages/netjsonconfig/backends/openwisp/templates/uninstall.sh 644 /bin/sh python3-netjsonconfig.noarch: E: non-executable-script /usr/lib/python3.6/site-packages/netjsonconfig/backends/openwisp/templates/vpn_script_down.sh 644 /bin/sh python3-netjsonconfig.noarch: E: non-executable-script /usr/lib/python3.6/site-packages/netjsonconfig/backends/openwisp/templates/vpn_script_up.sh 644 /bin/sh python3-netjsonconfig.noarch: W: no-manual-page-for-binary netjsonconfig 2 packages and 0 specfiles checked; 10 errors, 1 warnings. Note from reviewer: As it is also mentioned in the SPEC file the non-executable-scripts in site-packages/netjsonconfig/backends/*/templates are templates and the shebang is intentional, netjsonconfig will set the correct permissions upon usage. Requires -------- python2-netjsonconfig (rpmlib, GLIBC filtered): python(abi) python-ipaddress python-jsonschema python2-jinja2 python2-six python3-netjsonconfig (rpmlib, GLIBC filtered): /usr/bin/python3 python(abi) python3-jinja2 python3-jsonschema python3-six Provides -------- python2-netjsonconfig: python-netjsonconfig python2-netjsonconfig python2.7dist(netjsonconfig) python2dist(netjsonconfig) python3-netjsonconfig: python3-netjsonconfig python3.6dist(netjsonconfig) python3dist(netjsonconfig) Source checksums ---------------- https://files.pythonhosted.org/packages/source/n/netjsonconfig/netjsonconfig-0.5.3.tar.gz : CHECKSUM(SHA256) this package : da004ff34eda17df721cc718c5f5269ad2dccba09453054179ce0c7b8f7be41e CHECKSUM(SHA256) upstream package : da004ff34eda17df721cc718c5f5269ad2dccba09453054179ce0c7b8f7be41e Generated by fedora-review 0.6.1 (f03e4e7) last change: 2016-05-02 Command line :/usr/bin/fedora-review -b 1391457 -m fedora-rawhide-x86_64 Buildroot used: fedora-rawhide-x86_64
There has been a new release as well but it is not a blocker. Please update it to the latest version when you can. Package is accepted.
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/python-netjsonconfig
The package has been built.