[PATCHv2 2/3] busybox: update to 1.33

Wed Jan 6 06:40:19 EST 2021

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.

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.