[PATCHv2 2/3] busybox: update to 1.33

[Rosen Penev]

On Wed, Jan 6, 2021 at 3:40 AM Hannu Nyman <hannu.nyman at iki.fi> wrote:
>
> Hannu Nyman kirjoitti 6.1.2021 klo 12.08:
> > Rosen Penev kirjoitti 6.1.2021 klo 0.49:
> >> On Tue, Jan 5, 2021 at 12:20 PM Hannu Nyman<hannu.nyman at iki.fi>  wrote:
> >>> Hannu Nyman kirjoitti 5.1.2021 klo 16.34:
> >>>> Rosen Penev kirjoitti 5.1.2021 klo 0.29:
> >>>>> It seems the config section is not refreshed either in this patch.
> >>>>> That is, the other Config.in files. No idea how to update those.
> >>>> ...
> >>>>
> >>>> Or do you think that there is something else wrong reagrding the other
> >>>> defaults in Config-defaults.in in your patch?
> >>>> To me your patch looks sensible otherwise, except this IPV6 change.
> >>>>
> >>> Yeah, now I got what you meant. I tried your patch set, and then I also run
> >>> the "refresh config " scripts.
> >>>
> >>> Your patch set is missing all the updates to the sourced Config.in files,
> >>> which changes become visible once you run the config refreshing commands.
> >>>
> >>> I got them refreshed after the first ath79 build with
> >>>
> >>>     cd package/utils/busybox/config/
> >>>     ../convert_menuconfig.pl
> >>> ../../../../build_dir/target-mips_24kc_musl/busybox-default/busybox-1.33.0
> >>>     cd ..
> >>>     ./convert_defaults.pl <
> >>> ../../../build_dir/target-mips_24kc_musl/busybox-default/busybox-1.33.0/.config
> >>>
> >>>   > Config-defaults.in
> >> That's only for one of the config files, not the others.
> >
> >
> > Worked ok for all relevant ones, I think.
> >
> > Or what is your worry here?
> >
> > (that is the way it has been done with the past 7 version bumps, to my
> > knowledge.)
> >
> >
> >>
> >>> And then I edited by hand the IPv6 option in Config-defaults.in to get back
> >>> the OpenWrt speciality that deviates from busybox defaults.
> >>> I also manually added the needed quotes to source lines that the scripts
> >>> removed.
> >>>
> >>> I added your commits and my fix to my Github fork, so it is easier to see
> >>> and
> >>> compare.
> >>> https://github.com/hnyman/openwrt/commits/bb1330fix
> >>>
> >>> I think that you should add the following fixes to your v2 patch series:
> >>> https://github.com/hnyman/openwrt/commit/95f54c3f4d9501a42eda7be4e3bd971a1887f8ac
> >>>
> >>>
> >>> That is meant to be squashed into your main patch.
> >> Latest
> >> patch:https://github.com/neheb/openwrt/commit/139dc12498db287660bfad27b94e137afa4de9fa
> >>
> >> I think that should be fine.
> >>> I will compile that into a firmware and test it.
> >
> > I have good news and bad news:
> >
> > Good news is that your new patch is identical as the outcome of (your old
> > patch + my fix).
> > I compiled it yesterday for ath79/WNDR3700 and it worked ok. Good so far.
> >
> >
> > Bad news is that something has changed between 1.31 and 1.33 regarding
> > shell options, and booting ipq806x/R7800 fails with 1.33.0.
> >
> > I get an error in serial console in preinit phase and the R7800 router
> > never boots properly.
> >
> > [ 5.711407] kmodloader: done loading kernel modules from /etc/modules-boot.d/*
> > [ 5.721717] init: - preinit -
> > /etc/preinit: /lib/functions.sh: line 29: syntax error: support for
> > $((arith)) is disabled
> > [ 6.107332] procd: - early -
> > [ 6.107390] procd: - watchdog -
> >
> >
> > I think that I have narrowed is down to config options changes regarding
> > ash shell, and the enabling of arithmetics via BUSYBOX_CONFIG_FEATURE_SH_MATH.
> >
> > I suspect that reason for failure is that BUSYBOX_CONFIG_SHELL_ASH is
> > hidden in :
> >
> > config BUSYBOX_CONFIG_SHELL_ASH
> >     bool #hidden option
> >
> >
> > That leads our script to parse it wrongly and the section about common
> > options remains empty/unselectable in menuconfig, as there is SHELL_ASH
> > instead of BUSYBOX_CONFIG_SHELL_ASH in shell/Config.in file. So this
> > section remain hidden/disabled:
> >
> >  comment "Options common to all shells"
> > -if ASH || BUSYBOX_CONFIG_HUSH || BUSYBOX_CONFIG_SH_IS_ASH ||
> > BUSYBOX_CONFIG_BASH_IS_ASH || BUSYBOX_CONFIG_SH_IS_HUSH ||
> > BUSYBOX_CONFIG_BASH_IS_HUSH
> > +if SHELL_ASH || BUSYBOX_CONFIG_SHELL_HUSH
> >
> > That leads into missed dependency here, where the syntax has changed:
> >
> > config BUSYBOX_CONFIG_FEATURE_SH_MATH
> >     bool "POSIX math support"
> >     default BUSYBOX_DEFAULT_FEATURE_SH_MATH
> > -    depends on BUSYBOX_CONFIG_ASH || BUSYBOX_CONFIG_HUSH ||
> > BUSYBOX_CONFIG_SH_IS_ASH || BUSYBOX_CONFIG_BASH_IS_ASH ||
> > BUSYBOX_CONFIG_SH_IS_HUSH || BUSYBOX_CONFIG_BASH_IS_HUSH
> > +    depends on BUSYBOX_CONFIG_SHELL_ASH || BUSYBOX_CONFIG_SHELL_HUSH
> >     help
> >     Enable math support in the shell via $((...)) syntax.
> >
> >
> > So, some changes are still needed.
> >
> > I will test and come back.
> >
> >
>
> I added two more commits to
>
> https://github.com/hnyman/openwrt/commits/bb1330fix
>
>
> The first one addresses the shell math problem. Editing shell/Config.in helps
> menuconfig and defconfig to interprete the config correctly. I solved it for
> now, but the logic might easily get accidentally changed again at the next
> version bump and config file refresh. It might be better in the long run to
> change the underlying symbol definition by removing the hidden attribute from
> BUSYBOX_CONFIG_SHELL_ASH in shell/Config.in, but I have not tested it yet.
>
> https://github.com/hnyman/openwrt/commit/4fde43b539777e644e667c67bde74a9306bfd6cc
>
> That made R7800 to boot again normally.
>
>
> The second one is a fix for accidentally deleting two new OpenWrt specific
> conditional logic items in the config refresh. They have been introduced by
> 547f1ec25 and f1410902e in 2019/2020, and they get deleted (similarly as the
> IPV6 logic) by the config refresh script.  (This patch should be squashed to
> the main patch to revert the earlier error of deleting that logic.)
>
> https://github.com/hnyman/openwrt/commit/84605304a5742c083cc19ed6918e02a9f3e6103d
>
> Please test with a few different routers.
I've only tested with malta which showed no errors. It's very
unfortunate that malta does not break like real hardware.
>
>
> We have now at least three of these OpenWrt specific logic changes (IPV6,
> TRUNCATE/bcm53xx and REGEX/GLIBC). Too bad that our busybox config script
> misses those changes. I wonder if we should look into patching tat logic
> directly to busybx sources instead fo modifying the logic in Config* files.
> Config* files will be again refreshed in the next busybox version upgrades,
> and it is easy to miss these changes to be corrected.
It does look fragile, yes. I've never dealt with Busybox' build system.
>
>