Skip to content
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

posix: add Kconfig and cmake for application conformance #67132

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

cfriedt
Copy link
Member

@cfriedt cfriedt commented Jan 3, 2024

TL;DR

This change reduces the manual definitions needed to build POSIX applications and libraries in Zephyr, in downstream projects, and integrated modules by using Kconfig to allow the application to more cleanly specify POSIX application conformance macros.

This is an extension of #67181 and has already been a part of the POSIX Roadmap for LTSv3 for some time (along with adopting implementation conformance macros).

Overview

IEEE 1003.1-2017 (and many versions preceding it) requires a conformant application to define _POSIX_C_SOURCE and / or _XOPEN_SOURCE to specific values. This is used to ensure that the correct function prototypes are made visible to the application from the implementation.

Currently, _POSIX_C_SOURCE=200809L is hard-coded for at least Newlib and Picolibc in order to elicit some desired prototypes and declarations. However, since POSIX is optional in Zephyr, those application conformance macros should really not be hard-coded anywhere.

We do want to avoid exposing non-POSIX code to POSIX APIs in order to avoid potential namespace collisions and so on. Additionally, the kernel is specifically required to not have any extensions to the C programming language other than what is listed in Rule A.4 of the coding guidelines. In practice, we know that it's safe, since we're currently building all of Zephyr (including the kernel and core libraries) with these extensions (and in fact the entire POSIX API) already exposed. However, we should really not do that in order to be consistent with our own policies.

User Interface

Below are some screenshots showing the menuconfig options.

POSIX API Support
Screenshot 2024-02-01 at 7 27 36 AM

Appplication Conformance
Screenshot 2024-02-01 at 7 28 07 AM

Selection for _POSIX_C_SOURCE
Screenshot 2024-02-01 at 7 27 52 AM

Selection for _XOPEN_SOURCE
Screenshot 2024-02-01 at 7 28 15 AM

Help
Screenshot 2024-02-01 at 7 28 24 AM

Risks (of not merging this PR)

There is a chance that all POSIX applications and libraries, even those downstream, will break once the hard-coded values are removed.

To make the transition easier, with this change, we provide

  • CONFIG_POSIX_C_SOURCE_CHOICE, and
  • CONFIG_XOPEN_SOURCE_CHOICE

That allows the user to specify the application conformance level in Zephyr's native configuration language, in a way that scales with Zephyr.

By default, the conformance level is CONFIG_POSIX_C_SOURCE_CHOICE_NONE (i.e. POSIX API should not be exposed to the application). The same is true for _XOPEN_SOURCE.

A CONFIG_POSIX_C_SOURCE_CHOICE_LATEST option is provided, that uses the recommended application conformance level (as required by the latest supported POSIX standard).

Individual standard application conformance levels are also provided, so that the user may choose, e.g. CONFIG_POSIX_C_SOURCE_CHOICE_200809L.

POSIX Libraries and Applications

Libraries and applications that even optionally support the POSIX API should use one of the following to be consistent with the application-specified conformance level.

# For application-facing libraries
zephyr_posix_library_compile_options([<minimum>])
zephyr_xopen_library_compile_options([<minimum>])
# For applications
target_posix_library_compile_options(<target> [<minimum>])
target_xopen_library_compile_options(<target> [<minimum>])

Each function supports optionally specifying a minimum conformance level. So if the user specifies 200112L and the library requires 200809L, the library will be compiled as 200809L. While most of POSIX is backward-compatible, it is generally better to avoid specifying a minimum conformance level since that could potentially introduce using mixed application conformance levels ("unspecified" behaviour according to IEEE 1003.1-2017). There should only be a handful of situations where that is done as a workaround.

If the user selects no conformance option, then these cmake functions will have no effect. There is no need to guard them with any conditions.

POSIX applications that define CONFIG_POSIX_API=y will already be using the latest application conformance level.

With that, Zephyr libraries that support the POSIX API (even conditionally) can define the required application conformance macros with a single CMake Line.

POSIX applications and libraries may opt-out of using these helpers entirely and manage local compile options. However, again, using an inconsistent application conformance level is "unspecified" (i.e. will probably work, but no promises).

Non-POSIX libraries that Need POSIX support

