-
Notifications
You must be signed in to change notification settings - Fork 8.2k
posix: deprecate CONFIG_POSIX_API and remove zephyr/posix include prefix #97855
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
posix: deprecate CONFIG_POSIX_API and remove zephyr/posix include prefix #97855
Conversation
d6f7dd9 to
c82f505
Compare
c82f505 to
21ab57c
Compare
|
The following west manifest projects have changed revision in this Pull Request:
✅ All manifest checks OK Note: This message is automatically posted and updated by the Manifest GitHub Action. |
b9dac21 to
dc6abc8
Compare
|
e68fe2e to
191fb79
Compare
It wasn't quite that easy, but this seems to have worked and given me the opportunity to fix something that was missed up until this point as well. Namely, that There are additional workarounds needed in order to avoid using shims.
This should be sufficient to satisfy one of Alberto's pending comments mentioned here. I will not be shuffling headers around to some unrelated folder at this time, and I do not see the need to do so in the future. However, headers required as workarounds for issues in Picolibc can be removed as Picolibc is improved over time. |
|
Hey @carlescufi The
It's not really a prerequisite, as demonstrated by the last month of successful CI runs. As long as there are non-broken headers for the network stack to use, everything is fine, and it works identically. What's nicely demonstrated here, is that it's quite straight-forward to make the problem of refactoring the network stack a separable one, due to the well-established interface. I.e. as long as the interface is consistent and functionality is the same, the implementation may be modified. Since reworking the network stack has been demonstrated to be a separable problem, there is no need to defer this change to pay off that technical debt. |
aescolar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this is better.
But note we'd want to do the same for the other libCs too, not just picolibc.
Note that placing these new headers in a separate folders and only including them in a more fine grained way than CONFIG_POSIX_BASE_DEFINITIONS OR CONFIG_EVENTFD would help limiting side-effects as described in #99075.
CONFIG_XOPEN_STREAMS does not follow the pattern of other XSI Option Groups, where the Option Group name is not the same as the feature test macro that indicates it is supported by the implementation. Deprecate CONFIG_XOPEN_STREAMS and rename it to CONFIG_XSI_STREAMS. For more information, please see https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/\ V1_chap02.html#tag_02_01_05_09 Signed-off-by: Chris Friedt <[email protected]>
Deprecate the CONFIG_XOPEN_STREAMS Kconfig option in favour of CONFIG_XSI_STREAMS, which matches the naming conventions of other XSI Kconfig options and the actual Option Group name in the specification. Please use CONFIG_XSI_STREAMS instead. Signed-off-by: Chris Friedt <[email protected]>
This change mitigates some code sonar issues (from SonarQube). 1. explicit 'U' after 0xffffffff > A cast shall not remove any const or volatile qualification from the > type of a pointer or reference c:S859 2. check return value from call to pthread_barrierattr_getpshared() > Unused assignments should be removed c:S1854 Signed-off-by: Chris Friedt <[email protected]>
This mitigates a sonarqube warning "Value stored to 'cvalue' is never read" > Unused assignments should be removed c:S1854 https://sonarcloud.io/project/issues?pullRequest=97152&\ open=AZnAZi4C7BgzESPFIGwf&id=zephyrproject-rtos_zephyr Signed-off-by: Chris Friedt <[email protected]>
This change mitigates a sonarqube warning about uninitialized variables. "1st function call argument is an uninitialized value" > Variables should be initialized before use c:S836 https://sonarcloud.io/project/issues?pullRequest=97152&\ open=AZnAZi0m7BgzESPFIGwe&id=zephyrproject-rtos_zephyr Signed-off-by: Chris Friedt <[email protected]>
There is no need to pull in a POSIX header file into these files, so remove it. Signed-off-by: Chris Friedt <[email protected]>
If C libraries provide conformant POSIX headers, then use the headers provided by the C library. Otherwise, C library maintainers may add `include/zephyr/posix` to their standard search path. This change removes the requirement to have `<zephyr/posix/...>` prefixing all standard POSIX includes in Zephyr. The main reason that it was required was due to conflicts between C library time.h, signal.h and the fact that Zephyr posix headers were being somewhat ad-hoc mixed with libc posix headers. Now, any (even partially) POSIX-conformant C libraries are expected to provide their own types, constants, and headers, and Zephyr's implementation uses those definitions. If any definitions are missing, we define them on a per-libc basis. This can also be done out-of-tree, which allows for implementors to take advantage of the `CONFIG_TC_PROVIDES_POSIX..` options. This is likely the last major commit required before we deprecate the `CONFIG_POSIX_API` Kconfig option in favour of POSIX subprofiling \o/ Signed-off-by: Chris Friedt <[email protected]>
Include standard headers for POSIX types and functions instead of making implicit declarations. Signed-off-by: Chris Friedt <[email protected]>
Remove the duplicate Kconfig option `CONFIG_POSIX_SYSTEM_INTERFACES` in `lib/posix/Kconfig.profile` that shadows the option in `lib/posix/Kconfig`. Signed-off-by: Chris Friedt <[email protected]>
This change deprecates the Kconfig option `CONFIG_POSIX_API`. It is slated for removal in Zephyr v4.5.0. Signed-off-by: Chris Friedt <[email protected]>
Add release notes about the deprecation of CONFIG_POSIX_API. User should instead use one of the pre-defined POSIX subprofiles CONFIG_POSIX_AEP_CHOICE_BASE CONFIG_POSIX_AEP_CHOICE_PSE51 CONFIG_POSIX_AEP_CHOICE_PSE52 CONFIG_POSIX_AEP_CHOICE_PSE53 and libraries should depend on CONFIG_POSIX_SYSTEM_INTERFACES as well as other POSIX Option Groups, as needed. For more information, please see https://pubs.opengroup.org/onlinepubs/9799919799/xrat/V4_subprofiles.html Signed-off-by: Chris Friedt <[email protected]>
The specification requires that unistd.h (or a file that is included via unistd.h) declare version macros and symbolic constants. Feature test macros declarations are also removed from `lib/posix/options/CMakeLists.txt`. Closes zephyrproject-rtos#97949 Signed-off-by: Chris Friedt <[email protected]>
Disagree. The only reason that including the path was made conditional was because it was your preference, which, in my opinion, is not sufficient technical justification. I called it a workaround for the workaround, because the original workaround was to correct inconsistencies in C libraries, but then the workaround for the workaround was for you, since you seem to prefer broken headers, which introduces more inconsistence and tech debt. For a technical justification, we have to consider why the path is included in the first place. It's included to provide a way to supply POSIX headers when Picolibc (and other C libraries) does not (either partially or fully) provide the header or required definitions (i.e. to have a consistent compilation environment). Of course, this is only necessary when POSIX is configured, due to work done over the last few years. I should take a step back - that's only true until some source file includes a POSIX header without configuring POSIX, which is historically extremely likely in the Zephyr project. So having shim files available unconditionally would be more consistent, just like they are unconditionally available from the C library (even if only partially complete as provided by the C library). Now, inconsistence aside, assuming that it is possible to fix the C library over time to be more conformant, then shim headers can be removed over time as well (as well as the path to include shim headers). If that assumption does not hold, which is the case for (at least Zephyr's SDK) newlib, minimal libc, IAR, ARCMWDT, most commercial toolchains, etc), then shim headers, and the include paths, should remain as long as they are required.
What side effects? There are none described in #99075. If "side effects" mean having functional code, that is, by definition, not a side effect, but an intentional and direct effect. The conditions for including the shim header path are by no means meant to be permanent. Personally, I would have not added them at all due to aforementioned concerns with inconsistence, had it not been for @nashif's suggestion, and @carlescufi's follow-up, squarely meant to satisfy you. So from that perspective, you seem to want the rest of the Zephyr community to suffer with additional technical debt beyond the release cycle due to your preference. So no, "we" do not want to do the same for the other libCs, and I still question the value in making the shim header path conditional for Picolibc as well. |
191fb79 to
60fb460
Compare
|
|
I created #99075 to facilitate discussing the more fundamental issues. As there seems to be a disagreement about how the POSIX_API should relate to the C library, its integration, and how optional or not it should be. |



Deprecation of
CONFIG_POSIX_APIDeprecate
CONFIG_POSIX_API.Users may choose one of the pre-defined POSIX subprofiles, for simplicity.
Libraries should depend on CONFIG_POSIX_SYSTEM_INTERFACES as well as other POSIX Option Groups, as needed.
Custom Configurations
Of course, instead of, or in addition to, using a pre-defined POSIX subprofile, users can still fully customize their POSIX configuration via Kconfig.
Zephyr's POSIX Kconfig option names correspond 1:1 with official POSIX Option Group names. Simply add a
CONFIG_at the beginning of an Option Group to get the corresponding Kconfig option.For more information on POSIX Subprofiling Option Groups, please see
https://pubs.opengroup.org/onlinepubs/9799919799/xrat/V4_subprofiles.html
Removal of
zephyr/posix/prefix for standard includesThe majority of the changes in this category (~156 files) are mechanical and are contained in the commit posix: remove zephyr/posix in header prefixes. They are approximately equivalent to running this command (with some adjustments).
Originally from #97152, but included here for simplicity, these changes mean that we no longer need to have
include/zephyr/posixin the library search path for C libraries that have (at least partially) conformant POSIX headers. In other words, we use the C library's POSIX headers.Originally added to mitigate C-library conflicts, the
<zephyr/posix/...h>prefix on standard includes has been a thorn in our sides for several years. Since these changes remove the conflicts, opting to use C-library headers (if they exist), we no longer need to namespace those in Zephyr.Depends on
Note
Compliance issues are false positives
Fixes #97949
Fixes #98661
SUMMARY
Summary:
@nordicjm
_POSIX_C_SOURCEand_XOPEN_SOURCEsince the last commit was not exactly trivial@aescolar
modules/liblc3/CMakeLists.txthas lots of cmake formatting issues, unrelated to this PR_GNU_SOURCEinstead of__GNU_SOURCE_GNU_SOURCEinstead of__GNU_SOURCEMBEDTLS_TEST_SW_INET_PTONasctime_r()testsGETENTROPY_MAXCONFIG_CONFIG_ARCH_POSIXas conditional in vlan.cCONFIG_NATIVE_LIBCinstead@nashif
qemu_leon3issuefork()toscripts/coccinelle/symbols.txtlib/libc/picolibc/CMakeLists.txt@jhedberg
tests/arch/arm/arm_user_stack_test@carlescufi