Bug 209894 - security/clamav-unofficial-sigs: Update to 5.3.2
Summary: security/clamav-unofficial-sigs: Update to 5.3.2
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Kurt Jaeger
URL:
Keywords: patch, patch-ready
Depends on:
Blocks:
 
Reported: 2016-05-31 14:50 UTC by uros
Modified: 2016-06-05 09:19 UTC (History)
2 users (show)

See Also:
vlad-fbsd: maintainer-feedback+


Attachments
Upgrade patch (8.77 KB, patch)
2016-05-31 14:50 UTC, uros
vlad-fbsd: maintainer-approval-
Details | Diff
Maintainer patch to the 5.3.2 version (12.75 KB, patch)
2016-06-03 18:27 UTC, Marko Njezic
sf: maintainer-approval+
Details | Diff
Corrected patch to the 5.3.2 version (12.89 KB, patch)
2016-06-04 14:49 UTC, Marko Njezic
sf: maintainer-approval+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description uros 2016-05-31 14:50:26 UTC
Created attachment 170870 [details]
Upgrade patch

Attached is a patch for security/clamav-unofficial-sigs port that does the following:

1. Upgrade clamav-unofficial-sigs to the latest 5.3.2
2. Remove cron, man, logrotate files as they are now installed through script
3. Renamed clamav-unofficial-sigs.sh to clamav-unofficial-sigs
4. Minor changes in config (logging_enabled="no" moved to os.conf)
Comment 1 Marko Njezic 2016-05-31 16:51:54 UTC
The main reason why I did not submit an update to one of the latest versions is because there were quite a lot of the disruptive changes and released versions were sometimes followed with immediate fixes and I would prefer for all the changes to stabilize first.

I took a brief look at your patch and there are some changes that I can't approve:

- There's no need to rename the script.
- Manual page should be installed by the port. I know that the script now generates manual page and it should generate one during port build stage. I'm also aware that this is not really straightforward, since the script would require some changes in order to be able to generate manual page inside work directory.
- logging_enabled="no" should be kept in master.conf and log_file_path should be set to "/var/log", since that was the location chosen from day one of the port.
- You have hardcoded /usr/local/etc/ prefix in your patch
- You have removed pkg_mgr and pkg_rm variables from the clamav-unofficial-sigs.sh script
- You have changed port dependency from gnupg to gnupg1

Besides this, port documentation (pkg-message) needs updating.

I would also like to make a change to the default configuration to disable yara rules by default (like they were in 5.0.X series).
Comment 2 VK 2016-06-02 00:27:49 UTC
Comment on attachment 170870 [details]
Upgrade patch

Patch rejected per Marko's comment above. Marko, please note that as a maintainer you can approve or reject such patches. ;)
Comment 3 uros 2016-06-03 08:25:07 UTC
Ok, I fixed most of the points here. But before final patch, let me ask a few questions

- Could pkg_mgr and pkg_rm variables be put inside os.conf ?
- I tried doing a few things about man page but none of them worked. Below is what I have in mind. Just missing the part to put it in stage dir.

${CP} ${WRKSRC}/config/os.freebsd.conf ${WRKSRC}/config/os.conf
        ${REINPLACE_CMD} -e 's|#user_configuration_complete="yes"|user_configuration_complete="yes"|g' \
                -e 's|#yararulesproject_dbs_rating=""|man_dir="${WRKSRC}/man/man8"|g' \
                ${WRKSRC}/config/user.conf
        ${INSTALL_SCRIPT} ${WRKSRC}/clamav-unofficial-sigs.sh ${WRKSRC}/clamav-unofficial-sigs.sh.temp
        ${WRKSRC}/clamav-unofficial-sigs.sh.temp -c ${WRKSRC}/config --install-man

