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