[MacPorts] #56002: muniversal portgroup: treat merger_* list variables as lists!

[MacPorts]

#56002: muniversal portgroup: treat merger_* list variables as lists!
------------------------+------------------------
 Reporter:  ryandesign  |      Owner:  ryandesign
     Type:  defect      |     Status:  new
 Priority:  Normal      |  Milestone:
Component:  ports       |    Version:
 Keywords:              |       Port:  muniversal
------------------------+------------------------
 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.

--
Ticket URL: <https://trac.macports.org/ticket/56002>
MacPorts <https://www.macports.org/>
Ports system for macOS