[138845] trunk/dports/mail

[Ryan Schmidt]

> On Jul 21, 2015, at 5:19 AM, vince at macports.org wrote:
> 
> Revision
> 138845
> Author
> vince at macports.org
> Date
> 2015-07-21 03:19:00 -0700 (Tue, 21 Jul 2015)
> Log Message
> 
> cyrus5-imapd: initial commit
> Added Paths
> 
> 	• trunk/dports/mail/cyrus5-imapd/
> 	• trunk/dports/mail/cyrus5-imapd/Portfile
> 	• trunk/dports/mail/cyrus5-imapd/files/
> 	• trunk/dports/mail/cyrus5-imapd/files/patch-Makefile_in.diff
> 	• trunk/dports/mail/cyrus5-imapd/files/patch-util_h.diff
> Diff
> 
> Added: trunk/dports/mail/cyrus5-imapd/Portfile (0 => 138845)

> +# $Id: Portfile 129759 2014-12-19 23:59:34Z devans at macports.org $

Looks like you've forgotten to set the svn:keywords and svn:eol-style properties on this file. Use Subversion's autoprops feature to set them automatically in the future.

https://trac.macports.org/wiki/CommittersTipsAndTricks#SetsvnpropertiesautomaticallyonnewPortfiles


> +# Check for any db4* installation
> +if {[glob ${prefix}/include/db4*] != ""} {
> +    set db_list [lsort [glob ${prefix}/include/db4*]]
> +    set current_db [lindex ${db_list} end]
> +    # Caveat: this works for db44/db46/db48 but will have to be patched for db410
> +    set db_ver [string index [lindex [split ${current_db} "/"] end] end]
> +} else {
> +    depends_lib-append   port:db48
> +}

This causes the port to build differently depending on which db* port the user has installed. I realize that was your intention, but it's not what we want; we want ports that always build the same (for a given combination of variants, port version, port revision, OS version, architecture, and other variables). In this case, you should offer variants to allow the user to choose the db version.

Usually you would then just pick one of those variants to be the default. If you want to be clever and try to pick a default variant based on which db port the user already has installed, I suppose you can do that, but it's not what we usually do.

Make sure you declare a dependency on the db version you use. In this case, you're not declaring a dependency on any db version, unless the user didn't have one installed when the port was installed.

You could also forgo offering the user a choice, and just pick a version of db to be used by the port.


> +pre-fetch {
> +    ui_msg "Selected db4${db_ver}"
> +}

This debugging message shouldn't be printed.


> +post-patch {
> +    foreach dir {contrib contrib/cyrus-graphtools.1.0/cgi-bin contrib/cyrus-graphtools.1.0/script perl/imap/examples perl/sieve/scripts tools} {
> +        eval reinplace "s|^#!.*perl|#!${prefix}/bin/perl|" [glob -d ${worksrcpath}/${dir} *.pl]
> +    }
> +}

The expand operator ("{*}") should be used instead of eval:

reinplace "s|^#!.*perl|#!${prefix}/bin/perl|" {*}[glob -d ${worksrcpath}/${dir} *.pl]


> +startupitem.create  yes
> +startupitem.name    cyrus
> +startupitem.requires Disks Network "System Log"
> +startupitem.start   "${prefix}/bin/master -d"
> +startupitem.stop    "pidfile=${prefix}/var/run/cyrus-master.pid
> +            if \[ -f \${pidfile} \]; then
> +               kill -TERM \$(cat \${pidfile})
> +            fi"

If startupitem.executable can be used instead of startupitem.start and startupitem.stop, that would be better. It looks like the "-d" flag means to start in daemon mode, so all you probably need to do is set "startupitem.executable ${prefix}/bin/master" (without the "-d" flag).

This port really installs a program called "master"? That sounds like a naming collision just waiting to happen.


> +post-destroot   {
> +        #add_users cyrus group=cyrus home=${prefix}/var/imap shell=/usr/bin/false passwd="\*"

If you want to add users, the add_users invocation should be in the main part of the portfile, outside of any phase. And you probably do want to add users, since you're assuming elsewhere in the port that the user exists. Actually you're assuming the user "_cyrus" (with an underscore) exists, so you should create it first.


> +        xinstall -m 755 -v ${worksrcpath}/tools/mknewsgroups ${destroot}${prefix}/bin
> +        xinstall -m 755 -v ${worksrcpath}/tools/dohash ${destroot}${prefix}/bin
> +        xinstall -m 755 -v ${worksrcpath}/tools/rehash ${destroot}${prefix}/bin
> +        xinstall -m 755 -v ${worksrcpath}/tools/upgradesieve ${destroot}${prefix}/bin

Note that you can combine these four commands into a single command:

xinstall -W ${worksrcpath}/tools/ mknewsgroups dohash rehash upgradesieve ${destroot}${prefix}/bin


> +        eval delete [glob ${destroot}${prefix}/lib/perl5/*/*/perllocal.pod]

Here again, please use the expand operator instead of eval:

delete {*}[glob ${destroot}${prefix}/lib/perl5/*/*/perllocal.pod]

But the fact that you're using asterisks here in place of the perl version number indicates you don't know for what perl version perl modules will be installed. This is another case of the port building differently depending on the user's system -- in this case, depending on which variant of the perl5 port the user has selected. 

Instead, you should pick one of these options:

* move the perl parts to a separate set of subports named for the perl version, such as p5.16-cyrus5-imapd, p5.18-cyrus5-imapd, p5.20-cyrus5-imapd and p5.22-cyrus5-imapd (perhaps this would be a separate port using the perl5 portgroup; I'm not sure if the perl5 portgroup is designed to be able to be used to add additional subports to an existing port like cyrus5-imapd)
* pick a single perl version for which to offer the modules (such as perl5.22, which we are trying to make our new default perl version in MacPorts) and make sure to change "perl5" to "perl5.22" everywhere in the port
* offer variants for the user to select which version of perl to build the modules for, picking perl5.22 by default (or, if you insist, trying to be clever and picking the default based on which perl version the user already has installed, though that's not what we usually do)


> +variant sieve description {Use sieve} {
> +        configure.args-delete --disable-sieve
> +        configure.args-append --enable-sieve
> +}

> +variant snmp description {Use Net SNMP} {
> +        depends_lib-append      port:net-snmp
> +        configure.args-delete   --with-snmp=no
> +        configure.args-append   --with-snmp=yes
> +}

Consider using -replace instead of -append and -delete:


configure.args-replace --disable-sieve --enable-sieve


configure.args-replace --with-snmp=no --with-snmp=yes