2015-04-29 14:58:41

by Valentin Rothberg

[permalink] [raw]
Subject: [PATCH] checkkconfigsymbols.py: add option -i to ignore files

Sometimes a user might be interested to filter certain reports (e.g.,
the many defconfigs). Now, this can be achieved by specifying a Python
regex with -i / --ignore.

Signed-off-by: Valentin Rothberg <[email protected]>
---
scripts/checkkconfigsymbols.py | 32 ++++++++++++++++++++++++++------
1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/scripts/checkkconfigsymbols.py b/scripts/checkkconfigsymbols.py
index 74086a583d8d..f35c8ac5d9a0 100755
--- a/scripts/checkkconfigsymbols.py
+++ b/scripts/checkkconfigsymbols.py
@@ -58,6 +58,12 @@ def parse_options():
"input format bases on Git log's "
"\'commmit1..commit2\'.")

+ parser.add_option('-i', '--ignore', dest='ignore', action='store',
+ default="",
+ help="Ignore files matching this pattern. Note that "
+ "the pattern needs to be a Python regex. To "
+ "ignore defconfigs, specify -i '.*defconfig'.")
+
parser.add_option('', '--force', dest='force', action='store_true',
default=False,
help="Reset current Git tree even when it's dirty.")
@@ -80,6 +86,12 @@ def parse_options():
"'--force' if you\nwant to ignore this warning and "
"continue.")

+ if opts.ignore:
+ try:
+ re.match(opts.ignore, "this/is/just/a/test.c")
+ except:
+ sys.exit("Please specify a valid Python regex.")
+
return opts


@@ -105,11 +117,11 @@ def main():

# get undefined items before the commit
execute("git reset --hard %s" % commit_a)
- undefined_a = check_symbols()
+ undefined_a = check_symbols(opts.ignore)

# get undefined items for the commit
execute("git reset --hard %s" % commit_b)
- undefined_b = check_symbols()
+ undefined_b = check_symbols(opts.ignore)

# report cases that are present for the commit but not before
for feature in sorted(undefined_b):
@@ -129,7 +141,7 @@ def main():

# default to check the entire tree
else:
- undefined = check_symbols()
+ undefined = check_symbols(opts.ignore)
for feature in sorted(undefined):
files = sorted(undefined.get(feature))
print "%s\t%s" % (feature, ", ".join(files))
@@ -160,9 +172,10 @@ def get_head():
return stdout.strip('\n')


