| Message ID | 20240403121807.2087547-1-robimarko@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/1] kconfig/lxdialog: fix check() with GCC14 | expand |
Hi Robert, > GCC14 now treats implicit int types as error so when check() from > check-lxdialog.sh is called to check whether we can link against ncurses > it will fail silently and the help text indicating to install ncurses is > printed. > However, this is not due to missing ncurses but once the stderr redirect > to /dev/null is removed we can see the root cause: > <stdin>:2:1: error: return type defaults to ‘int’ [-Wimplicit-int] > So, in order for menuconfig to work with GCC14 lets just specify the > return type of main() as int. Good catch. Not only gcc-14, but also clang-16 fails due -Wimplicit-int. (clang-14 was ok). Reviewed-by: Petr Vorel <petr.vorel@gmail.com> Tested-by: Petr Vorel <petr.vorel@gmail.com> Kind regards, Petr > Signed-off-by: Robert Marko <robimarko@gmail.com> > --- > support/kconfig/lxdialog/check-lxdialog.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > diff --git a/support/kconfig/lxdialog/check-lxdialog.sh b/support/kconfig/lxdialog/check-lxdialog.sh > index 16cd9a3186..27d6c30a57 100755 > --- a/support/kconfig/lxdialog/check-lxdialog.sh > +++ b/support/kconfig/lxdialog/check-lxdialog.sh > @@ -48,7 +48,7 @@ trap "rm -f $tmp" 0 1 2 3 15 > check() { > $cc -x c - -o $tmp 2>/dev/null <<'EOF' > #include CURSES_LOC > -main() {} > +int main() {} > EOF > if [ $? != 0 ]; then > echo " *** Unable to find the ncurses libraries or the" 1>&2
Hello Robert, On Wed, 3 Apr 2024 14:18:07 +0200 Robert Marko <robimarko@gmail.com> wrote: > GCC14 now treats implicit int types as error so when check() from > check-lxdialog.sh is called to check whether we can link against ncurses > it will fail silently and the help text indicating to install ncurses is > printed. > > However, this is not due to missing ncurses but once the stderr redirect > to /dev/null is removed we can see the root cause: > <stdin>:2:1: error: return type defaults to ‘int’ [-Wimplicit-int] > > So, in order for menuconfig to work with GCC14 lets just specify the > return type of main() as int. > > Signed-off-by: Robert Marko <robimarko@gmail.com> Thanks a lot for the patch. However, making changes in support/kconfig/ in Buildroot requires a bit more care. Indeed, this code is taken from the Linux kernel code, and we try to keep a well-defined list of changes on top of the original code from the kernel. As explained in support/kconfig/README.buildroot, the current code in support/kconfig/ is taken from Linux 4.17-rc2, on top of which the patches in support/kconfig/patches/ have been applied. So from this, you have two options: (1) Add a new patch in support/kconfig/patches/ which implements the change you need for GCC14 compatibility (2) Update the code in support/kconfig/ to the latest code available in the Linux kernel scripts/kconfig directory, rebase our patches, and verify it all works. Obviously, (1) is the easiest path. (2) is nicer long term, as we benefit from all new features/fixes added by the Linux kernel developers. Turns out that in the upstream kernel, the check-lxdialog.sh script has been removed and replaced by something different, which no longer has this main() function definition. Could you have a look at implementing either (1) or (2) ? Thanks a lot! Thomas
On Mon, 8 Apr 2024 at 22:36, Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > > Hello Robert, > > On Wed, 3 Apr 2024 14:18:07 +0200 > Robert Marko <robimarko@gmail.com> wrote: > > > GCC14 now treats implicit int types as error so when check() from > > check-lxdialog.sh is called to check whether we can link against ncurses > > it will fail silently and the help text indicating to install ncurses is > > printed. > > > > However, this is not due to missing ncurses but once the stderr redirect > > to /dev/null is removed we can see the root cause: > > <stdin>:2:1: error: return type defaults to ‘int’ [-Wimplicit-int] > > > > So, in order for menuconfig to work with GCC14 lets just specify the > > return type of main() as int. > > > > Signed-off-by: Robert Marko <robimarko@gmail.com> > > Thanks a lot for the patch. However, making changes in support/kconfig/ > in Buildroot requires a bit more care. Indeed, this code is taken from > the Linux kernel code, and we try to keep a well-defined list of > changes on top of the original code from the kernel. > > As explained in support/kconfig/README.buildroot, the current code in > support/kconfig/ is taken from Linux 4.17-rc2, on top of which the > patches in support/kconfig/patches/ have been applied. > > So from this, you have two options: > > (1) Add a new patch in support/kconfig/patches/ which implements the > change you need for GCC14 compatibility > > (2) Update the code in support/kconfig/ to the latest code available in > the Linux kernel scripts/kconfig directory, rebase our patches, and > verify it all works. > > Obviously, (1) is the easiest path. (2) is nicer long term, as we > benefit from all new features/fixes added by the Linux kernel > developers. Turns out that in the upstream kernel, the > check-lxdialog.sh script has been removed and replaced by something > different, which no longer has this main() function definition. > > Could you have a look at implementing either (1) or (2) ? Hi Thomas, I can only implement solution 1 currently, will respin for v2. Regards, Robert > > Thanks a lot! > > Thomas > -- > Thomas Petazzoni, co-owner and CEO, Bootlin > Embedded Linux and Kernel engineering and training > https://bootlin.com
Hi Robert, Thomas, > Hello Robert, > On Wed, 3 Apr 2024 14:18:07 +0200 > Robert Marko <robimarko@gmail.com> wrote: > > GCC14 now treats implicit int types as error so when check() from > > check-lxdialog.sh is called to check whether we can link against ncurses > > it will fail silently and the help text indicating to install ncurses is > > printed. > > However, this is not due to missing ncurses but once the stderr redirect > > to /dev/null is removed we can see the root cause: > > <stdin>:2:1: error: return type defaults to ‘int’ [-Wimplicit-int] > > So, in order for menuconfig to work with GCC14 lets just specify the > > return type of main() as int. > > Signed-off-by: Robert Marko <robimarko@gmail.com> > Thanks a lot for the patch. However, making changes in support/kconfig/ > in Buildroot requires a bit more care. Indeed, this code is taken from > the Linux kernel code, and we try to keep a well-defined list of > changes on top of the original code from the kernel. > As explained in support/kconfig/README.buildroot, the current code in > support/kconfig/ is taken from Linux 4.17-rc2, on top of which the > patches in support/kconfig/patches/ have been applied. I'm sorry for a wrong review, I completely forget Buildroot uses patches also for kconfig. > So from this, you have two options: > (1) Add a new patch in support/kconfig/patches/ which implements the > change you need for GCC14 compatibility > (2) Update the code in support/kconfig/ to the latest code available in > the Linux kernel scripts/kconfig directory, rebase our patches, and > verify it all works. > Obviously, (1) is the easiest path. (2) is nicer long term, as we > benefit from all new features/fixes added by the Linux kernel > developers. Turns out that in the upstream kernel, the > check-lxdialog.sh script has been removed and replaced by something > different, which no longer has this main() function definition. FYI I gave up kconfig update after big rewrites in the Linux kernel made by Masahiro Yamada (the new kbuild and kconfig maintainer), because the code changes were quite big but kconfig in Buildroot is IMHO not missing any significant usability function. But I can be wrong, Thomas let me know if you miss something significant which would justify the update. Obviously we would get some fixes, but also major rewrite, which would likely require changes in other parts of Buildroot. check-lxdialog.sh was removed in 1c5af5cf9308 ("kconfig: refactor ncurses package checks for building mconf and nconf") in v4.18, but major rewrite come in 4.19 and 5.0 + kbuild updates in 5.1, changes continue in 5.10, 5.13, later fixes in 6.x kernels... Some of these changes moved storing toolchain capabilities in .config (while Buildroot uses does something similar, I'm not sure whether kernel functionality could be used without big changes in Buildroot). Kind regards, Petr > Could you have a look at implementing either (1) or (2) ? > Thanks a lot! > Thomas
On Mon, 8 Apr 2024 23:38:07 +0200 Petr Vorel <petr.vorel@gmail.com> wrote: > FYI I gave up kconfig update after big rewrites in the Linux kernel > made by Masahiro Yamada (the new kbuild and kconfig maintainer), because the > code changes were quite big but kconfig in Buildroot is IMHO not missing any > significant usability function. But I can be wrong, Thomas let me know if you > miss something significant which would justify the update. Obviously we would > get some fixes, but also major rewrite, which would likely require changes in > other parts of Buildroot. I have no idea what are the new interesting features that have been added in the upstream scripts/kconfig/ (if any). However, keeping our support/kconfig/ reasonably up-to-date is not only about new features, it's also about staying compatible with reasonably recent libraries/tools (more recent ncurses, Gtk, Qt, more recent GCC, like this issue illustrates). Best regards, Thomas
Thomas, Petr, All, On 2024-04-09 21:46 +0200, Thomas Petazzoni via buildroot spake thusly: > On Mon, 8 Apr 2024 23:38:07 +0200 > Petr Vorel <petr.vorel@gmail.com> wrote: > > > FYI I gave up kconfig update after big rewrites in the Linux kernel > > made by Masahiro Yamada (the new kbuild and kconfig maintainer), because the > > code changes were quite big but kconfig in Buildroot is IMHO not missing any > > significant usability function. But I can be wrong, Thomas let me know if you > > miss something significant which would justify the update. Obviously we would > > get some fixes, but also major rewrite, which would likely require changes in > > other parts of Buildroot. > > I have no idea what are the new interesting features that have been > added in the upstream scripts/kconfig/ (if any). However, keeping our > support/kconfig/ reasonably up-to-date is not only about new features, > it's also about staying compatible with reasonably recent > libraries/tools (more recent ncurses, Gtk, Qt, more recent GCC, like > this issue illustrates). The kernel's kconfig has dropped the possibility to include files from within a choice, so what we're doing now in a few places will not work; - toolchain/toolchain-external/Config.in - package/openssl/Config.in and so on... Unless that was reverted upstream... Regards, Yann E. MORIN.
Hi Yann, Thomas, all, > Thomas, Petr, All, > On 2024-04-09 21:46 +0200, Thomas Petazzoni via buildroot spake thusly: > > On Mon, 8 Apr 2024 23:38:07 +0200 > > Petr Vorel <petr.vorel@gmail.com> wrote: > > > FYI I gave up kconfig update after big rewrites in the Linux kernel > > > made by Masahiro Yamada (the new kbuild and kconfig maintainer), because the > > > code changes were quite big but kconfig in Buildroot is IMHO not missing any > > > significant usability function. But I can be wrong, Thomas let me know if you > > > miss something significant which would justify the update. Obviously we would > > > get some fixes, but also major rewrite, which would likely require changes in > > > other parts of Buildroot. > > I have no idea what are the new interesting features that have been > > added in the upstream scripts/kconfig/ (if any). However, keeping our > > support/kconfig/ reasonably up-to-date is not only about new features, > > it's also about staying compatible with reasonably recent > > libraries/tools (more recent ncurses, Gtk, Qt, more recent GCC, like > > this issue illustrates). > The kernel's kconfig has dropped the possibility to include files from > within a choice, so what we're doing now in a few places will not work; > - toolchain/toolchain-external/Config.in > - package/openssl/Config.in Ah, you mean choice within choice .. endchoice. Thanks for info. That's unfortunate. Any idea which commits reverted it? I'm not sure if it will be possible to easily revert it back (there are many changes in kconfig). Also replacing 'source' with some placeholder and using some preprocessing in extra make target (dependency for gconfig/xconfig/menuconfig/nconfig/config) might not be as trivial as it looks like. > and so on... Do you remember any other problem? > Unless that was reverted upstream... I doubt, but that needs to be find out. Also, maybe slightly easier way could be to use init-kconfig [1], used in kdevops [2] (imported via 'git subtree' into the usual scripts/kconfig/ directory). At least we could try to migrate at least some of our patches there. Kind regards, Petr > Regards, > Yann E. MORIN. [1] https://github.com/mcgrof/init-kconfig [2] https://github.com/mcgrof/kdevops
Petr, All, > > The kernel's kconfig has dropped the possibility to include files from > > within a choice, so what we're doing now in a few places will not work; > > - toolchain/toolchain-external/Config.in > > - package/openssl/Config.in > Ah, you mean choice within choice .. endchoice. Thanks for info. That's ^^^^^^ s/choice/source/ Yes, "source" in a choice...endchoice block. > unfortunate. Any idea which commits reverted it? That was commit in the kernel, 4 years ago: 09d5873e4d1f7 kconfig: allow only 'config', 'comment', and 'if' inside 'choice' > I'm not sure if it will be > possible to easily revert it back (there are many changes in kconfig). Indeed. > Also replacing 'source' with some placeholder and using some preprocessing > in extra make target (dependency for gconfig/xconfig/menuconfig/nconfig/config) > might not be as trivial as it looks like. No, it is not... > Do you remember any other problem? Not right now. There were a few other major changes, but AFAICS we would not be impacted. > Also, maybe slightly easier way could be to use init-kconfig [1], init-kconfig has not been touched in 2 years. There is no point in switching from our mothballed copy, to another mothballed copy... And in any case, they advertise as being "a passive fork of Linux kconfig", and their "goal is not to fork kconfig and evolve it separately". Regards, Yann E. MORIN.
diff --git a/support/kconfig/lxdialog/check-lxdialog.sh b/support/kconfig/lxdialog/check-lxdialog.sh index 16cd9a3186..27d6c30a57 100755 --- a/support/kconfig/lxdialog/check-lxdialog.sh +++ b/support/kconfig/lxdialog/check-lxdialog.sh @@ -48,7 +48,7 @@ trap "rm -f $tmp" 0 1 2 3 15 check() { $cc -x c - -o $tmp 2>/dev/null <<'EOF' #include CURSES_LOC -main() {} +int main() {} EOF if [ $? != 0 ]; then echo " *** Unable to find the ncurses libraries or the" 1>&2
GCC14 now treats implicit int types as error so when check() from check-lxdialog.sh is called to check whether we can link against ncurses it will fail silently and the help text indicating to install ncurses is printed. However, this is not due to missing ncurses but once the stderr redirect to /dev/null is removed we can see the root cause: <stdin>:2:1: error: return type defaults to ‘int’ [-Wimplicit-int] So, in order for menuconfig to work with GCC14 lets just specify the return type of main() as int. Signed-off-by: Robert Marko <robimarko@gmail.com> --- support/kconfig/lxdialog/check-lxdialog.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)