> On Jul 21, 2015, at 5:19 AM, [email protected] wrote:
> 
> Revision
> 138845
> Author
> [email protected]
> 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 [email protected] $

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



_______________________________________________
macports-dev mailing list
[email protected]
https://lists.macosforge.org/mailman/listinfo/macports-dev

Reply via email to