| Message ID | 20240913171205.22126-6-david.hunter.linux@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/7] linux-kbuild: fix: config option can be bool | expand |
On Sat, Sep 14, 2024 at 2:12 AM David Hunter <david.hunter.linux@gmail.com> wrote: > > Properly implement the config entries that are within the choice keyword > for kconfig. Currently, the script only stops the previous config entry > when a choice keyword is encountered. > > When the keyword "choice" is encountered, do the following: > - distribute the lines immediately following the "choice" > keyword to each config entry inside the "choice" section. > - process the config entries with the distributed lines. > > Signed-off-by: David Hunter <david.hunter.linux@gmail.com> > --- > scripts/kconfig/streamline_config.pl | 40 ++++++++++++++++++++++++++-- > 1 file changed, 38 insertions(+), 2 deletions(-) > > diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl > index 26e544744579..593df824ead7 100755 > --- a/scripts/kconfig/streamline_config.pl > +++ b/scripts/kconfig/streamline_config.pl > @@ -162,6 +162,10 @@ sub read_kconfig { > > my $source = "$ksource/$kconfig"; > my $last_source = ""; > + my $choice_activated = 0; > + my $distribute = 0; > + my $dist_string; > + > > # Check for any environment variables used > while ($source =~ /\$\((\w+)\)/ && $last_source ne $source) { > @@ -214,6 +218,19 @@ sub read_kconfig { > $state = "DEP"; > } > > + if($choice_activated) { > + $distribute = 0; > + my $config_lines = "$_\n" . "$dist_string"; > + my $tmpconfig = ".choice.kconfig"; > + open (my $FH, '>', $tmpconfig); > + print $FH $config_lines; > + close($FH); > + > + read_kconfig($tmpconfig); > + unlink($tmpconfig) or die "Can't delete $tmpconfig: $!\n"; This is ugly. Please do not use the temp file. I believe the only benefit to parse 'choice' block is to propagate its 'depends on' down to member configs. See how the 'if' statement is handled. -- Best Regards Masahiro Yamada
On 9/23/24 23:46, Masahiro Yamada wrote: > > This is ugly. > Please do not use the temp file. > Understood. I found a way to do it that is much more in line with the style of the script and it avoids using the temp file. Oddly enough it involves changing less lines of code as well. I will be submitting the changes soon. Right now, I am just finishing up the change logs for all the patches. My question here is "Is there a general rule as to why the first version was bad?" For example, is it considered bad practice to make temporary files? Thanks, David Hunter
Apologies, I was a little too quick with the send button on my previous email. I forgot that I also wanted to respond to this part as well. On 9/23/24 23:46, Masahiro Yamada wrote: > I believe the only benefit to parse 'choice' block > is to propagate its 'depends on' down to member configs. I am not quite sure this statement is correct. I definitely agree that the main reason to parse the "choice" block is to propagate the "depends"; however, I believe that there is also information for defaults as well as for prompts inside the "choice" blocks. I made a shell script that reads all of the choice blocks in the various Kconfig files. I wanted to know how many of each type of information is in there. Here are the results: - select: 0 - prompts: 152 - defaults: 156 - depends: 72 Also, I should note that while there are no selects inside of a choice, I am unaware if this will always be the case in the future. Here is a link to the shell script that I made: https://github.com/dshunter107/linux-tools/blob/main/check_propagation.sh Thanks, David Hunter
version 2: https://lore.kernel.org/all/20241014141345.5680-6-david.hunter.linux@gmail.com/
diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl index 26e544744579..593df824ead7 100755 --- a/scripts/kconfig/streamline_config.pl +++ b/scripts/kconfig/streamline_config.pl @@ -162,6 +162,10 @@ sub read_kconfig { my $source = "$ksource/$kconfig"; my $last_source = ""; + my $choice_activated = 0; + my $distribute = 0; + my $dist_string; + # Check for any environment variables used while ($source =~ /\$\((\w+)\)/ && $last_source ne $source) { @@ -214,6 +218,19 @@ sub read_kconfig { $state = "DEP"; } + if($choice_activated) { + $distribute = 0; + my $config_lines = "$_\n" . "$dist_string"; + my $tmpconfig = ".choice.kconfig"; + open (my $FH, '>', $tmpconfig); + print $FH $config_lines; + close($FH); + + read_kconfig($tmpconfig); + unlink($tmpconfig) or die "Can't delete $tmpconfig: $!\n"; + } + + # collect the depends for the config } elsif ($state eq "NEW" && /^\s*depends\s+on\s+(.*)$/) { $state = "DEP"; @@ -258,8 +275,27 @@ sub read_kconfig { $iflevel-- if ($iflevel); # stop on "help" and keywords that end a menu entry - } elsif (/^\s*(---)?help(---)?\s*$/ || /^(comment|choice|menu)\b/) { - $state = "NONE"; + } elsif (/^\s*(---)?help(---)?\s*$/ || /^(comment|menu)\b/) { + $state = "NONE"; + + # for choice, distribute the lines before each config entry + # to each config entry + } elsif (/^\s*choice\b/) { + $state = "CHOICE"; + $choice_activated = 1; + $distribute = 1; + } elsif(/^\s*endchoice/) { + $choice_activated = 0; + $dist_string = ""; + } + + if($choice_activated && $distribute) { + # do not put 'choice' inside of string to distribute + if($state eq "CHOICE") { + $state = "NONE"; + } else { + $dist_string .= "$_\n"; + } } } close($kinfile);
Properly implement the config entries that are within the choice keyword for kconfig. Currently, the script only stops the previous config entry when a choice keyword is encountered. When the keyword "choice" is encountered, do the following: - distribute the lines immediately following the "choice" keyword to each config entry inside the "choice" section. - process the config entries with the distributed lines. Signed-off-by: David Hunter <david.hunter.linux@gmail.com> --- scripts/kconfig/streamline_config.pl | 40 ++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-)