#38091: alpine @2.00_4: update to 2.11 -----------------------+---------------------- Reporter: larryv@… | Owner: john@… Type: update | Status: closed Priority: Normal | Milestone: Component: ports | Version: Resolution: fixed | Keywords: haspatch Port: alpine | -----------------------+---------------------- Comment (by larryv@…): Replying to [comment:37 jerryyhom@…]:

Re: [133173], considering I am new to MP, and I mean no disrespect, could you explain why transposing negative variants into positive ones is helpful here?

See the final paragraph of PortfileRecipes#default_variants. We want the act of //enabling// variants to //enable// features, not //disable// them. An implementation bug was the only reason negative variants were ever necessary. The vast majority of ports no longer have negative variants; `alpine` was just neglected until now. Today, the correct way to enable optional features by default is with the `default_variants` portfile option.

From a maintainer's perspective, feels like the changes introduce byzantine rules. At first, I thought it was simply double negatives to positive, but after hours of scratching my head, it seems more like triple negatives to keep the same build behavior? I am losing count, is it quadruple negatives? Overall, seems unnecessary and obfuscating.

It’s only the transitional code that’s confusing. The logic of positive variants is actually much //less// confusing than that of negative variants. - Since the `without_tcl` variant is deprecated, it should no longer be a default variant. {{{ #!diff --- a/dports/mail/alpine/Portfile +++ b/dports/mail/alpine/Portfile @@ -34,5 +34,3 @@ reinplace "s|__PREFIX__|${prefix}|" ${worksrcpath}/imap/Makefile } -default_variants +without_tcl - }}} - Without any variants selected, the port should have the minimal possible set of dependencies. {{{ #!diff --- a/dports/mail/alpine/Portfile +++ b/dports/mail/alpine/Portfile @@ -39,6 +37,3 @@ -depends_lib port:openssl \ +depends_lib port:gettext \ port:libiconv \ - port:gettext \ - port:openldap \ - port:ncurses \ - port:cyrus-sasl2 + port:ncurses }}} - Some build systems automatically enable optional features if they detect that the necessary dependencies are present. This is not acceptable to us, as it makes the builds irreproducible. The port needs to //explicitly disable// optional features if the variants that enable those optional features are //not// selected. {{{ #!diff --- a/dports/mail/alpine/Portfile +++ b/dports/mail/alpine/Portfile @@ -53,11 +48,16 @@ configure.args -with-lib-dir=${prefix}/lib \ --with-ssl-include-dir=${prefix}/include/openssl \ --with-ssl-lib-dir=${prefix}/lib \ --with-local-password-cache-method \ - --with-debug-level=0 + --with-debug-level=0 \ + --without-krb5 \ + --without-ldap \ + --without-ssl \ + --without-tcl variant universal {} use_parallel_build no +build.env SSLTYPE=none build.args CC=${configure.cc} \ EXTRACFLAGS="[get_canonical_archflags cc]" \ EXTRALDFLAGS="[get_canonical_archflags ld]" \ }}} - We turn the negative variants into dummy variants but keep them around for migration purposes. We usually give users about a year to upgrade their ports and switch over; then we remove the old stuff. {{{ #!diff --- a/dports/mail/alpine/Portfile +++ b/dports/mail/alpine/Portfile @@ -65,21 +65,8 @@ -variant without_krb5 description {Disable Kerberos5 support} { - depends_lib-delete port:cyrus-sasl2 - configure.args-append --without-krb5 -} - -variant without_ldap description {Disable LDAP support} { - depends_lib-delete port:openldap - configure.args-append --without-ldap -} - -variant without_ssl description {Disable SSL support} { - depends_lib-delete port:openssl - configure.args-append --without-ssl - build.env-append SSLTYPE=none -} - -variant without_tcl description {Disable TCL support (disables Alpine Web)} { - configure.args-append --without-tcl -} +# TODO: Remove legacy variants after 2016-02-20. + +variant without_krb5 conflicts kerberos description {Legacy variant} {} +variant without_ldap conflicts ldap description {Legacy variant} {} +variant without_ssl conflicts ssl description {Legacy variant} {} +variant without_tcl conflicts tcl description {Legacy variant} {} }}} - Users who already have `alpine` installed need to be migrated to the new variants in a sane way. Ideally, we’d just set `default_variants +kerberos +ldap +ssl` and be done with it, but that would cause variant conflicts for users who have `alpine` installed with `+without_krb5`, `+without_ldap`, or `+without_ssl`. So we only set new default variants if the user doesn’t already have one of the old ones selected. {{{ #!div class="" I know this is confusing. Think about the first conditional. If a user has `alpine +without_krb5` installed, we don’t want to set `+kerberos` as a default for them because they’ve already explicitly chosen to forego Kerberos support. So we only make `+kerberos` a default if they have //not// set the `without_krb5` variant. The same goes for the rest. (We have to treat `without_tcl` / `tcl` a little differently because `+without_tcl` was the previous default.) The logical contortions one has to endure to reason about negative variants (“`if {![variant_isset without_ldap]}`”? what?) are actually a very good argument //against// their use. In any case, you need not worry about it, as it’s only transitional. }}} {{{ #!diff --- a/dports/mail/alpine/Portfile +++ b/dports/mail/alpine/Portfile @@ -86,4 +73,23 @@ +# TODO: Replace this morass after 2016-02-20. +#default_variants +kerberos +ldap +ssl + +if {![variant_isset without_krb5]} { + default_variants +kerberos +} +if {![variant_isset without_ldap]} { + default_variants +ldap +} +if {![variant_isset without_ssl]} { + default_variants +ssl +} +if {![variant_isset tcl]} { + default_variants +without_tcl +} +if {![variant_isset without_tcl]} { + default_variants +tcl +} + variant passfile description {Enable password files support} { configure.args-delete --with-local-password-cache-method configure.args-append --with-passfile=".pine.pwd" } }}} - Finally, the positive variants modify the default port configuration to //enable// the features they control. Unfortunately, Alpine’s build system disables features using negative options, so we end up having to delete them to enable the features. {{{ #!diff --- a/dports/mail/alpine/Portfile +++ b/dports/mail/alpine/Portfile @@ -86,4 +92,25 @@ variant passfile description {Enable password files support} { configure.args-delete --with-local-password-cache-method configure.args-append --with-passfile=".pine.pwd" } + +variant kerberos description {Kerberos support} { + depends_lib-append port:cyrus-sasl2 + configure.args-delete --without-krb5 +} + +variant ldap description {LDAP support} { + depends_lib-append port:openldap + configure.args-delete --without-ldap +} + +variant ssl description {OpenSSL support} { + depends_lib-append port:openssl + configure.args-delete --without-ssl + build.env-delete SSLTYPE=none +} + +variant tcl description {Tcl support (required by Alpine Web} { + # Should we force MacPorts' Tcl? + configure.args-delete --without-tcl +} }}} -- Ticket URL: <https://trac.macports.org/ticket/38091#comment:38> MacPorts <https://www.macports.org/> Ports system for OS X