snowleopard_fixes portgroup behavior changed, causing some ports to fail to build

[ryandesign (Ryan Carsten Schmidt)]

The snowleopard_fixes portgroup's behavior changed in [40ae4799e2e766c658b20d09d23830cdab5adfcc/macports-ports] in which the use of port::register_callback was introduced. The reason given for the change was so that ports that use the portgroup are not forced to use depends_lib-append instead of depends_lib.

This effectively makes the code of the portgroup execute at the end of the portfile's code, rather than at the point where the portgroup is included, which is usually at the beginning. This is not how other portgroups work, and is therefore confusing.

This change causes some ports to fail to build, namely those that need access to the changes the portgroup is making. For example, the moreutils port does not have a configure phase, so it gets the value of ${configure.ldflags} to set flags for the build phase.

Now that the portgroup no longer changes configure.ldflags until after the portfile has been executed, the portfile doesn't have access to the changed variable, so the build fails​ because -lsnowleopardfixes isn't in the ldflags. It succeeded before the change​.

The moreutils port could adapt to this changed behavior by enclosing the build.args-append command inside a pre-build {...} block. But if the point of making the portgroup use port::register_callback was to avoid having to change portfiles, then it fails at that goal.

I suggest the use of port::register_callback be reverted. The fact that its name contains : characters also indicates it was not intended to be called from portfiles or portgroups.

Ports that want the portgroup's code to be executed at the end of the portfile can include the portgroup at the end of the portfile.

[kencu (Ken)]

Easy enough. I copied that block after I saw Marcus' cxx11 1.1 PortGroup changes that implemented that idea for the exact reason. FIgured he knew more about it that I did, and it looked good.

Should he revert that too? I don't understand why it would work with one but not the other.

I await your feedback.

[ryandesign (Ryan Carsten Schmidt)]

Ok yes, I was just thinking that it would probably be ok if the procedure that's called later only adds the dependencies, and it looks like that's what Marcus did in cxx11 1.1. The flags are modified immediately (when the portgroup is included). If snowleopard_fixes did the same that should work.

Up to now, we've told portfile authors to append to dependencies if they include a portgroup, but time and again we find portfile authors who don't do that. If we can find a way to make it so that portfile authors don't need to do that, as I think this fix is trying to do, that's great. And we should do it in all the portgroups. But first maybe we should make port::register_callback available in the portfile's namespace so that we don't need to use a proc name with :s.

[kencu (Ken)]

How then would you optionally change flags depending on PortGroup variables? Like using the snowleopard_fixes.addheader variable to optionally add flags?

[kencu (Ken)]

Oh, I think I see. I thought the logic would have to be done in a callback once the variable was known, but not necessarily so.

I think I can see how to fix this up.

[kencu (Ken)]

How about like this:

options snowleopard_fixes.addheader
default snowleopard_fixes.addheader {no}

proc add_libsnowleopardfixes {} {
	depends_lib-append          port:snowleopardfixes
        configure.ldflags-append   -lsnowleopardfixes
}

if {${os.platform} eq "darwin" && ${os.major} < 11} {

    if {${snowleopard_fixes.addheader} eq "yes"} {
        configure.cxxflags-append -include ${prefix}/include/snowleopardfixes.h
        configure.cflags-append   -include ${prefix}/include/snowleopardfixes.h
    }

    # do not force all Portfiles to switch from depends_lib to depends_lib-append
    port::register_callback add_libsnowleopardfixes
}

[ryandesign (Ryan Carsten Schmidt)]

Ah yes. That won't work unless the portfile author sets snowleopard_fixes.addheader before including the portgroup, which would look unusual. (The same way that the obsolete-1.0 portgroup requires the portfile author to set replaced_by before including the portgroup, and that looks unusual.)

The solution is to create an option_proc that's called whenever the snowleopard_fixes.addheader option's value is changed; in that proc, you then add or remove flags as needed depending on the value the option is being set to.

grep the groups directory for option_proc for some examples.

[ryandesign (Ryan Carsten Schmidt)]

Now that the moreutils port has switched from the snowleopard_fixes portgroup to the legacysupport portgroup it is able to build on older systems again. I guess when all ports have been moved to this new portgroup (have they already?) and the old portgroup is deleted, then this ticket can be closed.

[kencu (Ken)]

SL fixes PG no longer is active.