On Mon, 14 Oct 2024 10:13:36 -0400 David Hunter <david.hunter.linux@xxxxxxxxx> wrote: > An assumption made in this script is that the config options do not need > to be processed because they will simply be in the new config file. This > assumption is incorrect. > > Process the config entries set to "y" because those config entries might > have dependencies set to "m". If a config entry is set to "m" and is not > loaded directly into the machine, the script will currently turn off > that config entry; however, if that turned off config entry is a > dependency for a "y" option. that means the config entry set to "y" > will also be turned off later when the conf executive file is called. > > Here is a model of the problem (arrows show dependency): > > Original config file > Config_1 (m) <-- Config_2 (y) > > Config_1 is not loaded in this example, so it is turned off. > After scripts/kconfig/streamline_config.pl, but before scripts/kconfig/conf > Config_1 (n) <-- Config_2 (y) > > After scripts/kconfig/conf > Config_1 (n) <-- Config_2 (n) Hmm, can you show the example of this. My worry here is that there are a lot of cases that have: Config_1 (m) <-- Config_2 (y) Where Config_2 is just some binary option to modify the module set by Config_1. If we keep Config_1 because there's some 'y' that selects it, we are going to keep a lot of unused modules around. > > It should also be noted that any module in the dependency chain will > also be turned off, even if that module is loaded directly onto the > computer. Here is an example: > > Original config file > Config_1 (m) <-- Config_2 (y) <-- Config_3 (m) If Config_3 is used, then it should add Config_2 as a dependency. I guess that's what this patch does. > > Config_3 will be loaded in this example. > After scripts/kconfig/streamline_config.pl, but before scripts/kconfig/conf > Config_1 (n) <-- Config_2 (y) <-- Config_3 (m) > > After scripts/kconfig/conf > Config_1 (n) <-- Config_2 (n) <-- Config_3 (n) > > Signed-off-by: David Hunter <david.hunter.linux@xxxxxxxxx> > --- > V1 https://lore.kernel.org/all/20240913171205.22126-8-david.hunter.linux@xxxxxxxxx/ > > V2 > - change subject > - changed part of the code that only processed config options > set to "m" so that config options set to "y" are processed. I > forgot to put this in v1 when making the series patch. > - removed an unneccessary condition that would have skipped a > dependency loop. > --- > scripts/kconfig/streamline_config.pl | 15 +++++++-------- > 1 file changed, 7 insertions(+), 8 deletions(-) > > diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl > index 4149c4b344db..c3b434220c9f 100755 > --- a/scripts/kconfig/streamline_config.pl > +++ b/scripts/kconfig/streamline_config.pl > @@ -473,6 +473,11 @@ foreach my $line (@config_file) { > > if (/(CONFIG_[$valid]*)=(m|y)/) { > $orig_configs{$1} = $2; > + # all configs options set to 'y' need to be processed > + if ($2 eq "y") { > + $configs{$1}= $2; > + } > + > } > } > > @@ -499,8 +504,8 @@ sub parse_config_depends > > $p =~ s/^[^$valid]*[$valid]+//; > > - # We only need to process if the depend config is a module > - if (!defined($orig_configs{$conf}) || $orig_configs{$conf} eq "y") { > + # Make sure that this config exists in the current .config file > + if (!defined($orig_configs{$conf})) { > next; > } > > @@ -600,12 +605,6 @@ sub loop_depend { > > forloop: > foreach my $config (keys %configs) { > - > - # If this config is not a module, we do not need to process it > - if (defined($orig_configs{$config}) && $orig_configs{$config} ne "m") { > - next forloop; > - } > - OK, and I guess you tests show this doesn't have any other issues. I'll check it out. Thanks, -- Steve > $config =~ s/^CONFIG_//; > $depconfig = $config; >