For libraries that are not application-facing, such as things built in arch/posix, there is absolutely no need to adopt the semantics used by application conformance macros, since those libraries are not considered part of the application (they are a platform implementation detail, insulated by the Zephyr API).

It is, however, recommended to not use global compile options, so as to avoid conflicting with the rest of the Zephyr build.

Source files may manually undefine and redefine _POSIX_C_SOURCE or _XOPEN_SOURCE explicitly as well (assuming CI compliance errors are ignored), but this option does not scale as easily.

Four cmake extensions are provided that can be used to set the conformance level via local compile options. These options do not affect global compile options.

Deprecation Process for Hard-coded _POSIX_C_SOURCE?

Because POSIX is optional in Zephyr, and because _POSIX_C_SOURCE is choice typically made by the application, there may need to be a deprecation process for removing instances where _POSIX_C_SOURCE is define globally. The main reason for that, is that if we were to suddenly remove it, many applications (even out-of-tree applications) would fail to build (in fact, we are already seeing this happen today).

For that, we could follow the usual deprecation process. A good suggestion would be to surround the hard-coded value in CMake with a new Kconfig option that is already deprecated. E.g.

diff --git a/lib/posix/Kconfig.app_conf b/lib/posix/Kconfig.app_conf
index 5f072fc7f7..9c2fcda1d7 100644
--- a/lib/posix/Kconfig.app_conf
+++ b/lib/posix/Kconfig.app_conf
@@ -2,6 +2,17 @@
 #
 # SPDX-License-Identifier: Apache-2.0
 
+config POSIX_AUTO_DECLARE_APP_CONFORMANCE
+	bool "Automatically declare APP Conformance"
+	select DEPRECATED
+      default y
+	help
+	  Due to _POSIX_C_SOURCE and perhaps other reserved macros being
+	  unconditionally defined within certain areas, and since POSIX
+	  support is optional within Zephyr, we must deprecate that
+	  behaviour in a graceful way, so as to not break downstream
+	  users.
+
 # The latest versions from IEEE 1003.1-2017
 config POSIX_C_SOURCE_LATEST_VERSION
 	int
@@ -15,6 +25,7 @@ config XOPEN_SOURCE_LATEST_VERSION
 choice POSIX_C_SOURCE_CHOICE
 	prompt "POSIX API application conformance"
 	default POSIX_C_SOURCE_CHOICE_DEFAULT if POSIX_API
+	default POSIX_C_SOURCE_CHOICE_DEFAULT if POSIX_AUTO_DECLARE_APP_CONFORMANCE
 	default POSIX_C_SOURCE_CHOICE_NONE
 	help
 	  POSIX API application conformance level.
@@ -69,6 +80,7 @@ endchoice
 choice XOPEN_SOURCE_CHOICE
 	prompt "POSIX X/Open application conformance"
 	default XOPEN_SOURCE_CHOICE_DEFAULT if POSIX_API
+	default XOPEN_SOURCE_CHOICE_DEFAULT if POSIX_AUTO_DECLARE_APP_CONFORMANCE
 	default XOPEN_SOURCE_CHOICE_NONE
 	help
 	  X/Open application conformance level.

With that, existing applications will not be required to immediately modify CMake or Kconfig. The _POSIX_C_SOURCE variable will still be globally defined (if CONFIG_POSIX_AUTO_DECLARE_APP_CONFORMANCE is not explicitly n-selected).

And then, subsequently, after 2 releases, the deprecated option would be removed and users could make the recommended changes.

@cfriedt cfriedt force-pushed the posix-api-application-conformance-level branch 6 times, most recently from 948ccf5 to eb380b8 Compare January 11, 2024 00:21
@cfriedt cfriedt requested review from ycsin and tejlmand January 11, 2024 00:22
@cfriedt cfriedt force-pushed the posix-api-application-conformance-level branch 3 times, most recently from 448ea33 to 4991b3d Compare January 11, 2024 01:04
@cfriedt cfriedt force-pushed the posix-api-application-conformance-level branch 2 times, most recently from 9ff68c7 to 8558059 Compare January 11, 2024 14:57
@cfriedt cfriedt changed the title posix: add Kconfig and cmake for application conformance [RFC] posix: add Kconfig and cmake for application conformance Jan 16, 2024
@cfriedt cfriedt added area: Build System RFC Request For Comments: want input from the community area: POSIX POSIX API Library Architecture Review Discussion in the Architecture WG required labels Jan 16, 2024
@cfriedt
Copy link
Member Author

cfriedt commented Jan 16, 2024

Not passing all tests yet, but will open up for review & inclusion in the arch meeting.

@cfriedt cfriedt marked this pull request as ready for review January 16, 2024 13:55
@zephyrbot zephyrbot added the area: Samples Samples label Jan 16, 2024
Copy link
Member

@aescolar aescolar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As already mentioned in other RFCs and PRs, I don't think this is the way to go.
Mainly: This is adding quite a lot of complexity, but not really improving things.
With or without this PR neither Zephyr components, library or app developers, should be setting these macros globally.

I would much rather clean up the use in tree either with the approach of #67040 or #67052, and leave it at that.


Other issues:

  • I don't see any need for applications to influence how a library which uses any of these APIs sets these macros and viceversa.
  • This PR does not solve what is described in the description>overview.

"POSIX applications that define CONFIG_POSIX_API=y will already be using the latest application conformance level."

  • I would not set these macros just because a user selected CONFIG_POSIX_API. I would still expect the user to set them.

Related:

cmake/modules/extensions.cmake Show resolved Hide resolved
@cfriedt cfriedt dismissed stale reviews from kgugala, ycsin, and jgl-meta via d628d9a March 26, 2024 11:50
@cfriedt cfriedt force-pushed the posix-api-application-conformance-level branch from 4735526 to d628d9a Compare March 26, 2024 11:51
cfriedt added 3 commits March 26, 2024 09:09
Summary:

This change reduces the manual definitions needed to build POSIX
applications and libraries in Zephyr, in downstream projects,
and integrated modules.

Details:

IEEE 1003.1 (many versions) require a conformant application to
define _POSIX_C_SOURCE to a specific value.

Similarly, a strictly conforming XSI application (which includes
additional features) is required to define _XOPEN_SOURCE to
a specific value.

Currently, _POSIX_C_SOURCE=200809L is hard-coded for at least
Newlib. However, since POSIX is optional in Zephyr, those
application conformance macros should really not be hard-coded.

There is a chance that all POSIX applications and libraries,
even those downstream, will break once the hard-coded values are
removed.

Provide a Kconfig choice that allows the user to specify the
application conformance level in a way that scales with Zephyr.

