Opened 5 years ago

Last modified 2 years ago

#57539 new defect

legacysupport-1.0: Let ports specify what symbols they need

Reported by: ryandesign (Ryan Carsten Schmidt) Owned by:
Priority: Normal Milestone:
Component: ports Version:
Keywords: Cc: cjones051073 (Chris Jones), kencu (Ken), mascguy (Christopher Nielsen)
Port: legacysupport

Description

The legacysupport-1.0 portgroup currently defaults to doing its thing (adding the port:legacy-support dependency and manipulating the flags) on OS X 10.11 and earlier. Presumably this is for ports that require the clock_gettime functionality which was new in macOS 10.12. But many ports don't need that; many ports are only using this portgroup to add strnlen or strndup for Mac OS X 10.6 or older. Linking with the legacy library is unnecessary Mac OS X 10.7 or later in those cases, but the portgroup will still do it because the portgroup doesn't know not to.

The portgroup provides the legacysupport.newest_darwin_requires_legacy option, which defaults to 15 and which ports can override. But this requires each Portfile author to know which versions of macOS introduced support for which symbols. It would be better if Portfile authors didn't have to know that.

How about if the portgroup allowed the Portfile author to specify which symbols they require? Knowledge about which versions of macOS first introduced support for which symbols can be added to the portgroup, so that it can set legacysupport.newest_darwin_requires_legacy to the correct value automatically.

Change History (12)

comment:1 Changed 5 years ago by kencu (Ken)

in the build sources for the library, the symbols are disabled on systems that have them already.

For example, the strnlen.c source will be actually built only on 10.6 and earlier. On 10.7 and later, it is bypassed by a guard, and there is no code built.

Similar for all the other replaced functions.

It took me a while to notice this.

Last edited 5 years ago by kencu (Ken) (previous) (diff)

comment:2 Changed 5 years ago by cjones051073 (Chris Jones)

As Ken says, on OSX10.7 and newer the only symbols the library currently provides is clock_gettime, and only for OSX10.11 and older where it is missing. On OSX10.12 and newer the PortGroup does nothing.

So there is never a problem of the library override symbols provided by the system.

comment:3 Changed 5 years ago by ryandesign (Ryan Carsten Schmidt)

That's great, but if a port only needs e.g. strnlen, there is zero purpose to linking with the library on 10.7 or later, and the portfile author should not have to know that strnlen was released in 10.7; the portfile author should only need to specify that they need legacy support to provide an implementation of strnlen on those systems that require that.

comment:4 Changed 5 years ago by cjones051073 (Chris Jones)

The linker should already be dealing with this. It should only actually link the library if the target requires one of the symbols it provides. Otherwise the linkage should be dropped and not appear in the end binaries.

Last edited 5 years ago by cjones051073 (Chris Jones) (previous) (diff)

comment:5 Changed 5 years ago by kencu (Ken)

So to be clear, on 10.7, the library does not have any symbols for strnlen -- strnlen is not compiled into the library on that OS, nor are any of the other legacy symbols.

On 10.7+, the library at present would only have the symbol for clock_gettime, which would be ignored by the linker unless something actually called for it.

Turning the library linking on and off totally is a rather more complicated project of many options to turn on and off individual symbols, and I would tend to agree with Chris here, it would not appear to serve any purpose for the added complexity.

comment:6 in reply to:  4 Changed 5 years ago by ryandesign (Ryan Carsten Schmidt)

Replying to cjones051073:

The linker should already be dealing with this. It should only actually link the library if the target requires one of the symbols it provides. Otherwise the linkage should be dropped and not appear in the end binaries.

Ah, I didn't consider that. Still, at the very least, an unneeded dependency is added. You already have legacysupport.newest_darwin_requires_legacy for specifying that the dependency etc. should not be added. I'm suggesting a more user-friendly way of specifying that.

comment:7 Changed 5 years ago by cjones051073 (Chris Jones)

I only added legacysupport.newest_darwin_requires_legacy to act in corner cases where it is really needed. The majority of ports only need to add the PortGroup and that is it, nothing else, and I was aiming at keeping it simple like that. Requiring that ports that use it also have to list the specific functions they are going to use just seems like over engineering things a bit, to me.

comment:8 in reply to:  7 ; Changed 5 years ago by ryandesign (Ryan Carsten Schmidt)

Replying to cjones051073:

I only added legacysupport.newest_darwin_requires_legacy to act in corner cases where it is really needed.

Hmm. Did such a problem actually occur? What were the circumstances?

The majority of ports only need to add the PortGroup and that is it, nothing else, and I was aiming at keeping it simple like that. Requiring that ports that use it also have to list the specific functions they are going to use just seems like over engineering things a bit, to me.

I guess I just don't like unnecessary dependencies.

comment:9 in reply to:  8 ; Changed 5 years ago by cjones051073 (Chris Jones)

Replying to ryandesign:

Replying to cjones051073:

I only added legacysupport.newest_darwin_requires_legacy to act in corner cases where it is really needed.

Hmm. Did such a problem actually occur? What were the circumstances?

bladeRF, for instance. That port had some specific home-grown work arounds for clock_gettime that conflicted (as they did not use quite the right types) with those I added in the library. So I needed to turn this off for 10.7 and above.

The majority of ports only need to add the PortGroup and that is it, nothing else, and I was aiming at keeping it simple like that. Requiring that ports that use it also have to list the specific functions they are going to use just seems like over engineering things a bit, to me.

