Description of problem: While reviewing frr I came across the below: ~~~ 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 ~~~ The above got reported even if frr.spec contains: ~~~ %post %systemd_post frr.service ... %postun %systemd_postun_with_restart frr.service ... %preun %systemd_preun frr.service ~~~ How reproducible: Always. Steps to Reproduce: 1. Run fedora-review on frr Actual results: The below gets reported: ~~~ - 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 ~~~ Expected results: No report of the above. Additional info: Please, see the below comment containing thorough investigation results: https://bugzilla.redhat.com/show_bug.cgi?id=1702720#c10 Copy&pasting the comment from there: ============================================================ 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 ~~~ ============================================================
This bug appears to have been reported against 'rawhide' during the Fedora 31 development cycle. Changing version to '31'.
This bug appears to have been reported against 'rawhide' during the Fedora 31 development cycle. Changing version to 31.
This message is a reminder that Fedora 31 is nearing its end of life. Fedora will stop maintaining and issuing updates for Fedora 31 on 2020-11-24. It is Fedora's policy to close all bug reports from releases that are no longer maintained. At that time this bug will be closed as EOL if it remains open with a Fedora 'version' of '31'. Package Maintainer: If you wish for this bug to remain open because you plan to fix it in a currently maintained version, simply change the 'version' to a later Fedora version. Thank you for reporting this issue and we are sorry that we were not able to fix it before Fedora 31 is end of life. If you would still like to see this bug fixed and are able to reproduce it against a later version of Fedora, you are encouraged change the 'version' to a later Fedora version prior this bug is closed as described in the policy above. Although we aim to fix as many bugs as possible during every release's lifetime, sometimes those efforts are overtaken by events. Often a more recent Fedora release includes newer upstream software that fixes bugs or makes them obsolete.
Fedora 31 changed to end-of-life (EOL) status on 2020-11-24. Fedora 31 is no longer maintained, which means that it will not receive any further security or bug fix updates. As a result we are closing this bug. If you can reproduce this bug against a currently maintained version of Fedora please feel free to reopen this bug against that version. If you are unable to reopen this bug, please file a new report against the current release. If you experience problems, please add a comment to this bug. Thank you for reporting this bug and we are sorry it could not be fixed.