Bug 220978 - mail/dcc-dccd: Mixes data and code, does not respect hier(7)
Summary: mail/dcc-dccd: Mixes data and code, does not respect hier(7)
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Piotr Kubaj
URL:
Keywords: needs-qa
Depends on:
Blocks:
 
Reported: 2017-07-24 20:17 UTC by Mikhail T.
Modified: 2019-06-10 20:05 UTC (History)
3 users (show)

See Also:
pkubaj: maintainer-feedback+
koobs: merge-quarterly?


Attachments
do it (10.70 KB, patch)
2017-07-24 20:17 UTC, Mikhail T.
no flags Details | Diff
Update to 1.3.163 and respect more hier(7) (13.81 KB, patch)
2019-04-03 21:22 UTC, Kai Knoblich
kai: maintainer-approval? (pkubaj)
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mikhail T. 2017-07-24 20:17:34 UTC
Created attachment 184675 [details]
do it

The current version of the port mixes together the scripts/binaries and the data. Maintaining read-only ${PREFIX} becomes impossible because the port tries to change ${PREFX}/dcc/map and other data.

Fortunately, vendor's configure allows for the "home directory" to be different from where the executables are...

The attached patch does the following:

 1. Introduces the DCCDATA knob pointing at /var/dcc (which is the vendor's default)
 2. Changes the destination of data files throughout the port to point there
 3. Changes port's own ${PREFIX}/dcc/libexec to simply ${PREFIX}/libexec -- as hier(7) suggests
 4. Teaches vendor's Makefiles to use "install -s" instead of strip-ing in the port's post-install
 5. Eliminates the vendor's own MD5 implementation -- the code is already linked with our -lmd, but was *still* compiling its own md5.c

Potential further improvements:
 . Change the various scripts in the port's files/*.in to share the single dcc_home-knob instead of each script looking for its own foo_home
 . Find a better location for cgi-bin -- ${PREFIX}/www ?
Comment 1 Richard Gallamore freebsd_committer freebsd_triage 2017-07-31 17:11:38 UTC
(In reply to Mikhail T. from comment #0)
Apologies, I should have provided more details on some previous commit messages.

The ALT_HOME option was removed which changed the home directory to /var/dcc because this breaks standardized FreeBSD layout. Depending on maintainers decision, changing this would be fine, however it will need to be changed to a different directory. DATADIR instead perhaps?

Also can you provide some QA on this patch?
Comment 2 Mikhail T. 2017-07-31 21:48:44 UTC
DATADIR is still under PREFIX, which shouldn't need to be modifiable at runtime -- that is, it should be possible to run with it mounted read-only and/or shared among multiple systems. That, which is routinely modified, should be under /var someplace.

I'm not sure, why it had to change from the vendor's default, but the new location ought to be under /var still...
Comment 3 Piotr Kubaj freebsd_committer freebsd_triage 2017-08-02 08:14:58 UTC
I'd rather keep it in /var, if /usr/local/dcc is not ok. If /var/dcc will not do, would /var/db/dcc be fine?
Comment 4 Richard Gallamore freebsd_committer freebsd_triage 2017-08-04 02:12:28 UTC
(In reply to Piotr Kubaj from comment #3)
Keeping /usr/local/dcc is fine, /var/db/dcc should work as well. /var/dcc wont be acceptable however. I know this is the vendor's default for dcc-dccd, but this simply is not an acceptable location for any FreeBSD port to reside.



I have a couple of questions about this patch,

1: DCCDATA knob seems more like a complete change of DCCHOME. Is this supposed to be an option? It would probably be better to add an option simply changing the DCCHOME var instead. For example,
ALT_HOME_VAR=      DCCHOME=/var/db/dcc
ALT_HOME_VAR_OFF=  DCCHOME=${PREFIX}/dcc
* The pkg-message has %%DCCHOME%%/dcc_conf while this was changed in pkg-plist.
3: I like this change, but I am worried about some of the scripts not working correctly. This will need to be verified.
4: Looks good
5: with the configure_arg, --without-DCC-MD5 this shouldn't be linked at all correct? removing it completely as you have should be fine.


As for further improvements:
 Changing files/*.in should be as easy as adding %%DCCHOME%%/DCCHOME= to the file/SUB_LIST.

I don't think there really is a standard directory for cgi-bin. Don't know enough about the port to know if this is important enough to move.
Comment 5 Richard Gallamore freebsd_committer freebsd_triage 2017-08-04 02:24:01 UTC
My suggestion with _VAR should actually be _VARS.
Comment 6 Mikhail T. 2017-08-11 18:52:08 UTC
(In reply to Piotr Kubaj from comment #3)
> If /var/dcc will not do, would /var/db/dcc be fine?

Yes, I think so. But only for the data-files, that the software needs to modify at runtime. The binaries -- executables and libraries, and config-files still do belong under PREFIX.

> DCCDATA knob seems more like a complete change of DCCHOME.

Yes and no :) It separates binaries from the data, which used to both go into the same place. I do not see, why it should be an option at all...

What may be an option is using the same "home" for all of the services. Or simply change the rc.d scripts to check for multiple rc-vars: dccX_home and then, if not set, the dcc_home.
Comment 7 Kai Knoblich freebsd_committer freebsd_triage 2019-03-11 16:47:17 UTC
Hi all, 

this bug seems a bit stale but still relevant in some way. If nobody is working on this at the moment I can merge Mikhail's patch into bug #223992 (if Piotr as maintainer approves it).
Comment 8 Piotr Kubaj freebsd_committer freebsd_triage 2019-03-14 14:40:30 UTC
(In reply to Kai Knoblich from comment #7)
Yes, sounds reasonable.
Comment 9 Kai Knoblich freebsd_committer freebsd_triage 2019-04-03 21:22:27 UTC
Created attachment 203357 [details]
Update to 1.3.163 and respect more hier(7)

(In reply to Piotr Kubaj from comment #8)

Here's preliminary patch that has following items:

- Contains your changes from bug #223992
- Contains also the changes from mi@ given in this PR (using /var/db/dcc instead of /var/db)
- Sorted variables according to the recent version of the PHB
- Fix for pkg-plist when building with poudriere -t (I did also s/unexec/postunexec/ while I was here)

It builds fine on 11.2-RELEASE with all options off/on and mixed variations of it but I was not able to do some runtime test, yet.

If all those changes makes sense I'll do some run-time testing the next days.
Comment 10 commit-hook freebsd_committer freebsd_triage 2019-06-10 20:05:21 UTC
A commit references this bug:

Author: pkubaj
Date: Mon Jun 10 20:04:48 UTC 2019
New revision: 503924
URL: https://svnweb.freebsd.org/changeset/ports/503924

Log:
  mail/dcc-dccd: update to 2.3.164, use /var/db/dcc instead of /var/db, various cleanups

  PR:		220978
  Submitted by:	kai
  Reported by:	mi@ALDAN.algebra.com
  Approved by:	mat (mentor)
  Differential Revision:	https://reviews.freebsd.org/D20155

Changes:
  head/mail/dcc-dccd/Makefile
  head/mail/dcc-dccd/distinfo
  head/mail/dcc-dccd/files/dccd.in
  head/mail/dcc-dccd/files/dccgrey.in
  head/mail/dcc-dccd/files/dccifd.in
  head/mail/dcc-dccd/files/dccm.in
  head/mail/dcc-dccd/files/pkg-install.in
  head/mail/dcc-dccd/files/pkg-message.in
  head/mail/dcc-dccd/pkg-descr
  head/mail/dcc-dccd/pkg-plist