Changeset 135550


Ignore:
Timestamp:
Apr 26, 2015, 2:54:46 AM (4 years ago)
Author:
larryv@…
Message:

muniversal-1.0: Set options with option proc (#47388)

A portfile command sets its associated option to a list composed of its
arguments. Accessing the associated global variable then yields that
list. Thus, for a portfile option foo,

foo 1 2 3
set bar $foo
foo $bar

does not restore foo to a list containing 1, 2, and 3. Instead, foo
is set to a list containing a list containing 1, 2, and 3. One way to
correct this is with eval:

foo 1 2 3
set bar $foo
eval foo $bar

The use of eval is generally discouraged because it's very easy to
create subtle injection bugs/vulnerabilities.[1] Tcl 8.5 argument
expansion permits an alternative:

foo 1 2 3
set bar $foo
foo {*}$bar

It's not particularly obvious when the double substitution[2] provided
by eval/{*} is required. This has caused much confusion in both base
and portfile code.

Another option (more palatable in base code) is to dispense with the
portfile command magic altogether and use the option proc directly.

option foo {1 2 3}
set bar [option foo]
option foo $bar

The option proc behaves more or less like set and is thus easier to
understand, if more verbose.

This is the approach I've taken to handling the bugs reported in #47388
and #47420. The muniversal-1.0 portgroup looks nothing like a portfile
at this point, so I think the use of portfile commands just muddles the
reader's mental model of what's going on. The simplicity of option is
preferable here.

[1] http://wiki.tcl.tk/1017
[2] http://wiki.tcl.tk/1535

File:
1 edited

Legend:

Unmodified
Added
Removed
  • trunk/dports/_resources/port1.0/group/muniversal-1.0.tcl

    r134863 r135550  
    291291            if { [string match "${worksrcpath}/*" ${configure.dir}] } {
    292292                # The configure directory is inside the source directory, so put in the new source directory name.
    293                 configure.dir [string map "${worksrcpath} ${worksrcpath}-${arch}" ${configure.dir}]
     293                option configure.dir [string map "${worksrcpath} ${worksrcpath}-${arch}" ${configure.dir}]
    294294            } else {
    295295                # The configure directory is outside the source directory, so give it a new name by appending ${arch}.
    296                 configure.dir ${configure.dir}-${arch}
     296                option configure.dir ${configure.dir}-${arch}
    297297                if { ![file exists ${configure.dir}] } {
    298298                    file mkdir ${configure.dir}
     
    303303            if { [string match "${worksrcpath}/*" ${autoreconf.dir}] } {
    304304                # The autoreconf directory is inside the source directory, so put in the new source directory name.
    305                 autoreconf.dir [string map "${worksrcpath} ${worksrcpath}-${arch}" ${autoreconf.dir}]
     305                option autoreconf.dir [string map "${worksrcpath} ${worksrcpath}-${arch}" ${autoreconf.dir}]
    306306            } else {
    307307                # The autoreconf directory is outside the source directory, so give it a new name by appending ${arch}.
    308                 autoreconf.dir ${autoreconf.dir}-${arch}
     308                option autoreconf.dir ${autoreconf.dir}-${arch}
    309309                if { ![file exists ${autoreconf.dir}] } {
    310310                    file mkdir ${autoreconf.dir}
     
    315315
    316316            # Undo changes to the configure related variables
    317             autoreconf.dir      {*}${autoreconf_dir_save}
    318             configure.dir       {*}${configure_dir_save}
    319             configure.compiler  {*}${configure_compiler_save}
    320             configure.f90       {*}${configure_f90_save}
    321             configure.f77       {*}${configure_f77_save}
    322             configure.fc        {*}${configure_fc_save}
    323             configure.cc        {*}${configure_cc_save}
    324             configure.cxx       {*}${configure_cxx_save}
    325             configure.objc      {*}${configure_objc_save}
     317            option autoreconf.dir       ${autoreconf_dir_save}
     318            option configure.dir        ${configure_dir_save}
     319            option configure.compiler   ${configure_compiler_save}
     320            option configure.f90        ${configure_f90_save}
     321            option configure.f77        ${configure_f77_save}
     322            option configure.fc         ${configure_fc_save}
     323            option configure.cc         ${configure_cc_save}
     324            option configure.cxx        ${configure_cxx_save}
     325            option configure.objc       ${configure_objc_save}
    326326            if { [info exists merger_configure_args(${arch})] } {
    327327                configure.args-delete  $merger_configure_args(${arch})
     
    371371            if { [string match "${worksrcpath}/*" ${build.dir}] } {
    372372                # The build directory is inside the source directory, so put in the new source directory name.
    373                 build.dir [string map "${worksrcpath} ${worksrcpath}-${arch}" ${build.dir}]
     373                option build.dir [string map "${worksrcpath} ${worksrcpath}-${arch}" ${build.dir}]
    374374            } else {
    375375                # The build directory is outside the source directory, so give it a new name by appending ${arch}.
    376                 build.dir ${build.dir}-${arch}
     376                option build.dir ${build.dir}-${arch}
    377377                if { ![file exists ${build.dir}] } {
    378378                    file mkdir ${build.dir}
     
    382382            portbuild::build_main
    383383
    384             build.dir ${build_dir_save}
     384            option build.dir ${build_dir_save}
    385385            if { [info exists merger_build_args(${arch})] } {
    386386                build.args-delete $merger_build_args(${arch})
     
    397397            copy ${destroot} ${workpath}/destroot-${arch}
    398398            set destdirSave ${destroot.destdir}
    399             destroot.destdir [string map "${destroot} ${workpath}/destroot-${arch}" ${destroot.destdir}]
     399            option destroot.destdir [string map "${destroot} ${workpath}/destroot-${arch}" ${destroot.destdir}]
    400400
    401401            if { [info exists merger_destroot_env(${arch})] } {
     
    408408            if { [string match "${worksrcpath}/*" ${destroot.dir}] } {
    409409                # The destroot directory is inside the source directory, so put in the new source directory name.
    410                 destroot.dir [string map "${worksrcpath} ${worksrcpath}-${arch}" ${destroot.dir}]
     410                option destroot.dir [string map "${worksrcpath} ${worksrcpath}-${arch}" ${destroot.dir}]
    411411            } else {
    412412                # The destroot directory is outside the source directory, so give it a new name by appending ${arch}.
    413                 destroot.dir ${destroot.dir}-${arch}
     413                option destroot.dir ${destroot.dir}-${arch}
    414414                if { ![file exists ${destroot.dir}] } {
    415415                    file mkdir ${destroot.dir}
     
    419419            portdestroot::destroot_main
    420420
    421             destroot.dir ${destroot_dir_save}
     421            option destroot.dir ${destroot_dir_save}
    422422            if { [info exists merger_destroot_args(${arch})] } {
    423423                destroot.args-delete $merger_destroot_args(${arch})
     
    426426                destroot.env-delete  $merger_destroot_env(${arch})
    427427            }
    428             destroot.destdir ${destdirSave}
     428            option destroot.destdir ${destdirSave}
    429429        }
    430430        delete ${destroot}
     
    720720                if { [string match "${worksrcpath}/*" ${test.dir}] } {
    721721                    # The test directory is inside the source directory, so put in the new source directory name.
    722                     test.dir [string map "${worksrcpath} ${worksrcpath}-${arch}" ${test.dir}]
     722                    option test.dir [string map "${worksrcpath} ${worksrcpath}-${arch}" ${test.dir}]
    723723                } else {
    724724                    # The test directory is outside the source directory, so give it a new name by appending ${arch}.
    725                     test.dir ${test.dir}-${arch}
     725                    option test.dir ${test.dir}-${arch}
    726726                    if { ![file exists ${test.dir}] } {
    727727                        file mkdir ${test.dir}
     
    731731                porttest::test_main
    732732
    733                 test.dir ${test_dir_save}
     733                option test.dir ${test_dir_save}
    734734            }
    735735        }
Note: See TracChangeset for help on using the changeset viewer.