By default, the conformance level is NONE (i.e. POSIX API should
not be exposed to the application.

With this change:

* a user may select the DEFAULT conformance level (which uses
  the latest specified by IEEE 1003.1). A user may select any
  other specific version via Kconfig choice.

* 4 cmake extensions are provided that can be used to set the
  conformance level via local compile options. These options do
  not affect global compile options.

The CMakeLists.txt files for libraries that even optionally support
the POSIX API should use one of the following to be consistent with
the application-specified conformance level.

zephyr_posix_library_compile_options()
zephyr_xopen_library_compile_options()

Each function supports 1 argument - namely the minimum version.

zephyr_posix_library_compile_options(200809L)
zephyr_xopen_library_compile_options(700)

Applications can similarly take advantage of these features via

target_posix_library_compile_options(<target> [<minimum>])
target_xopen_library_compile_options(<target> [<minimum>])

With that, Zephyr libraries that support the POSIX API (even
conditionally) can define the required application conformance
macros with a single CMake Line. If NONE is selected for a library
or application that has optional use of the POSIX API, the same
cmake functions can be used unchanged, and their effect will be
to do nothing.

POSIX applications that define CONFIG_POSIX_API=y will already
be using the latest application conformance level. They may
optionally use

CONFIG_POSIX_C_SOURCE_CHOICE_200112L=y

to specify a different application conformance level.

Source files may manually undefine and redefine _POSIX_C_SOURCE
or _XOPEN_SOURCE explicitly as well, but this option does not
scale as easily.

POSIX applications and libraries may opt-out of using these
helpers as well and manage local compile options manually.

Signed-off-by: Chris Friedt <chrisfriedt@gmail.com>
Use the new application conformance helpers in the POSIX
testsuite.

Signed-off-by: Chris Friedt <chrisfriedt@gmail.com>
Use the new application conformance helpers for POSIX sample
applications.

Signed-off-by: Chris Friedt <chrisfriedt@gmail.com>
@cfriedt cfriedt force-pushed the posix-api-application-conformance-level branch from d628d9a to 4bc9020 Compare March 26, 2024 13:09
@yashi
Copy link
Collaborator

yashi commented Mar 26, 2024

I think I now understand what you are trying to say. You seem to be suggesting that:

"Even though it's not explicitly required by IEEE 1003.1-2017 and practically no operating system with a significant level of complexity (like those based on Linux) does this, it's still be a good idea to compile all compilation units with a single _POSIX_C_SOURCE value (reasons are stated above). This PR provides an option for users who want to do that."

If this is the case, I do not want any module or in-tree libraries to adopt target_posix_library_compile_options() because if they do, they will be compiled with untested or unexpected definitions when app developers select a certain conformance level for their application.

Do you have a plan to support all variations for modules and libraries that adopt target_posix_library_compile_options()?

@henrikbrixandersen
Copy link
Member

I think I now understand what you are trying to say. You seem to be suggesting that:

"Even though it's not explicitly required by IEEE 1003.1-2017 and practically no operating system with a significant level of complexity (like those based on Linux) does this, it's still be a good idea to compile all compilation units with a single _POSIX_C_SOURCE value (reasons are stated above). This PR provides an option for users who want to do that."

If this is the case, I do not want any module or in-tree libraries to adopt target_posix_library_compile_options() because if they do, they will be compiled with untested or unexpected definitions when app developers select a certain conformance level for their application.

This is exactly my concern with this PR.

@cfriedt
Copy link
Member Author

cfriedt commented Mar 26, 2024

I think I now understand what you are trying to say. You seem to be suggesting that:

compile all compilation units with a single _POSIX_C_SOURCE value (reasons are stated above). This PR provides an option for users who want to do that."

Not all. Again, only compilation units that opt-in, and those that are "above the line". For example, tests.

However, if anyone would still prefer to simply define the posix level in source, this is still supported.

Additionally, if a module or compilation unit should be compiled with a minimum value for either _POSIX_C_SOURCE or _XOPEN_SOURCE, they can do so with an argument - e.g. target_posix_library_compile_options(200809L).

they will be compiled with untested or unexpected definitions when app developers select a certain conformance level for their application.

Again, that is not the case. As mentioned above (and previously, several times), if a certain module or library requires a specific minimum conformance value, it can be added via a parameter to target_posix_library_compile_options(<parameter>).

There seems to be some misconception (also addressing @henrikbrixandersen) that this PR is attempting to add some kind of policy, which is very much not the case. I want to make that 100% clear. I would rather set up customers for success than making it extremely difficult to test a given conformance level.

Use it, don't use it. It's up to you. We are essentially adding a variable to make life easier for those who care about posix and supporting multiple conformance levels. Chances are, if you don't care about strict conformance, then this PR does not affect you in the least.

Do you have a plan to support all variations for modules and libraries that adopt target_posix_library_compile_options()?

That is already supported.

@yashi
Copy link
Collaborator

yashi commented Mar 27, 2024

compilation units that opt-in, and those that are "above the line". For example, tests.

I've been asking you many times about this, and failing. What do you include in the "above the line"?

The best I've got from you so far is

[#67132] is mainly targeting libraries that use the POSIX API - e.g. thrift, c11 threads, c++11 threads (soon). greybus-for-zephyr used Zephyr's POSIX API.

https://discord.com/channels/720317445772017664/1215177757839986758/1215689579160674405
(issue number typo fix is mine)

I would rather set up customers for success than making it extremely difficult to test a given conformance level.

ditto.

@cfriedt
Copy link
Member Author

cfriedt commented Mar 27, 2024

I've been asking you many times about this, and failing. What do you include in the "above the line"?

Above the kernel and OS services. In that area, consumers of the POSIX API. Same as last time I answered that question for you 22 days ago in this very PR.

@cfriedt cfriedt removed RFC Request For Comments: want input from the community Architecture Review Discussion in the Architecture WG required labels Mar 27, 2024
@cfriedt cfriedt changed the title [RFC] posix: add Kconfig and cmake for application conformance posix: add Kconfig and cmake for application conformance Mar 27, 2024
Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the Stale label Sep 26, 2024
@cfriedt cfriedt removed the Stale label Sep 27, 2024
Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the Stale label Nov 27, 2024
@cfriedt cfriedt removed the Stale label Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.