serf1: re-enable universal variant

[ryandesign (Ryan Carsten Schmidt)]

r108691 updated serf1 to 1.3.0 and switched to its scons build system, which brought with it use_configure no, which removes the universal variant, thereby dooming all ports depending on serf1 to non-universality. Please reinstate the universal variant.

At minimum, you'll need to add this line before the first time you invoke the get_canonical_archflags proc:

variant universal {}

However that alone does not appear to be sufficient. The port tries to communicate the archflags to the build system via the ARCHFLAGS environment variable, but grepping the serf source for ARCHFLAGS does not reveal any hits, so I'm not sure whether this is intended to work. (Is this a standard scons environment variable?)

My own opinion is that the scons build system is difficult to use, especially in MacPorts. That might be partially remedied by the creation of a scons portgroup to abstract away some of the implementation details.

[blair (Blair Zajac)]

I just copied this section from mongodb's port:

build.env       ARCHFLAGS="[get_canonical_archflags]" \
                CPPFLAGS="${configure.cppflags}" \
                LDFLAGS="${configure.ldflags}"

I found CFLAGS in SConstruct and it's next to PREFIX and the other flags set via build.target. Maybe we remove ARCHFLAGS and set CFLAGS where PREFIX is set in build.target. Can you see if that works?

Do you have systems with universal builds?

[ryandesign (Ryan Carsten Schmidt)]

Yes, my primary system defaults to universal builds. But it's trivial for you to try it out too; just run "sudo port install serf1 +universal" and MacPorts will universalize dependencies as necessary.

[blair (Blair Zajac)]

I'd rather not switch my system to universal. I'll update the Portfile here and can you try it?

[cooljeanius (Eric Gallager)]

Cc Me!

[cooljeanius (Eric Gallager)]

Fink actually also had some issues with serf1 and scons ignoring arch flags... let me see if I can find how they solved it again...

[jmroot (Joshua Root)]

Replying to blair@…:

I just copied this section from mongodb's port:

Mongodb patches the SConstruct to make that work.

[jpenney (Jason Penney)]

Cc Me!

[Serge3leo (Serguei E. Leontiev)]

Cc Me!

[Serge3leo (Serguei E. Leontiev)]

Portfile probably contains a few errors:

• ARCHFLAGS not required to build serf;
• CFLAGS is not defined properly;
• serf build requires LINKFLAGS, not LDFLAGS;
• serf SConstruct can't use environment variables, it requires command-line arguments;

For add build universal variant should fix the Portfile:

variant universal {
    build.args CC=${configure.cc} \
        CFLAGS=[string map {" " ","} "${configure.universal_cflags}"] \
        CPPFLAGS=[string map {" " ","} "${configure.universal_cppflags}"] \
        LINKFLAGS=[string map {" " ","} "${configure.universal_ldflags}"]
}

build.cmd       ${prefix}/bin/scons
build.args      CC=${configure.cc} \
                CFLAGS=[string map {" " ","} "${configure.cflags}"] \
                CPPFLAGS=[string map {" " ","} "${configure.cppflags}"] \
                LINKFLAGS=[string map {" " ","} "${configure.ldflags}"]

-- Sorry for my best English

[ryandesign (Ryan Carsten Schmidt)]

This (though untested) would be shorter, and more accurate in that the universal build would get the regular c-, cpp- and ldflags too, and the non-universal build would get correct -arch flags too:

variant universal {}

build.cmd       ${prefix}/bin/scons
build.args      CC=${configure.cc} \
                CFLAGS=[string map {" " ","} "${configure.cflags} [get_canonical_archflags cc]"] \
                CPPFLAGS=[string map {" " ","} "${configure.cppflags}"] \
                LINKFLAGS=[string map {" " ","} "${configure.ldflags} [get_canonical_archflags ld]"]

That's assuming it's correct to concatenate these flags with commas instead of spaces. I've never seen that before and it seems weird to me. But we've already established that scons is needlessly weird.

[Serge3leo (Serguei E. Leontiev)]

Replying to ryandesign@…:

That's assuming it's correct to concatenate these flags with commas instead of spaces. I've never seen that before and it seems weird to me. But we've already established that scons is needlessly weird.

Not scons, but serf-1.3.0/SConstruct :) See function RawListVariable() at serf-1.3.0/SConstruct:40 and CFLAGS at line 89

[larryv (Lawrence Velázquez)]

It would be cleaner to use join for this.

[join "${configure.cflags},[get_canonical_archflags cc]" ,]

[blair (Blair Zajac)]

So is the following correct, incorporating all the feedback?

variant universal {}

build.cmd       ${prefix}/bin/scons
build.args      CC=${configure.cc} \
                CPPFLAGS=[join "${configure.cppflags}" ,] \
                CFLAGS=[join "${configure.cflags},[get_canonical_archflags cc]" ,] \
                LINKFLAGS=[join "${configure.ldflags},[get_canonical_archflags ld]" ,]

Ryan, feel free to commit any changes you want.

[jeremyhu (Jeremy Huddleston Sequoia)]

Cc Me!

[blair (Blair Zajac)]

When this is fixed, does svn need to have its revisions bumped to recompile so it picks up the universal pieces?

[ryandesign (Ryan Carsten Schmidt)]

It appears to work. Committed in r108804.

No, other ports should not need any changes.

[blair (Blair Zajac)]

Thanks.

So the pre-fixed version would, say, only have x86_64 bits while the universal build would have x86_64 and i386? But how could Subversion link the i386 version against libserf-1.dylib when it only contains x86_64? Or it only cares about the symbols and the symbols are there for both arches?

[ryandesign (Ryan Carsten Schmidt)]

Replying to blair@…:

So the pre-fixed version would, say, only have x86_64 bits

Right, whatever was specified for build_arch in the user's macports.conf.

while the universal build would have x86_64 and i386?

Right, whatever was specified for universal_archs in the user's macports.conf.

But how could Subversion link the i386 version against libserf-1.dylib when it only contains x86_64? Or it only cares about the symbols and the symbols are there for both arches?

It could not. MacPorts knows this and checks that the architectures match before allowing a build. If a user had tried to install or upgrade subversion +universal, MacPorts would have exited with an error explaining that it could not be done because dependency serf1 did not have a universal variant. And now it does so now it should work again.

[blair (Blair Zajac)]

OK, great, thanks for the explanation.