[87550] trunk/dports/lang/qore-pgsql-module/Portfile

[Ryan Schmidt]

On Nov 25, 2011, at 08:35, pvanek at macports.org wrote:

> Revision: 87550
>          http://trac.macports.org/changeset/87550
> Author:   pvanek at macports.org
> Date:     2011-11-25 06:35:17 -0800 (Fri, 25 Nov 2011)
> Log Message:
> -----------
> qore-pgsql-module 1.0.6: version bump, and psqlXX variants introduced
> 
> Modified Paths:
> --------------
>    trunk/dports/lang/qore-pgsql-module/Portfile
> 
> Modified: trunk/dports/lang/qore-pgsql-module/Portfile
> ===================================================================
> --- trunk/dports/lang/qore-pgsql-module/Portfile	2011-11-25 12:49:35 UTC (rev 87549)
> +++ trunk/dports/lang/qore-pgsql-module/Portfile	2011-11-25 14:35:17 UTC (rev 87550)
> @@ -4,7 +4,7 @@
> PortSystem          1.0
> 
> name                qore-pgsql-module
> -version             1.0.5
> +version             1.0.6
> categories          lang
> maintainers         davidnichols

Did you have David Nichols' permission to update this port?


> +variant psql83 conflicts psql84 psql90 psql91 \
> +description {Enable Postgre SQL Driver version 8.3} {}
> +
> +variant psql84 conflicts psql83 psql90 psql91 \
> +description {Enable Postgre SQL Driver version 8.4} {}
> +
> +variant psql90 conflicts psql83 psql84 psql91 \
> +description {Enable Postgre SQL Driver version 9.0 (default if none selected)} {}
> +
> +variant psql91 conflicts psql83 psql84 psql90 \
> +description {Enable Postgre SQL Driver version 9.1} {}

Please remove this "default if none selected" wording from the description, and actually make it a default variant:

if {![variant_isset psql83] && ![variant_isset psql84] && ![variant_isset psql90] && ![variant_isset psql91]} {
	default_variants +psql90
}

Why not make 9.1 the default? It's the latest stable version.

I wish we'd standardize on how we name our postgresql variants. I'd recommend matching the name of the ports: postgresql90, etc.


> +set psql_version "90"
> +if {[variant_isset psql83]} {
> +    set psql_version "83"
> +} elseif {[variant_isset psql84]} {
> +    set psql_version "84"
> +} elseif {[variant_isset psql90]} {
> +    set psql_version "90"
> +} elseif {[variant_isset psql91]} {
> +    set psql_version "91"
> +}

Why declare empty variants and then have these if statements? Why not just set the variable directly in the variant declarations? It would be much clearer and simpler. I know why: because the variable wouldn't be set in time for the dependency declaration. So why not just write e.g. "depends_lib-append postgresql90" in the variant declaration instead of creating a variable?


> depends_lib         port:qore \
> -                    port:postgresql84
> +                    port:postgresql${psql_version}

So you've added the correct dependency... but how does the port know to actually use that version of postgresql then? You haven't supplied configure args or environment variables or a patch or anything.