Re: [docs] [PATCH v2 1/2] bitbake-setup: update: add a --update-bb-conf option

["Antonin Godard" <antonin.godard@bootlin.com>]

Hi,

On Mon Nov 17, 2025 at 12:52 PM CET, Alexander Kanavin wrote:
> On Mon, 17 Nov 2025 at 09:21, Antonin Godard via
> lists.yoctoproject.org
> <antonin.godard=bootlin.com@lists.yoctoproject.org> wrote:
>> -        bitbake_config_diff = get_diff(backup_bitbake_confdir, bitbake_confdir)
>
> ... replaced by:
>
>> +        try:
>> +            conf_diff = bb.process.run(f'git diff --no-index "{backup_bitbake_confdir}" "{bitbake_confdir}"')[0]
>> +        except bb.process.ExecutionError as e:
>> +            if e.exitcode == 1:
>> +                conf_diff = e.stdout
>> +            else:
>> +                raise e
>
> Why? I think the function helper should be reused to keep the main
> code more succinct.

Correct, while writing this I removed the previous statement, then did not
realize get_diff() could be used on two directories - will revert.

>> +    _extra_msg_info = " (--update-bb-conf=yes)"
>
> I'd say it's better to print appropriate messages directly in the
> logic branches (even if they differ only slightly), rather than set
> and tweak a variable.
>
>> +    if update_bb_conf == "no":
>> +        print('Ignoring upstream bitbake configuration changes (--update-bb-conf=no)')
>> +        print(f'Leaving the upstream configuration in {upstream_bitbake_confdir}')
>> +        os.rename(bitbake_confdir, upstream_bitbake_confdir)
>> +        os.rename(backup_bitbake_confdir, bitbake_confdir)
>> +    elif conf_diff:
>
> The first check should be if bitbake configuration has changed at all,
> and the new config should be removed if so.
>
> See the proposal here:
> https://lists.yoctoproject.org/g/docs/message/8056
>
> So the overall logic should be:
>
> if os.path.exists(backup_bitbake_confdir):
>     ... (obtain the diff)
>     if not conf_diff:
>         print("New bitbake configuration from upstream is the same as the old one, no need to update it.")
>         os.rmtree(new)
>         os.rename(backup, main)
>         return
>     else:
>        print('Upstream bitbake configuration changes were found:')
>        print(conf_diff)
>        if update_bb_conf == 'prompt':
>           ... ask for yes or no
>
>     if update_bb_conf == 'no':
>        ... (the block quoted above)
>        return
>
> ... (write readme and print instructions and notice where the old config is)

Much simpler indeed :)

>
>>  def build_update(top_dir, settings, args, d):
>> -    build_status(top_dir, settings, args, d, update=True)
>> +    build_status(top_dir, settings, args, d, update=True, update_bb_conf=args.update_bb_conf)
>
> args are already passed in, so no need to pass args.update_bb_conf as
> a separate argument. build_status() can just obtain it from args.

Agreed, although the status command does not have the --update-bb-conf, but I'll
catch that case.

Thanks!
Antonin

-- 
Antonin Godard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com