1126228 - Directory (--directory) argument for |mach warnings-list| and |mach warnings-summary|

Status: RESOLVED FIXED

(Firefox Build System :: Mach Core, enhancement)

Description

[Mitchell Field [:Mitch]]

mach's warnings-list and warnings-summary commands could have a --directory argument to narrow down which warnings are where.

Comment 1

[Mitchell Field [:Mitch]]

Attachment: Patch v1

This patch adds a directory argument (--directory, -C) for |mach warnings-list| and |mach warnings-summary|.
It's a bit rough; directories are always recursed and relative paths aren't handled.
I've also added a warning flags argument (--flags) for warnings-list while I was at it. Example for MSVC warning C4267 in nsprpub:

>mach warnings-list --directory nsprpub --flags C4267

Comment 2

[Gregory Szorc [:gps]]

Comment on attachment 8555172 [details] [diff] [review]
Patch v1

Review of attachment 8555172 [details] [diff] [review]:
-----------------------------------------------------------------

Nice feature addition!

Just minor nits in the code.

Should be a rubber stamp on next review.

::: python/mozbuild/mozbuild/compilation/warnings.py
@@ +153,5 @@
>          types = {}
>          for value in self._files.values():
>              for warning in value['warnings']:
> +                if dirpath is not None and not warning['filename'].startswith(dirpath):
> +                    continue

if dirpath and not ...

I can't remember if the paths on Windows are normalized to posix style or not. You should use mozpack.path to do path normalization is a platform independent way. https://dxr.mozilla.org/mozilla-central/source/python/mozbuild/mozpack/path.py

::: python/mozbuild/mozbuild/mach_commands.py
@@ +626,3 @@
>          database = self.database
>  
> +        if directory is not None:

if directory:

@@ +626,5 @@
>          database = self.database
>  
> +        if directory is not None:
> +            if directory.startswith('/'):
> +                directory = directory[1:]

This assumes posix line endings. But it's a command argument, so that's probably fine since we develop in a posix-like shell on windows.

@@ +629,5 @@
> +            if directory.startswith('/'):
> +                directory = directory[1:]
> +
> +            dirpath = os.path.join(self.topsrcdir, directory)
> +            print(dirpath)

did you mean for this print statement to be in the final output?

@@ +669,5 @@
> +
> +            dirpath = mozpath.join(self.topsrcdir, directory)
> +            if not os.path.isdir(dirpath):
> +                print('Specified directory not found.')
> +                return 1

There is a bit of duplicate code here. Extract into its own function?

@@ +671,5 @@
> +            if not os.path.isdir(dirpath):
> +                print('Specified directory not found.')
> +                return 1
> +
> +        if flags is not None:

if flags

@@ +673,5 @@
> +                return 1
> +
> +        if flags is not None:
> +            # Flatten lists of flags
> +            flags = list(itertools.chain(*[flaglist.split(',') for flaglist in flags]))

You probably want a set() here. Membership testing in sets is much, much faster in Python.

@@ +681,5 @@
>  
>              if filename.startswith(self.topsrcdir):
>                  filename = filename[len(self.topsrcdir) + 1:]
>  
> +            if directory is not None:

if directory:

@@ +685,5 @@
> +            if directory is not None:
> +                if not filename.startswith(directory):
> +                    continue
> +
> +            if flags is not None and warning['flag'] not in flags:

if flags and ...

Comment 3

[Mitchell Field [:Mitch]]

Attachment: Patch v2

I believe the nits have been fixed.

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8e3baacbe31b

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8e3baacbe31b