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):
/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)
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 follow-up: 8 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 follow-up: 6 Changed 7 months ago by ryandesign (Ryan Carsten Schmidt)
Cc: | mascguy added |
---|---|
Owner: | changed from mascguy to barracuda156 |
Port: | legacy-support, R-nloptr → R-nloptr, legacy-support |
Summary: | legacy-support: stpncpy: error: expected parameter declarator → R-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 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 follow-up: 9 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 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 follow-up: 10 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 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 follow-up: 13 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 follow-up: 18 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 64This 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 follow-up: 15 Changed 7 months ago by kencu (Ken)
the new stpncpy macro is coming from the 10.7 SDK.
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
comment:15 Changed 7 months ago by fhgwright (Fred Wright)
Replying to kencu:
the new stpncpy macro is coming from the 10.7 SDK.
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.
comment:18 follow-ups: 20 21 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 64This 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 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 follow-up: 22 Changed 7 months ago by fhgwright (Fred Wright)
Replying to ryandesign:
A replacement for
xcrun --show-sdk-path
? I assume it would be justecho $SDKROOT
. MacPorts setsSDKROOT
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 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 justecho $SDKROOT
. MacPorts setsSDKROOT
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:
comment:24 follow-up: 26 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 follow-up: 27 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 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. Makingstpncpy()
"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 tostpncpy()
, but rather a general expansion of thelegacy-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 follow-up: 29 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 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 7 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
comment:31 follow-up: 32 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 Changed 4 months ago by mascguy (Christopher Nielsen)
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Replying to fhgwright:
Since this build is now working on the 10.6 buildbots, this ticket should be closed.
Great, closing.
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.