Opened 6 years ago

Closed 6 years ago

#56002 closed defect (fixed)

muniversal portgroup: treat merger_* list variables as lists!

Reported by: ryandesign (Ryan Carsten Schmidt) Owned by: ryandesign (Ryan Carsten Schmidt)
Priority: Normal Milestone:
Component: ports Version:
Keywords: Cc:
Port: muniversal

Description

The muniversal portgroup has a number of associative array variables whose elements should be treated as lists (because the MacPorts base options they're associated with are treated as lists) but aren't:

  • merger_build_args
  • merger_build_env
  • merger_configure_args
  • merger_configure_cflags
  • merger_configure_cppflags
  • merger_configure_cxxflags
  • merger_configure_env
  • merger_configure_ldflags
  • merger_configure_objcflags
  • merger_destroot_args
  • merger_destroot_env
  • merger_test_args
  • merger_test_env

This results in ports having to use nonstandard quoting characters and convoluted escaping when passing in multiple values.


Consider port babel @0.1.44 which does this:

if {![variant_isset universal]} {
    configure.env-append CC_FOR_BUILD="${configure.cc} ${configure.cc_archflags}"
} else {
    foreach arch ${configure.universal_archs} {
        lappend merger_configure_env(${arch})  CC_FOR_BUILD='${configure.cc} -arch ${arch}'
    }
}

This works, but note the use of the nonstandard quote character ' when setting merger_configure_env(${arch}). If the standard quote character " is used instead, CC_FOR_BUILD is not added to the environment at all.


Consider leveldb @1.19 which does this:

if { [variant_isset universal] } {
    foreach arch ${configure.universal_archs} {
        set merger_build_env(${arch}) "CFLAGS=\"-arch ${arch}\" CXXFLAGS=\"-arch ${arch}\" LDFLAGS=\"-arch ${arch} -L${prefix}/lib\""
    }
} else {
    build.env-append \
        CFLAGS="[get_canonical_archflags cc]" \
        CXXFLAGS="[get_canonical_archflags cxx]" \
        LDFLAGS="[get_canonical_archflags ld] -L${prefix}/lib"
}

This works, but there's a lot of inconvenient escaping needed to shoehorn all of the desired variables into a single string to pass to merger_build_env(${arch}). If the more natural lappend is used to append individual arguments, again the variables silently don't end up in the environment.


Consider mlt @6.4.1 which does this:

if {[variant_isset universal]} {
    foreach arch ${configure.universal_archs} {
        lappend merger_build_env(${arch})   {*}"CFLAGS='${configure.cflags} -arch ${arch}'"
        lappend merger_build_env(${arch})   {*}"CXXFLAGS='${configure.cxxflags} -arch ${arch}'"
        lappend merger_build_env(${arch})   {*}"LDFLAGS='${configure.ldflags} -L${worksrcpath}-${arch}/src/framework -L${prefix}/lib -arch ${arch}'"
    }
} else {
    build.env-append                        CFLAGS="${configure.cflags} [get_canonical_archflags cc]"
    build.env-append                        CXXFLAGS="${configure.cxxflags} [get_canonical_archflags cxx]"
    build.env-append                        LDFLAGS="${configure.ldflags} -L${worksrcpath}/src/framework -L${prefix}/lib [get_canonical_archflags ld]"
}

Having to use the expansion operator and enclose the entire flag value in quotes is certainly strange enough that many developers wouldn't know to do that.


Consider lp_solve @5.5.23 which does this:

if {[variant_isset universal]} {
    set merger_must_run_binaries yes
    foreach arch ${configure.universal_archs} {
        lappend merger_build_args(${arch}) CC='${configure.cc} -arch ${arch}'
    }
} else {
    build.args-append CC='${configure.cc} ${configure.cc_archflags}'
}

MacPorts base handles *.args differently than *.env. Again note the unfortunate use of the nonstandard quote character '. It works:

Executing:  cd "..." && /usr/bin/make -j8 -w all PREFIX="/opt/local" CC='/usr/bin/clang -arch x86_64'

But if it's changed to ", the portgroup introduces incorrect escaping, which causes make to fail with a usage message.

Executing:  cd "..." && /usr/bin/make -j8 -w all PREFIX="/opt/local" CC=\"/usr/bin/clang -arch x86_64\" 

It becomes even more problematic if one wants to include a list of more than one item. Demonstrating this using ${configure.ldflags}:

        lappend merger_build_args(${arch}) CC='${configure.cc} ${configure.ldflags} -arch ${arch}'

Result:

Executing:  cd "..." && /usr/bin/make -j8 -w all PREFIX="/opt/local" CC='/usr/bin/clang {-L/opt/local/lib -Wl,-headerpad_max_install_names} -arch x86_64' 

All of these issues and more can be fixed in the portgroup by simply expanding the list with {*} before using it to append to or delete from the associated MacPorts base option. I've tested several ports that use these merger_* variables and they seem to be unaffected by the change, which makes sense, since in Tcl a list is a string, so all the existing usage of passing a single string would be treated as a list of one item which is fine.

Change History (1)

comment:1 Changed 6 years ago by ryandesign (Ryan Carsten Schmidt)

Resolution: fixed
Status: newclosed

In d8cb2dcd9a0d9bae5a0bd46f6b1bc6da7b365cf4/macports-ports:

muniversal-1.0.tcl: treat merger_* list variables as lists

Closes: #56002

Note: See TracTickets for help on using tickets.