[82234] users/pixilla/dports

[Bradley Giesbrecht]

Thank you very much for your comments.

I am wanting to close my old tickets. This is one of them.
https://trac.macports.org/ticket/26195

On Aug 10, 2011, at 11:40 PM, Ryan Schmidt wrote:

> On Aug 10, 2011, at 20:30, pixilla at macports.org wrote:
> 
>> Revision: 82234
>>         http://trac.macports.org/changeset/82234
>> Author:   pixilla at macports.org
>> Date:     2011-08-10 18:30:10 -0700 (Wed, 10 Aug 2011)
>> Log Message:
>> -----------
>> sysutils/logrotate: Upgrade to 3.8.0.
>> 
>> Added Paths:
>> -----------
>>   users/pixilla/dports/sysutils/
>>   users/pixilla/dports/sysutils/logrotate/
>>   users/pixilla/dports/sysutils/logrotate/Portfile
>>   users/pixilla/dports/sysutils/logrotate/files/
>>   users/pixilla/dports/sysutils/logrotate/files/patch-Makefile.diff
>>   users/pixilla/dports/sysutils/logrotate/files/patch-config.c.diff
>>   users/pixilla/dports/sysutils/logrotate/files/patch-examples-logrotate-default.diff
>>   users/pixilla/dports/sysutils/logrotate/files/patch-logrotate.c.diff
> 
> I know this is a private port in your users directory for now, but some comments:
> 
> 
>> +maintainers         markd pixilla.com:brad
> 
> May want to use your MacPorts handle here.

Thanks. The ticket attachment predates my MacPorts handle.

>> +use_configure       no
> 
> This probably means you need to manually handle UsingTheRightCompiler and use [get_canonical_archflags] somewhere.

Right.

>> +destroot.keepdirs   ${destroot}${prefix}/etc/logrotate.d \
>> +                    ${destroot}${prefix}/var/run/logrotate
>> +
>> +post-destroot {
>> +		xinstall -m 755 ${worksrcpath}/examples/logrotate-default \
>> +			${destroot}${prefix}/etc/logrotate.conf.sample
>> +    xinstall -d ${destroot}${prefix}/etc/logrotate.d
>> +    xinstall -d ${destroot}${prefix}/var/run/logrotate
>> +}
>> +
>> +post-activate {
>> +    delete  ${prefix}/etc/logrotate.d/.turd_${name} \
>> +            ${prefix}/var/run/logrotate/.turd_${name}
>> +}
> 
> It's a bit odd to be deleting the turds... does their presence really cause a problem?

I'm not sure about the var/run/logrotate turd, but the etc/logrotate file globs the contents of etc/logrotate.d. I think there was a problem with that. I'll remove it and test. This is to old for me to remember.

>> +variant bzip2 description {change default compression to bzip} {
>> +    build.env-append    COMPRESS_COMMAND="${prefix}/bin/bzip2" \
>> +                        COMPRESS_EXT=".bz2" \
>> +                        UNCOMPRESS_COMMAND="${prefix}/bin/bunzip2"
>> +    depends_run-delete  port:gzip
>> +    depends_run-append  port:bzip2
>> +}
>> 
> 
>> +default_variants    bzip2
> 
> You need a + before the default variant name.

Right.

> But moreover, you're offering the user a choice between gzip and bzip2. (And not a choice between bzip2 on or off.) In other words, if considered in the context of a GUI preferences dialog, this choice might be represented as two radio buttons; it would not be represented as a checkbox. Standalone variants usually represent checkbox-like on-or-off choices. To implement radio-button-like choices in variants, I find it makes more sense to make two variants that are conflicting with one another. To do this, remove port:gzip from the port's global depends_run, and then:
> 
> 
> variant bzip2 conflicts gzip description {Use bzip2 compression by default} {
>    build.env-append    COMPRESS_COMMAND="${prefix}/bin/bzip2" \
>                        COMPRESS_EXT=".bz2" \
>                        UNCOMPRESS_COMMAND="${prefix}/bin/bunzip2"
>    depends_run-append  port:bzip2
> }
> 
> variant gzip conflicts bzip2 description {Use gzip compression by default} {
>    depends_run-append  port:gzip
> }
> 
> if {![variant_isset gzip]} {
>    default_variants +bzip2
>    variant_set bzip2
> }
> 
> 
> The variant_set line ensures that even if the user requests to install the port with neither variant (-gzip -bzip2), the bzip2 variant will still be used. (It doesn't make sense to have "all radio buttons off".)

Ah, right. So using variant_set prevents -bzip2, right?


Regards,
Bradley Giesbrecht (pixilla)