[libcamera-devel,8/8] utils: checkstyle.py: Add header add checker
diff mbox series

Message ID 20201224122855.22200-9-laurent.pinchart@ideasonboard.com
State Accepted
Delegated to: Laurent Pinchart
Headers show
Series
  • checkstyle.py: Ensure meson.build is updated when adding header
Related show

Commit Message

Laurent Pinchart Dec. 24, 2020, 12:28 p.m. UTC
Add a commit checker that ensures that all header files added to the
libcamera includes (public or internal) are accompanied by a
corresponding update of the meson.build file in the same directory.

Here's the output of the new checker when run against a commit that
forgot to update meson.build.

    $ ./utils/checkstyle.py b3383da79f1d
    ---------------------------------------------------------------------------------
    b3383da79f1d513b0d76db220a7104e1c1035e30 libcamera: buffer: Create a MappedBuffer
    ---------------------------------------------------------------------------------
    Header include/libcamera/internal/buffer.h added without corresponding update to include/libcamera/internal/meson.build
    ---
    1 potential issue detected, please review

In theory we could extend the checker to cover .cpp files too, but the
issue will be quite noticeable as meson won't build the file if
meson.build isn't updated. Header files are more tricky as problems
would only occur at when installing the headers (for public headers), or
would result in race conditions in the build. Both of those issues are
harder to catch.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 utils/checkstyle.py | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

Comments

Niklas Söderlund Dec. 27, 2020, 10:51 a.m. UTC | #1
Hi Laurent,

Thanks for the new checker, if only I could create one for my bad 
spelling ;-P

On 2020-12-24 14:28:55 +0200, Laurent Pinchart wrote:
> Add a commit checker that ensures that all header files added to the
> libcamera includes (public or internal) are accompanied by a
> corresponding update of the meson.build file in the same directory.
> 
> Here's the output of the new checker when run against a commit that
> forgot to update meson.build.
> 
>     $ ./utils/checkstyle.py b3383da79f1d
>     ---------------------------------------------------------------------------------
>     b3383da79f1d513b0d76db220a7104e1c1035e30 libcamera: buffer: Create a MappedBuffer
>     ---------------------------------------------------------------------------------
>     Header include/libcamera/internal/buffer.h added without corresponding update to include/libcamera/internal/meson.build
>     ---
>     1 potential issue detected, please review
> 
> In theory we could extend the checker to cover .cpp files too, but the
> issue will be quite noticeable as meson won't build the file if
> meson.build isn't updated. Header files are more tricky as problems
> would only occur at when installing the headers (for public headers), or
> would result in race conditions in the build. Both of those issues are
> harder to catch.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  utils/checkstyle.py | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/utils/checkstyle.py b/utils/checkstyle.py
> index c0a6b7ab06cd..e618db937c2b 100755
> --- a/utils/checkstyle.py
> +++ b/utils/checkstyle.py
> @@ -320,6 +320,50 @@ class CommitIssue(object):
>          self.msg = msg
>  
>  
> +class HeaderAddChecker(CommitChecker):
> +    @classmethod
> +    def check(cls, commit, top_level):
> +        issues = []
> +
> +        meson_files = [f for f in commit.files('M')
> +                       if os.path.basename(f) == 'meson.build']
> +
> +        for filename in commit.files('A'):
> +            if not filename.startswith('include/libcamera/') or \
> +               not filename.endswith('.h'):
> +                continue
> +
> +            meson = os.path.dirname(filename) + '/meson.build'
> +            header = os.path.basename(filename)
> +
> +            issue = CommitIssue('Header %s added without corresponding update to %s' %
> +                                (filename, meson))
> +
> +            if meson not in meson_files:
> +                issues.append(issue)
> +                continue
> +
> +            diff = commit.get_diff(top_level, meson)
> +            found = False
> +
> +            for hunk in diff:
> +                for line in hunk.lines:
> +                    if line[0] != '+':
> +                        continue
> +
> +                    if line.find("'%s'" % header) != -1:
> +                        found = True
> +                        break
> +
> +                if found:
> +                    break
> +
> +            if not found:
> +                issues.append(issue)
> +
> +        return issues
> +
> +
>  # ------------------------------------------------------------------------------
>  # Style Checkers
>  #
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Kieran Bingham Dec. 29, 2020, 2:56 p.m. UTC | #2
Hi Niklas,

On 27/12/2020 10:51, Niklas Söderlund wrote:
> Hi Laurent,
> 
> Thanks for the new checker, if only I could create one for my bad 
> spelling ;-P

I started one,trying to use the python 'enchant' package, but the
performance was ... absolutely appalling :-(

I look forward to the one you create ;-)

--
Kieran



