clamav-server: destroot patch failed

[ryandesign (Ryan Carsten Schmidt)]

​https://build.macports.org/builders/ports-10.15_x86_64-builder/builds/3026/steps/install-port/logs/stdio

DEBUG: Executing org.macports.destroot (clamav-server)
xinstall: mkdir /opt/local/var/macports/build/_opt_bblocal_var_buildworker_ports_build_ports_sysutils_clamav-server/clamav-server/work/destroot/opt/local/var/clamav
xinstall: mkdir /opt/local/var/macports/build/_opt_bblocal_var_buildworker_ports_build_ports_sysutils_clamav-server/clamav-server/work/destroot/opt/local/var/log/clamav
xinstall: mkdir /opt/local/var/macports/build/_opt_bblocal_var_buildworker_ports_build_ports_sysutils_clamav-server/clamav-server/work/destroot/opt/local/var/run/clamav
xinstall: mkdir /opt/local/var/macports/build/_opt_bblocal_var_buildworker_ports_build_ports_sysutils_clamav-server/clamav-server/work/destroot/opt/local/share/clamav
xinstall: /opt/local/etc/clamd.conf.sample -> /opt/local/var/macports/build/_opt_bblocal_var_buildworker_ports_build_ports_sysutils_clamav-server/clamav-server/work/destroot/opt/local/etc/clamd.conf.macports
DEBUG: system -W /opt/local/var/macports/build/_opt_bblocal_var_buildworker_ports_build_ports_sysutils_clamav-server/clamav-server/work/destroot/opt/local: patch -p1 ./etc/clamd.conf.macports /opt/bblocal/var/buildworker/ports/build/ports/sysutils/clamav-server/files/patch-etc-clamd-conf-macports.diff
patching file ./etc/clamd.conf.macports
Hunk #6 FAILED at 156.
Hunk #7 succeeded at 176 (offset -1 lines).
Hunk #8 succeeded at 302 (offset -1 lines).
Hunk #9 succeeded at 319 with fuzz 1 (offset -1 lines).
Hunk #10 succeeded at 335 (offset -1 lines).
1 out of 10 hunks FAILED -- saving rejects to file ./etc/clamd.conf.macports.rej
Command failed: patch -p1 ./etc/clamd.conf.macports /opt/bblocal/var/buildworker/ports/build/ports/sysutils/clamav-server/files/patch-etc-clamd-conf-macports.diff
Exit code: 1
Error: Failed to destroot clamav-server: command execution failed

The hunk that's failing is:

@@ -156,7 +156,7 @@
 #SendBufTimeout 200

 # Maximum number of queued items (including those being processed by
-# MaxThreads threads)
+MaxThreads 6
 # It is recommended to have this value at least twice MaxThreads if possible.
 # WARNING: you shouldn't increase this too much to avoid running out  of file
 # descriptors,

because there's now a period after the closing parenthesis.

But why did this build successfully in June? It looks like this is because the file it's patching here (a copy of /opt/local/etc/clamd.conf.sample) doesn't come from this port; it comes from the clamav port. This is a pretty strange situation. Ports don't usually patch other ports' files. If it's going to continue to be that way, then that means that clamav-server should be revbumped (and possibly the patch will need to be changed) anytime the clamav port is updated in such a way that this conf file is changed. So at minimum a note should be added to the clamav port advising its maintainer of that.

And more importantly, why is this hunk in the patchfile at all? It looks totally wrong, like the result of a bad global search-replace. (MaxThreads 6 already appears in the correct place earlier in the file.)

[danielluke (Daniel J. Luke)]

I don’t know. I didn’t make the clamav-server port and I don’t maintain it.

[essandess (Steve Smith)]

I originally had line-by-line reinplace commands, but these were replaced by patch files in the review. See ​https://github.com/macports/macports-ports/pull/4587#discussion_r293100109

Perhaps we should go back to reinplace commands. I’m not a fan of patch files myself.

@l2dy @mf2k @ryandesign any guidance on this to mitigate future glitches?

[essandess (Steve Smith)]

For reference, here’s all the deleted reinplace commands: ​https://github.com/macports/macports-ports/compare/1a5c38cd128e905f59231e1eefd68b6023f7efea..63d81f5600219e190a2c0b6ba3d46607eb01ae39

[ryandesign (Ryan Carsten Schmidt)]

Replying to danielluke:

I don’t know. I didn’t make the clamav-server port and I don’t maintain it.

I know, but you maintain the clamav port whose file is involved here.

[ryandesign (Ryan Carsten Schmidt)]

Replying to essandess:

I originally had line-by-line reinplace commands, but these were replaced by patch files in the review. See ​https://github.com/macports/macports-ports/pull/4587#discussion_r293100109

Right, and the discussion there explained the advantage of patchfiles, including that exactly what is happening now will happen: that the patch will fail to apply if the file being patched changes. You want this, so that you have the opportunity to revise the patch in the appropriate way. If you had used reinplaces instead, this would not have happened and the port would have installed, even though the file being patched had changed in some way, possibly in a way that made the reinplaces fail to work properly. In this specific instance, I admit that it would not have made a difference, but that's no guarantee about what will happen with future changes. My guidance is in the ticket description.

[essandess (Steve Smith)]

Thanks. I’m honestly still not convinced that patch files are the best approach in this specific scenario where the edits are all keyword/value settings in a .conf file. In this example, someone added a period to the end of a comment, and the whole thing breaks.

But I’ll stick with patch files if that’s the consensus, and hopefully find a more robust way of using them.

Do you or anyone know of a patch command settings that ignores comment lines?

My guidance is in the ticket description.

I apologize, but I’m not conversant enough in MacPorts design to act on that description.

What’s a revbump? What code or edits must take place to create it?

What command should be used to create the patch file to avoid the issue you identified? I’m still not clear about the source of the patch file problem.

Replying to ryandesign:

Right, and the discussion there explained the advantage of patchfiles, including that exactly what is happening now will happen: that the patch will fail to apply if the file being patched changes. You want this, so that you have the opportunity to revise the patch in the appropriate way. If you had used reinplaces instead, this would not have happened and the port would have installed, even though the file being patched had changed in some way, possibly in a way that made the reinplaces fail to work properly. In this specific instance, I admit that it would not have made a difference, but that's no guarantee about what will happen with future changes. My guidance is in the ticket description.

[essandess (Steve Smith)]

Replying to ryandesign:

I know, but you maintain the clamav port whose file is involved here.

FYSA, the file clamd.conf.sample comes from upstream. We all just use it. clamav-server depends on clamav, so it necessarily used the one installed by the parent part.

[essandess (Steve Smith)]

Would anyone object to this approach, which generates the patch file for {{{.conf}} on the fly:

1. Stick a clamav-server specific .conf into the port.
2. Use the command diff -NaurdwB -I '^ *#.*' to create a patch file from the upstream clamav.conf and the clamav-server specific one. This ignores comments and just captures keyword-value diffs.
3. Patch the upstream clamav.conf using this patch file.

That’s pretty much how I update any commented .conf files by hand.

[essandess (Steve Smith)]

Fixed in ​https://github.com/macports/macports-ports/pull/5871

Also, note that the file clamd.conf.macports is created by the port clam-server and is not shared with the parent clamav, which creates the file clamd.conf.sample.

[essandess (Steve Smith)]

In eedf91a939dd2bb2a0bce7b226e90b6e071e2e9f/macports-ports (master):

clamav-server: Bugfix patch files

• Fixes: #59743