https://bugzilla.redhat.com/show_bug.cgi?id=1702720 --- Comment #10 from Martin Osvald 🛹 <mosvald@xxxxxxxxxx> --- 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 _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx
