zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.88k stars 6.63k forks source link

POSIX kconfigs show up although POSIX APIs is not being used #75843

Open nashif opened 4 months ago

nashif commented 4 months ago

Describe the bug Building a hello world result in many POSIX kconfigs being enabled or showing in the resulting .config

To Reproduce Steps to reproduce the behavior:

  1. west build -b qemu_x86 samples/hello_world
  2. grep ^CONFIG_POSIX build/zephyr/.config CONFIG_POSIX_AEP_CHOICE_NONE=y CONFIG_POSIX_OPEN_MAX=4 CONFIG_POSIX_PAGE_SIZE=0x1000 CONFIG_POSIX_LIMITS_RTSIG_MAX=0 CONFIG_POSIX_MAX_FDS=4 CONFIG_POSIX_MAX_OPEN_FILES=4

Expected behavior

When a feature is disabled, it should not contaminate resulting .config with unrelated options. All Kconfigs of that feature shall be guarded

Impact

Confusion and potential leakge of features...

Logs and console output

grep ^CONFIG_POSIX build/zephyr/.config CONFIG_POSIX_AEP_CHOICE_NONE=y CONFIG_POSIX_OPEN_MAX=4 CONFIG_POSIX_PAGE_SIZE=0x1000 CONFIG_POSIX_LIMITS_RTSIG_MAX=0 CONFIG_POSIX_MAX_FDS=4 CONFIG_POSIX_MAX_OPEN_FILES=4

Environment (please complete the following information):

nashif commented 4 months ago

also note many deprecated options that do not start with CONFIG_POSIX...


#
# Deprecated POSIX options
#
CONFIG_EVENTFD_MAX=0
# CONFIG_FNMATCH is not set
# CONFIG_GETENTROPY is not set
# CONFIG_GETOPT is not set
CONFIG_MAX_PTHREAD_COUNT=0
CONFIG_MAX_PTHREAD_KEY_COUNT=0
CONFIG_MAX_TIMER_COUNT=0
CONFIG_MSG_COUNT_MAX=0
# CONFIG_POSIX_CLOCK is not set
# CONFIG_POSIX_CONFSTR is not set
# CONFIG_POSIX_ENV is not set
# CONFIG_POSIX_FS is not set
CONFIG_POSIX_LIMITS_RTSIG_MAX=0
CONFIG_POSIX_MAX_FDS=4
CONFIG_POSIX_MAX_OPEN_FILES=4
# CONFIG_POSIX_MQUEUE is not set
# CONFIG_POSIX_PUTMSG is not set
# CONFIG_POSIX_SIGNAL is not set
# CONFIG_POSIX_SYSCONF is not set
# CONFIG_POSIX_SYSLOG is not set
# CONFIG_POSIX_UNAME is not set
# CONFIG_PTHREAD is not set
# CONFIG_PTHREAD_BARRIER is not set
# CONFIG_PTHREAD_COND is not set
# CONFIG_PTHREAD_IPC is not set
# CONFIG_PTHREAD_KEY is not set
# CONFIG_PTHREAD_MUTEX is not set
# CONFIG_PTHREAD_RWLOCK is not set
# CONFIG_PTHREAD_SPINLOCK is not set
# CONFIG_TIMER is not set
CONFIG_TIMER_DELAYTIMER_MAX=0
CONFIG_SEM_NAMELEN_MAX=0
CONFIG_SEM_VALUE_MAX=0
# end of Deprecated POSIX options
# end of POSIX Options

#
# POSIX Shell Utilities
                                                                                                                               1039,1        77%
cfriedt commented 4 months ago

CONFIG_POSIX_AEP_CHOICE_NONE=y is expected (it's the default choice, saying not to enable POSIX).

It might be possible to move these

CONFIG_POSIX_OPEN_MAX=4
CONFIG_POSIX_PAGE_SIZE=0x1000

These are deprecated (not going to change until they are removed).

CONFIG_POSIX_LIMITS_RTSIG_MAX=0
CONFIG_POSIX_MAX_FDS=4
CONFIG_POSIX_MAX_OPEN_FILES=4
cfriedt commented 4 months ago

also note many deprecated options that do not start with CONFIG_POSIX...

... which is one reason why they are deprecated.

cfriedt commented 4 months ago

This is hardly a bug... deprecated Kconfig options following the regular deprecation process?

cfriedt commented 4 months ago

Also, FYI, CONFIG_POSIX_API will be deprecated after v3.7.0

nashif commented 4 months ago

CONFIG_POSIX_AEP_CHOICE_NONE=y is expected (it's the default choice, saying not to enable POSIX).

ok, so this is what is replacing CONFIG_POSIX_API?

It might be possible to move these

CONFIG_POSIX_OPEN_MAX=4
CONFIG_POSIX_PAGE_SIZE=0x1000

I do not know about moving and if that is needed, what we need is some guard somewhere or make them depend on something else and show up only when the parent option is enabled.

These are deprecated (not going to change until they are removed).

CONFIG_POSIX_LIMITS_RTSIG_MAX=0
CONFIG_POSIX_MAX_FDS=4
CONFIG_POSIX_MAX_OPEN_FILES=4

No need to change, but didnt those depend on POSIX_API before they were deprecated? Kconfig.deprecated could maybe depend on POSIX_API so they do not show up in each generated .config.

nashif commented 4 months ago

This is hardly a bug... deprecated Kconfig options following the regular deprecation process?

when kconfig options show up whether the feqture is enabled or not, it is a bug. Kconfigs of features need to be contained and guarded. Not following how the a kconfig option being deprecated changes anything.

cfriedt commented 4 months ago

CONFIG_POSIX_AEP_CHOICE_NONE=y is expected (it's the default choice, saying not to enable POSIX).

ok, so this is what is replacing CONFIG_POSIX_API?

CONFIG_POSIX_AEP_CHOICE*

and probably a new Kconfig option that adds <zephyr/posix/..> to the default search path.

No need to change, but didnt those depend on POSIX_API before they were deprecated? Kconfig.deprecated could maybe depend on POSIX_API so they do not show up in each generated .config.

Actually, many of them did not :-)

But, that might be a fair compromise. I'll make a PR.

cfriedt commented 4 months ago

Made some additional notes in the PR, but CONFIG_POSIX_API can't be used for gating anything (it was also not previously used for that). It was mainly used for default-y-selection of common options and for adjusting the default include search path.

github-actions[bot] commented 2 months ago

This issue 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 issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

github-actions[bot] commented 2 days ago

This issue 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 issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.