9:44 a.m.
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(a)fedoraproject.org
Reporter: mruprich(a)redhat.com
QA Contact: extras-qa(a)fedoraproject.org
CC: package-review(a)lists.fedoraproject.org
Target Milestone: ---
Classification: Fedora
Spec URL:
https://copr-be.cloud.fedoraproject.org/results/mruprich/frr/fedora-rawhi...
SRPM URL:
https://copr-be.cloud.fedoraproject.org/results/mruprich/frr/fedora-rawhi...
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
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
9:45 a.m.
New subject: [Bug 1702720] Review Request: frr - routing daemon
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
Michal Ruprich <mruprich(a)redhat.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Assignee|nobody(a)fedoraproject.org |mosvald(a)redhat.com
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
9:46 a.m.
New subject: [Bug 1702720] Review Request: frr - routing daemon
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
Martin Osvald 🛹 <mosvald(a)redhat.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Flags| |fedora-review?
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
9:54 a.m.
New subject: [Bug 1702720] Review Request: frr - routing daemon
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
--- Comment #1 from Michal Ruprich <mruprich(a)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.
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
4:05 a.m.
New subject: [Bug 1702720] Review Request: frr - routing daemon
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
Fabian Affolter <mail(a)fabian-affolter.ch> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |mail(a)fabian-affolter.ch
--- Comment #2 from Fabian Affolter <mail(a)fabian-affolter.ch> ---
Both URLs are 404.
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
5:16 a.m.
New subject: [Bug 1702720] Review Request: frr - routing daemon
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
--- Comment #3 from Michal Ruprich <mruprich(a)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-rawhi...
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
4:37 a.m.
New subject: [Bug 1702720] Review Request: frr - routing daemon
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
Martin Osvald 🛹 <mosvald(a)redhat.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Flags| |needinfo?(mruprich(a)redhat.c
| |om)
--- Comment #4 from Martin Osvald 🛹 <mosvald(a)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.
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
8:28 a.m.
New subject: [Bug 1702720] Review Request: frr - routing daemon
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
David Lamparter <equinox-redhatbugz(a)diac24.net> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |equinox-redhatbugz(a)diac24.n
| |et
--- Comment #5 from David Lamparter <equinox-redhatbugz(a)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(a)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.
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
8:57 a.m.
New subject: [Bug 1702720] Review Request: frr - routing daemon
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
Michal Ruprich 🐧 <mruprich(a)redhat.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Flags|needinfo?(mruprich(a)redhat.c |
|om) |
--- Comment #6 from Michal Ruprich 🐧 <mruprich(a)redhat.com> ---
Hi David, thanks for the feedback. I will incorporate the suggestions and
update it to 7.1 before we release it.
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
2:52 a.m.
New subject: [Bug 1702720] Review Request: frr - routing daemon
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
--- Comment #7 from Michal Ruprich 🐧 <mruprich(a)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.
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
3:04 a.m.
New subject: [Bug 1702720] Review Request: frr - routing daemon
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
--- Comment #8 from Michal Ruprich 🐧 <mruprich(a)redhat.com> ---
Sorry I forgot to add a link to the new build:
https://copr-be.cloud.fedoraproject.org/results/mruprich/frr/fedora-rawhi...
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
12:21 p.m.
New subject: [Bug 1702720] Review Request: frr - routing daemon
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
--- Comment #9 from Martin Osvald 🛹 <mosvald(a)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.
------------------------------------------------------------
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
3:13 a.m.
New subject: [Bug 1702720] Review Request: frr - routing daemon
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
--- Comment #10 from Martin Osvald 🛹 <mosvald(a)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
~~~
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
3:15 a.m.
New subject: [Bug 1702720] Review Request: frr - routing daemon
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
Martin Osvald 🛹 <mosvald(a)redhat.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Flags| |needinfo?(mruprich(a)redhat.c
| |om)
--- Comment #11 from Martin Osvald 🛹 <mosvald(a)redhat.com> ---
Michal, pls, see the comment 9 and comment 10 to find out about the remaining
issues needed to be fixed.
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
5:32 a.m.
New subject: [Bug 1702720] Review Request: frr - routing daemon
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
Michal Ruprich 🐧 <mruprich(a)redhat.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Flags|needinfo?(mruprich(a)redhat.c |
|om) |
--- Comment #12 from Michal Ruprich 🐧 <mruprich(a)redhat.com> ---
Latest build in COPR here:
https://copr-be.cloud.fedoraproject.org/results/mruprich/frr/fedora-rawhi...
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.
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
2:54 a.m.
New subject: [Bug 1702720] Review Request: frr - routing daemon
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
Martin Osvald 🛹 <mosvald(a)redhat.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Flags| |needinfo?(mruprich(a)redhat.c
| |om)
--- Comment #13 from Martin Osvald 🛹 <mosvald(a)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()
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
6:39 a.m.
New subject: [Bug 1702720] Review Request: frr - routing daemon
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
Michal Ruprich 🐧 <mruprich(a)redhat.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Flags|needinfo?(mruprich(a)redhat.c |
|om) |
--- Comment #14 from Michal Ruprich 🐧 <mruprich(a)redhat.com> ---
Thanks Martin,
hopefully this is the final version:
https://copr-be.cloud.fedoraproject.org/results/mruprich/frr/fedora-rawhi...
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
7:46 a.m.
New subject: [Bug 1702720] Review Request: frr - routing daemon
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
Martin Osvald 🛹 <mosvald(a)redhat.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Flags|fedora-review? |fedora-review+
--- Comment #15 from Martin Osvald 🛹 <mosvald(a)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
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
9:06 a.m.
New subject: [Bug 1702720] Review Request: frr - routing daemon
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
--- Comment #16 from Mohan Boddu <mboddu(a)bhujji.com> ---
(fedscm-admin): The Pagure repository was created at
https://src.fedoraproject.org/rpms/frr
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
9 a.m.
New subject: [Bug 1702720] Review Request: frr - routing daemon
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
Fedora Update System <updates(a)fedoraproject.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |MODIFIED
--- Comment #17 from Fedora Update System <updates(a)fedoraproject.org> ---
FEDORA-2019-6a069524b6 has been submitted as an update to Fedora 30.
https://bodhi.fedoraproject.org/updates/FEDORA-2019-6a069524b6
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
8:10 p.m.
New subject: [Bug 1702720] Review Request: frr - routing daemon
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
Fedora Update System <updates(a)fedoraproject.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|MODIFIED |ON_QA
--- Comment #18 from Fedora Update System <updates(a)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
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
8:14 p.m.
New subject: [Bug 1702720] Review Request: frr - routing daemon
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
Fedora Update System <updates(a)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(a)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.
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
6:54 p.m.
New subject: [Bug 1702720] Review Request: frr - routing daemon
https://bugzilla.redhat.com/show_bug.cgi?id=1702720
Robert Scheck <redhat-bugzilla(a)linuxnetz.de> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |redhat-bugzilla(a)linuxnetz.d
| |e
--- Comment #20 from Robert Scheck <redhat-bugzilla(a)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/463257a2ddcf53c9d227f35c3d071...)
but it still passed the review. Please treat this as simple "heads up" for
future package reviews - thank you :)
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
1451
days inactive
1826
days old
package-review@lists.fedoraproject.org
22 comments
1 participants
participants (1)
-
bugzilla＠redhat.com