https://bugzilla.redhat.com/show_bug.cgi?id=1702720
Bug ID: 1702720 Summary: Review Request: frr - routing daemon Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: mruprich@redhat.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://copr-be.cloud.fedoraproject.org/results/mruprich/frr/fedora-rawhide-...
SRPM URL: https://copr-be.cloud.fedoraproject.org/results/mruprich/frr/fedora-rawhide-...
Description: FRRouting is free software that manages TCP/IP based routing protocols. It takes a multi-server and multi-threaded approach to resolve the current complexity of the Internet.
FRRouting supports BGP4, OSPFv2, OSPFv3, ISIS, RIP, RIPng, PIM, NHRP, PBR, EIGRP and BFD.
FRRouting is a fork of Quagga.
Fedora Account System Username: mruprich
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
Michal Ruprich mruprich@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|nobody@fedoraproject.org |mosvald@redhat.com
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
Martin Osvald 🛹 mosvald@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review?
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
--- Comment #1 from Michal Ruprich mruprich@redhat.com --- Fedora-review tool will complain about "Upstream MD5sum check error" but that is ok - there are two modules in frr that have some licensing issues and to avoid any problems, I am providing FRR without them. These modules are ldpd and babeld. These were removed from the original tar and new tar was created, hence the md5sum error.
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
Fabian Affolter mail@fabian-affolter.ch changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |mail@fabian-affolter.ch
--- Comment #2 from Fabian Affolter mail@fabian-affolter.ch --- Both URLs are 404.
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
--- Comment #3 from Michal Ruprich mruprich@redhat.com --- (In reply to Fabian Affolter from comment #2)
Both URLs are 404.
It seems that after some time COPR deletes these files. I made a new build and both files should be here: https://copr-be.cloud.fedoraproject.org/results/mruprich/frr/fedora-rawhide-...
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
Martin Osvald 🛹 mosvald@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(mruprich@redhat.c | |om)
--- Comment #4 from Martin Osvald 🛹 mosvald@redhat.com ---
The below needs to be fixed:
- the first three are related, more info in rpmlint section, the files there looks to be better candidate for -devel subpackage than -contrib as adding files only into /usr/share/doc even not documentation at all (scripts, relocatable files used during compilation (.o), and unstripped DSOs):
[!] /usr/share/doc/frr-contrib/tools contains object/relocatable files [!] /usr/share/doc/frr-contrib/tools/.libs contains shared object files/DSOs [!]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 122880 bytes in 15 files. /usr/share/doc/frr-contrib consumes 912K [!] userdel && groupdel in %pre
Package Review ==============
Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed
Issues: ======= - Texinfo files are installed using install-info in %post and %preun if package has .info files. Note: Texinfo .info file(s) in frr See: https://docs.fedoraproject.org/en-US/packaging- guidelines/Scriptlets/#_texinfo - Sources used to build the package match the upstream source, as provided in the spec URL. Note: Upstream MD5sum check error, diff is in /home/mosvald/Work/redhat/src/pkg-review/frr/review-frr/diff.txt See: https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/ - systemd_post is invoked in %post, systemd_preun in %preun, and systemd_postun in %postun for Systemd service files. Note: Systemd service file(s) in frr See: https://docs.fedoraproject.org/en-US/packaging- guidelines/Scriptlets/#_scriptlets
===== MUST items =====
C/C++: [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [x]: Development (unversioned) .so files in -devel subpackage, if present. Note: Unversioned so-files in private %_libdir subdirectory (see attachment). Verify they are not in ld path. [x]: If your application is a C or C++ application you must list a BuildRequires against gcc, gcc-c++ or clang. [x]: Header files in -devel subpackage, if present. [x]: Package does not contain any libtool archives (.la) [x]: Rpath absent or only used for internal libs.
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", "GPL (v2 or later)", "Expat License", "GPL (v2 or later) (with incorrect FSF address)", "ISC License GPL (v2 or later) (with incorrect FSF address)", "ISC License", "GPL (v3 or later)", "ISC License GPL (v2 or later)", "BSD 3-clause "New" or "Revised" License", "ISC License BSD 2-clause "Simplified" License", "BSD 2-clause "Simplified" License", "*No copyright* GPL (v2 or later)", "BSD (unspecified)", "GNU Lesser General Public License (v2.1 or later)", "GNU General Public License", "FSF Unlimited License (with Retention) GNU General Public License", "FSF Unlimited License (with Retention)", "GPL (with incorrect FSF address)", "*No copyright* Public domain", "*No copyright* GNU General Public License", "Expat License GPL (v2 or later)". 395 files have unknown license. Detailed output of licensecheck in /home/mosvald/Work/redhat/src/pkg- review/frr/review-frr/licensecheck.txt [x]: License file installed when any subpackage combination is installed. [x]: Package requires other packages for directories it uses. Note: No known owner of /usr/lib64/frr/modules, /usr/lib/frr, /usr/share/yang [x]: Package must own all directories that it creates. Note: Directories without known owners: /etc/logrotate.d, /usr/lib/frr, /usr/share/yang, /usr/lib64/frr/modules [x]: %build honors applicable compiler flags or justifies otherwise. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [x]: Each %files section contains %defattr if rpm < 4.4 Note: %defattr present but not needed [-]: 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. [x]: Package contains systemd file(s) if in need. [x]: Useful -debuginfo package or justification otherwise. [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 122880 bytes in 15 files.
/usr/share/doc/frr-contrib consumes 912K
[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 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]: %config files are marked noreplace or the reason is justified. [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]: No %config files under /usr. [x]: Package does not use a name that already exists. [x]: Package is not relocatable. [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
Perl: [-]: Package contains the mandatory BuildRequires and Requires:. Note: Requires: perl(:MODULE_COMPAT_%(eval "`%{__perl} -V:version`"; echo $version)) missing?
===== SHOULD items =====
Generic: [!]: Uses parallel make %{?_smp_mflags} macro. [x]: 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 frr [x]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [-]: Patches link to upstream bugs/comments/lists or are otherwise justified. [-]: 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. [?]: Files in /run, var/run and /var/lock uses tmpfiles.d when appropriate [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 debuginfo package(s). Note: No rpmlint messages. [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. [x]: Package should not use obsolete m4 macros [x]: Spec file according to URL is the same as in SRPM.
Rpmlint ------- Checking: frr-7.0-1.fc31.x86_64.rpm frr-contrib-7.0-1.fc31.x86_64.rpm frr-devel-7.0-1.fc31.x86_64.rpm frr-debuginfo-7.0-1.fc31.x86_64.rpm frr-debugsource-7.0-1.fc31.x86_64.rpm frr-7.0-1.fc31.src.rpm frr.x86_64: E: description-line-too-long C FRRouting supports BGP4, OSPFv2, OSPFv3, ISIS, RIP, RIPng, PIM, NHRP, PBR, EIGRP and BFD. frr.x86_64: W: obsolete-not-provided frr-sysvinit frr.x86_64: W: obsolete-not-provided quagga frr.x86_64: E: missing-call-to-setgroups-before-setuid /usr/bin/vtysh frr.x86_64: W: non-standard-uid /etc/frr frr frr.x86_64: W: non-standard-gid /etc/frr frr frr.x86_64: W: non-conffile-in-etc /etc/frr/daemons frr.x86_64: W: non-standard-uid /run/frr frr frr.x86_64: W: non-standard-gid /run/frr frr frr.x86_64: E: incorrect-fsf-address /usr/lib/frr/frr-reload.py frr.x86_64: W: non-standard-uid /var/log/frr frr frr.x86_64: W: non-standard-gid /var/log/frr frr frr.x86_64: W: dangerous-command-in-%pre userdel frr.x86_64: W: dangerous-command-in-%post chown frr-contrib.x86_64: E: arch-dependent-file-in-usr-share /usr/share/doc/frr-contrib/tools/.libs/gen_northbound_callbacks frr-contrib.x86_64: W: unstripped-binary-or-object /usr/share/doc/frr-contrib/tools/.libs/gen_northbound_callbacks frr-contrib.x86_64: W: binaryinfo-readelf-failed /usr/share/doc/frr-contrib/tools/.libs/gen_northbound_callbacks readelf: /tmp/rpmlint.frr-contrib-7.0-1.fc31.x86_64.rpm.iy21zrke/usr/share/doc/frr-contrib/tools/.libs/gen_northbound_callbacks: Error: No such file frr-contrib.x86_64: W: binaryinfo-tail-failed /usr/share/doc/frr-contrib/tools/.libs/gen_northbound_callbacks: [Errno 2] No such file or directory: '/tmp/rpmlint.frr-contrib-7.0-1.fc31.x86_64.rpm.iy21zrke/usr/share/doc/frr-contrib/tools/.libs/gen_northbound_callbacks' frr-contrib.x86_64: E: arch-dependent-file-in-usr-share /usr/share/doc/frr-contrib/tools/.libs/gen_yang_deviations frr-contrib.x86_64: W: unstripped-binary-or-object /usr/share/doc/frr-contrib/tools/.libs/gen_yang_deviations frr-contrib.x86_64: W: binaryinfo-readelf-failed /usr/share/doc/frr-contrib/tools/.libs/gen_yang_deviations readelf: /tmp/rpmlint.frr-contrib-7.0-1.fc31.x86_64.rpm.iy21zrke/usr/share/doc/frr-contrib/tools/.libs/gen_yang_deviations: Error: No such file frr-contrib.x86_64: W: binaryinfo-tail-failed /usr/share/doc/frr-contrib/tools/.libs/gen_yang_deviations: [Errno 2] No such file or directory: '/tmp/rpmlint.frr-contrib-7.0-1.fc31.x86_64.rpm.iy21zrke/usr/share/doc/frr-contrib/tools/.libs/gen_yang_deviations' frr-contrib.x86_64: E: arch-dependent-file-in-usr-share /usr/share/doc/frr-contrib/tools/.libs/permutations frr-contrib.x86_64: W: unstripped-binary-or-object /usr/share/doc/frr-contrib/tools/.libs/permutations frr-contrib.x86_64: W: binaryinfo-readelf-failed /usr/share/doc/frr-contrib/tools/.libs/permutations readelf: /tmp/rpmlint.frr-contrib-7.0-1.fc31.x86_64.rpm.iy21zrke/usr/share/doc/frr-contrib/tools/.libs/permutations: Error: No such file frr-contrib.x86_64: W: binaryinfo-tail-failed /usr/share/doc/frr-contrib/tools/.libs/permutations: [Errno 2] No such file or directory: '/tmp/rpmlint.frr-contrib-7.0-1.fc31.x86_64.rpm.iy21zrke/usr/share/doc/frr-contrib/tools/.libs/permutations' frr-contrib.x86_64: E: arch-dependent-file-in-usr-share /usr/share/doc/frr-contrib/tools/gen_northbound_callbacks.o frr-contrib.x86_64: E: arch-dependent-file-in-usr-share /usr/share/doc/frr-contrib/tools/gen_yang_deviations.o frr-contrib.x86_64: E: arch-dependent-file-in-usr-share /usr/share/doc/frr-contrib/tools/permutations.o frr-contrib.x86_64: E: arch-dependent-file-in-usr-share /usr/share/doc/frr-contrib/tools/ssd frr-contrib.x86_64: W: unstripped-binary-or-object /usr/share/doc/frr-contrib/tools/ssd frr-contrib.x86_64: W: binaryinfo-readelf-failed /usr/share/doc/frr-contrib/tools/ssd readelf: /tmp/rpmlint.frr-contrib-7.0-1.fc31.x86_64.rpm.iy21zrke/usr/share/doc/frr-contrib/tools/ssd: Error: No such file frr-contrib.x86_64: W: binaryinfo-tail-failed /usr/share/doc/frr-contrib/tools/ssd: [Errno 2] No such file or directory: '/tmp/rpmlint.frr-contrib-7.0-1.fc31.x86_64.rpm.iy21zrke/usr/share/doc/frr-contrib/tools/ssd' frr-contrib.x86_64: E: arch-dependent-file-in-usr-share /usr/share/doc/frr-contrib/tools/start-stop-daemon.o frr-contrib.x86_64: E: non-standard-dir-perm /usr/share/doc/frr-contrib/tools 644 frr-contrib.x86_64: W: hidden-file-or-dir /usr/share/doc/frr-contrib/tools/.deps frr-contrib.x86_64: E: non-standard-dir-perm /usr/share/doc/frr-contrib/tools/.deps 644 frr-contrib.x86_64: W: hidden-file-or-dir /usr/share/doc/frr-contrib/tools/.deps frr-contrib.x86_64: W: hidden-file-or-dir /usr/share/doc/frr-contrib/tools/.deps/.dirstamp frr-contrib.x86_64: E: zero-length /usr/share/doc/frr-contrib/tools/.deps/.dirstamp frr-contrib.x86_64: W: hidden-file-or-dir /usr/share/doc/frr-contrib/tools/.dirstamp frr-contrib.x86_64: E: zero-length /usr/share/doc/frr-contrib/tools/.dirstamp frr-contrib.x86_64: W: hidden-file-or-dir /usr/share/doc/frr-contrib/tools/.libs frr-contrib.x86_64: E: non-standard-dir-perm /usr/share/doc/frr-contrib/tools/.libs 644 frr-contrib.x86_64: W: hidden-file-or-dir /usr/share/doc/frr-contrib/tools/.libs frr-contrib.x86_64: E: non-standard-dir-perm /usr/share/doc/frr-contrib/tools/etc 644 frr-contrib.x86_64: E: non-standard-dir-perm /usr/share/doc/frr-contrib/tools/etc/frr 644 frr-contrib.x86_64: E: non-standard-dir-perm /usr/share/doc/frr-contrib/tools/etc/iproute2 644 frr-contrib.x86_64: E: non-standard-dir-perm /usr/share/doc/frr-contrib/tools/etc/iproute2/rt_protos.d 644 frr-contrib.x86_64: E: non-standard-dir-perm /usr/share/doc/frr-contrib/tools/etc/rsyslog.d 644 frr-devel.x86_64: W: only-non-binary-in-usr-lib frr-devel.x86_64: E: incorrect-fsf-address /usr/include/frr/ipaddr.h frr-devel.x86_64: E: incorrect-fsf-address /usr/include/frr/lua.h frr-devel.x86_64: E: incorrect-fsf-address /usr/include/frr/pbr.h frr-devel.x86_64: E: incorrect-fsf-address /usr/include/frr/sbuf.h frr-devel.x86_64: E: incorrect-fsf-address /usr/include/frr/version.h frr-devel.x86_64: E: incorrect-fsf-address /usr/include/frr/vlan.h frr-devel.x86_64: E: incorrect-fsf-address /usr/include/frr/vrf_int.h frr-devel.x86_64: E: incorrect-fsf-address /usr/include/frr/vxlan.h frr.src: E: description-line-too-long C FRRouting supports BGP4, OSPFv2, OSPFv3, ISIS, RIP, RIPng, PIM, NHRP, PBR, EIGRP and BFD. frr.src:6: E: hardcoded-library-path in /usr/lib/frr frr.src:33: W: unversioned-explicit-obsoletes frr-sysvinit frr.src:33: W: unversioned-explicit-obsoletes quagga frr.src:5: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 5) 6 packages and 0 specfiles checked; 31 errors, 33 warnings.
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
David Lamparter equinox-redhatbugz@diac24.net changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |equinox-redhatbugz@diac24.n | |et
--- Comment #5 from David Lamparter equinox-redhatbugz@diac24.net --- Hi, upstream maintainer here with feedback.
%global vty_group frrvt
Typo: the group is canonically named "frrvty", note the missing "y". Of course you could rename it but all our docs refer to "frrvty".
(Please don't inherit the historical "quaggavt" - not sure if that was a typo too or some 8-char limit...)
%package devel Summary: Header and object files for frr development
This entire subpackage is useless, FRR does not maintain a stable API or support out-of-tree development of software using libfrr. Just discard the headers and .so/.la files.
%configure \ --sbindir=%{frr_libdir} \ --sysconfdir=%{_sysconfdir}/frr \ --libdir=%{_libdir}/frr \ --libexecdir=%{_libexecdir}/frr \ --localstatedir=%{_localstatedir}/run/frr \ --enable-snmp=agentx \ --enable-multipath=64 \ --enable-vtysh=yes \ --enable-ospfclient=no \ --enable-ospfapi=no \ --enable-user=frr \ --enable-group=frr \ --enable-vty-group=%vty_group \ --enable-rtadv \ --disable-exampledir \ --enable-systemd=yes \ --enable-static=no \ --disable-ldpd \ --disable-babeld \ --enable-sharpd \
sharpd is used for development/testing purposes only and probably shouldn't be packaged.
--with-moduledir=%{_libdir}/frr/modules \ --enable-fpm
install -p -m 644 %{_builddir}/%{name}-%{frrversion}/redhat/daemons %{buildroot}/etc/frr/daemons install -p -m 644 %{_builddir}/%{name}-%{frrversion}/redhat/frr.service %{buildroot}%{_unitdir}/frr.service install -p -m 755 %{_builddir}/%{name}-%{frrversion}/redhat/frr.init %{buildroot}%{frr_libdir}/frr
The redhat/{frr.init,frr.service,daemons} files included in 7.0 are deprecated. Please use tools/{frrinit,frrcommon,watchfrr}.sh, tools/frr.service and tools/etc/frr/daemons instead. (Note this is one of changes in 7.0.1, the old files are gone.)
getent group frrvt >/dev/null 2>&1 || groupadd -r -g %vty_gid frrvt >/dev/null 2>&1 || :
Same typo in group name.
# Create dummy files if they don't exist so basic functions can be used. if [ ! -e %{_sysconfdir}/frr/zebra.conf ]; then echo "hostname `hostname`" > %{_sysconfdir}/frr/zebra.conf chown frr:frr %{_sysconfdir}/frr/zebra.conf chmod 640 %{_sysconfdir}/frr/zebra.conf fi
Split-config (zebra.conf, ospfd.conf) is slowly going the way of the dodo. Please create frr.conf instead.
%files contrib %defattr(-,root,root) %doc COPYING %attr(0644,root,root) tools
Most of the "tools/" content is development tools, not useful to the user. In all honesty, I wouldn't package any of it, except the init scripts & tools/etc bits.
%files devel %defattr(-,root,root) %doc COPYING %dir %{_libdir}/frr/ %{_libdir}/frr/*.so %dir %{_includedir}/frr %{_includedir}/frr/*.h %dir %{_includedir}/frr/ospfd %{_includedir}/frr/ospfd/*.h %dir %{_includedir}/frr/eigrpd %{_includedir}/frr/eigrpd/*.h
As noted above, all of these files could just be discarded. There is no point in a -devel package for FRR.
%changelog
- Wed May 29 2019 Michal Ruprich mruprich@redhat.com - 7.0-1
- Resolves: #1657029 - Add FRR as a replacement of Quagga in RHEL 8
Note 7.0.1 and 7.1 are available upstream.
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
Michal Ruprich 🐧 mruprich@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(mruprich@redhat.c | |om) |
--- Comment #6 from Michal Ruprich 🐧 mruprich@redhat.com --- Hi David, thanks for the feedback. I will incorporate the suggestions and update it to 7.1 before we release it.
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
--- Comment #7 from Michal Ruprich 🐧 mruprich@redhat.com --- Hi Martin,
I created a new build with the newer upstream version 7.1 and included suggestions from David. Could you take a look at this? I ran it through the review-tool and so far it seems good. The original [!] are gone now:
[!] /usr/share/doc/frr-contrib/tools contains object/relocatable files [!] /usr/share/doc/frr-contrib/tools/.libs contains shared object files/DSOs
I removed the contrib subpackage.
[!]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 122880 bytes in 15 files. /usr/share/doc/frr-contrib consumes 912K
The doc files were so big because all the files from tools directory were included as %doc as well. These are gone now.
[!] userdel && groupdel in %pre
I removed the userdel and groupdel.
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
--- Comment #8 from Michal Ruprich 🐧 mruprich@redhat.com --- Sorry I forgot to add a link to the new build:
https://copr-be.cloud.fedoraproject.org/results/mruprich/frr/fedora-rawhide-...
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
--- Comment #9 from Martin Osvald 🛹 mosvald@redhat.com --- There are still some issues reported by fedora-review which should get fixed (my notes inside '---' parts):
~~~ Issues: ======= - Package installs properly. Note: Installation errors (see attachment) See: https://docs.fedoraproject.org/en-US/packaging-guidelines/ ~~~
------------------------------------------------------------ relates to:
~~~ ERROR: Command failed: # /usr/bin/dnf --installroot /var/lib/mock/fedora-rawhide-x86_64/root/ --releasever 31 --setopt=deltarpm=False --disableplugin=local --disableplugin=spacewalk install /home/mosvald/review-frr/results/frr-debuginfo-7.1-1.fc31.x86_64.rpm /home/mosvald/review-frr/results/frr-debugsource-7.1-1.fc31.x86_64.rpm /home/mosvald/review-frr/results/frr-7.1-1.fc31.x86_64.rpm ~~~
dnf output:
~~~ Running transaction Preparing :
1/1 Running scriptlet: frr-7.1-1.fc31.x86_64
1/1 useradd: group frr exists - if you want to add this user to that group, use -g. usermod: group '%{vty_group}' does not exist error: %prein(frr-7.1-1.fc31.x86_64) scriptlet failed, exit status 6
Error in PREIN scriptlet in rpm package frr Verifying : frr-7.1-1.fc31.x86_64
1/1
Failed: frr-7.1-1.fc31.x86_64
Error: Transaction failed # ~~~
related specfile code:
~~~ 108 %pre 109 getent group frrvty >/dev/null 2>&1 || groupadd -r frrvty >/dev/null 2>&1 || : 110 getent group frr >/dev/null 2>&1 || groupadd frr >/dev/null 2>&1 || : 111 getent passwd frr >/dev/null 2>&1 || useradd -M -r -s /sbin/nologin \ 112 -c "FRRouting routing suite" -d %{_localstatedir}/run/frr frr || : 113 usermod -aG %{vty_group} frr ~~~
notes:
- frr is not system group (-r) even it owns files under /{etc,run,var/log}/frr, frr chowned:
~~~ Rpmlint ------- ... frr.x86_64: W: non-standard-uid /etc/frr frr frr.x86_64: W: non-standard-gid /etc/frr frr frr.x86_64: W: non-standard-uid /run/frr frr frr.x86_64: W: non-standard-gid /run/frr frr frr.x86_64: W: non-standard-uid /var/log/frr frr frr.x86_64: W: non-standard-gid /var/log/frr frr ~~~
- missing '-g frr' in useradd - %{vty_group} doesn't get substituted - exit 0 could be added at the end of %pre after solving all the above ------------------------------------------------------------
~~~ Issues: ======= ... - Texinfo files are installed using install-info in %post and %preun if package has .info files. Note: Texinfo .info file(s) in frr See: https://docs.fedoraproject.org/en-US/packaging- guidelines/Scriptlets/#_texinfo ... ~~~
------------------------------------------------------------ It happens because install-info command isn't mentioned in %preun, but %postun instead.
Related code:
/usr/share/fedora-review/plugins/generic.py: ~~~ 42 def in_list(what, list_): 43 """ test if 'what' is in each item in list_. """ 44 for item in list_: 45 if not item: 46 return False 47 if what not in item: 48 return False 49 return True ... 1918 class CheckInfoInstall(GenericCheckBase): 1919 """ Check that info files are properly installed. """ 1920 1921 def __init__(self, base): 1922 GenericCheckBase.__init__(self, base) 1923 self.url = ( 1924 "https://docs.fedoraproject.org/en-US" 1925 "/packaging-guidelines/Scriptlets/#_texinfo" 1926 ) 1927 self.text = ( 1928 "Texinfo files are installed using install-info" 1929 " in %post and %preun if package has .info files." 1930 ) 1931 self.automatic = True 1932 self.type = "MUST" 1933 1934 def run(self): 1935 using = [] 1936 failed = False 1937 for pkg in self.spec.packages: 1938 if self.rpms.find("/usr/share/info/*", pkg): 1939 using.append(pkg) 1940 rpm_pkg = self.rpms.get(pkg) 1941 if not in_list("install-info", [rpm_pkg.post, rpm_pkg.preun]): 1942 failed = True 1943 if not using: 1944 self.set_passed(self.NA) 1945 return 1946 text = "Texinfo .info file(s) in " + ", ".join(using) 1947 self.set_passed(self.FAIL if failed else self.PENDING, text) ~~~
I added extra DBG statements into in_list():
~~~ 42 def in_list(what, list_): 43 """ test if 'what' is in each item in list_. """ 44 for item in list_: 45 print("ITEM", item) 46 print("WHAT", what) 47 if not item: 48 print("FALSE1!") 49 return False 50 if what not in item: 51 print("FALSE2!") 52 return False 53 print("TRUE!") 54 return True ~~~
and the below is what I obtained for it:
~~~ ITEM /bin/sh
if [ $1 -eq 1 ] && [ -x /usr/bin/systemctl ] ; then # Initial installation /usr/bin/systemctl --no-reload preset frr.service || : fi
if [ -f /usr/share/info/frr.inf* ]; then install-info /usr/share/info/frr.info /usr/share/info/dir || : fi
# Create dummy files if they don't exist so basic functions can be used. if [ ! -e /etc/frr/frr.conf ]; then echo "hostname `hostname`" > /etc/frr/frr.conf chown frr:frr /etc/frr/frr.conf chmod 640 /etc/frr/frr.conf fi WHAT install-info ITEM /bin/sh
if [ $1 -eq 0 ] && [ -x /usr/bin/systemctl ] ; then # Package removal, not upgrade /usr/bin/systemctl --no-reload disable --now frr.service || : fi WHAT install-info FALSE2! ~~~
From the above, we can see that "install-info" gets found in %post section, but not in %preun (it's in %postun instead).
If we move it from %postun to %preun, this Issue disappears. ------------------------------------------------------------
~~~ Issues: ======= ... - systemd_post is invoked in %post, systemd_preun in %preun, and systemd_postun in %postun for Systemd service files. Note: Systemd service file(s) in frr See: https://docs.fedoraproject.org/en-US/packaging- guidelines/Scriptlets/#_scriptlets ~~~
------------------------------------------------------------ TODO:
I haven't managed to finish the investigation for it today, but I expect something similar to the above Texinfo problem.
Related code:
/usr/share/fedora-review/plugins/generic.py: ~~~ 2002 class CheckSystemdUnitdirScriplets(GenericCheckBase): 2003 """ Check that Systemd unitdir scriplets are run if required. """ 2004 2005 def __init__(self, base): 2006 GenericCheckBase.__init__(self, base) 2007 self.url = ( 2008 "https://docs.fedoraproject.org/en-US" 2009 "/packaging-guidelines/Scriptlets/#_scriptlets" 2010 ) 2011 self.text = ( 2012 "systemd_post is invoked in %post, systemd_preun in" 2013 " %preun, and systemd_postun in %postun for Systemd" 2014 " service files." 2015 ) 2016 self.automatic = True 2017 self.type = "MUST" 2018 2019 def run(self): 2020 using = [] 2021 failed = False 2022 systemd_post_re = re.compile( 2023 re.escape(rpm.expandMacro("%systemd_post .*.service")).replace( 2024 r".*", ".*" 2025 )[2:-4], 2026 re.M, 2027 ) 2028 systemd_preun_re = re.compile( 2029 re.escape(rpm.expandMacro("%systemd_preun .*.service")).replace( 2030 r".*", ".*" 2031 )[2:-4], 2032 re.M, 2033 ) 2034 for pkg in self.spec.packages: 2035 if self.rpms.find("/usr/lib/systemd/system/*", pkg): 2036 using.append(pkg) 2037 rpmpkg = self.rpms.get(pkg) 2038 if not systemd_post_re.search( 2039 str(rpmpkg.post) 2040 ) or not systemd_preun_re.search(str(rpmpkg.preun)): 2041 failed = True 2042 2043 if not using: 2044 self.set_passed(self.NA) 2045 return 2046 text = "Systemd service file(s) in " + ", ".join(using) 2047 self.set_passed(self.FAIL if failed else self.PASS, text) ~~~
by just quickly looking at the regexps I can see that it looks for the presence of:
"%systemd_post" in %post "%systemd_preun" in %preun
but %postun contains:
~~~ 129 %postun 130 %systemd_postun_with_restart frr.service ~~~
but simply renaming it to %systemd_postun, doesn't solve it. I don't get the logic between lines #2034 - #2041 yet. ------------------------------------------------------------
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
--- Comment #10 from Martin Osvald 🛹 mosvald@redhat.com --- this is a follow up to comment 9...
regarding the remaining issue:
~~~ Issues: ======= ... - systemd_post is invoked in %post, systemd_preun in %preun, and systemd_postun in %postun for Systemd service files. Note: Systemd service file(s) in frr See: https://docs.fedoraproject.org/en-US/packaging- guidelines/Scriptlets/#_scriptlets ~~~
It looks like a fedora-review bug to me. If we add extra debug statements, we find out that lines #2023 & #2029 (comment 9) containing:
rpm.expandMacro("%systemd_post .*.service") rpm.expandMacro("%systemd_preun .*.service")
expand to:
~~~ if [ $1 -eq 1 ] ; then # Initial installation systemctl --no-reload preset .*.service >/dev/null 2>&1 || : fi
~~~
and (respectively):
~~~ if [ $1 -eq 0 ] ; then # Package removal, not upgrade systemctl --no-reload disable --now .*.service >/dev/null 2>&1 || : fi
~~~
whereas the %post and %preun from lines #2039 & #2040 (comment 9):
str(rpmpkg.post) str(rpmpkg.preun)
expand to:
~~~ /bin/sh
if [ $1 -eq 1 ] && [ -x /usr/bin/systemctl ] ; then # Initial installation /usr/bin/systemctl --no-reload preset frr.service || : fi
if [ -f /usr/share/info/frr.inf* ]; then install-info /usr/share/info/frr.info /usr/share/info/dir || : fi
# Create dummy files if they don't exist so basic functions can be used. if [ ! -e /etc/frr/frr.conf ]; then echo "hostname `hostname`" > /etc/frr/frr.conf chown frr:frr /etc/frr/frr.conf chmod 640 /etc/frr/frr.conf fi ~~~
and (respectively):
~~~ /bin/sh
if [ $1 -eq 0 ] && [ -x /usr/bin/systemctl ] ; then # Package removal, not upgrade /usr/bin/systemctl --no-reload disable --now frr.service || : fi ~~~
while it is not easy to spot it at first, there is a missing part ">/dev/null 2>&1" in them which are present in the output from rpm.expandMacro().
This leads to setting 'failed' to true at line #2041 (comment 9) and reporting it as missing.
I don't have time to dig further to find out what exactly is behind this, that the actual code in frr.spec:
~~~ 115 %post 116 %systemd_post frr.service ... 139 %preun 140 %systemd_preun frr.service ~~~
doesn't resolve to the same output as from rpm.expandMacro(), but that what's happening.
Additional info:
Code with debug statements and related parts:
/usr/lib/python3.7/site-packages/FedoraReview/rpm_file.py ~~~ 96 def _scriptlet(self, prog_tag, script_tag): 97 """ Return inline -p script, script or None. """ 98 self.init() 99 prog = self.header_to_str(prog_tag) 100 script = self.header_to_str(script_tag) 101 print("PROG", prog) 102 print("SCRIPT", script) 103 if prog and script: 104 return prog + script 105 if prog: 106 return prog 107 return script ... 124 @property 125 def preun(self): 126 """ Return preUn scriptlet. """ 127 return self._scriptlet(rpm.RPMTAG_PREUNPROG, rpm.RPMTAG_PREUN) 128 129 @property 130 def post(self): 131 """ Return post scriptlet. """ 132 return self._scriptlet(rpm.RPMTAG_POSTINPROG, rpm.RPMTAG_POSTIN) ~~~
/usr/share/fedora-review/plugins/generic.py: ~~~ 2012 class CheckSystemdUnitdirScriplets(GenericCheckBase): 2013 """ Check that Systemd unitdir scriplets are run if required. """ 2014 2015 def __init__(self, base): 2016 GenericCheckBase.__init__(self, base) 2017 self.url = ( 2018 "https://docs.fedoraproject.org/en-US" 2019 "/packaging-guidelines/Scriptlets/#_scriptlets" 2020 ) 2021 self.text = ( 2022 "systemd_post is invoked in %post, systemd_preun in" 2023 " %preun, and systemd_postun in %postun for Systemd" 2024 " service files." 2025 ) 2026 self.automatic = True 2027 self.type = "MUST" 2028 2029 def run(self): 2030 using = [] 2031 failed = False 2032 systemd_post_re = re.compile( 2033 re.escape(rpm.expandMacro("%systemd_post .*.service")).replace( 2034 r".*", ".*" 2035 )[2:-4], 2036 re.M, 2037 ) 2038 systemd_preun_re = re.compile( 2039 re.escape(rpm.expandMacro("%systemd_preun .*.service")).replace( 2040 r".*", ".*" 2041 )[2:-4], 2042 re.M, 2043 ) 2044 print("SYSTEMD_POST_RE", systemd_post_re) 2045 print("SYSTEMD_PREUN_RE", systemd_preun_re) 2046 print("EXPANDMACRO POST", rpm.expandMacro("%systemd_post .*.service")) 2047 print("EXPANDMACRO PREUN", rpm.expandMacro("%systemd_preun .*.service")) 2048 for pkg in self.spec.packages: 2049 print("PKG", pkg) 2050 if self.rpms.find("/usr/lib/systemd/system/*", pkg): 2051 using.append(pkg) 2052 rpmpkg = self.rpms.get(pkg) 2053 print("STR(RPMKG.POST)", str(rpmpkg.post)) 2054 print("STR(RPMKG.PREUN)", str(rpmpkg.preun)) 2055 if not systemd_post_re.search( 2056 str(rpmpkg.post) 2057 ) or not systemd_preun_re.search(str(rpmpkg.preun)): 2058 print("FAILED") 2059 failed = True 2060 2061 if not using: 2062 self.set_passed(self.NA) 2063 return 2064 text = "Systemd service file(s) in " + ", ".join(using) 2065 self.set_passed(self.FAIL if failed else self.PASS, text) ~~~
debug output from the above altered code:
~~~ SYSTEMD_POST_RE re.compile('if\ \[\ \$1\ \-eq\ 1\ \]\ ;\ then\ \\n\ \ \ \ \ \ \ \ \#\ Initial\ installation\ \\n\ \ \ \ \ \ \ \ systemctl\ \-\-no\-reload\ preset\ .*\.service\ >/dev/nul, re.MULTILINE) SYSTEMD_PREUN_RE re.compile('if\ \[\ \$1\ \-eq\ 0\ \]\ ;\ then\ \\n\ \ \ \ \ \ \ \ \#\ Package\ removal,\ not\ upgrade\ \\n\ \ \ \ \ \ \ \ systemctl\ \-\-no\-reload\ disable\ \-\-now\, re.MULTILINE) EXPANDMACRO POST if [ $1 -eq 1 ] ; then # Initial installation systemctl --no-reload preset .*.service >/dev/null 2>&1 || : fi
EXPANDMACRO PREUN if [ $1 -eq 0 ] ; then # Package removal, not upgrade systemctl --no-reload disable --now .*.service >/dev/null 2>&1 || : fi
PKG frr PROG /bin/sh SCRIPT
if [ $1 -eq 1 ] && [ -x /usr/bin/systemctl ] ; then # Initial installation /usr/bin/systemctl --no-reload preset frr.service || : fi
if [ -f /usr/share/info/frr.inf* ]; then install-info /usr/share/info/frr.info /usr/share/info/dir || : fi
# Create dummy files if they don't exist so basic functions can be used. if [ ! -e /etc/frr/frr.conf ]; then echo "hostname `hostname`" > /etc/frr/frr.conf chown frr:frr /etc/frr/frr.conf chmod 640 /etc/frr/frr.conf fi STR(RPMKG.POST) /bin/sh
if [ $1 -eq 1 ] && [ -x /usr/bin/systemctl ] ; then # Initial installation /usr/bin/systemctl --no-reload preset frr.service || : fi
if [ -f /usr/share/info/frr.inf* ]; then install-info /usr/share/info/frr.info /usr/share/info/dir || : fi
# Create dummy files if they don't exist so basic functions can be used. if [ ! -e /etc/frr/frr.conf ]; then echo "hostname `hostname`" > /etc/frr/frr.conf chown frr:frr /etc/frr/frr.conf chmod 640 /etc/frr/frr.conf fi PROG /bin/sh SCRIPT
if [ $1 -eq 0 ] && [ -x /usr/bin/systemctl ] ; then # Package removal, not upgrade /usr/bin/systemctl --no-reload disable --now frr.service || : fi STR(RPMKG.PREUN) /bin/sh
if [ $1 -eq 0 ] && [ -x /usr/bin/systemctl ] ; then # Package removal, not upgrade /usr/bin/systemctl --no-reload disable --now frr.service || : fi PROG /bin/sh SCRIPT
if [ $1 -eq 1 ] && [ -x /usr/bin/systemctl ] ; then # Initial installation /usr/bin/systemctl --no-reload preset frr.service || : fi
if [ -f /usr/share/info/frr.inf* ]; then install-info /usr/share/info/frr.info /usr/share/info/dir || : fi
# Create dummy files if they don't exist so basic functions can be used. if [ ! -e /etc/frr/frr.conf ]; then echo "hostname `hostname`" > /etc/frr/frr.conf chown frr:frr /etc/frr/frr.conf chmod 640 /etc/frr/frr.conf fi FAILED PKG frr-debuginfo PKG frr-debugsource ~~~
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
Martin Osvald 🛹 mosvald@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(mruprich@redhat.c | |om)
--- Comment #11 from Martin Osvald 🛹 mosvald@redhat.com --- Michal, pls, see the comment 9 and comment 10 to find out about the remaining issues needed to be fixed.
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
Michal Ruprich 🐧 mruprich@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(mruprich@redhat.c | |om) |
--- Comment #12 from Michal Ruprich 🐧 mruprich@redhat.com --- Latest build in COPR here: https://copr-be.cloud.fedoraproject.org/results/mruprich/frr/fedora-rawhide-...
I moved the install-info. As for the systemd macros - we discussed this with Martin and this seems to be a bug in fedora-review tool.
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
Martin Osvald 🛹 mosvald@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(mruprich@redhat.c | |om)
--- Comment #13 from Martin Osvald 🛹 mosvald@redhat.com --- It probably got overlooked in my messy comment 9 as it contained several issues mentioned close together so it probably got overlooked:
- frr is not system group (-r) even it owns files under /{etc,run,var/log}/frr:
~~~ $ grep non-standard ~/review-frr/review.txt frr.x86_64: W: non-standard-uid /etc/frr frr frr.x86_64: W: non-standard-gid /etc/frr frr frr.x86_64: W: non-standard-uid /run/frr frr frr.x86_64: W: non-standard-gid /run/frr frr frr.x86_64: W: non-standard-uid /var/log/frr frr frr.x86_64: W: non-standard-gid /var/log/frr frr $ ~~~
While the above warnings don't directly relate to the missing '-r' option, but to the fact that frr is not defined in hardcoded DEFAULT_STANDARD_USERS/DEFAULT_STANDARD_GROUPS or in setup/uidgid file, I still think groupadd for frr should contain '-r' option like it is for frrvty:
~~~ %pre getent group frrvty >/dev/null 2>&1 || groupadd -r frrvty >/dev/null 2>&1 || : getent group frr >/dev/null 2>&1 || groupadd frr >/dev/null 2>&1 || : ~~~
Otherwise, I filled two new BZs for fedora-review regarding the broken link to texinfo scriplet documentation and the other one regarding strange macro expansions for %systemd_*:
Bug 1725569 - fedora-review points to non-existent texinfo documentation Bug 1725584 - fedora-review: strange behavior in CheckSystemdUnitdirScriplets()
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
Michal Ruprich 🐧 mruprich@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(mruprich@redhat.c | |om) |
--- Comment #14 from Michal Ruprich 🐧 mruprich@redhat.com --- Thanks Martin,
hopefully this is the final version: https://copr-be.cloud.fedoraproject.org/results/mruprich/frr/fedora-rawhide-...
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
Martin Osvald 🛹 mosvald@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+
--- Comment #15 from Martin Osvald 🛹 mosvald@redhat.com --- It's OK from my side now, giving it fedora-review+
Below is a fresh output from fedora-review (went through the manual review needed items again ([ ])). There are few reported issues, but either we opened a new BZs for them (considering them as process review bugs) or found them as not problematic.
Package Review ==============
Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed [X] = Passed manual review
Issues: ======= - Sources used to build the package match the upstream source, as provided in the spec URL. Note: Upstream MD5sum check error, diff is in /home/mosvald/review- frr/diff.txt See: https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/ - systemd_post is invoked in %post, systemd_preun in %preun, and systemd_postun in %postun for Systemd service files. Note: Systemd service file(s) in frr See: https://docs.fedoraproject.org/en-US/packaging- guidelines/Scriptlets/#_scriptlets
===== MUST items =====
C/C++: [X]: Package does not contain kernel modules. [X]: Package contains no static executables. [X]: Development (unversioned) .so files in -devel subpackage, if present. Note: Unversioned so-files in private %_libdir subdirectory (see attachment). Verify they are not in ld path. [x]: If your application is a C or C++ application you must list a BuildRequires against gcc, gcc-c++ or clang. [x]: Header files in -devel subpackage, if present. [x]: Package does not contain any libtool archives (.la) [x]: Rpath absent or only used for internal libs.
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", "GPL (v2 or later)", "Expat License", "GPL (v2 or later) (with incorrect FSF address)", "ISC License GPL (v2 or later) (with incorrect FSF address)", "ISC License", "GPL (v3 or later)", "ISC License GPL (v2 or later)", "BSD 3-clause "New" or "Revised" License", "ISC License BSD 2-clause "Simplified" License", "BSD 2-clause "Simplified" License", "*No copyright* GPL (v2 or later)", "GNU Lesser General Public License (v2.1 or later)", "BSD (unspecified)", "GNU General Public License", "FSF Unlimited License (with Retention) GNU General Public License", "FSF Unlimited License (with Retention)", "GPL (with incorrect FSF address)", "*No copyright* Public domain", "*No copyright* GNU General Public License", "Expat License GPL (v2 or later)". 400 files have unknown license. Detailed output of licensecheck in /home/mosvald/review-frr/licensecheck.txt [X]: License file installed when any subpackage combination is installed. [X]: Package requires other packages for directories it uses. Note: No known owner of /usr/share/yang, /usr/lib/frr, /usr/lib64/frr/modules [X]: Package must own all directories that it creates. Note: Directories without known owners: /usr/lib/frr, /usr/share/yang, /usr/lib64/frr/modules, /etc/logrotate.d [X]: %build honors applicable compiler flags or justifies otherwise. [X]: Package contains no bundled libraries without FPC exception. [X]: Changelog in prescribed format. [X]: Sources contain only permissible code or content. [X]: Each %files section contains %defattr if rpm < 4.4 Note: %defattr present but not needed [-]: 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]: Texinfo files are installed using install-info in %post and %preun if package has .info files. Note: Texinfo .info file(s) in frr [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. [X]: Package contains systemd file(s) if in need. [X]: Useful -debuginfo package or justification otherwise. [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 61440 bytes in 13 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 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]: %config files are marked noreplace or the reason is justified. [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]: No %config files under /usr. [x]: Package does not use a name that already exists. [x]: Package is not relocatable. [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
===== SHOULD items =====
Generic: [!]: Uses parallel make %{?_smp_mflags} macro. [X]: 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 frr [X]: Package functions as described. [X]: Latest version is packaged. [X]: Package does not include license text files separate from upstream. [X]: Patches link to upstream bugs/comments/lists or are otherwise justified. [?]: 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. [-]: Files in /run, var/run and /var/lock uses tmpfiles.d when appropriate [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 debuginfo package(s). Note: No rpmlint messages. [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. [x]: Package should not use obsolete m4 macros [x]: Spec file according to URL is the same as in SRPM.
Rpmlint ------- Checking: frr-7.1-1.fc31.x86_64.rpm frr-debuginfo-7.1-1.fc31.x86_64.rpm frr-debugsource-7.1-1.fc31.x86_64.rpm frr-7.1-1.fc31.src.rpm frr.x86_64: E: description-line-too-long C FRRouting supports BGP4, OSPFv2, OSPFv3, ISIS, RIP, RIPng, PIM, NHRP, PBR, EIGRP and BFD. frr.x86_64: E: missing-call-to-setgroups-before-setuid /usr/bin/vtysh frr.x86_64: W: non-standard-uid /etc/frr frr frr.x86_64: W: non-standard-gid /etc/frr frr frr.x86_64: W: non-conffile-in-etc /etc/frr/daemons frr.x86_64: W: non-standard-uid /run/frr frr frr.x86_64: W: non-standard-gid /run/frr frr frr.x86_64: E: incorrect-fsf-address /usr/lib/frr/frr-reload.py frr.x86_64: W: non-standard-uid /var/log/frr frr frr.x86_64: W: non-standard-gid /var/log/frr frr frr.x86_64: W: dangerous-command-in-%post chown frr.src: E: description-line-too-long C FRRouting supports BGP4, OSPFv2, OSPFv3, ISIS, RIP, RIPng, PIM, NHRP, PBR, EIGRP and BFD. frr.src:2: E: hardcoded-library-path in /usr/lib/frr frr.src:82: W: mixed-use-of-spaces-and-tabs (spaces: line 82, tab: line 1) 4 packages and 0 specfiles checked; 5 errors, 9 warnings.
Rpmlint (debuginfo) ------------------- Checking: frr-debuginfo-7.1-1.fc31.x86_64.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings.
Rpmlint (installed packages) ---------------------------- Cannot parse rpmlint output:
Unversioned so-files -------------------- frr: /usr/lib64/frr/modules/bgpd_snmp.so frr: /usr/lib64/frr/modules/ospf6d_snmp.so frr: /usr/lib64/frr/modules/ospfd_snmp.so frr: /usr/lib64/frr/modules/ripd_snmp.so frr: /usr/lib64/frr/modules/zebra_fpm.so frr: /usr/lib64/frr/modules/zebra_irdp.so frr: /usr/lib64/frr/modules/zebra_snmp.so
Source checksums ---------------- https://github.com/FRRouting/frr/releases/download/frr-7.1/frr-7.1.tar.gz : CHECKSUM(SHA256) this package : 290ba09436353be69a266121749c04e076b9ed6fd4d262dc857e7249566c7677 CHECKSUM(SHA256) upstream package : 96513465a8837b02cf93c1efd9fa88ac06e1189e0fc6a6573f24092773631797 diff -r also reports differences
Requires -------- frr (rpmlib, GLIBC filtered): /bin/sh /sbin/install-info /usr/bin/bash /usr/bin/python3 /usr/bin/sh config(frr) libc.so.6()(64bit) libcap.so.2()(64bit) libcares.so.2()(64bit) libcrypt.so.2()(64bit) libcrypt.so.2(XCRYPT_2.0)(64bit) libdl.so.2()(64bit) libfrr.so.0()(64bit) libfrrsnmp.so.0()(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libgcc_s.so.1(GCC_3.3.1)(64bit) libjson-c.so.4()(64bit) libm.so.6()(64bit) libnetsnmp.so.35()(64bit) libnetsnmpagent.so.35()(64bit) libnetsnmpmibs.so.35()(64bit) libpthread.so.0()(64bit) libreadline.so.8()(64bit) librt.so.1()(64bit) libsystemd.so.0()(64bit) libsystemd.so.0(LIBSYSTEMD_209)(64bit) libyang libyang.so.0.16()(64bit) ncurses net-snmp rtld(GNU_HASH) systemd
frr-debuginfo (rpmlib, GLIBC filtered):
frr-debugsource (rpmlib, GLIBC filtered):
Provides -------- frr: config(frr) frr frr(x86-64) libfrr.so.0()(64bit) libfrrsnmp.so.0()(64bit) routingdaemon
frr-debuginfo: debuginfo(build-id) frr-debuginfo frr-debuginfo(x86-64)
frr-debugsource: frr-debugsource frr-debugsource(x86-64)
Generated by fedora-review 0.7.2 (65d36bb) last change: 2019-04-09 Command line :/usr/bin/fedora-review -n frr Buildroot used: fedora-rawhide-x86_64 Active plugins: C/C++, Generic, Shell-api Disabled plugins: fonts, Java, Haskell, SugarActivity, Ocaml, R, PHP, Perl, Python Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
--- Comment #16 from Mohan Boddu mboddu@bhujji.com --- (fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/frr
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |MODIFIED
--- Comment #17 from Fedora Update System updates@fedoraproject.org --- FEDORA-2019-6a069524b6 has been submitted as an update to Fedora 30. https://bodhi.fedoraproject.org/updates/FEDORA-2019-6a069524b6
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |ON_QA
--- Comment #18 from Fedora Update System updates@fedoraproject.org --- frr-7.1-1.fc30 has been pushed to the Fedora 30 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-6a069524b6
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Resolution|--- |ERRATA Last Closed| |2019-07-30 01:14:35
--- Comment #19 from Fedora Update System updates@fedoraproject.org --- frr-7.1-1.fc30 has been pushed to the Fedora 30 stable repository. If problems still persist, please make note of it in this bug report.
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
Robert Scheck redhat-bugzilla@linuxnetz.de changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |redhat-bugzilla@linuxnetz.d | |e
--- Comment #20 from Robert Scheck redhat-bugzilla@linuxnetz.de --- Martin, this package review was unfortunately not as good as it could have been, because bug #1830808, bug #1830806, bug #1830800, bug #1830798 and bug #1830797 already existed while the package review happened (as per https://src.fedoraproject.org/rpms/frr/blob/463257a2ddcf53c9d227f35c3d071b3c...) but it still passed the review. Please treat this as simple "heads up" for future package reviews - thank you :)
package-review@lists.fedoraproject.org
-
bugzilla＠redhat.com