Opened 7 years ago
Closed 6 years ago
#55469 closed defect (fixed)
snowleopard_fixes portgroup behavior changed, causing some ports to fail to build
Reported by: | ryandesign (Ryan Carsten Schmidt) | Owned by: | kencu (Ken) |
---|---|---|---|
Priority: | Normal | Milestone: | |
Component: | ports | Version: | |
Keywords: | Cc: | svensen | |
Port: |
Description (last modified by 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.
Attachments (2)
Change History (12)
Changed 7 years ago by ryandesign (Ryan Carsten Schmidt)
Attachment: | moreutils-main.log added |
---|
Changed 7 years ago by ryandesign (Ryan Carsten Schmidt)
Attachment: | moreutils-port__register_callback-main.log added |
---|
comment:1 Changed 7 years ago by ryandesign (Ryan Carsten Schmidt)
Description: | modified (diff) |
---|
comment:2 Changed 7 years ago by kencu (Ken)
comment:3 Changed 7 years ago by 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.
comment:4 Changed 7 years ago by kencu (Ken)
How then would you optionally change flags depending on PortGroup variables? Like using the snowleopard_fixes.addheader
variable to optionally add flags?
comment:5 Changed 7 years ago by 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.
comment:6 Changed 7 years ago by kencu (Ken)
How about like this:
options snowleopard_fixes.addheader default snowleopard_fixes.addheader {no} proc add_libsnowleopardfixes {} { depends_lib-append port:snowleopardfixes } if {${os.platform} eq "darwin" && ${os.major} < 11} { configure.ldflags-append -lsnowleopardfixes 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 }
comment:7 Changed 7 years ago by 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.
comment:8 Changed 7 years ago by svensen
Cc: | svensen added |
---|
comment:9 Changed 6 years ago by 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.
comment:10 Changed 6 years ago by kencu (Ken)
Resolution: | → fixed |
---|---|
Status: | new → closed |
SL fixes PG no longer is active.
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.