Let me know what you think and I can update the patch with all the changes.
Comment 4 Marko Njezic 2016-06-03 18:23:51 UTC
(In reply to Uros from comment #3)

- pkg_mgr and pkg_rm variables should be kept away from user visible configuration files.
- While your code snippet related to manual page generation is actually unnecessary complex and somewhat fragile, it has given me an idea how to handle this during port build stage. I'll upload a complete patch shortly.
Comment 5 Marko Njezic 2016-06-03 18:27:17 UTC
Created attachment 170981 [details]
Maintainer patch to the 5.3.2 version

Attached is a patch that should be used to update security/clamav-unofficial-sigs port to the latest released version 5.3.2. Most of the changes are quite straightforward, only "tricky" bit is part of the code that generates manual page during port build stage. A dummy configuration file is provided so that the script can run and generate manual page.
Comment 6 uros 2016-06-04 08:13:23 UTC
Tested and works fine.
Comment 7 uros 2016-06-04 08:50:50 UTC
I also run this with poudriere but with errors

======================================================================
===>   clamav-unofficial-sigs-5.3.2 depends on executable: bash - found
===>   Returning to build of clamav-unofficial-sigs-5.3.2
===========================================================================
=======================<phase: lib-depends    >============================
===========================================================================
=======================<phase: configure      >============================
===>  Configuring for clamav-unofficial-sigs-5.3.2
===========================================================================
=======================<phase: build          >============================
===>  Building for clamav-unofficial-sigs-5.3.2
(cd /wrkdirs/usr/ports/security/clamav-unofficial-sigs/work/clamav-unofficial-sigs-5.3.2 && /usr/local/bin/bash ./clamav-unofficial-sigs.sh -c manpage.conf --install-man)
*** Error code 1

Stop.
make: stopped in /usr/ports/security/clamav-unofficial-sigs
====>> Cleaning up wrkdir
Comment 8 Marko Njezic 2016-06-04 14:46:18 UTC
(In reply to Uros from comment #7)

The script was checking for existence of ClamAV and other related binaries, which aren't really necessary for building the man page. :-) Setting BUILD_DEPENDS to be the same as RUN_DEPENDS will fix the problem. I'll upload the new patch shortly.
Comment 9 Marko Njezic 2016-06-04 14:49:14 UTC
Created attachment 171010 [details]
Corrected patch to the 5.3.2 version

Attached is corrected patch that should be used to update security/clamav-unofficial-sigs port to the latest released version 5.3.2. The only difference compared to previous patch is that BUILD_DEPENDS and RUN_DEPENDS are set to the same value. Port now builds cleanly using poudriere.

This time, I have uploaded svn diff file to make the patch easier to apply.

Added file:
files/manpage.conf

Removed files:
files/patch-cron.d_clamav-unofficial-sigs
files/patch-logrotate.d_clamav-unofficial-sigs
Comment 10 Kurt Jaeger freebsd_committer freebsd_triage 2016-06-05 08:37:53 UTC
testbuilds are Ok.
Comment 11 commit-hook freebsd_committer freebsd_triage 2016-06-05 08:43:11 UTC
A commit references this bug:

Author: pi
Date: Sun Jun  5 08:42:35 UTC 2016
New revision: 416386
URL: https://svnweb.freebsd.org/changeset/ports/416386

Log:
  security/clamav-unofficial-sigs: 5.0.5 -> 5.3.2

  Changes:
    https://github.com/extremeshok/clamav-unofficial-sigs/releases

  PR:		209894
  Submitted by:	uros.gruber@gmail.com, Marko Njezic <sf@maxempire.com> (maintainer)

Changes:
  head/security/clamav-unofficial-sigs/Makefile
  head/security/clamav-unofficial-sigs/distinfo
  head/security/clamav-unofficial-sigs/files/manpage.conf
  head/security/clamav-unofficial-sigs/files/patch-clamav-unofficial-sigs.sh
  head/security/clamav-unofficial-sigs/files/patch-config_master.conf
  head/security/clamav-unofficial-sigs/files/patch-config_os.freebsd.conf
  head/security/clamav-unofficial-sigs/files/patch-cron.d_clamav-unofficial-sigs
  head/security/clamav-unofficial-sigs/files/patch-logrotate.d_clamav-unofficial-sigs
  head/security/clamav-unofficial-sigs/files/pkg-message.in
  head/security/clamav-unofficial-sigs/pkg-plist
Comment 12 Kurt Jaeger freebsd_committer freebsd_triage 2016-06-05 08:48:22 UTC
(In reply to Marko Njezic from comment #1)
> - There's no need to rename the script.

I really like to avoid clutter with files with .sh extensions in /usr/local/bin,
so maybe that change isn't that bad ?

Anyway, update is now committed.
Comment 13 Marko Njezic 2016-06-05 09:19:34 UTC
(In reply to Kurt Jaeger from comment #12)

The only reason why I kept the script name (and a few other old changes) is consistency with previous versions of the port. If we change the name now, automatic invocation of the script via cron will stop working for the old users of the script (and we know that users usually don't read UPDATING file before updating, only when they stumble upon an error).

The ideal opportunity for script name change was when the port was first updated to the backwards incompatible version from the 5.X series, but that opportunity was missed. The next time when upstream releases a backwards incompatible version will be the perfect time to perform the name change.