I guess I just don't like unnecessary dependencies.

Also please bear in mind the library does not allow individual features to be turned on or off. There can only be one library on any given system so it has to satisfy what *all* ports might require.

This means, in practise there are only 3 different states, that are determined on a Darwin level, not a feature level.

  • 10.6 or older. Has everything in the library.
  • 10.7 to 10.11. Has clock_gettime only.
  • 10.12+ Has nothing.

Thats the only states possible.

Say we added an option to allow individual ports to list the features they wanted. As explained above this would *not* actually map to what the library provided. Say a port said it needed 'strnlen' only. This would mean, on 10.6 and older it would get everything, not just the requested feature. For me this is misleading, and why I am not really in favour of adding what you suggest. I think the current option, that does it at the Darwin level, is actually clearer and not misleading, because its the only option that properly maps to what the library actually can or can not provide.

comment:10 Changed 2 years ago by mascguy (Christopher Nielsen)

Cc: mascguy added

comment:11 in reply to:  9 ; Changed 2 years ago by barracuda156

Replying to cjones051073:

Replying to ryandesign:

Replying to cjones051073:

I only added legacysupport.newest_darwin_requires_legacy to act in corner cases where it is really needed.

Hmm. Did such a problem actually occur? What were the circumstances?

bladeRF, for instance. That port had some specific home-grown work arounds for clock_gettime that conflicted (as they did not use quite the right types) with those I added in the library. So I needed to turn this off for 10.7 and above.

The majority of ports only need to add the PortGroup and that is it, nothing else, and I was aiming at keeping it simple like that. Requiring that ports that use it also have to list the specific functions they are going to use just seems like over engineering things a bit, to me.

I guess I just don't like unnecessary dependencies.

Also please bear in mind the library does not allow individual features to be turned on or off. There can only be one library on any given system so it has to satisfy what *all* ports might require.

This means, in practise there are only 3 different states, that are determined on a Darwin level, not a feature level.

  • 10.6 or older. Has everything in the library.
  • 10.7 to 10.11. Has clock_gettime only.
  • 10.12+ Has nothing.

Thats the only states possible.

Say we added an option to allow individual ports to list the features they wanted. As explained above this would *not* actually map to what the library provided. Say a port said it needed 'strnlen' only. This would mean, on 10.6 and older it would get everything, not just the requested feature. For me this is misleading, and why I am not really in favour of adding what you suggest. I think the current option, that does it at the Darwin level, is actually clearer and not misleading, because its the only option that properly maps to what the library actually can or can not provide.

Does it have CLOCK_PROCESS_CPUTIME_ID and CLOCK_THREAD_CPUTIME_ID? These are supposed to be a part of CLOCK_GETTIME, however I try to build gstreamer1 now, and it fails:

 ../gstreamer-1.20.1/plugins/tracers/gstrusage.c
../gstreamer-1.20.1/plugins/tracers/gstrusage.c: In function 'do_stats':
../gstreamer-1.20.1/plugins/tracers/gstrusage.c:172:25: error: 'CLOCK_PROCESS_CPUTIME_ID' undeclared (first use in this function)
  172 |     if (!clock_gettime (CLOCK_PROCESS_CPUTIME_ID, &now)) {
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~
../gstreamer-1.20.1/plugins/tracers/gstrusage.c:172:25: note: each undeclared identifier is reported only once for each function it appears in
../gstreamer-1.20.1/plugins/tracers/gstrusage.c:184:25: error: 'CLOCK_THREAD_CPUTIME_ID' undeclared (first use in this function)
  184 |     if (!clock_gettime (CLOCK_THREAD_CPUTIME_ID, &now)) {
      |                 

The following added to Portfile:

PortGroup			legacysupport 1.1
legacysupport.newest_darwin_requires_legacy 15

comment:12 in reply to:  11 Changed 2 years ago by cjones051073 (Chris Jones)

Does it have CLOCK_PROCESS_CPUTIME_ID and CLOCK_THREAD_CPUTIME_ID? These are supposed to be a part of CLOCK_GETTIME, however I try to build gstreamer1 now, and it fails:

 ../gstreamer-1.20.1/plugins/tracers/gstrusage.c
../gstreamer-1.20.1/plugins/tracers/gstrusage.c: In function 'do_stats':
../gstreamer-1.20.1/plugins/tracers/gstrusage.c:172:25: error: 'CLOCK_PROCESS_CPUTIME_ID' undeclared (first use in this function)
  172 |     if (!clock_gettime (CLOCK_PROCESS_CPUTIME_ID, &now)) {
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~
../gstreamer-1.20.1/plugins/tracers/gstrusage.c:172:25: note: each undeclared identifier is reported only once for each function it appears in
../gstreamer-1.20.1/plugins/tracers/gstrusage.c:184:25: error: 'CLOCK_THREAD_CPUTIME_ID' undeclared (first use in this function)
  184 |     if (!clock_gettime (CLOCK_THREAD_CPUTIME_ID, &now)) {
      |                 

The following added to Portfile:

PortGroup			legacysupport 1.1
legacysupport.newest_darwin_requires_legacy 15

No, those options are currently not defined. See

https://github.com/macports/macports-legacy-support/blob/master/include/time.h

and the local implementation in

https://github.com/macports/macports-legacy-support/blob/master/src/time.c

Please feel free to open a PR implementing these options, if you know how they can be added.

Note: See TracTickets for help on using tickets.