-def check_symbols():
+def check_symbols(ignore):
"""Find undefined Kconfig symbols and return a dict with the symbol as key
- and a list of referencing files as value."""
+ and a list of referencing files as value. Files matching %ignore are not
+ checked for undefined symbols."""
source_files = []
kconfig_files = []
defined_features = set()
@@ -185,10 +198,17 @@ def check_symbols():
source_files.append(gitfile)

for sfile in source_files:
+ if ignore and re.match(ignore, sfile):
+ # do not check files matching %ignore
+ continue
parse_source_file(sfile, referenced_features)

for kfile in kconfig_files:
- parse_kconfig_file(kfile, defined_features, referenced_features)
+ if ignore and re.match(ignore, kfile):
+ # do not collect references for files matching %ignore
+ parse_kconfig_file(kfile, defined_features, dict())
+ else:
+ parse_kconfig_file(kfile, defined_features, referenced_features)

undefined = {} # {feature: [files]}
for feature in sorted(referenced_features):
--
2.1.4


2015-05-01 19:31:42

by Paul Bolle

[permalink] [raw]
Subject: Re: [PATCH] checkkconfigsymbols.py: add option -i to ignore files

Valentin Rothberg schreef op wo 29-04-2015 om 16:58 [+0200]:
> Sometimes a user might be interested to filter certain reports (e.g.,
> the many defconfigs).

Is this actually useful outside of filtering out defconfigs?

> Now, this can be achieved by specifying a Python
> regex with -i / --ignore.

Patch hijack: it's been my view for some time now that almost all
defconfigs are outdated in one way or another. And they are outdated
because they are not, as far as I can tell, updated regularly. So I
wonder whether there are any guidelines for defconfigs. What is their
purpose? What exactly can one expect when using a defconfig?


Paul Bolle

2015-05-01 19:45:37

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] checkkconfigsymbols.py: add option -i to ignore files

On Fri, May 01, 2015 at 09:31:36PM +0200, Paul Bolle wrote:
> Valentin Rothberg schreef op wo 29-04-2015 om 16:58 [+0200]:
> > Sometimes a user might be interested to filter certain reports (e.g.,
> > the many defconfigs).
>
> Is this actually useful outside of filtering out defconfigs?
>
> > Now, this can be achieved by specifying a Python
> > regex with -i / --ignore.
>
> Patch hijack: it's been my view for some time now that almost all
> defconfigs are outdated in one way or another. And they are outdated
> because they are not, as far as I can tell, updated regularly. So I
> wonder whether there are any guidelines for defconfigs. What is their
> purpose? What exactly can one expect when using a defconfig?

When ever I bring this up, someone says, "but we need it to define how
to select the options for my arch / board!" How real this is is really
unknown to me but the arches other than x86 seem to insist that they are
relevant.

thanks,

greg k-h

2015-05-01 20:13:36

by Valentin Rothberg

[permalink] [raw]
Subject: Re: [PATCH] checkkconfigsymbols.py: add option -i to ignore files

On Fri, May 1, 2015 at 9:31 PM, Paul Bolle <[email protected]> wrote:
> Valentin Rothberg schreef op wo 29-04-2015 om 16:58 [+0200]:
>> Sometimes a user might be interested to filter certain reports (e.g.,
>> the many defconfigs).
>
> Is this actually useful outside of filtering out defconfigs?

It's a regex, so we can filter entire paths as well (e.g., -i
'arch/.*' to ignore all issues in arch/). Until now, I only used it
to get rid of all the defconfigs.

>> Now, this can be achieved by specifying a Python
>> regex with -i / --ignore.
>
> Patch hijack: it's been my view for some time now that almost all
> defconfigs are outdated in one way or another. And they are outdated
> because they are not, as far as I can tell, updated regularly. So I
> wonder whether there are any guidelines for defconfigs. What is their
> purpose? What exactly can one expect when using a defconfig?
>

As far as I know, it's really hard to manually configure certain
boards. With defconfigs, only few people have to go through the fire.
Two years ago I tried to manually select a kernel configuration for my
Nexus 7 and failed desperately since some feature constraints are just
not visible/understandable from the menu. Not that I am an ARM
developer, but there I understood the need to have a defconfig : )

Kind regards,
Valentin

> Paul Bolle
>

2015-05-01 20:30:42

by Paul Bolle

[permalink] [raw]
Subject: Re: [PATCH] checkkconfigsymbols.py: add option -i to ignore files

Greg KH schreef op vr 01-05-2015 om 21:45 [+0200]:
> On Fri, May 01, 2015 at 09:31:36PM +0200, Paul Bolle wrote:
> > Patch hijack: it's been my view for some time now that almost all
> > defconfigs are outdated in one way or another. And they are outdated
> > because they are not, as far as I can tell, updated regularly. So I
> > wonder whether there are any guidelines for defconfigs. What is their
> > purpose? What exactly can one expect when using a defconfig?
>
> When ever I bring this up, someone says, "but we need it to define how
> to select the options for my arch / board!" How real this is is really
> unknown to me but the arches other than x86 seem to insist that they are
> relevant.

Perhaps I should, just for fun, rebuild v4.1-rc1 using a x86 defconfig
and see what happens. Note that I'm typing this on v4.1-rc1 using an
x86 .config based on what Fedora ships for 21 (which is, on its turn,
based on 3.19.5).


Paul Bolle

2015-05-01 20:45:36

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] checkkconfigsymbols.py: add option -i to ignore files

On Fri, May 01, 2015 at 10:30:35PM +0200, Paul Bolle wrote:
> Greg KH schreef op vr 01-05-2015 om 21:45 [+0200]:
> > On Fri, May 01, 2015 at 09:31:36PM +0200, Paul Bolle wrote:
> > > Patch hijack: it's been my view for some time now that almost all
> > > defconfigs are outdated in one way or another. And they are outdated
> > > because they are not, as far as I can tell, updated regularly. So I
> > > wonder whether there are any guidelines for defconfigs. What is their
> > > purpose? What exactly can one expect when using a defconfig?
> >
> > When ever I bring this up, someone says, "but we need it to define how
> > to select the options for my arch / board!" How real this is is really
> > unknown to me but the arches other than x86 seem to insist that they are
> > relevant.
>
> Perhaps I should, just for fun, rebuild v4.1-rc1 using a x86 defconfig
> and see what happens. Note that I'm typing this on v4.1-rc1 using an
> x86 .config based on what Fedora ships for 21 (which is, on its turn,
> based on 3.19.5).

We know x86 defconfig doesn't really do much, if anything, these days.
It used to be just a copy of Linus's configuration file, but given that
the last time it was seriously updated was 2009, I doubt that it can
even boot a modern x86 machine these days :)

good luck!

greg k-h

2015-05-01 20:49:20

by Paul Bolle

[permalink] [raw]
Subject: Re: [PATCH] checkkconfigsymbols.py: add option -i to ignore files

[Added Russell, because I, sort, of drop his name.]

Valentin Rothberg schreef op vr 01-05-2015 om 22:13 [+0200]:
> On Fri, May 1, 2015 at 9:31 PM, Paul Bolle <[email protected]> wrote:
> > Valentin Rothberg schreef op wo 29-04-2015 om 16:58 [+0200]:
> >> Sometimes a user might be interested to filter certain reports (e.g.,
> >> the many defconfigs).
> >
> > Is this actually useful outside of filtering out defconfigs?
>
> It's a regex, so we can filter entire paths as well (e.g., -i
> 'arch/.*' to ignore all issues in arch/). Until now, I only used it
> to get rid of all the defconfigs.

So, perhaps we're better off by just skipping defconfigs?

> As far as I know, it's really hard to manually configure certain
> boards. With defconfigs, only few people have to go through the fire.
> Two years ago I tried to manually select a kernel configuration for my
> Nexus 7 and failed desperately since some feature constraints are just
> not visible/understandable from the menu. Not that I am an ARM
> developer, but there I understood the need to have a defconfig : )

See https://lkml.org/lkml/2012/1/18/355 . Manually configuring from
scratch is, I think, simply not doable. About the only advice I'd dare
to give someone would be: somehow get a .config that works for your
machine, however old that .config might be, and use it as your base.
Probably by doing
yes "" | make oldconfig >/dev/null

So I guess my question is: is a defconfig to be considered a ".config
that works for your machine"? And, yes, I realize "works" is a very
broad goal.


Paul Bolle

2015-05-01 21:05:24

by Valentin Rothberg

[permalink] [raw]
Subject: Re: [PATCH] checkkconfigsymbols.py: add option -i to ignore files

On Fri, May 1, 2015 at 10:49 PM, Paul Bolle <[email protected]> wrote:
> [Added Russell, because I, sort, of drop his name.]
>
> Valentin Rothberg schreef op vr 01-05-2015 om 22:13 [+0200]:
>> On Fri, May 1, 2015 at 9:31 PM, Paul Bolle <[email protected]> wrote:
>> > Valentin Rothberg schreef op wo 29-04-2015 om 16:58 [+0200]:
>> >> Sometimes a user might be interested to filter certain reports (e.g.,
>> >> the many defconfigs).
>> >
>> > Is this actually useful outside of filtering out defconfigs?
>>
>> It's a regex, so we can filter entire paths as well (e.g., -i
>> 'arch/.*' to ignore all issues in arch/). Until now, I only used it
>> to get rid of all the defconfigs.
>
> So, perhaps we're better off by just skipping defconfigs?

I remember Greg writing that some people do actually clean up some
defconfigs, so I'd like to keep the door open.

When I do the daily ' $ checkkconfigsymbols.py --diff
yesterday..today-next ' I see regularly that patches that remove a
Kconfig option do not remove references in defconfigs. Ideally (i.e.,
in my ideal world), people check their patch with checkkconfigsymbols
and see that some defconfigs are dirty and also fix that.

Kind regards,
Valentin

>> As far as I know, it's really hard to manually configure certain
>> boards. With defconfigs, only few people have to go through the fire.
>> Two years ago I tried to manually select a kernel configuration for my
>> Nexus 7 and failed desperately since some feature constraints are just
>> not visible/understandable from the menu. Not that I am an ARM
>> developer, but there I understood the need to have a defconfig : )
>
> See https://lkml.org/lkml/2012/1/18/355 . Manually configuring from
> scratch is, I think, simply not doable. About the only advice I'd dare
> to give someone would be: somehow get a .config that works for your
> machine, however old that .config might be, and use it as your base.
> Probably by doing
> yes "" | make oldconfig >/dev/null
>
> So I guess my question is: is a defconfig to be considered a ".config
> that works for your machine"? And, yes, I realize "works" is a very
> broad goal.
>
>
> Paul Bolle
>