Buildroot: Target to update config fragments in kconfig based packages

This patch adds the new target above which implements the update routine as detailled in the head of this patch series.

Signed-off-by Marcel Patzlaff <[email protected]>

Hello Marcel,

Thanks for working on this complex topic, as Arnout said.

On my side, I find the semantic of "let's update the _last_ fragment" to be a bit weird and complex to understand. Why the last one, and not any other ?

I'm not sure it's possible to have something that updates fragments with a sensible semantic.

However, what would be useful would be a way to dump the difference between the current configuration, and what the combination of the defconfig+fragments provide.

I.e, you start Buildroot with omap2plus_defconfig + some specific fragments as the kernel configuration.

You run "make linux-show-config-diff", which returns empty.

Then you tweak the configuration with "make linux-menuconfig", and run "make linux-show-config-diff", which shows you the lines that you need to put in one of your fragments to preserve the configuration tweaks you have done.

Of course, the name "linux-show-config-diff" is just made up here, and some more thought is needed to find a good name. But overall, I believe the semantic would be much clearer, and doesn't make any assumption on whether we have one or several fragments, and whether the last fragment or any other fragment needs to be updated.

What do you think about this? Would this also fill your needs?

Hello Thomas,

Quoted 4 lines

On my side, I find the semantic of "let's update the _last_ fragment" to be a bit weird and complex to understand. Why the last one, and not any other ?

Quoted 2 lines

I'm not sure it's possible to have something that updates fragments with a sensible semantic.

Well, it should just reflect what the merge_config script does: the last file in the list of files to be merged always overrides settings done in files before. So this means, that the last fragment is basically the most significate one.

When I think about it, this should also be documented somewhere. Because the result with configuration fragments A, B, C could be different than B, A, C for example. So it should definitely be told, that the order of fragments matters and that the fragments are best used to specialise the configuration of other "layers" before.

Quoted 20 lines

However, what would be useful would be a way to dump the difference between the current configuration, and what the combination of the defconfig+fragments provide.

I.e, you start Buildroot with omap2plus_defconfig + some specific fragments as the kernel configuration.

You run "make linux-show-config-diff", which returns empty.

Then you tweak the configuration with "make linux-menuconfig", and run "make linux-show-config-diff", which shows you the lines that you need to put in one of your fragments to preserve the configuration tweaks you have done.

Of course, the name "linux-show-config-diff" is just made up here, and some more thought is needed to find a good name. But overall, I believe the semantic would be much clearer, and doesn't make any assumption on whether we have one or several fragments, and whether the last fragment or any other fragment needs to be updated.

Quoted 1 line

What do you think about this? Would this also fill your needs?

Despite the fact, that you now have to put this diff manually somewhere it would not change things. Because this diff, as I pointed out above, is in general only useful for the last fragment.

Updating any other fragment in the list would require either to have no dependencies between fragments at all (too restrictive at least for my use cases and also I'm not sure that this is feasible) or to have a deep understanding of the dependencies.

From my perspective, neither option is that encouraging, so I do not want to propose a way to update any fragment other than the last one.

Quoted 1 line

Well, it should just reflect what the merge_config script does: the

Quoted 1 line

file in the list of files to be merged always overrides settings done

Quoted 2 lines

files before. So this means, that the last fragment is basically the most significate one.

Fragments are not about "priority", they are typically about splitting by topic different aspects. So it is not because the last fragment overrides the configuration options of other fragments that a configuration change you just did to enable driver FOO should go in the last fragment.

Quoted 1 line

Updating any other fragment in the list would require either to have

Quoted 2 lines

dependencies between fragments at all (too restrictive at least for my use cases and also I'm not sure that this is feasible) or to have a

Quoted 1 line

understanding of the dependencies.

When someone is using fragments, then they understand what they mean to split the configuration in different snippets. People who don't understand that will typically use a single monolithic .config file.

Quoted 2 lines

From my perspective, neither option is that encouraging, so I do not want to propose a way to update any fragment other than the last one.

My position is that updating anything is not correct, because you cannot know whether it is the last fragment or any of the other fragment that needs to be updated. The only sane semantic is to show what the differences are, and let the user figure out where these configuration differences should be propagated.

Quoted 4 lines

This patch adds the new target above which implements the update routine as detailled in the head of this patch series.

Signed-off-by Marcel Patzlaff <[email protected]>

Thanks for working on this complex topic, as Arnout said.

On my side, I find the semantic of "let's update the _last_ fragment" to be a bit weird and complex to understand. Why the last one, and not any other ?

I'm not sure it's possible to have something that updates fragments with a sensible semantic.

However, what would be useful would be a way to dump the difference between the current configuration, and what the combination of the defconfig+fragments provide.

I.e, you start Buildroot with omap2plus_defconfig + some specific fragments as the kernel configuration.

You run "make linux-show-config-diff", which returns empty.

Then you tweak the configuration with "make linux-menuconfig", and run "make linux-show-config-diff", which shows you the lines that you need to put in one of your fragments to preserve the configuration tweaks you have done.

Of course, the name "linux-show-config-diff" is just made up here, and some more thought is needed to find a good name. But overall, I believe the semantic would be much clearer, and doesn't make any assumption on whether we have one or several fragments, and whether the last fragment or any other fragment needs to be updated.

What do you think about this? Would this also fill your needs?

I would just suggest that we do not add any new rule, but trying to update the defconfig when there are fragments would fail as it currently does, but also would display the delta if there is one. I.e.:

    $ make linux-menuconfig
    [change stuff]
    $ make linux-update-defconfig
    Unable to perform linux-update-defconfig when fragment files are set
    Configuration changes that you want to propagate to one of the fragments:
    -CONFIG_FOO=y
    +# CONFIG_BAR is unset
    linux/linux.mk:511: recipe for target 'linux-update-defconfig' failed
    make[1]: *** [linux-update-defconfig] Error 1

Adding Arnout and Peter in Cc, in case they want to give their opinion on the patch series.

Quoted 4 lines · Yann E. MORIN

I would just suggest that we do not add any new rule, but trying to update the defconfig when there are fragments would fail as it currently does, but also would display the delta if there is one. I.e.:

Quoted 9 lines

    $ make linux-menuconfig
    [change stuff]
    $ make linux-update-defconfig
    Unable to perform linux-update-defconfig when fragment files are set
    Configuration changes that you want to propagate to one of the fragments:
    -CONFIG_FOO=y
    +# CONFIG_BAR is unset
    linux/linux.mk:511: recipe for target 'linux-update-defconfig' failed
    make[1]: *** [linux-update-defconfig] Error 1

I think we could do that in *addition* to having a new rule. Indeed, when you know what you're doing, having to run something that doesn't make sense ("make linux-update-defconfig") and which causes a failure is a bit silly. I'd rather run "make linux-diff-config" (or whatever name we chose).

Marcel, do you think you could rework your patch series to go in the direction of showing a diff rather than arbitrarily adjusting the last fragment ?

Thanks!

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Hello,

Adding Arnout and Peter in Cc, in case they want to give their opinion on the patch series.

Quoted 4 lines · Yann E. MORIN

I would just suggest that we do not add any new rule, but trying to update the defconfig when there are fragments would fail as it currently does, but also would display the delta if there is one. I.e.:

Quoted 9 lines

    $ make linux-menuconfig
    [change stuff]
    $ make linux-update-defconfig
    Unable to perform linux-update-defconfig when fragment files are set
    Configuration changes that you want to propagate to one of the fragments:
    -CONFIG_FOO=y
    +# CONFIG_BAR is unset
    linux/linux.mk:511: recipe for target 'linux-update-defconfig' failed
    make[1]: *** [linux-update-defconfig] Error 1

I think we could do that in *addition* to having a new rule. Indeed, when you know what you're doing, having to run something that doesn't make sense ("make linux-update-defconfig") and which causes a failure is a bit silly. I'd rather run "make linux-diff-config" (or whatever name we chose).

Marcel, do you think you could rework your patch series to go in the direction of showing a diff rather than arbitrarily adjusting the last fragment ?

Quoted 3 lines

Marcel, do you think you could rework your patch series to go in the direction of showing a diff rather than arbitrarily adjusting the last fragment ?

Note that the diff really applies to the last fragment only, in the sense that
if the last fragment is setting CONFIG_BAR, the "is unset" bit has to come after it. I don't think there is a way to say anything sensible automatically except for the last fragment.

Quoted 8 lines

Quoted 3 lines

Marcel, do you think you could rework your patch series to go in the direction of showing a diff rather than arbitrarily adjusting the last fragment ?

Note that the diff really applies to the last fragment only, in the sense that
if the last fragment is setting CONFIG_BAR, the "is unset" bit has to come after it. I don't think there is a way to say anything sensible automatically except for the last fragment.

Yes, the diff cannot be blindly applied to anything but the last fragment. But the idea in showing the diff is not for the user to apply it blindly, but rather to determine which parts of the diff should be reflected to each fragment.

Best regards,

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Indeed, it is not to be considered to be a diff of any of the fragment, but of the configuration *as a whole*.

That's why I initially did not write a full diff, but only the delta snippet, not the context, because it is a diff, not a patch.

But I think we should instead use the scripts/diffconfig script that is in the linux kernel tree, and which outputs looks like:

    $ cat config.000
    CONFIG_FOO=y
    $ cat config.001
    CONFIG_FOO is not set
    $ ./scripts/diffconfig config.000 config.001
     FOO y -> n

which IMNSHO is exactly the message we want to convey.

Quoted 16 lines

Indeed, it is not to be considered to be a diff of any of the fragment, but of the configuration *as a whole*.

That's why I initially did not write a full diff, but only the delta snippet, not the context, because it is a diff, not a patch.

But I think we should instead use the scripts/diffconfig script that is in the linux kernel tree, and which outputs looks like:

    $ cat config.000
    CONFIG_FOO=y
    $ cat config.001
    CONFIG_FOO is not set
    $ ./scripts/diffconfig config.000 config.001
     FOO y -> n

Quoted 1 line

which IMNSHO is exactly the message we want to convey.

Indeed, this looks very good, and diffconfig is an existing tool.

Quoted 1 line

Indeed, this looks very good, and diffconfig is an existing tool.

Note that this was the diffconfig tool frm the kernel. The one we have behave differently (and badly I think), given the same config files as input:

    $ ./utils/diffconfig linux-config.000 linux-config.001
    +IG_FOO n

Which is not nice... :-/