-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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: standardize kconfig options #73047
posix: standardize kconfig options #73047
Conversation
b0ed626
to
2da46dc
Compare
Just kicking CI again. It looks fine locally for me. |
c48f5af
to
071c1ce
Compare
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.
I think this is a much needed clean up, thanks for this, some more comments.
3939558
to
3e8cd31
Compare
432df23
to
c5c834a
Compare
@aescolar - your change requests have been made. Is there anything else? |
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.
Refresh +1
Friendly ping to @peter-mitsis @andyross @jukkar . Can I solicit more refresh +1s? |
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.
Love the standard names. Not a big fan of renaming churn in general (makes bisection across long history a high impedance process because you need to remember what the conventions were at each point), but if we're going to do it "all in a single PR" is for sure the best way.
The Zephyr 3.7 migration guide still has this:
should it be refactored so you don't need to recursively read the migration guide? |
#define PTHREAD_RWLOCK_INITIALIZER (-1) | ||
#define POSIX_RWLOCK_INITIALIZER (-1) |
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.
hey @cfriedt is this correct? I bumped into a library that was using PTHREAD_RWLOCK_INITIALIZER
(boringssl) and now does not build anymore, I see that /usr/include/pthread.h
on my system defines PTHREAD_RWLOCK_INITIALIZER
as well
If the change is intentional does this need a migration guide entry?
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.
@fabiobaltieri - good find. PTHREAD_RWLOCK_INITIALIZER
is the standard macro name. Are you good to submit a fix?
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.
yeah I can do that
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.
there you go #73808
Another issue I bumped into this: the sysconfig change somehow collides with a macro in the stm32 hals, try:
I think there's a macro collision for |
@fabiobaltieri - I believe this was fixed a while ago by Erwan. Did you |
Probably, yea. @SeppoTakalo are you good to submit a fix? |
~/zephyrproject/zephyr$ git fetch
~/zephyrproject/zephyr$ git checkout --detach main
HEAD is now at d3081e2f30f net: lwm2m: On write, use server selected block size
~/zephyrproject/zephyr$ west update
~/zephyrproject/zephyr$ west build -p -b nucleo_f410rb samples/posix/uname/ -DCONFIG_POSIX_SYSCONF_IMPL_FULL=y
<broken> Looks like it's still a problem, maybe manifest is out of date? |
Need to update the manifest with zephyrproject-rtos/hal_stm32#209 |
Ah, I do have that commit, but that just removes the path from the include path list, the header is still being included:
@erwango could you look into it? There's quite a few explicit |
Upstream migrated to a new set of posix options in zephyrproject-rtos/zephyr#73047. Change the fpmcu options to match the new ones, get rid of few deprecation messages: warning: Deprecated symbol GETENTROPY is enabled. warning: Deprecated symbol POSIX_CLOCK is enabled. warning: Deprecated symbol PTHREAD is enabled. warning: Deprecated symbol PTHREAD_IPC is enabled. warning: Deprecated symbol PTHREAD_KEY is enabled. warning: Deprecated symbol PTHREAD_MUTEX is enabled. BUG=none TEST=cq dry run Change-Id: I092bc50c37611023eff394478470e9a2af6e2059 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/5604463 Code-Coverage: Zoss <[email protected]> Tested-by: Fabio Baltieri <[email protected]> Commit-Queue: Fabio Baltieri <[email protected]> Reviewed-by: Patryk Duda <[email protected]>
With the update of the Kconfig symbols to new naming scheme, CONFIG_PTHREAD does not exist anymore. Use the new CONFIG_POSIX_THREADS instead Follow-up: zephyrproject-rtos#73047 Signed-off-by: Pierrick Guillaume <[email protected]>
With the update of the Kconfig symbols to new naming scheme, CONFIG_PTHREAD does not exist anymore. Use the new CONFIG_POSIX_THREADS instead Follow-up: #73047 Signed-off-by: Pierrick Guillaume <[email protected]>
With the update of the Kconfig symbols to new naming scheme, CONFIG_PTHREAD does not exist anymore. Use the new CONFIG_POSIX_THREADS instead Follow-up: zephyrproject-rtos#73047 Signed-off-by: Pierrick Guillaume <[email protected]>
This change (outlined as part of #51211) deprecates several POSIX Kconfig variables (linked below) in favour of their more normative replacements defined by IEEE 1003.1-2017.
Release Notes
Doc Preview
Out-of-tree users of the POSIX API should use the provided script to automatically update to the new Kconfig options.
$ python $ZEPHYR_BASE/scripts/utils/migrate_posix_kconfigs.py -r root_path
See also
Migration Guide
DEPRECATED
added to older symbols following policyFixes #66132