[packaging] Failing patch application upon update in sm's RPM triggerin on XenServer 8

[stormi]

The patch application in sm.spec fails if the patch has already been applied previously, with:

warning: %triggerin(sm-3.2.3-1.1.xcpng8.3.x86_64) scriptlet failed, exit status 1
Non-fatal <unknown> scriptlet failure in rpm package sm-3.2.3-1.1.xcpng8.3.x86_64

This comes from :

# Mark processes that should be moved to the data path
%triggerin -- libcgroup-tools
( patch -tsN -r - -d / -p0 )>/dev/null << 'EOF'
--- /etc/cgrules.conf→  2018-04-11 02:33:52.000000000 +0000
+++ /tmp/cgrules.conf→  2024-01-26 17:30:29.204242549 +0000
@@ -7,4 +7,6 @@
 #@student→     cpu,memory→     usergroup/student/
 #peter→→       cpu→    →       test1/
 #%→    →       memory→ →       test2/
+*:tapdisk→     cpu,cpuacct→    vm.slice/
+%→     →       blkio→  →       vm.slice/
 # End of file
EOF

One way to fix it is doing the same as what is done in xenserver-release.spec:

diff --git a/SPECS/sm.spec b/SPECS/sm.spec
index 922133c..8338be8 100644
--- a/SPECS/sm.spec
+++ b/SPECS/sm.spec
@@ -90,7 +90,7 @@ make install DESTDIR=%{buildroot}

 # Mark processes that should be moved to the data path
 %triggerin -- libcgroup-tools
-( patch -tsN -r - -d / -p0 )>/dev/null << 'EOF'
+( patch -tsN -r - -d / -p0 || : )>/dev/null << 'EOF'
 --- /etc/cgrules.conf  2018-04-11 02:33:52.000000000 +0000
 +++ /tmp/cgrules.conf  2024-01-26 17:30:29.204242549 +0000
 @@ -7,4 +7,6 @@

However, I don't like this solution, because it hides away real failures, and this has already bitten XenServer in the past (SSH cipher lists not updated in configuration files).

When there is no other solution but patching a file, I prefer to use a different approach: if the patch can apply in reverse mode (with -R --dry-run), then don't attempt to apply it. Otherwise, apply it but don't hide error messages. I should probably make it a RPM macro for simpler spec files.

diff --git a/SPECS/sm.spec b/SPECS/sm.spec
index 922133c..baecb9e 100644
--- a/SPECS/sm.spec
+++ b/SPECS/sm.spec
@@ -88,21 +88,32 @@ make -C misc/fairlock
 make -C misc/fairlock install DESTDIR=%{buildroot}
 make install DESTDIR=%{buildroot}

 # Mark processes that should be moved to the data path
 %triggerin -- libcgroup-tools
-( patch -tsN -r - -d / -p0 )>/dev/null << 'EOF'
+CGRULES_PATCH=$(cat << 'EOF'
 --- /etc/cgrules.conf  2018-04-11 02:33:52.000000000 +0000
 +++ /tmp/cgrules.conf  2024-01-26 17:30:29.204242549 +0000
 @@ -7,4 +7,6 @@
  #@student cpu,memory  usergroup/student/
  #peter        cpu     test1/
  #%        memory      test2/
 +*:tapdisk cpu,cpuacct vm.slice/
 +%     blkio       vm.slice/
  # End of file
 EOF
+)
+
+# Do not apply patch if it was already applied
+if ! echo "$CGRULES_PATCH" | patch --dry-run -RsN -d / -p1 >/dev/null; then
+    # Apply patch. Output NOT redirected to /dev/null so that error messages are displayed
+    if ! echo "$CGRULES_PATCH" | patch -tsN -r - -d / -p1; then
+        echo "Error: failed to apply patch:"
+        echo "$CGRULES_PATCH"
+        false
+    fi
+fi

 %pre
 # Remove sm-multipath on install or upgrade, to ensure it goes
 [ ! -x /sbin/chkconfig ] || chkconfig --del sm-multipath || :

[MarkSymsCtx]

@stormi ~please send a PR~. Ah, sorry, you can't as it's an internal spec repo, I'll create a ticket for us to do it.