| Message ID | 20230817012007.131868-1-senozhatsky@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] kconfig: introduce listunknownconfig | expand |
On (23/08/17 10:19), Sergey Senozhatsky wrote: > The listunknownconfig option reads old .config and lists > all unrecognized symbols. This is especially useful for > continuous kernel uprevs when some symbols can be either > removed or renamed between kernel releases (which can go > unnoticed otherwise). > > A recent real-life example of such a symbol rename > that quietly disabled some drivers after kernel uprev > is MFD_RK808 rename. > > Example: > Suppose old .config has the following two options which > were removed from the recent kernel: > > $ cat .config > CONFIG_DISABLE_BUGS=y Uh, I see what happened there. The correct output is $ cat .config CONFIG_DISABLE_BUGS=y # CONFIG_ENABLE_WINAPI is not set
On Thu, Aug 17, 2023 at 5:30 PM Sergey Senozhatsky <senozhatsky@chromium.org> wrote: > > Example: > Suppose old .config has the following two options which > were removed from the recent kernel: > > $ cat .config > CONFIG_DISABLE_BUGS=y > > Running `make listunknownconfig` produces the following > list of unrecognized symbols: > > .config:6:warning: unknown symbol: DISABLE_BUGS > .config:7:warning: unknown unset symbol: ENABLE_WINAPI > > Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org> A new target is not what I like to see. We decided to add KCONFIG_VERBOSE, which will be used to warn options accidentally disabled or downgraded. https://lore.kernel.org/linux-kbuild/20230809002436.18079-1-sunying@nj.iscas.ac.cn/T/#u -- Best Regards Masahiro Yamada
On (23/08/20 08:19), Masahiro Yamada wrote: > > Example: > > Suppose old .config has the following two options which > > were removed from the recent kernel: > > > > $ cat .config > > CONFIG_DISABLE_BUGS=y > > > > Running `make listunknownconfig` produces the following > > list of unrecognized symbols: > > > > .config:6:warning: unknown symbol: DISABLE_BUGS > > .config:7:warning: unknown unset symbol: ENABLE_WINAPI > > > > Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org> > > > A new target is not what I like to see. > > > We decided to add KCONFIG_VERBOSE, which will be used to > warn options accidentally disabled or downgraded. That doesn't seem cover the cases that I'm concerned with. I don't see anything related to "!sym" in the patch. What will KCONFIG_VERBOSE do if it reads the following config file? // assuming that both config options were valid and existed in the old // kernel, but were removed/renamed in the new kernel $ cat .config CONFIG_DISABLE_BUGS=y # CONFIG_ENABLE_WINAPI is not set I'd like to see warnings for both lines, even for config that is not set, because it maybe we set by a build script depending on USE flags for instance, so that build target may still refer to non-existent config.
On (23/08/20 11:45), Sergey Senozhatsky wrote: > > > > A new target is not what I like to see. > > > > > > We decided to add KCONFIG_VERBOSE, which will be used to > > warn options accidentally disabled or downgraded. > > That doesn't seem cover the cases that I'm concerned with. I don't see > anything related to "!sym" in the patch. > > What will KCONFIG_VERBOSE do if it reads the following config file? > > // assuming that both config options were valid and existed in the old > // kernel, but were removed/renamed in the new kernel > > $ cat .config > CONFIG_DISABLE_BUGS=y > # CONFIG_ENABLE_WINAPI is not set > > > I'd like to see warnings for both lines, even for config that is not > set, because it maybe we set by a build script depending on USE flags > for instance, so that build target may still refer to non-existent > config. It's also important to exit with an error when non-existent config symbols are detected. Because that is an error. We are looking at a broken kernel from the end user PoV: the kernel will compile, get rolled out and at some point you'll start receiving bug reports of non-functioning peripheral hardware and whatnot (e.g. some specific USB devices whose config symbol has been renamed). So `make FOO-config` should never succeed if old config contains unrecognized symbols, it should never get to the compilation stage. That is not what KCONFIG_VERBOSE does (at least in its current form), as far as I can tell.
On Sun, Aug 20, 2023 at 1:20 PM Sergey Senozhatsky <senozhatsky@chromium.org> wrote: > > On (23/08/20 08:19), Masahiro Yamada wrote: > > > Example: > > > Suppose old .config has the following two options which > > > were removed from the recent kernel: > > > > > > $ cat .config > > > CONFIG_DISABLE_BUGS=y > > > > > > Running `make listunknownconfig` produces the following > > > list of unrecognized symbols: > > > > > > .config:6:warning: unknown symbol: DISABLE_BUGS > > > .config:7:warning: unknown unset symbol: ENABLE_WINAPI > > > > > > Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org> > > > > > > A new target is not what I like to see. > > > > > > We decided to add KCONFIG_VERBOSE, which will be used to > > warn options accidentally disabled or downgraded. > > That doesn't seem cover the cases that I'm concerned with. I don't see > anything related to "!sym" in the patch. > > What will KCONFIG_VERBOSE do if it reads the following config file? > > // assuming that both config options were valid and existed in the old > // kernel, but were removed/renamed in the new kernel > > $ cat .config > CONFIG_DISABLE_BUGS=y > # CONFIG_ENABLE_WINAPI is not set > > > I'd like to see warnings for both lines, even for config that is not > set, because it maybe we set by a build script depending on USE flags > for instance, so that build target may still refer to non-existent > config. I did not say Ying Sun's patch covered your case. I just meant I dislike your approach. After his patch is applied, please come back with a similar approach if you want to address your case in the mainline kernel.
On (23/08/20 14:11), Masahiro Yamada wrote: > > That doesn't seem cover the cases that I'm concerned with. I don't see > > anything related to "!sym" in the patch. > > > > What will KCONFIG_VERBOSE do if it reads the following config file? > > > > // assuming that both config options were valid and existed in the old > > // kernel, but were removed/renamed in the new kernel > > > > $ cat .config > > CONFIG_DISABLE_BUGS=y > > # CONFIG_ENABLE_WINAPI is not set > > > > > > I'd like to see warnings for both lines, even for config that is not > > set, because it maybe we set by a build script depending on USE flags > > for instance, so that build target may still refer to non-existent > > config. > > > I did not say Ying Sun's patch covered your case. > > I just meant I dislike your approach. Sure, OK. > After his patch is applied, please come back with a similar approach I guess Ying Sun's patch cannot be extended to cover these extra cases? What the preferred approach would be? Do we want a new KCONFIG_FOO env variable that changes behaviour of one of the targets? E.g. KCONFIG_LIST_MISSING=1 make oldconfig and then have conf list symbols and terminate with exit(1) if there are some unrecognized symbols? We have "listnew", so "listmissing" sort of fits as a new target. > if you want to address your case in the mainline kernel Yes please, we want an upstream solution for the problem in question.
On (23/08/20 16:21), Sergey Senozhatsky wrote: > What the preferred approach would be? Do we want a new KCONFIG_FOO env > variable that changes behaviour of one of the targets? E.g. > > KCONFIG_LIST_MISSING=1 make oldconfig > > and then have conf list symbols and terminate with exit(1) if there are > some unrecognized symbols? Will something like this be OK with you? KCONFIG_LIST_MISSING=1 make oldconfig .config:6:warning: unknown symbol: DISABLE_BUGS .config:7:warning: unknown unset symbol: ENABLE_WINAPI make[2]: *** [scripts/kconfig/Makefile:77: oldconfig] Error 1 --- diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c index fa2ae6f63352..b2c0bcf0e5c1 100644 --- a/scripts/kconfig/confdata.c +++ b/scripts/kconfig/confdata.c @@ -360,7 +360,9 @@ int conf_read_simple(const char *name, int def) char *p, *p2; struct symbol *sym; int i, def_flags; + const char *list_missing; + list_missing = getenv("KCONFIG_LIST_MISSING"); if (name) { in = zconf_fopen(name); } else { @@ -448,6 +450,12 @@ int conf_read_simple(const char *name, int def) if (def == S_DEF_USER) { sym = sym_find(line + 2 + strlen(CONFIG_)); if (!sym) { + if (list_missing) { + conf_warning("unknown unset symbol: %s", + line + 2 + strlen(CONFIG_)); + continue; + } + conf_set_changed(true); continue; } @@ -482,6 +490,12 @@ int conf_read_simple(const char *name, int def) sym = sym_find(line + strlen(CONFIG_)); if (!sym) { + if (list_missing) { + conf_warning("unknown symbol: %s", + line + strlen(CONFIG_)); + continue; + } + if (def == S_DEF_AUTO) /* * Reading from include/config/auto.conf @@ -530,6 +544,13 @@ int conf_read_simple(const char *name, int def) } free(line); fclose(in); + + if (list_missing) { + if (conf_warnings) + exit(1); + exit(0); + } + return 0; }
On Sun, Aug 20, 2023 at 5:15 PM Sergey Senozhatsky <senozhatsky@chromium.org> wrote: > > On (23/08/20 16:21), Sergey Senozhatsky wrote: > > What the preferred approach would be? Do we want a new KCONFIG_FOO env > > variable that changes behaviour of one of the targets? E.g. > > > > KCONFIG_LIST_MISSING=1 make oldconfig > > > > and then have conf list symbols and terminate with exit(1) if there are > > some unrecognized symbols? > > > Will something like this be OK with you? > > > KCONFIG_LIST_MISSING=1 make oldconfig > > .config:6:warning: unknown symbol: DISABLE_BUGS > .config:7:warning: unknown unset symbol: ENABLE_WINAPI > > make[2]: *** [scripts/kconfig/Makefile:77: oldconfig] Error 1 > Yup, much better than adding a new target. > --- > > diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c > index fa2ae6f63352..b2c0bcf0e5c1 100644 > --- a/scripts/kconfig/confdata.c > +++ b/scripts/kconfig/confdata.c > @@ -360,7 +360,9 @@ int conf_read_simple(const char *name, int def) > char *p, *p2; > struct symbol *sym; > int i, def_flags; > + const char *list_missing; > > + list_missing = getenv("KCONFIG_LIST_MISSING"); My (original) hope was to add a single switch, KCONFIG_VERBOSE, to address both: - A CONFIG option is hidden by unmet dependency (Ying Sun's case) - A CONFIG option no longer exists (your case) - Anything else we need to be careful > if (name) { > in = zconf_fopen(name); > } else { > @@ -448,6 +450,12 @@ int conf_read_simple(const char *name, int def) > if (def == S_DEF_USER) { > sym = sym_find(line + 2 + strlen(CONFIG_)); > if (!sym) { > + if (list_missing) { > + conf_warning("unknown unset symbol: %s", > + line + 2 + strlen(CONFIG_)); > + continue; > + } > + > conf_set_changed(true); > continue; > } > @@ -482,6 +490,12 @@ int conf_read_simple(const char *name, int def) > > sym = sym_find(line + strlen(CONFIG_)); > if (!sym) { > + if (list_missing) { > + conf_warning("unknown symbol: %s", > + line + strlen(CONFIG_)); > + continue; > + } > + This should be warned only if (def != S_DEF_AUTO), otherwise the same warning will be displayed twice. > if (def == S_DEF_AUTO) > /* > * Reading from include/config/auto.conf > @@ -530,6 +544,13 @@ int conf_read_simple(const char *name, int def) > } > free(line); > fclose(in); > + > + if (list_missing) { > + if (conf_warnings) > + exit(1); > + exit(0); > + } > + This is something different because you are making these errors instead of warnings. > return 0; > }
On (23/08/21 21:27), Masahiro Yamada wrote: > > + const char *list_missing; > > > > + list_missing = getenv("KCONFIG_LIST_MISSING"); > > > My (original) hope was to add a single switch, KCONFIG_VERBOSE, to address both: > > - A CONFIG option is hidden by unmet dependency (Ying Sun's case) > - A CONFIG option no longer exists (your case) > - Anything else we need to be careful So I see a "no longer existing option" as a terminal condition. In general there is no point in continuing the build because the build will not include some driver/functionality that is still expected to be there. (This has actually happened to us). [..] > > @@ -482,6 +490,12 @@ int conf_read_simple(const char *name, int def) > > > > sym = sym_find(line + strlen(CONFIG_)); > > if (!sym) { > > + if (list_missing) { > > + conf_warning("unknown symbol: %s", > > + line + strlen(CONFIG_)); > > + continue; > > + } > > + > > > This should be warned only if (def != S_DEF_AUTO), > otherwise the same warning will be displayed twice. Good point. > > @@ -530,6 +544,13 @@ int conf_read_simple(const char *name, int def) > > } > > free(line); > > fclose(in); > > + > > + if (list_missing) { > > + if (conf_warnings) > > + exit(1); > > + exit(0); > > + } > > + > > This is something different because you are making these > errors instead of warnings. Right. So "verbose" and "list missing" probably have slightly different requirements. When "verbose" complaints about a downgraded option, for instance, then you can regenerate the config and continue, because the symbol is still there. When "list missing" complaints about an option then we should stop and investigate. That option, for example, can be added by build infra based on some USE flags switch, etc. So there can be multiple parties that needs to be fixed. Apart from that we need to figure out what option (if any) replaces the gone symbol. "list missing" is basically "missed expectations", we need to do some work before we can build the kernel. I can "list missing" under KCONFIG_VERBOSE. It probably still better error exit if missing symbols are found. Thoughts?
On (23/08/21 21:27), Masahiro Yamada wrote: > > My (original) hope was to add a single switch, KCONFIG_VERBOSE, to address both: > > - A CONFIG option is hidden by unmet dependency (Ying Sun's case) > - A CONFIG option no longer exists (your case) > - Anything else we need to be careful A quick question: is it too late to suggest an alternative name? Could KCONFIG_SANITY_CHECKS be a little cleaner? Because we basically run sanity checks on the config. And one more question: those sanity checks seem very reasonable. Is there any reason we would not want to keep them ON by default? And those brave souls, that do not wish for the tool to very that the .config is sane and nothing will get downgraded/disabled, can always set KCONFIG_SANITY_CHECKS to 0.
On Tue, Aug 22, 2023 at 4:30 PM Sergey Senozhatsky <senozhatsky@chromium.org> wrote: > > On (23/08/21 21:27), Masahiro Yamada wrote: > > > > My (original) hope was to add a single switch, KCONFIG_VERBOSE, to address both: > > > > - A CONFIG option is hidden by unmet dependency (Ying Sun's case) > > - A CONFIG option no longer exists (your case) > > - Anything else we need to be careful > > A quick question: is it too late to suggest an alternative name? > Could KCONFIG_SANITY_CHECKS be a little cleaner? Because we basically > run sanity checks on the config. Ying's is not applied yet. So, it is not too late. But, I started to be a little worried because it is unpredictable how many KCONFIG_* env variables will increase until people are satisfied. > > And one more question: those sanity checks seem very reasonable. > Is there any reason we would not want to keep them ON by default? > And those brave souls, that do not wish for the tool to very that > the .config is sane and nothing will get downgraded/disabled, can > always set KCONFIG_SANITY_CHECKS to 0. Kconfig is meant to resolve the dependency without causing an error. If a feature is not available, it is automatically, silently hidden, and that works well. When a compiler does not support a particular feature, 'depends on $(cc-option )' hides that CONFIG option. Kconfig is meant to work like that. For your case, it is case-by-case. Let's say a stale code is removed from upstream. After "obj-$(CONFIG_FOO) += foo.o" is removed from upstream, CONFIG_FOO in the .config is a "don't care". If it were an error, all arch/*/configs/*_defconfig must be cleaned up at the same time. So, sometimes it is helpful, but sometimes noisy. For the MFD_RK808 case particularly, I believe Kconfig showed MFD_RK8XX_I2C as a new option. Or, when you bumped to a new kernel version, you could run 'make listnewconfig'. (See 17baab68d337a0bf4654091e2b4cd67c3fdb44d8. Redhat says they review every new config option.) If you had done a per-config review you would have noticed c20e8c5b1203af3726561ee5649b147194e0618e before spending time on run-time debugging.
On (23/08/24 10:00), Masahiro Yamada wrote: > For the MFD_RK808 case particularly, > I believe Kconfig showed MFD_RK8XX_I2C > as a new option. I think there were some other unmet dependencies for MFD_RK8XX_I2C and I don't think the new config was shown. But even if it was, we still would have no idea that this meant "MFD_RK808 is not available anymore and the corresponding code won't get compiled". So the "this is not recognized anymore" is still needed and is quite helpful. Would you be OK with "list missing" being a warning (not a terminal condition)?
Hi Masahiro, On Thu, Aug 24, 2023 at 10:00 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Tue, Aug 22, 2023 at 4:30 PM Sergey Senozhatsky > <senozhatsky@chromium.org> wrote: > > > > On (23/08/21 21:27), Masahiro Yamada wrote: > > > > > > My (original) hope was to add a single switch, KCONFIG_VERBOSE, to address both: > > > > > > - A CONFIG option is hidden by unmet dependency (Ying Sun's case) > > > - A CONFIG option no longer exists (your case) > > > - Anything else we need to be careful > > > > A quick question: is it too late to suggest an alternative name? > > Could KCONFIG_SANITY_CHECKS be a little cleaner? Because we basically > > run sanity checks on the config. > > > Ying's is not applied yet. So, it is not too late. > > But, I started to be a little worried > because it is unpredictable how many KCONFIG_* env > variables will increase until people are satisfied. > Is there really a problem with having those? There are a lot of different env variables affecting different parts of the kernel build. If they are useful, and even better, allow catching issues quickly, should we favor less options or usefulness for users? > > > > And one more question: those sanity checks seem very reasonable. > > Is there any reason we would not want to keep them ON by default? > > And those brave souls, that do not wish for the tool to very that > > the .config is sane and nothing will get downgraded/disabled, can > > always set KCONFIG_SANITY_CHECKS to 0. > > > Kconfig is meant to resolve the dependency without causing an error. > If a feature is not available, it is automatically, silently hidden, > and that works well. How do you come to the conclusion that it works well? I've heard many people unhappy about the way Kconfig works. How does one know that something is missing (and should maybe be fixed?) if Kconfig silently hides it? > > When a compiler does not support a particular feature, > 'depends on $(cc-option )' hides that CONFIG option. > Kconfig is meant to work like that. > > > > For your case, it is case-by-case. > > Let's say a stale code is removed from upstream. > > After "obj-$(CONFIG_FOO) += foo.o" is removed > from upstream, CONFIG_FOO in the .config is a "don't care". > > If it were an error, all arch/*/configs/*_defconfig > must be cleaned up at the same time. > I'd argue that clean up should actually happen. An identically named option could be added in the future again and mean something different and would end up accidentally selected by those old defconfigs. > > So, sometimes it is helpful, but sometimes noisy. > > > > > For the MFD_RK808 case particularly, > I believe Kconfig showed MFD_RK8XX_I2C > as a new option. Among tens or hundreds of other new options. Good luck making sure that you didn't miss it. > > Or, when you bumped to a new kernel version, > you could run 'make listnewconfig'. > (See 17baab68d337a0bf4654091e2b4cd67c3fdb44d8. > Redhat says they review every new config option.) > What is the listnewconfig supposed to show? Regardless of that, shouldn't we strive to automate things rather than just put people at those and wasting the time they could spend on something more productive? > > If you had done a per-config review > you would have noticed > c20e8c5b1203af3726561ee5649b147194e0618e > before spending time on run-time debugging. > Instead, I'd have spent time on researching every single new Kconfig option just to realize that I don't care about it, except the single one that I needed. In fact, it wouldn't have even guaranteed tracking down the problem, because I don't necessarily have to know all the config options that are necessary for my board - how do you associate some MFD driver Kconfig option with an SD/MMC controller not working? Best regards, Tomasz > > > > -- > Best Regards > Masahiro Yamada
On Thu, Aug 24, 2023 at 2:30 PM Tomasz Figa <tfiga@chromium.org> wrote: > > Hi Masahiro, > > On Thu, Aug 24, 2023 at 10:00 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > On Tue, Aug 22, 2023 at 4:30 PM Sergey Senozhatsky > > <senozhatsky@chromium.org> wrote: > > > > > > On (23/08/21 21:27), Masahiro Yamada wrote: > > > > > > > > My (original) hope was to add a single switch, KCONFIG_VERBOSE, to address both: > > > > > > > > - A CONFIG option is hidden by unmet dependency (Ying Sun's case) > > > > - A CONFIG option no longer exists (your case) > > > > - Anything else we need to be careful > > > > > > A quick question: is it too late to suggest an alternative name? > > > Could KCONFIG_SANITY_CHECKS be a little cleaner? Because we basically > > > run sanity checks on the config. > > > > > > Ying's is not applied yet. So, it is not too late. > > > > But, I started to be a little worried > > because it is unpredictable how many KCONFIG_* env > > variables will increase until people are satisfied. > > > > Is there really a problem with having those? There are a lot of > different env variables affecting different parts of the kernel build. > If they are useful, and even better, allow catching issues quickly, > should we favor less options or usefulness for users? I am considering how to implement it. One way is to add env variables as a new request arises. Sergey is doing two things by one option. KCONFIG_WARN_UNKNWON_SYMBOL : warn unknown symbol in input .config or defconfig KCONFIG_WARN_TO_ERROR : turn warnings into errors Another way is to handle those as command line options. -Wunknown-symbol -Werror (associated with W=e) -Wall (associated with W=1) $ make W=1e olddefconfig will work to sanity check. > > > > > > And one more question: those sanity checks seem very reasonable. > > > Is there any reason we would not want to keep them ON by default? > > > And those brave souls, that do not wish for the tool to very that > > > the .config is sane and nothing will get downgraded/disabled, can > > > always set KCONFIG_SANITY_CHECKS to 0. > > > > > > Kconfig is meant to resolve the dependency without causing an error. > > If a feature is not available, it is automatically, silently hidden, > > and that works well. > > How do you come to the conclusion that it works well? I've heard many > people unhappy about the way Kconfig works. How does one know that > something is missing (and should maybe be fixed?) if Kconfig silently > hides it? Kconfig has worked like that for a long time, but I do not know how to detect non-existing symbols. > > > > > When a compiler does not support a particular feature, > > 'depends on $(cc-option )' hides that CONFIG option. > > Kconfig is meant to work like that. > > > > > > > > For your case, it is case-by-case. > > > > Let's say a stale code is removed from upstream. > > > > After "obj-$(CONFIG_FOO) += foo.o" is removed > > from upstream, CONFIG_FOO in the .config is a "don't care". > > > > If it were an error, all arch/*/configs/*_defconfig > > must be cleaned up at the same time. > > > > I'd argue that clean up should actually happen. An identically named > option could be added in the future again and mean something different > and would end up accidentally selected by those old defconfigs. For renaming, I agree with you. All defconfig files should be updated to keep the equivalent behavior. For code removal, defconfig cleaning can be deferred because that would possibly cause conflicts across subsystems. Reusing the same CONFIG name for different meaning must be sorted out properly but that rarely happens, I guess. > > > > So, sometimes it is helpful, but sometimes noisy. > > > > > > > > > > For the MFD_RK808 case particularly, > > I believe Kconfig showed MFD_RK8XX_I2C > > as a new option. > > Among tens or hundreds of other new options. Good luck making sure > that you didn't miss it. > > > > > Or, when you bumped to a new kernel version, > > you could run 'make listnewconfig'. > > (See 17baab68d337a0bf4654091e2b4cd67c3fdb44d8. > > Redhat says they review every new config option.) > > > > What is the listnewconfig supposed to show? Documented in Documentation/kbuild/kconfig.rst line 16 - 34. > > Regardless of that, shouldn't we strive to automate things rather than > just put people at those and wasting the time they could spend on > something more productive? > > > > > If you had done a per-config review > > you would have noticed > > c20e8c5b1203af3726561ee5649b147194e0618e > > before spending time on run-time debugging. > > > > Instead, I'd have spent time on researching every single new Kconfig > option just to realize that I don't care about it, except the single > one that I needed. In fact, it wouldn't have even guaranteed tracking > down the problem, because I don't necessarily have to know all the > config options that are necessary for my board - how do you associate > some MFD driver Kconfig option with an SD/MMC controller not working? -- Best Regards Masahiro Yamada
On Thu, Aug 24, 2023 at 7:50 PM Sergey Senozhatsky <senozhatsky@chromium.org> wrote: > > On (23/08/24 10:00), Masahiro Yamada wrote: > > > For the MFD_RK808 case particularly, > > I believe Kconfig showed MFD_RK8XX_I2C > > as a new option. > > I think there were some other unmet dependencies for MFD_RK8XX_I2C and > I don't think the new config was shown. But even if it was, we still > would have no idea that this meant "MFD_RK808 is not available anymore > and the corresponding code won't get compiled". So the "this is not > recognized anymore" is still needed and is quite helpful. > > Would you be OK with "list missing" being a warning (not a terminal > condition)? I am fine with implementing both. But, I'd like to implement them as separate options. (one option for warn unknown symbols, another for for turning warnings into errors) As I replied to Tomasz, I am considering about env variables vs command line options associated with W= option. -- Best Regards Masahiro Yamada
On (23/08/26 10:10), Masahiro Yamada wrote: > > I am considering how to implement it. > > One way is to add env variables as a new request arises. > > Sergey is doing two things by one option. > > KCONFIG_WARN_UNKNWON_SYMBOL : warn unknown symbol in input .config > or defconfig > KCONFIG_WARN_TO_ERROR : turn warnings into errors > > Another way is to handle those as command line options. > > -Wunknown-symbol > -Werror (associated with W=e) > -Wall (associated with W=1) > > $ make W=1e olddefconfig > > will work to sanity check. Sounds good. Being able to choose whether those sanity checks are warnings or errors is quite handful. I don't have preferences as to implementation. Env variables seem to have very clear and descriptive names. Command line options look fine too. I'd probably prefer command line args.
On Sat, Aug 26, 2023 at 10:12 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Thu, Aug 24, 2023 at 7:50 PM Sergey Senozhatsky > <senozhatsky@chromium.org> wrote: > > > > On (23/08/24 10:00), Masahiro Yamada wrote: > > > > > For the MFD_RK808 case particularly, > > > I believe Kconfig showed MFD_RK8XX_I2C > > > as a new option. > > > > I think there were some other unmet dependencies for MFD_RK8XX_I2C and > > I don't think the new config was shown. But even if it was, we still > > would have no idea that this meant "MFD_RK808 is not available anymore > > and the corresponding code won't get compiled". So the "this is not > > recognized anymore" is still needed and is quite helpful. > > > > Would you be OK with "list missing" being a warning (not a terminal > > condition)? > > > I am fine with implementing both. > > But, I'd like to implement them as separate options. > (one option for warn unknown symbols, > another for for turning warnings into errors) > > > > As I replied to Tomasz, I am considering about > env variables vs command line options associated with W= option. With a little more thought, the command line option approach would require more code changes and efforts. So, I am OK with adding new env variables. Could you add two env variables? I think the first two hunks (show warnings for symbols not found in Kconfig) -> KCONFIG_WARN_UNKNOWN_SYMBOLS the last hunk (turn warnings into errors) -> KCONFIG_WERROR (You can suggest a better naming if you have, but I guess KCONFIG_WARN_* will be consistent in case more warning requests come up.) -- Best Regards Masahiro Yamada
On (23/08/26 14:38), Masahiro Yamada wrote: > > I am fine with implementing both. > > > > But, I'd like to implement them as separate options. > > (one option for warn unknown symbols, > > another for for turning warnings into errors) > > > > > > > > As I replied to Tomasz, I am considering about > > env variables vs command line options associated with W= option. > > > > With a little more thought, the command line option approach > would require more code changes and efforts. > Sounds good. > So, I am OK with adding new env variables. > Could you add two env variables? Absolutely. > I think > > the first two hunks (show warnings for symbols not found in Kconfig) > -> KCONFIG_WARN_UNKNOWN_SYMBOLS > > the last hunk (turn warnings into errors) > -> KCONFIG_WERROR > > (You can suggest a better naming if you have, but I guess > KCONFIG_WARN_* will be consistent in case > more warning requests come up.) Looks good. I'll send a patch in the coming days.
On Sat, Aug 26, 2023 at 10:11 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Thu, Aug 24, 2023 at 2:30 PM Tomasz Figa <tfiga@chromium.org> wrote: > > > > Hi Masahiro, > > > > On Thu, Aug 24, 2023 at 10:00 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > On Tue, Aug 22, 2023 at 4:30 PM Sergey Senozhatsky > > > <senozhatsky@chromium.org> wrote: > > > > > > > > On (23/08/21 21:27), Masahiro Yamada wrote: > > > > > > > > > > My (original) hope was to add a single switch, KCONFIG_VERBOSE, to address both: > > > > > > > > > > - A CONFIG option is hidden by unmet dependency (Ying Sun's case) > > > > > - A CONFIG option no longer exists (your case) > > > > > - Anything else we need to be careful > > > > > > > > A quick question: is it too late to suggest an alternative name? > > > > Could KCONFIG_SANITY_CHECKS be a little cleaner? Because we basically > > > > run sanity checks on the config. > > > > > > > > > Ying's is not applied yet. So, it is not too late. > > > > > > But, I started to be a little worried > > > because it is unpredictable how many KCONFIG_* env > > > variables will increase until people are satisfied. > > > > > > > Is there really a problem with having those? There are a lot of > > different env variables affecting different parts of the kernel build. > > If they are useful, and even better, allow catching issues quickly, > > should we favor less options or usefulness for users? > > > > I am considering how to implement it. > > > > One way is to add env variables as a new request arises. > > Sergey is doing two things by one option. > > > KCONFIG_WARN_UNKNWON_SYMBOL : warn unknown symbol in input .config > or defconfig > KCONFIG_WARN_TO_ERROR : turn warnings into errors > > > > Another way is to handle those as command line options. > > -Wunknown-symbol > -Werror (associated with W=e) > -Wall (associated with W=1) > > > > $ make W=1e olddefconfig > > > will work to sanity check. > > I see, I think I misunderstood your previous message, sorry. Agreed that there could be other approaches than an environment variable and a command line option could definitely work as well. I'll leave the details to you and Sergey, but ideally we would have something that is simple to use both in scripts (e.g. distro build systems) and in manual build for end users > > > > > > > > > > > And one more question: those sanity checks seem very reasonable. > > > > Is there any reason we would not want to keep them ON by default? > > > > And those brave souls, that do not wish for the tool to very that > > > > the .config is sane and nothing will get downgraded/disabled, can > > > > always set KCONFIG_SANITY_CHECKS to 0. > > > > > > > > > Kconfig is meant to resolve the dependency without causing an error. > > > If a feature is not available, it is automatically, silently hidden, > > > and that works well. > > > > How do you come to the conclusion that it works well? I've heard many > > people unhappy about the way Kconfig works. How does one know that > > something is missing (and should maybe be fixed?) if Kconfig silently > > hides it? > > > Kconfig has worked like that for a long time, but I do not know > how to detect non-existing symbols. > > I think a tool to detect symbols present in old config, but missing in new kernel solves the "upgraded config" part of the problem. The other part ("new config") would probably be solved by some kind of a tool that looks at the currently present hardware and spews a list of Kconfig options together with their dependencies, but arguably that's not something that would be a part of Kconfig itself. For the graphical configuration tools like menuconfig I could imagine that the options with unmet dependencies could be still displayed but greyed out, so at least one can open the help for the item and check which dependencies are missing. > > > > > > > > > > When a compiler does not support a particular feature, > > > 'depends on $(cc-option )' hides that CONFIG option. > > > Kconfig is meant to work like that. > > > > > > > > > > > > For your case, it is case-by-case. > > > > > > Let's say a stale code is removed from upstream. > > > > > > After "obj-$(CONFIG_FOO) += foo.o" is removed > > > from upstream, CONFIG_FOO in the .config is a "don't care". > > > > > > If it were an error, all arch/*/configs/*_defconfig > > > must be cleaned up at the same time. > > > > > > > I'd argue that clean up should actually happen. An identically named > > option could be added in the future again and mean something different > > and would end up accidentally selected by those old defconfigs. > > > For renaming, I agree with you. > All defconfig files should be updated to keep the equivalent behavior. > > For code removal, defconfig cleaning can be deferred because > that would possibly cause conflicts across subsystems. > True. In that case it sounds like the new behavior definitely needs to be optional. > Reusing the same CONFIG name for different meaning must be > sorted out properly but that rarely happens, I guess. > Sure, I have to admit that it was a bit of an imaginary case. :) > > > > > > > > So, sometimes it is helpful, but sometimes noisy. > > > > > > > > > > > > > > > For the MFD_RK808 case particularly, > > > I believe Kconfig showed MFD_RK8XX_I2C > > > as a new option. > > > > Among tens or hundreds of other new options. Good luck making sure > > that you didn't miss it. > > > > > > > > Or, when you bumped to a new kernel version, > > > you could run 'make listnewconfig'. > > > (See 17baab68d337a0bf4654091e2b4cd67c3fdb44d8. > > > Redhat says they review every new config option.) > > > > > > > What is the listnewconfig supposed to show? > > > Documented in Documentation/kbuild/kconfig.rst > line 16 - 34. > I see, thanks. Best regards, Tomasz
On Thu, Aug 31, 2023 at 11:30 AM Tomasz Figa <tfiga@chromium.org> wrote: > > On Sat, Aug 26, 2023 at 10:11 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > On Thu, Aug 24, 2023 at 2:30 PM Tomasz Figa <tfiga@chromium.org> wrote: > > > > > > Hi Masahiro, > > > > > > On Thu, Aug 24, 2023 at 10:00 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > On Tue, Aug 22, 2023 at 4:30 PM Sergey Senozhatsky > > > > <senozhatsky@chromium.org> wrote: > > > > > > > > > > On (23/08/21 21:27), Masahiro Yamada wrote: > > > > > > > > > > > > My (original) hope was to add a single switch, KCONFIG_VERBOSE, to address both: > > > > > > > > > > > > - A CONFIG option is hidden by unmet dependency (Ying Sun's case) > > > > > > - A CONFIG option no longer exists (your case) > > > > > > - Anything else we need to be careful > > > > > > > > > > A quick question: is it too late to suggest an alternative name? > > > > > Could KCONFIG_SANITY_CHECKS be a little cleaner? Because we basically > > > > > run sanity checks on the config. > > > > > > > > > > > > Ying's is not applied yet. So, it is not too late. > > > > > > > > But, I started to be a little worried > > > > because it is unpredictable how many KCONFIG_* env > > > > variables will increase until people are satisfied. > > > > > > > > > > Is there really a problem with having those? There are a lot of > > > different env variables affecting different parts of the kernel build. > > > If they are useful, and even better, allow catching issues quickly, > > > should we favor less options or usefulness for users? > > > > > > > > I am considering how to implement it. > > > > > > > > One way is to add env variables as a new request arises. > > > > Sergey is doing two things by one option. > > > > > > KCONFIG_WARN_UNKNWON_SYMBOL : warn unknown symbol in input .config > > or defconfig > > KCONFIG_WARN_TO_ERROR : turn warnings into errors > > > > > > > > Another way is to handle those as command line options. > > > > -Wunknown-symbol > > -Werror (associated with W=e) > > -Wall (associated with W=1) > > > > > > > > $ make W=1e olddefconfig > > > > > > will work to sanity check. > > > > > > I see, I think I misunderstood your previous message, sorry. Agreed > that there could be other approaches than an environment variable and > a command line option could definitely work as well. I'll leave the > details to you and Sergey, but ideally we would have something that is > simple to use both in scripts (e.g. distro build systems) and in > manual build for end users > > > > > > > > > > > > > > > > > And one more question: those sanity checks seem very reasonable. > > > > > Is there any reason we would not want to keep them ON by default? > > > > > And those brave souls, that do not wish for the tool to very that > > > > > the .config is sane and nothing will get downgraded/disabled, can > > > > > always set KCONFIG_SANITY_CHECKS to 0. > > > > > > > > > > > > Kconfig is meant to resolve the dependency without causing an error. > > > > If a feature is not available, it is automatically, silently hidden, > > > > and that works well. > > > > > > How do you come to the conclusion that it works well? I've heard many > > > people unhappy about the way Kconfig works. How does one know that > > > something is missing (and should maybe be fixed?) if Kconfig silently > > > hides it? > > > > > > Kconfig has worked like that for a long time, but I do not know > > how to detect non-existing symbols. > > > > > > I think a tool to detect symbols present in old config, but missing in > new kernel solves the "upgraded config" part of the problem. > > The other part ("new config") would probably be solved by some kind of > a tool that looks at the currently present hardware and spews a list > of Kconfig options together with their dependencies, but arguably > that's not something that would be a part of Kconfig itself. > > For the graphical configuration tools like menuconfig I could imagine > that the options with unmet dependencies could be still displayed but > greyed out, so at least one can open the help for the item and check > which dependencies are missing. Yes. That idea exists, and at least for xconfig, I got a patch to grey out hidden options. https://lore.kernel.org/linux-kbuild/20200708133015.12286-1-maxime.chretien@bootlin.com/ I liked the idea, and suggested improvements, but did not receive v2. Maybe I could revisit it when I have some time, but I always have TODOs more than my capacity. Anyway, I applied Sergey's patch, so the life of you guys will get a little easier. -- Best Regards Masahiro Yamada
On Fri, Sep 1, 2023 at 12:28 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Thu, Aug 31, 2023 at 11:30 AM Tomasz Figa <tfiga@chromium.org> wrote: > > > > On Sat, Aug 26, 2023 at 10:11 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > On Thu, Aug 24, 2023 at 2:30 PM Tomasz Figa <tfiga@chromium.org> wrote: > > > > > > > > Hi Masahiro, > > > > > > > > On Thu, Aug 24, 2023 at 10:00 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > > > On Tue, Aug 22, 2023 at 4:30 PM Sergey Senozhatsky > > > > > <senozhatsky@chromium.org> wrote: > > > > > > > > > > > > On (23/08/21 21:27), Masahiro Yamada wrote: > > > > > > > > > > > > > > My (original) hope was to add a single switch, KCONFIG_VERBOSE, to address both: > > > > > > > > > > > > > > - A CONFIG option is hidden by unmet dependency (Ying Sun's case) > > > > > > > - A CONFIG option no longer exists (your case) > > > > > > > - Anything else we need to be careful > > > > > > > > > > > > A quick question: is it too late to suggest an alternative name? > > > > > > Could KCONFIG_SANITY_CHECKS be a little cleaner? Because we basically > > > > > > run sanity checks on the config. > > > > > > > > > > > > > > > Ying's is not applied yet. So, it is not too late. > > > > > > > > > > But, I started to be a little worried > > > > > because it is unpredictable how many KCONFIG_* env > > > > > variables will increase until people are satisfied. > > > > > > > > > > > > > Is there really a problem with having those? There are a lot of > > > > different env variables affecting different parts of the kernel build. > > > > If they are useful, and even better, allow catching issues quickly, > > > > should we favor less options or usefulness for users? > > > > > > > > > > > > I am considering how to implement it. > > > > > > > > > > > > One way is to add env variables as a new request arises. > > > > > > Sergey is doing two things by one option. > > > > > > > > > KCONFIG_WARN_UNKNWON_SYMBOL : warn unknown symbol in input .config > > > or defconfig > > > KCONFIG_WARN_TO_ERROR : turn warnings into errors > > > > > > > > > > > > Another way is to handle those as command line options. > > > > > > -Wunknown-symbol > > > -Werror (associated with W=e) > > > -Wall (associated with W=1) > > > > > > > > > > > > $ make W=1e olddefconfig > > > > > > > > > will work to sanity check. > > > > > > > > > > I see, I think I misunderstood your previous message, sorry. Agreed > > that there could be other approaches than an environment variable and > > a command line option could definitely work as well. I'll leave the > > details to you and Sergey, but ideally we would have something that is > > simple to use both in scripts (e.g. distro build systems) and in > > manual build for end users > > > > > > > > > > > > > > > > > > > > > > > And one more question: those sanity checks seem very reasonable. > > > > > > Is there any reason we would not want to keep them ON by default? > > > > > > And those brave souls, that do not wish for the tool to very that > > > > > > the .config is sane and nothing will get downgraded/disabled, can > > > > > > always set KCONFIG_SANITY_CHECKS to 0. > > > > > > > > > > > > > > > Kconfig is meant to resolve the dependency without causing an error. > > > > > If a feature is not available, it is automatically, silently hidden, > > > > > and that works well. > > > > > > > > How do you come to the conclusion that it works well? I've heard many > > > > people unhappy about the way Kconfig works. How does one know that > > > > something is missing (and should maybe be fixed?) if Kconfig silently > > > > hides it? > > > > > > > > > Kconfig has worked like that for a long time, but I do not know > > > how to detect non-existing symbols. > > > > > > > > > > I think a tool to detect symbols present in old config, but missing in > > new kernel solves the "upgraded config" part of the problem. > > > > The other part ("new config") would probably be solved by some kind of > > a tool that looks at the currently present hardware and spews a list > > of Kconfig options together with their dependencies, but arguably > > that's not something that would be a part of Kconfig itself. > > > > For the graphical configuration tools like menuconfig I could imagine > > that the options with unmet dependencies could be still displayed but > > greyed out, so at least one can open the help for the item and check > > which dependencies are missing. > > > Yes. That idea exists, and at least for xconfig, > I got a patch to grey out hidden options. > > https://lore.kernel.org/linux-kbuild/20200708133015.12286-1-maxime.chretien@bootlin.com/ > > > I liked the idea, and suggested improvements, but did not receive v2. > > Maybe I could revisit it when I have some time, > but I always have TODOs more than my capacity. > Let me see if I can do something about it. It sounds like a very convenient thing, although I'd definitely prefer menuconfig over xconfig. > > > Anyway, I applied Sergey's patch, so the life of you guys > will get a little easier. Thanks a lot! Best regards, Tomasz
On Mon, Sep 4, 2023 at 2:10 PM Tomasz Figa <tfiga@chromium.org> wrote: > > On Fri, Sep 1, 2023 at 12:28 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > On Thu, Aug 31, 2023 at 11:30 AM Tomasz Figa <tfiga@chromium.org> wrote: > > > > > > On Sat, Aug 26, 2023 at 10:11 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > On Thu, Aug 24, 2023 at 2:30 PM Tomasz Figa <tfiga@chromium.org> wrote: > > > > > > > > > > Hi Masahiro, > > > > > > > > > > On Thu, Aug 24, 2023 at 10:00 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > > > > > On Tue, Aug 22, 2023 at 4:30 PM Sergey Senozhatsky > > > > > > <senozhatsky@chromium.org> wrote: > > > > > > > > > > > > > > On (23/08/21 21:27), Masahiro Yamada wrote: > > > > > > > > > > > > > > > > My (original) hope was to add a single switch, KCONFIG_VERBOSE, to address both: > > > > > > > > > > > > > > > > - A CONFIG option is hidden by unmet dependency (Ying Sun's case) > > > > > > > > - A CONFIG option no longer exists (your case) > > > > > > > > - Anything else we need to be careful > > > > > > > > > > > > > > A quick question: is it too late to suggest an alternative name? > > > > > > > Could KCONFIG_SANITY_CHECKS be a little cleaner? Because we basically > > > > > > > run sanity checks on the config. > > > > > > > > > > > > > > > > > > Ying's is not applied yet. So, it is not too late. > > > > > > > > > > > > But, I started to be a little worried > > > > > > because it is unpredictable how many KCONFIG_* env > > > > > > variables will increase until people are satisfied. > > > > > > > > > > > > > > > > Is there really a problem with having those? There are a lot of > > > > > different env variables affecting different parts of the kernel build. > > > > > If they are useful, and even better, allow catching issues quickly, > > > > > should we favor less options or usefulness for users? > > > > > > > > > > > > > > > > I am considering how to implement it. > > > > > > > > > > > > > > > > One way is to add env variables as a new request arises. > > > > > > > > Sergey is doing two things by one option. > > > > > > > > > > > > KCONFIG_WARN_UNKNWON_SYMBOL : warn unknown symbol in input .config > > > > or defconfig > > > > KCONFIG_WARN_TO_ERROR : turn warnings into errors > > > > > > > > > > > > > > > > Another way is to handle those as command line options. > > > > > > > > -Wunknown-symbol > > > > -Werror (associated with W=e) > > > > -Wall (associated with W=1) > > > > > > > > > > > > > > > > $ make W=1e olddefconfig > > > > > > > > > > > > will work to sanity check. > > > > > > > > > > > > > > I see, I think I misunderstood your previous message, sorry. Agreed > > > that there could be other approaches than an environment variable and > > > a command line option could definitely work as well. I'll leave the > > > details to you and Sergey, but ideally we would have something that is > > > simple to use both in scripts (e.g. distro build systems) and in > > > manual build for end users > > > > > > > > > > > > > > > > > > > > > > > > > > > > > And one more question: those sanity checks seem very reasonable. > > > > > > > Is there any reason we would not want to keep them ON by default? > > > > > > > And those brave souls, that do not wish for the tool to very that > > > > > > > the .config is sane and nothing will get downgraded/disabled, can > > > > > > > always set KCONFIG_SANITY_CHECKS to 0. > > > > > > > > > > > > > > > > > > Kconfig is meant to resolve the dependency without causing an error. > > > > > > If a feature is not available, it is automatically, silently hidden, > > > > > > and that works well. > > > > > > > > > > How do you come to the conclusion that it works well? I've heard many > > > > > people unhappy about the way Kconfig works. How does one know that > > > > > something is missing (and should maybe be fixed?) if Kconfig silently > > > > > hides it? > > > > > > > > > > > > Kconfig has worked like that for a long time, but I do not know > > > > how to detect non-existing symbols. > > > > > > > > > > > > > > I think a tool to detect symbols present in old config, but missing in > > > new kernel solves the "upgraded config" part of the problem. > > > > > > The other part ("new config") would probably be solved by some kind of > > > a tool that looks at the currently present hardware and spews a list > > > of Kconfig options together with their dependencies, but arguably > > > that's not something that would be a part of Kconfig itself. > > > > > > For the graphical configuration tools like menuconfig I could imagine > > > that the options with unmet dependencies could be still displayed but > > > greyed out, so at least one can open the help for the item and check > > > which dependencies are missing. > > > > > > Yes. That idea exists, and at least for xconfig, > > I got a patch to grey out hidden options. > > > > https://lore.kernel.org/linux-kbuild/20200708133015.12286-1-maxime.chretien@bootlin.com/ > > > > > > I liked the idea, and suggested improvements, but did not receive v2. > > > > Maybe I could revisit it when I have some time, > > but I always have TODOs more than my capacity. > > > > Let me see if I can do something about it. It sounds like a very > convenient thing, although I'd definitely prefer menuconfig over > xconfig. To close the loop, it took me a while to find some time to give it a stab, but here is a patch: https://lore.kernel.org/linux-kbuild/20231228054630.3595093-1-tfiga@chromium.org/T/#u One thing that may need to be added is a toggle for the new behavior if there are people who prefer all the options to look the same (or just the dim text is problematic for some reason). Best, Tomasz
diff --git a/Documentation/kbuild/kconfig.rst b/Documentation/kbuild/kconfig.rst index 6530ecd99da3..445c438dc741 100644 --- a/Documentation/kbuild/kconfig.rst +++ b/Documentation/kbuild/kconfig.rst @@ -29,6 +29,14 @@ To see a list of new config symbols, use:: and the config program will list any new symbols, one per line. +To see a list of config symbols that are not recognized anymore (e.g. +removed or renamed), use:: + + cp user/some/old.config .config + make listunknownconfig + +and the config program will list any unrecognized symbols, one per line. + Alternatively, you can use the brute force method:: make oldconfig diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile index af1c96198f49..942316ddebd9 100644 --- a/scripts/kconfig/Makefile +++ b/scripts/kconfig/Makefile @@ -69,7 +69,8 @@ localyesconfig localmodconfig: $(obj)/conf # deprecated for external use simple-targets := oldconfig allnoconfig allyesconfig allmodconfig \ alldefconfig randconfig listnewconfig olddefconfig syncconfig \ - helpnewconfig yes2modconfig mod2yesconfig mod2noconfig + helpnewconfig yes2modconfig mod2yesconfig mod2noconfig \ + listunknownconfig PHONY += $(simple-targets) @@ -141,6 +142,7 @@ help: @echo ' default value without prompting' @echo ' tinyconfig - Configure the tiniest possible kernel' @echo ' testconfig - Run Kconfig unit tests (requires python3 and pytest)' + @echo ' listunknownconfig - List unrecognized options' # =========================================================================== # object files used by all kconfig flavours diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c index 33d19e419908..e26aa491be00 100644 --- a/scripts/kconfig/conf.c +++ b/scripts/kconfig/conf.c @@ -36,6 +36,7 @@ enum input_mode { yes2modconfig, mod2yesconfig, mod2noconfig, + listunknownconfig, }; static enum input_mode input_mode = oldaskconfig; static int input_mode_opt; @@ -683,6 +684,7 @@ static const struct option long_opts[] = { {"yes2modconfig", no_argument, &input_mode_opt, yes2modconfig}, {"mod2yesconfig", no_argument, &input_mode_opt, mod2yesconfig}, {"mod2noconfig", no_argument, &input_mode_opt, mod2noconfig}, + {"listunknownconfig", no_argument, &input_mode_opt, listunknownconfig}, {NULL, 0, NULL, 0} }; @@ -712,6 +714,7 @@ static void conf_usage(const char *progname) printf(" --yes2modconfig Change answers from yes to mod if possible\n"); printf(" --mod2yesconfig Change answers from mod to yes if possible\n"); printf(" --mod2noconfig Change answers from mod to no if possible\n"); + printf(" --listunknownconfig List config options that do not exist anymore\n"); printf(" (If none of the above is given, --oldaskconfig is the default)\n"); } @@ -823,6 +826,11 @@ int main(int ac, char **av) exit(1); } break; + case listunknownconfig: + if (conf_read_list_unknown()) + exit(1); + exit(0); + break; default: break; } diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c index 992575f1e976..d387a4f08cef 100644 --- a/scripts/kconfig/confdata.c +++ b/scripts/kconfig/confdata.c @@ -341,6 +341,61 @@ static ssize_t compat_getline(char **lineptr, size_t *n, FILE *stream) return -1; } +int conf_read_list_unknown(void) +{ + FILE *in = NULL; + size_t line_asize = 0; + char *line = NULL; + char *p, *p2; + struct symbol *sym; + + conf_filename = conf_get_configname(); + in = zconf_fopen(conf_filename); + if (!in) + return -1; + + while (compat_getline(&line, &line_asize, in) != -1) { + conf_lineno++; + sym = NULL; + if (line[0] == '#') { + if (memcmp(line + 2, CONFIG_, strlen(CONFIG_))) + continue; + p = strchr(line + 2 + strlen(CONFIG_), ' '); + if (!p) + continue; + *p++ = 0; + sym = sym_find(line + 2 + strlen(CONFIG_)); + if (!sym) { + conf_warning("unknown unset symbol: %s", + line + 2 + strlen(CONFIG_)); + continue; + } + } else if (memcmp(line, CONFIG_, strlen(CONFIG_)) == 0) { + p = strchr(line + strlen(CONFIG_), '='); + if (!p) + continue; + *p++ = 0; + p2 = strchr(p, '\n'); + if (p2) { + *p2-- = 0; + if (*p2 == '\r') + *p2 = 0; + } + + sym = sym_find(line + strlen(CONFIG_)); + if (!sym) { + conf_warning("unknown symbol: %s", + line + strlen(CONFIG_)); + continue; + } + } + } + + free(line); + fclose(in); + return conf_warnings; +} + int conf_read_simple(const char *name, int def) { FILE *in = NULL; diff --git a/scripts/kconfig/lkc_proto.h b/scripts/kconfig/lkc_proto.h index edd1e617b25c..bb60b1669750 100644 --- a/scripts/kconfig/lkc_proto.h +++ b/scripts/kconfig/lkc_proto.h @@ -5,6 +5,7 @@ void conf_parse(const char *name); int conf_read(const char *name); int conf_read_simple(const char *name, int); +int conf_read_list_unknown(void); int conf_write_defconfig(const char *name); int conf_write(const char *name); int conf_write_autoconf(int overwrite);
The listunknownconfig option reads old .config and lists all unrecognized symbols. This is especially useful for continuous kernel uprevs when some symbols can be either removed or renamed between kernel releases (which can go unnoticed otherwise). A recent real-life example of such a symbol rename that quietly disabled some drivers after kernel uprev is MFD_RK808 rename. Example: Suppose old .config has the following two options which were removed from the recent kernel: $ cat .config CONFIG_DISABLE_BUGS=y Running `make listunknownconfig` produces the following list of unrecognized symbols: .config:6:warning: unknown symbol: DISABLE_BUGS .config:7:warning: unknown unset symbol: ENABLE_WINAPI Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org> --- Documentation/kbuild/kconfig.rst | 8 +++++ scripts/kconfig/Makefile | 4 ++- scripts/kconfig/conf.c | 8 +++++ scripts/kconfig/confdata.c | 55 ++++++++++++++++++++++++++++++++ scripts/kconfig/lkc_proto.h | 1 + 5 files changed, 75 insertions(+), 1 deletion(-)