> On 2020-12-24 14:28:55 +0200, Laurent Pinchart wrote:
>> Add a commit checker that ensures that all header files added to the
>> libcamera includes (public or internal) are accompanied by a
>> corresponding update of the meson.build file in the same directory.
>>
>> Here's the output of the new checker when run against a commit that
>> forgot to update meson.build.
>>
>>     $ ./utils/checkstyle.py b3383da79f1d
>>     ---------------------------------------------------------------------------------
>>     b3383da79f1d513b0d76db220a7104e1c1035e30 libcamera: buffer: Create a MappedBuffer
>>     ---------------------------------------------------------------------------------
>>     Header include/libcamera/internal/buffer.h added without corresponding update to include/libcamera/internal/meson.build
>>     ---
>>     1 potential issue detected, please review
>>
>> In theory we could extend the checker to cover .cpp files too, but the
>> issue will be quite noticeable as meson won't build the file if
>> meson.build isn't updated. Header files are more tricky as problems
>> would only occur at when installing the headers (for public headers), or
>> would result in race conditions in the build. Both of those issues are
>> harder to catch.
>>
>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> 
>> ---
>>  utils/checkstyle.py | 44 ++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 44 insertions(+)
>>
>> diff --git a/utils/checkstyle.py b/utils/checkstyle.py
>> index c0a6b7ab06cd..e618db937c2b 100755
>> --- a/utils/checkstyle.py
>> +++ b/utils/checkstyle.py
>> @@ -320,6 +320,50 @@ class CommitIssue(object):
>>          self.msg = msg
>>  
>>  
>> +class HeaderAddChecker(CommitChecker):
>> +    @classmethod
>> +    def check(cls, commit, top_level):
>> +        issues = []
>> +
>> +        meson_files = [f for f in commit.files('M')
>> +                       if os.path.basename(f) == 'meson.build']
>> +
>> +        for filename in commit.files('A'):
>> +            if not filename.startswith('include/libcamera/') or \
>> +               not filename.endswith('.h'):
>> +                continue
>> +
>> +            meson = os.path.dirname(filename) + '/meson.build'
>> +            header = os.path.basename(filename)
>> +
>> +            issue = CommitIssue('Header %s added without corresponding update to %s' %
>> +                                (filename, meson))
>> +
>> +            if meson not in meson_files:
>> +                issues.append(issue)
>> +                continue
>> +
>> +            diff = commit.get_diff(top_level, meson)
>> +            found = False
>> +
>> +            for hunk in diff:
>> +                for line in hunk.lines:
>> +                    if line[0] != '+':
>> +                        continue
>> +
>> +                    if line.find("'%s'" % header) != -1:
>> +                        found = True
>> +                        break
>> +
>> +                if found:
>> +                    break
>> +
>> +            if not found:
>> +                issues.append(issue)
>> +
>> +        return issues
>> +
>> +
>>  # ------------------------------------------------------------------------------
>>  # Style Checkers
>>  #
>> -- 
>> Regards,
>>
>> Laurent Pinchart
>>
>> _______________________________________________
>> libcamera-devel mailing list
>> libcamera-devel@lists.libcamera.org
>> https://lists.libcamera.org/listinfo/libcamera-devel
>

Patch
diff mbox series

diff --git a/utils/checkstyle.py b/utils/checkstyle.py
index c0a6b7ab06cd..e618db937c2b 100755
--- a/utils/checkstyle.py
+++ b/utils/checkstyle.py
@@ -320,6 +320,50 @@  class CommitIssue(object):
         self.msg = msg
 
 
+class HeaderAddChecker(CommitChecker):
+    @classmethod
+    def check(cls, commit, top_level):
+        issues = []
+
+        meson_files = [f for f in commit.files('M')
+                       if os.path.basename(f) == 'meson.build']
+
+        for filename in commit.files('A'):
+            if not filename.startswith('include/libcamera/') or \
+               not filename.endswith('.h'):
+                continue
+
+            meson = os.path.dirname(filename) + '/meson.build'
+            header = os.path.basename(filename)
+
+            issue = CommitIssue('Header %s added without corresponding update to %s' %
+                                (filename, meson))
+
+            if meson not in meson_files:
+                issues.append(issue)
+                continue
+
+            diff = commit.get_diff(top_level, meson)
+            found = False
+
+            for hunk in diff:
+                for line in hunk.lines:
+                    if line[0] != '+':
+                        continue
+
+                    if line.find("'%s'" % header) != -1:
+                        found = True
+                        break
+
+                if found:
+                    break
+
+            if not found:
+                issues.append(issue)
+
+        return issues
+
+
 # ------------------------------------------------------------------------------
 # Style Checkers
 #