Opened 7 months ago

Closed 4 months ago

#69838 closed defect (fixed)

R-nloptr: stpncpy: error: expected parameter declarator

Reported by: ryandesign (Ryan Carsten Schmidt) Owned by: barracuda156
Priority: Normal Milestone:
Component: ports Version: 2.9.3
Keywords: Cc: mascguy (Christopher Nielsen), fhgwright (Fred Wright)
Port: R-nloptr, legacy-support

Description

Building R-nloptr fails with this error that implicates the newly-added stpncpy implementation (#69571):

https://build.macports.org/builders/ports-10.6_x86_64-builder/builds/198028/steps/install-port/logs/stdio

/opt/local/include/LegacySupport/string.h:30:14: error: expected parameter declarator
extern char *stpncpy(char *dst, const char *src, size_t n);
             ^
/Developer/SDKs/MacOSX10.7.sdk/usr/include/secure/_string.h:110:5: note: expanded from macro 'stpncpy'
  ((__darwin_obsz0 (dest) != (size_t) -1)                               \
    ^
/Developer/SDKs/MacOSX10.7.sdk/usr/include/secure/_common.h:38:63: note: expanded from macro '__darwin_obsz0'
#define __darwin_obsz0(object) __builtin_object_size (object, 0)
                                                              ^

Change History (32)

comment:1 Changed 7 months ago by ryandesign (Ryan Carsten Schmidt)

Our 10.6 build machines do have a copy of the 10.7 SDK installed manually which was for the benefit of libsdl2 which could not build without it, but it is curious that R-nloptr is using it; nothing in the Portfile requests this.

comment:2 Changed 7 months ago by fhgwright (Fred Wright)

I suppose, in principle, that any simple (i.e., not playing any funny games) declarations in the legacy-support headers would be more appropriately conditioned on the SDK version rather than the target OS version, but stpncpy() is not unique in basing it on the deployment target. In particular, the preexisting strnlen(), strndup(), and memmem() are all handled in exactly the same way, including the exact same OS condition.

Is R-nloptr somehow forcing the 10.7 SDK for the express purpose of getting stpncpy()?

comment:3 Changed 7 months ago by kencu (Ken)

This is a very messy build, that uses a bunch of shell scripts and configure commands to ultimately decompress and build with cmake what seems to be the real source code here:

https://github.com/astamm/nloptr/blob/master/src/nlopt-src.tar.gz

that source code contains the CMakeLists.txt file.

as the build is not using any of the cmake PortGroup's settings, all the secret sauce that makes things work in MacPorts is not happening. So you are getting what the build thinks should be set for compilers, SDKs, etc, not what macports sets.

comment:4 Changed 7 months ago by fhgwright (Fred Wright)

Note that the error involves some bizarre stpncpy macro, which is not provided by either legacy-support or the 10.7 SDK.

The legacy-support declaration in strings.h:

extern char *stpncpy(char *dst, const char *src, size_t n);

The 10.7 SDK declaration in strings.h (assuming __DARWIN_C_LEVEL >= 200809L):

char    *stpncpy(char *, const char *, size_t) __OSX_AVAILABLE_STARTING(__MAC_10_7, __IPHONE_4_3);

My guess is that R-nloptr is providing its own definition of stpncpy() as a macro, based on an unverified (and now false) assumption that it's not provided by the "OS" in this case. This behavior seems like it's inherently incompatible with the philosophy of legacy-support. It's probably unrelated to the SDK selection, though that seems questionable as well.

The only way legacy-support is to blame for this is by working as intended.

As the saying goes, "be careful what you wish for (#69571)". :-)

comment:5 Changed 7 months ago by ryandesign (Ryan Carsten Schmidt)

Cc: mascguy added
Owner: changed from mascguy to barracuda156
Port: legacy-support, R-nloptrR-nloptr, legacy-support
Summary: legacy-support: stpncpy: error: expected parameter declaratorR-nloptr: stpncpy: error: expected parameter declarator

Ok, then I guess this is an R-nloptr bug. Can R-nloptr not use the 10.7 SDK on 10.6?

comment:6 in reply to:  5 Changed 7 months ago by barracuda156

Replying to ryandesign:

Ok, then I guess this is an R-nloptr bug. Can R-nloptr not use the 10.7 SDK on 10.6?

I never understood why later SDKs are made available for earlier OS versions by default. This seems very fragile and prone to generate errors, and here we go.

comment:7 Changed 7 months ago by ryandesign (Ryan Carsten Schmidt)

Whom are you referring to?

If you mean why is the 10.7 SDK installed on the 10.6 builders when that is not normally the case, I explained above.

If you mean why does this port's build system choose the 10.7 SDK on 10.6, you'll have to ask whoever made this port's build system.

comment:8 in reply to:  3 Changed 7 months ago by barracuda156

Replying to kencu:

This is a very messy build, that uses a bunch of shell scripts and configure commands to ultimately decompress and build with cmake what seems to be the real source code here:

https://github.com/astamm/nloptr/blob/master/src/nlopt-src.tar.gz

that source code contains the CMakeLists.txt file.

as the build is not using any of the cmake PortGroup's settings, all the secret sauce that makes things work in MacPorts is not happening. So you are getting what the build thinks should be set for compilers, SDKs, etc, not what macports sets.

R build system itself is mildly horrible, and package upstreams at times make it yet worse.

But I do not know how to use cmake PG inside R PG. Very few R ports use CMake, AFAIK, it is arguably not worth bothering to implement via PG, unless trivial. Perhaps rather patch this by hand to ensure it works correctly.

comment:9 in reply to:  7 ; Changed 7 months ago by barracuda156

Replying to ryandesign:

Whom are you referring to?

If you mean why is the 10.7 SDK installed on the 10.6 builders when that is not normally the case, I explained above.

I do not know why SDL was set up that way, tbh :)

If you mean why does this port's build system choose the 10.7 SDK on 10.6, you'll have to ask whoever made this port's build system.

I think nothing from our side is doing that, though obviously nothing prevented that from happening either. But it is not the default installation of Xcode, AFAIU, so nobody tried that. I have 10.6.8 x86_64, I think I have R-nloptr built there.

If it is not possible to avoid 10.7 SDK from buildbots system side, I can try installing it and figuring out how to prevent it from breaking things.

(Upstreams normally would not be testing anything aside of very recent macOS versions, so no surprise it may not work in some scenarios.)

comment:10 in reply to:  9 Changed 7 months ago by ryandesign (Ryan Carsten Schmidt)

Replying to barracuda156:

I do not know why SDL was set up that way, tbh :)

As I explained above, libsdl2 (well, now it's the libsdl2-snowleopard port) cannot build on 10.6 unless the 10.7 SDK is used. Now that there is a MacOSX10.7.sdk port, the libsdl2-snowleopard port might be changed to use that instead and we could uninstall the 10.7 SDKs from the system's SDK directory.

If you mean why does this port's build system choose the 10.7 SDK on 10.6, you'll have to ask whoever made this port's build system.

I think nothing from our side is doing that,

If by "our side" you mean this port's build system, then it's hard to tell what it's doing, since it uses silent rules, since the CMake flag -DCMAKE_VERBOSE_MAKEFILE=ON hasn't been used. This would have been used if it were building with the cmake portgroup. It may simply be selecting the newest installed SDK, which is what Apple usually recommends, though not what we usually do in MacPorts.

If it is not possible to avoid 10.7 SDK from buildbots system side,

There's nothing buildbot can do about this. It doesn't contain any feature to hide files from builds. You're thinking of trace mode, which we don't use on buildbot.

comment:11 Changed 7 months ago by kencu (Ken)

the details about libsdl2 and why it needs the 10.7 SDK on SnowLeopard are here #52210.

comment:12 Changed 7 months ago by ryandesign (Ryan Carsten Schmidt)

I do see in the log:

Warning in system2("xcrun", "--show-sdk-path", TRUE, TRUE) :
  running command ''xcrun' --show-sdk-path 2>&1' had status 64

This is #67314.

Somewhere in the build system of this port or in R generally there must be an instruction to run this command, and fallback code if it doesn't work, that must be responsible for selecting the 10.7 SDK.

comment:13 in reply to:  12 ; Changed 7 months ago by barracuda156

Replying to ryandesign:

I do see in the log:

Warning in system2("xcrun", "--show-sdk-path", TRUE, TRUE) :
  running command ''xcrun' --show-sdk-path 2>&1' had status 64

This is #67314.

Somewhere in the build system of this port or in R generally there must be an instruction to run this command, and fallback code if it doesn't work, that must be responsible for selecting the 10.7 SDK.

It there a canonical replacement for this (where just removal does not work)? I had quite a bit of pain sorting that for Qt5 builds on ppc (they still fail later on, but xcrun in their configure is nasty).

comment:14 Changed 7 months ago by kencu (Ken)

Last edited 7 months ago by kencu (Ken) (previous) (diff)

comment:15 in reply to:  14 Changed 7 months ago by fhgwright (Fred Wright)

Replying to kencu:

the new stpncpy macro is coming from the 10.7 SDK.

https://github.com/alexey-lysiuk/macos-sdk/blob/2e3a0a902a4d8e641d44224ef2860a4f1ebfd9d7/MacOSX10.7.sdk/usr/include/secure/_string.h#L109

I think we do need to add something to legacysupport for the new function, like is done for others here:

https://github.com/alexey-lysiuk/macos-sdk/blob/main/MacOSX10.6.sdk/usr/include/secure/_string.h

If the intent is to fully support _USE_FORTIFY_LEVEL, then this treatment would need to be applied to many functions, with stpncpy() being just the tip of the iceberg. OTOH, if ignoring _USE_FORTIFY_LEVEL is acceptable, then a simple #undef should be sufficient to fix the problem. It might make sense to do the latter in the short term, with more elaborate rework to come later.

comment:16 Changed 7 months ago by fhgwright (Fred Wright)

Note that, AFAIK, nothing in legacy-support currently honors _USE_FORTIFY_LEVEL, so the #undef approach would be consistent with that, and honoring _USE_FORTIFY_LEVEL should be considered an enhancement.

The fact that R-nloptr is opportunistically choosing an inappropriate SDK is also a bug, but it shouldn't cause this failure.

comment:17 Changed 7 months ago by kencu (Ken)

I didn't look through this exhaustively, but I think going from 10.6 to 10.7, the only additional function that needs this treatment is the new stpncpy.

Later on, if someday more secure string functions need to be added, there could be more.

So I would just add it like the others, myself. But you're the new maintainer of legacysupport, so you can decide how you want to play it out.

Last edited 7 months ago by kencu (Ken) (previous) (diff)

comment:18 in reply to:  13 ; Changed 7 months ago by ryandesign (Ryan Carsten Schmidt)

Replying to barracuda156:

Replying to ryandesign:

I do see in the log:

Warning in system2("xcrun", "--show-sdk-path", TRUE, TRUE) :
  running command ''xcrun' --show-sdk-path 2>&1' had status 64

This is #67314.

Somewhere in the build system of this port or in R generally there must be an instruction to run this command, and fallback code if it doesn't work, that must be responsible for selecting the 10.7 SDK.

It there a canonical replacement for this (where just removal does not work)? I had quite a bit of pain sorting that for Qt5 builds on ppc (they still fail later on, but xcrun in their configure is nasty).

A replacement for xcrun --show-sdk-path? I assume it would be just echo $SDKROOT. MacPorts sets SDKROOT to the SDK path when an SDK is needed and to the empty string when one is not needed.

comment:19 Changed 7 months ago by barracuda156

With R-nloptr, I get this now during the build, though it still seems to install fine:

-- Installing: /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_R_R-nloptr/R-nloptr/work/nloptr/src/nlopt/share/man/man3/nlopt_minimize_constrained.3
mv: rename nlopt/lib to nlopt/lib/lib: Invalid argument
configure: creating ./config.status
config.status: creating src/Makevars

comment:20 in reply to:  18 Changed 7 months ago by barracuda156

Replying to ryandesign:

Apparently that SDK code does nothing and exists for informational purposes only:

src/library/tools/R/install.R
@@ -2809,7 +2809,7 @@
         if (Sys.info()["sysname"] == "Darwin" &&
             (with_c|| with_f77 || with_f9x || with_cxx)) {
             ## report the SDK in use: we want to know what it is symlinked to
             sdk <- try(system2("xcrun", "--show-sdk-path", TRUE, TRUE), silent = TRUE)
             if(!inherits(sdk, "try-error")) {
                 sdk <- Sys.readlink(sdk)
                 message("using SDK: ", sQuote(sdk))

comment:21 in reply to:  18 ; Changed 7 months ago by fhgwright (Fred Wright)

Replying to ryandesign:

A replacement for xcrun --show-sdk-path? I assume it would be just echo $SDKROOT. MacPorts sets SDKROOT to the SDK path when an SDK is needed and to the empty string when one is not needed.

And overrides any value set by the user, even when it's included in extra_env, which can be inconvenient for some types of testing.

Replying to barracuda156:

With R-nloptr, I get this now during the build, though it still seems to install fine:

-- Installing: /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_R_R-nloptr/R-nloptr/work/nloptr/src/nlopt/share/man/man3/nlopt_minimize_constrained.3
mv: rename nlopt/lib to nlopt/lib/lib: Invalid argument
configure: creating ./config.status
config.status: creating src/Makevars

That mv command is clearly illegal, since mv has no built-in mkdir. You may want to see what it was trying to do unsuccessfully in this "successful" install.

I created a new ticket #69867 for the issue of legacy-support and nonstandard SDKs, so this one can stick to the R-nloptr SDK issue.

BTW, legacy-support ought to have its own component in the ticket system, since issues of this form are really related to the project, not the ports system.

comment:22 in reply to:  21 Changed 7 months ago by ryandesign (Ryan Carsten Schmidt)

Replying to fhgwright:

Replying to ryandesign:

A replacement for xcrun --show-sdk-path? I assume it would be just echo $SDKROOT. MacPorts sets SDKROOT to the SDK path when an SDK is needed and to the empty string when one is not needed.

And overrides any value set by the user, even when it's included in extra_env, which can be inconvenient for some types of testing.

That has nothing to do with this ticket, but if you want to choose a different SDK for testing purposes, I assume what you should do is e.g. configure.sdk_version=10.7 on the command line or set macosx_sdk_version in macports.conf.

comment:23 Changed 7 months ago by kencu (Ken)

If you don’t specifically pass in the SDK for cmake to use with CMAKE_OSX_SYSROOT, then cmake defaults to choosing the newest SDK that it can find:

https://github.com/Kitware/CMake/blob/f56eb6fa88625946961f80dadca9790695c6ccf7/Modules/Platform/Darwin-Initialize.cmake#L99

comment:24 Changed 7 months ago by kencu (Ken)

there are 5 ports that explicitly select the 10.7 SDK on 10.6. Some others, like this one, might implicitly use it.

not sure how many of those currently use stpncpy, but all are potentially broken until somebody gets around to adding the missing secure definition of stpncpy in the legacysupport headers.

Probably take about 5 minutes to do it :)

comment:25 Changed 7 months ago by ryandesign (Ryan Carsten Schmidt)

Yes but I can probably count on one hand the number of people who understand the legacy support code to the extent that they could make the change.

It sounds like we should do that, and the R-nloptr port should be fixed to specify more of the cmake parameters we expect (if it cannot use the cmake portgroup), and the ports that use the 10.7 SDK on 10.6 should be updated to use the MacOSX10.7.sdk port so that the 10.7 SDK can be removed from the system SDKs directory.

comment:26 in reply to:  24 ; Changed 7 months ago by fhgwright (Fred Wright)

Replying to kencu:

there are 5 ports that explicitly select the 10.7 SDK on 10.6. Some others, like this one, might implicitly use it.

not sure how many of those currently use stpncpy, but all are potentially broken until somebody gets around to adding the missing secure definition of stpncpy in the legacysupport headers.

At present, legacy-support does not provide "secure" definitions for anything. Making stpncpy() "like all the others" isn't the fix, because it is like all the others.

I filed a new ticket #69878 for adding "secure" support to legacy-support, which is not at all specific to stpncpy(), but rather a general expansion of the legacy-support headers. It's also worth noting that adding "secure" support is neither necessary nor sufficient to fix the issue here.

Probably take about 5 minutes to do it :)

Hardly.

comment:27 in reply to:  26 Changed 7 months ago by kencu (Ken)

Replying to fhgwright:

Replying to kencu:

there are 5 ports that explicitly select the 10.7 SDK on 10.6. Some others, like this one, might implicitly use it.

not sure how many of those currently use stpncpy, but all are potentially broken until somebody gets around to adding the missing secure definition of stpncpy in the legacysupport headers.

At present, legacy-support does not provide "secure" definitions for anything. Making stpncpy() "like all the others" isn't the fix, because it is like all the others.

I filed a new ticket #69878 for adding "secure" support to legacy-support, which is not at all specific to stpncpy(), but rather a general expansion of the legacy-support headers. It's also worth noting that adding "secure" support is neither necessary nor sufficient to fix the issue here.

Probably take about 5 minutes to do it :)

Hardly.

OK. It's currently April 30, 2024.

Let's see how many years this gets left broken to make some kind of point ;>

Luckily, I fork legacysupport for my own uses.

comment:28 Changed 7 months ago by ryandesign (Ryan Carsten Schmidt)

Ken, if you know how to fix it and it will take you 5 minutes, please fix it today.

comment:29 in reply to:  28 Changed 7 months ago by mascguy (Christopher Nielsen)

Replying to ryandesign:

Ken, if you know how to fix it and it will take you 5 minutes, please fix it today.

Agreed! And if possible, we'd like to accomodate folks' needs for the library, rather than them maintaining forked versions.

comment:30 Changed 6 months ago by fhgwright (Fred Wright)

It turns out that stpncpy is the only function affected by this particular form of collision, though not for the reason that Ken implied. There are four string.h functions provided by legacy-support prior to 10.7. There are ten string.h functions with "secure" wrappers in the 10.7 SDK. But only stpncpy is in the intersection between those two sets.

Actually editing in the one-line fix probably indeed took less than five minutes, but determining the fix took longer, determining that the fix was only relevant to stpncpy took longer, verifying that the fix actually fixed the problem took longer, and of course testing the result on all the platforms took longer.

See: https://github.com/macports/macports-legacy-support/pull/85

Last edited 6 months ago by fhgwright (Fred Wright) (previous) (diff)

comment:31 Changed 6 months ago by fhgwright (Fred Wright)

Since this build is now working on the 10.6 buildbots, this ticket should be closed.

comment:32 in reply to:  31 Changed 4 months ago by mascguy (Christopher Nielsen)

Resolution: fixed
Status: assignedclosed

Replying to fhgwright:

Since this build is now working on the 10.6 buildbots, this ticket should be closed.

Great, closing.

Note: See TracTickets for help on using tickets.