[33588] trunk/dports/security/cracklib

[Ryan Schmidt]

On Jan 30, 2008, at 19:01, June Tate-Gans wrote:

> On Wed, Jan 30, 2008 at 5:42 PM, Ryan Schmidt wrote:
>
>> In this diff, it's hard to see what functional changes you made,
>>  because you also reformatted the whitespace of the file. In the
>>  future, could you please commit whitespace changes in a separate
>>  revision from functional changes? Thanks.
>
> Absolutely. The trick in this case, though, was that I literally
> ripped out most of the original portfile, so a diff was actually
> rather insufficient to show what it was I did. I will keep this in
> mind the next time I send out a revision, though -- I know how painful
> diffs can be. My apologies for the additional eye-strain.
>
>>  It's not necessary to set the extract.suffix to .tar.gz since that's
>>  the default already.
>
> Noted. I'll remove that from the portfile.
>
>>  You shouldn't define a "largedict" variant, then use
>>  "default_variants +largedict" to enable it always. This makes it
>>  difficult for the user to disable this functionality, should they
>>  want to. (The user can "sudo port install cracklib -largedict" but
>>  next time they want to "sudo port upgrade" the +largedict variant
>>  will be selected again.) Rather, you should write the port so that
>>  this is the default functionality, and then define a "no_largedict"
>>  variant to disable it.
>
> Mm. I thought about this for a while before submitting the patch.
> Thing is, cracklib's functionality is severely reduced without that
> dictionary, and it seemed to be the upstream maintainer's intention to
> include it, which is why it is the default. Your idea makes quite a
> lot of sense, though, and I will include it soon. FYI: the macports
> guide doesn't mention anything about the upgrade issue you mentioned
> above in the variants section, which is why I didn't consider that
> aspect.

The guide mentions it here:

http://guide.macports.org/#reference.variants.user-selected

"Due to a bug in the current MacPorts base default_variants shouldn't  
be used at the moment as they cause problems while upgrading ports."


>>  The largedict variant has the path /opt/local hardcoded into the
>>  portfile. MacPorts might be installed into a different prefix so the
>>  variable ${prefix} should always be used instead.
>
> Greh -- thought I managed to nix most of those. I'll fix that in the
> next revision. My apologies.
>
> Thanks for the human-linting -- it was quite informative. =o)

What can I say, I like keeping an eye on the commit mails. :)