Created attachment 168684 [details] upgrade security/clamav-unofficial-sigs from 3.7.2 to 5.0.3 This is update of security/clamav-unofficial-sigs to version 5.0.3
Testbuilds are fine.
*** Bug 201060 has been marked as a duplicate of this bug. ***
My version of this port is little different, I don't use hardcoded paths (ie. to bash). I'm also using additional patches. I'd like to take over maintainership of security/clamav-unofficial-sigs, current maintainer is no longer interested in this port. svn diff and build logs are attached. Version 5.x changes config schema and location. I think that this information should go to /usr/ports/UPDATING. I'm thinking of something like this: YYYYMMDD: AFFECTS: users of security/clamav-unofficial-sigs AUTHOR: Lukasz Wasikowski <lukasz@wasikowski.net> This version of clamav-unofficial-sigs is eXtremeSHOK's fork. Configuration file location has changed from %PREFIX%/clamav-unofficial-sigs.conf to %PREFIX%/clamav-unofficial-sigs/ directory. master.conf and os.conf holds default values, local changes should be placed in user.conf.
Created attachment 168770 [details] clamav-unofficial-sigs 5.0.3 (eXtremeSHOK fork) Patch to 5.0.3 with no hardcoded paths and some additional patches.
Created attachment 168771 [details] clamav-unofficial-sigs 5.0.3 (eXtremeSHOK fork) - testport 9.3 poudriere testport on FreeBSD 9.3.
Created attachment 168772 [details] clamav-unofficial-sigs 5.0.3 (eXtremeSHOK fork) - testport 10.2 poudriere testport on FreeBSD 10.2.
Testbuilds@work
I'll take a look at the proposed patches in the next couple of days.
(In reply to sf from comment #8) I would like to put it in the tree today, because otherwise the 2016q2 will still have the old code, which is very bad because of all the crypto trojans.
(In reply to Kurt Jaeger from comment #7) Testbuilds are all fine. If I compare the two versions from tad and lukasz, I do not see that much of a difference. Lukasz, can you elaborate on what is so much different from tad's version, given that tad really had the first real update ?
tad's version assumes that /usr/local is the only valid prefix which is ok in most cases, but not in all of them. My version use the correct prefix, that's the main difference. Other differences are minor. I've noticed a missing piece in my patch, updated version is coming shortly.
Created attachment 168799 [details] clamav-unofficial-sigs 5.0.3 (eXtremeSHOK fork) - updated patch Replaced dig with host.
(In reply to Lukasz Wasikowski from comment #12) testbuilds are fine with that version, too.
Since Kurt wanted to push this update, I dedicated some time to look at the proposed patches as well as review the forked version of the script. While the update patches are more or less fine, there are some bugs in the forked script related to configuration files loading, which result in wrong parameters being loaded. I don't know how no one has not noticed that the working directory location is not being respected and that the files are being stored in two different locations. There are also issues with logging (also related to the log file location parameter). Therefore, the port should not be updated to the forked version at this moment. I will create a working patch to update the port to the forked version and will upload it soon.
(In reply to sf from comment #14) Logging bug I reported to upstream 2 days ago. work_dir issue is also reported. None of those are critical, but of course those bugs should be fixed.
Thanks for working on this. I normally do not do run-tests, as this requires knowledge about the port itself. So indeed, non-working things might have slipped in.
There is a new release of this software (version 5.0.4). It fixes logging bug (according to author, I didn't tested it yet), and adds one of my patches. I'll prepare a new diff shortly.
(In reply to Lukasz Wasikowski from comment #17) It seems that work_dir bug is also fixed.
Created attachment 168812 [details] clamav-unofficial-sigs 5.0.43 (eXtremeSHOK fork) This versions fixes bugs mentioned in https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=208326#c14 logging bug: https://github.com/extremeshok/clamav-unofficial-sigs/issues/57 work_dir bug: https://github.com/extremeshok/clamav-unofficial-sigs/issues/62
Created attachment 168813 [details] clamav-unofficial-sigs 5.0.4 (eXtremeSHOK fork) Updated patch (leaving MAINTAINER unchanged). This versions fixes bugs mentioned in https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=208326#c14 logging bug: https://github.com/extremeshok/clamav-unofficial-sigs/issues/57 work_dir bug: https://github.com/extremeshok/clamav-unofficial-sigs/issues/62
Created attachment 168816 [details] MAINTAINER update of the port to the forked version 5.0.4 I've attached a patch that should be used to update the port the forked version of the script. UPDATING file should also mention that the new version of the port is a forked version and that the configuration files have been changed. The latest version 5.0.4 does fix the work directory bug, but the logging options are still not respected properly and you may end with multiple log files during script startup (i.e. set logging_enabled to no in user.conf and you'll still end up with created log file with entries related to previously loaded configuration files). I worked around this by adding a note to the user.conf that states that logging options should only be changed in master.conf and removed log_file_path option from os.conf file to avoid any confusion. Besides this, I also changed the logging location to /var/log and disabled logging by default, like it has been done in the port so far. I also removed pkg_* options from os.conf file, since they should be patched directly in the script and not be user editable. I would like to thank both Talal Al Dik and Lukasz Wasikowski for spending their time to provide provide patches to update the port the currently maintained version.
testbuilds@work
(In reply to sf from comment #21) I think that suggesting user to modify master.conf is a bad practice. If user modify master.conf he will stuck with modified version during upgrades and script will eventually fail when minimal config version will change. Also what was wrong with pkg_* variables in os.conf? os.conf should not be user modified anyway. I don't think that patching code to achieve something, that can be set in a configuration file, is the best way to go. Log file in /var/log/ instead of /var/log/clamav/ (where clamav's and freshclam's logs are stored) is a matter of choice, but IMHO /var/log/clamav/ is better suited for clamav-unofficial-sigs log. That's my $0.02, thank you for porting this in the first place :)
A commit references this bug: Author: pi Date: Thu Mar 31 13:36:54 UTC 2016 New revision: 412222 URL: https://svnweb.freebsd.org/changeset/ports/412222 Log: security/clamav-unofficial-sigs: 3.7.2 -> 5.0.4, extremeshok fork Changes: (long!) https://github.com/extremeshok/clamav-unofficial-sigs/releases This version of clamav-unofficial-sigs is eXtremeSHOK's fork. Configuration file location has changed from %PREFIX%/clamav-unofficial-sigs.conf to %PREFIX%/clamav-unofficial-sigs/ master.conf and os.conf hold default values, local changes should be placed in user.conf. PR: 208326 Submitted by: Talal Al Dik <tad@vif.com>, Lukasz Wasikowski <lukasz@wasikowski.net> Approved by: sf@maxempire.com (maintainer) Changes: head/UPDATING head/security/clamav-unofficial-sigs/Makefile head/security/clamav-unofficial-sigs/distinfo head/security/clamav-unofficial-sigs/files/patch-clamav-unofficial-sigs.conf head/security/clamav-unofficial-sigs/files/patch-clamav-unofficial-sigs.sh head/security/clamav-unofficial-sigs/files/patch-clamd-status.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-config_user.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-descr head/security/clamav-unofficial-sigs/pkg-plist
Committed, thanks for all involved!
(In reply to Kurt Jaeger from comment #26) Kurt, this commit has hardcoded paths to /usr/local/ in patch-clamav-unofficial-sigs.sh and in patch-cron.d_clamav-unofficial-sigs
(In reply to Lukasz Wasikowski from comment #24) I'm also not very happy with suggestion to use master.conf, but I don't have time to fix actual script and the way it reads the configuration from multiple files. Hopefully, it will be fixed upstream in the future. If you are interested in this and have some time, it would be good to create another issue in the project's issue tracker that mentions the problem with logging_enable. Regarding pkg_* variables, they were never meant to be configuration variables and that's why they actually did not exist anywhere. They were meant to be used by package maintainers, who patched them in the script itself. Having them in configuration file that can be edited by user and having no actual description what they do can only lead to user confusion. Log file has been stored in /var/log since the day one of this port's lifetime and there is no need to change that now.
(In reply to Kurt Jaeger from comment #26) As noticed by Lukasz, %%PREFIX%% variable got somehow expanded in your commit in two patch files: patch-clamav-unofficial-sigs.sh and patch-cron.d_clamav-unofficial-sigs You should commit an update to those two files.
(In reply to Marko Njezic from comment #28) You don't have time to fix logging issue, so you've decided to patch it in a way that will break this script for users who will follow advice you've put in user.conf. I'm sorry, but I don't see much sense in that, especially that the bug you "fixed" is very minor and easily could be left intact. I'll try to find some time to dig into that, but something tells me that maybe this work should be done by port maintainer ;) pkg_* variables naturally belongs to os.conf, as those are OS specific. I can agree that some comment saying "don't touch this" would be nice. I'll send patch to upstream.
Created attachment 168828 [details] Missing %%PREFIX%% restored
A commit references this bug: Author: pi Date: Thu Mar 31 14:56:36 UTC 2016 New revision: 412230 URL: https://svnweb.freebsd.org/changeset/ports/412230 Log: security/clamav-unofficial-sigs: fix PREFIX in two patches PR: 208326 Submitted by: Lukasz Wasikowski <lukasz@wasikowski.net> Approved by: Marko Njezic <sf@maxempire.com> (maintainer) Changes: head/security/clamav-unofficial-sigs/Makefile head/security/clamav-unofficial-sigs/files/patch-clamav-unofficial-sigs.sh head/security/clamav-unofficial-sigs/files/patch-cron.d_clamav-unofficial-sigs
(In reply to Lukasz Wasikowski from comment #30) It is not my job to fix forked scripts with newly introduced bugs. And I do not agree with some of the completely unnecessary changes introduced by the fork. The only reason why I devoted my time to this is because Kurt wanted to push this update so that it appears in quarterly packages update. I could have easily left everything as is and not update the port, since there is no actual update. Forked version is very different thing from the original and new port with a different name would have actually been much better. The issues related to logging are not minor at all. If you have had left everything intact you would have been left with two log files, one in /var/log/clamav-unofficial-sigs that says that os.conf has been loaded successfully and another one with the rest of entries in /var/log/clamav. The same goes for logging_enabled, if you set it to no in user.conf, you would still get a log file since it is evaluated too late in the process. This just leaves a mess of files all around. pkg_* are not configuration parameters and typical users should not even see them.