Re: [Patch] BaseTools/Scripts/PatchCheck.py: Remove submodule false positives

["Laszlo Ersek" <lersek@redhat.com>]

On 01/24/20 00:10, Michael D Kinney wrote:
> https://bugzilla.tianocore.org/show_bug.cgi?id=2484
> https://bugzilla.tianocore.org/show_bug.cgi?id=2485
> 
> Update PatchCheck to not enforce no tabs and not enforce CR/LF
> line endings for .gitmodules files.  These files are updated by
> git when a git submodule command is used and the updates by git
> use tab characters and LF line endings.
> 
> Also update patch check to not enforce CR/LF line endings for
> patch lines that create a submodule directory.  These patch
> lines use LF line endings.  The git submodule directory is
> added as a new file with attributes 160000 that can be detected
> by looking for the pattern "new file mode 160000".
> 
> Cc: Bob Feng <bob.c.feng@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> ---
>  BaseTools/Scripts/PatchCheck.py | 23 +++++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/BaseTools/Scripts/PatchCheck.py b/BaseTools/Scripts/PatchCheck.py
> index 173d4517e0..6823cc69bb 100755
> --- a/BaseTools/Scripts/PatchCheck.py
> +++ b/BaseTools/Scripts/PatchCheck.py
> @@ -341,7 +341,21 @@ class GitDiffCheck:
>                  self.state = PRE_PATCH
>                  self.filename = line[13:].split(' ', 1)[0]
>                  self.is_newfile = False
> -                self.force_crlf = not self.filename.endswith('.sh')
> +                self.force_crlf = True
> +                self.force_notabs = True
> +                if self.filename.endswith('.sh'):
> +                    #
> +                    # Do not enforce CR/LF line endings for linux shell scripts.
> +                    #
> +                    self.force_crlf = False
> +                if self.filename == '.gitmodules':
> +                    #
> +                    # .gitmodules is updated by git and uses tabs and LF line
> +                    # endings.  Do not enforce no tabs and do not enforce
> +                    # CR/LF line endings.
> +                    #
> +                    self.force_crlf = False
> +                    self.force_notabs = False
>              elif len(line.rstrip()) != 0:
>                  self.format_error("didn't find diff command")
>              self.line_num += 1
> @@ -355,6 +369,11 @@ class GitDiffCheck:
>                  self.binary = True
>                  if self.is_newfile:
>                      self.new_bin.append(self.filename)
> +            elif line.startswith('new file mode 160000'):
> +                #
> +                # New submodule.  Do not enforce CR/LF line endings
> +                #
> +                self.force_crlf = False
>              else:
>                  ok = False
>                  self.is_newfile = self.newfile_prefix_re.match(line)
> @@ -429,7 +448,7 @@ class GitDiffCheck:
>          if self.force_crlf and eol != '\r\n':
>              self.added_line_error('Line ending (%s) is not CRLF' % repr(eol),
>                                    line)
> -        if '\t' in line:
> +        if self.force_notabs and '\t' in line:
>              self.added_line_error('Tab character used', line)
>          if len(stripped) < len(line):
>              self.added_line_error('Trailing whitespace found', line)
> 

https://github.com/git/git/blob/master/Documentation/technical/index-format.txt

  32-bit mode, split into (high to low bits)

    4-bit object type
      valid values in binary are 1000 (regular file), 1010 (symbolic link)
      and 1110 (gitlink)

    3-bit unused

    9-bit unix permission. Only 0755 and 0644 are valid for regular files.
    Symbolic links and gitlinks have value 0 in this field.

Octal 160000 is E000 hexadecimal. That makes the unused bits and the unix permission bits (12 bits in total) zero, and 0xE is 1110 binary, so the object type is "gitlink".

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo