[PATCH] sort-makefile-lines.py: Allow '_' in name and "^# name"

[PATCH] sort-makefile-lines.py: Allow '_' in name and "^# name"

[H.J. Lu]

'_' is used in Makefile variable names and many variables end with
"^# name".  Relax sort-makefile-lines.py to allow '_' in name and
"^# name" as variable end.  This fixes BZ #31385.
---
 scripts/sort-makefile-lines.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/scripts/sort-makefile-lines.py b/scripts/sort-makefile-lines.py
index f65ee40e27..ea02412d67 100755
--- a/scripts/sort-makefile-lines.py
+++ b/scripts/sort-makefile-lines.py
@@ -129,7 +129,7 @@ def sort_makefile_lines():
     for i in range(len(lines)):
         # Look for things like "var = \", "var := \" or "var += \"
         # to start the sorted list.
-        var = re.search(r'^([a-zA-Z0-9-]*) [\+:]?\= \\$', lines[i])
+        var = re.search(r'^([a-zA-Z0-9-_]*) [\+:]?\= \\$', lines[i])
         if var:
             # Remember the index and the name.
             startmarks.append((i, var.group(1)))
@@ -139,8 +139,8 @@ def sort_makefile_lines():
     # must have the matching comment name for it to be valid.
     rangemarks = []
     for sm in startmarks:
-        # Look for things like "  # var" to end the sorted list.
-        reg = r'^  # ' + sm[1] + r'$'
+        # Look for things like "^ *# var" to end the sorted list.
+        reg = r'^ *# ' + sm[1] + r'$'
         for j in range(sm[0] + 1, len(lines)):
             if re.search(reg, lines[j]):
                 # Remember the block to sort (inclusive).
-- 
2.43.0

Re: [PATCH] sort-makefile-lines.py: Allow '_' in name and "^# name"

[Florian Weimer]

* H. J. Lu:

> '_' is used in Makefile variable names and many variables end with
> "^# name".  Relax sort-makefile-lines.py to allow '_' in name and
> "^# name" as variable end.  This fixes BZ #31385.
> ---
>  scripts/sort-makefile-lines.py | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/sort-makefile-lines.py b/scripts/sort-makefile-lines.py
> index f65ee40e27..ea02412d67 100755
> --- a/scripts/sort-makefile-lines.py
> +++ b/scripts/sort-makefile-lines.py
> @@ -129,7 +129,7 @@ def sort_makefile_lines():
>      for i in range(len(lines)):
>          # Look for things like "var = \", "var := \" or "var += \"
>          # to start the sorted list.
> -        var = re.search(r'^([a-zA-Z0-9-]*) [\+:]?\= \\$', lines[i])
> +        var = re.search(r'^([a-zA-Z0-9-_]*) [\+:]?\= \\$', lines[i])
>          if var:
>              # Remember the index and the name.
>              startmarks.append((i, var.group(1)))

Please keep the literal - at the end of the bracket expression.  I think
it's easier to read even if it may be semantically the same.

Thanks,
Florian

Re: [PATCH] sort-makefile-lines.py: Allow '_' in name and "^# name"

[H.J. Lu]

On Thu, Feb 15, 2024 at 4:16 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > '_' is used in Makefile variable names and many variables end with
> > "^# name".  Relax sort-makefile-lines.py to allow '_' in name and
> > "^# name" as variable end.  This fixes BZ #31385.
> > ---
> >  scripts/sort-makefile-lines.py | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/scripts/sort-makefile-lines.py b/scripts/sort-makefile-lines.py
> > index f65ee40e27..ea02412d67 100755
> > --- a/scripts/sort-makefile-lines.py
> > +++ b/scripts/sort-makefile-lines.py
> > @@ -129,7 +129,7 @@ def sort_makefile_lines():
> >      for i in range(len(lines)):
> >          # Look for things like "var = \", "var := \" or "var += \"
> >          # to start the sorted list.
> > -        var = re.search(r'^([a-zA-Z0-9-]*) [\+:]?\= \\$', lines[i])
> > +        var = re.search(r'^([a-zA-Z0-9-_]*) [\+:]?\= \\$', lines[i])
> >          if var:
> >              # Remember the index and the name.
> >              startmarks.append((i, var.group(1)))
>
> Please keep the literal - at the end of the bracket expression.  I think
> it's easier to read even if it may be semantically the same.

Did you mean like this?

diff --git a/scripts/sort-makefile-lines.py b/scripts/sort-makefile-lines.py
index f65ee40e27..d791789671 100755
--- a/scripts/sort-makefile-lines.py
+++ b/scripts/sort-makefile-lines.py
@@ -129,7 +129,7 @@ def sort_makefile_lines():
     for i in range(len(lines)):
         # Look for things like "var = \", "var := \" or "var += \"
         # to start the sorted list.
-        var = re.search(r'^([a-zA-Z0-9-]*) [\+:]?\= \\$', lines[i])
+        var = re.search(r'^([-_a-zA-Z0-9]*) [\+:]?\= \\$', lines[i])
         if var:
             # Remember the index and the name.
             startmarks.append((i, var.group(1)))

> Thanks,
> Florian
>


-- 
H.J.

Re: [PATCH] sort-makefile-lines.py: Allow '_' in name and "^# name"

[Carlos O'Donell]

On 2/15/24 07:19, H.J. Lu wrote:
> On Thu, Feb 15, 2024 at 4:16 AM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * H. J. Lu:
>>
>>> '_' is used in Makefile variable names and many variables end with
>>> "^# name".  Relax sort-makefile-lines.py to allow '_' in name and
>>> "^# name" as variable end.  This fixes BZ #31385.
>>> ---
>>>  scripts/sort-makefile-lines.py | 6 +++---
>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/scripts/sort-makefile-lines.py b/scripts/sort-makefile-lines.py
>>> index f65ee40e27..ea02412d67 100755
>>> --- a/scripts/sort-makefile-lines.py
>>> +++ b/scripts/sort-makefile-lines.py
>>> @@ -129,7 +129,7 @@ def sort_makefile_lines():
>>>      for i in range(len(lines)):
>>>          # Look for things like "var = \", "var := \" or "var += \"
>>>          # to start the sorted list.
>>> -        var = re.search(r'^([a-zA-Z0-9-]*) [\+:]?\= \\$', lines[i])
>>> +        var = re.search(r'^([a-zA-Z0-9-_]*) [\+:]?\= \\$', lines[i])
>>>          if var:
>>>              # Remember the index and the name.
>>>              startmarks.append((i, var.group(1)))
>>
>> Please keep the literal - at the end of the bracket expression.  I think
>> it's easier to read even if it may be semantically the same.
> 
> Did you mean like this?
> 
> diff --git a/scripts/sort-makefile-lines.py b/scripts/sort-makefile-lines.py
> index f65ee40e27..d791789671 100755
> --- a/scripts/sort-makefile-lines.py
> +++ b/scripts/sort-makefile-lines.py
> @@ -129,7 +129,7 @@ def sort_makefile_lines():
>      for i in range(len(lines)):
>          # Look for things like "var = \", "var := \" or "var += \"
>          # to start the sorted list.
> -        var = re.search(r'^([a-zA-Z0-9-]*) [\+:]?\= \\$', lines[i])
> +        var = re.search(r'^([-_a-zA-Z0-9]*) [\+:]?\= \\$', lines[i])

Yes, exactly, I agree with Florian here you want to show that the literal '-' and '_'
are not part of a range in the character class. So either start or end is fine with
me.

Looking forward to v2. Thanks for fixing this!

>          if var:
>              # Remember the index and the name.
>              startmarks.append((i, var.group(1)))
> 
>> Thanks,
>> Florian
>>
> 
> 

-- 
Cheers,
Carlos.

Re: [PATCH] sort-makefile-lines.py: Allow '_' in name and "^# name"

[Andreas Schwab]

On Feb 15 2024, H.J. Lu wrote:

> @@ -139,8 +139,8 @@ def sort_makefile_lines():
>      # must have the matching comment name for it to be valid.
>      rangemarks = []
>      for sm in startmarks:
> -        # Look for things like "  # var" to end the sorted list.
> -        reg = r'^  # ' + sm[1] + r'$'
> +        # Look for things like "^ *# var" to end the sorted list.
> +        reg = r'^ *# ' + sm[1] + r'$'

The comment is a bit confusing now, as the actual regexp is "^ *# var$".
I think the old comment still fits.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH] sort-makefile-lines.py: Allow '_' in name and "^# name"

[H.J. Lu]

On Thu, Feb 15, 2024 at 4:24 AM Carlos O'Donell <carlos@redhat.com> wrote:
>
> On 2/15/24 07:19, H.J. Lu wrote:
> > On Thu, Feb 15, 2024 at 4:16 AM Florian Weimer <fweimer@redhat.com> wrote:
> >>
> >> * H. J. Lu:
> >>
> >>> '_' is used in Makefile variable names and many variables end with
> >>> "^# name".  Relax sort-makefile-lines.py to allow '_' in name and
> >>> "^# name" as variable end.  This fixes BZ #31385.
> >>> ---
> >>>  scripts/sort-makefile-lines.py | 6 +++---
> >>>  1 file changed, 3 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/scripts/sort-makefile-lines.py b/scripts/sort-makefile-lines.py
> >>> index f65ee40e27..ea02412d67 100755
> >>> --- a/scripts/sort-makefile-lines.py
> >>> +++ b/scripts/sort-makefile-lines.py
> >>> @@ -129,7 +129,7 @@ def sort_makefile_lines():
> >>>      for i in range(len(lines)):
> >>>          # Look for things like "var = \", "var := \" or "var += \"
> >>>          # to start the sorted list.
> >>> -        var = re.search(r'^([a-zA-Z0-9-]*) [\+:]?\= \\$', lines[i])
> >>> +        var = re.search(r'^([a-zA-Z0-9-_]*) [\+:]?\= \\$', lines[i])
> >>>          if var:
> >>>              # Remember the index and the name.
> >>>              startmarks.append((i, var.group(1)))
> >>
> >> Please keep the literal - at the end of the bracket expression.  I think
> >> it's easier to read even if it may be semantically the same.
> >
> > Did you mean like this?
> >
> > diff --git a/scripts/sort-makefile-lines.py b/scripts/sort-makefile-lines.py
> > index f65ee40e27..d791789671 100755
> > --- a/scripts/sort-makefile-lines.py
> > +++ b/scripts/sort-makefile-lines.py
> > @@ -129,7 +129,7 @@ def sort_makefile_lines():
> >      for i in range(len(lines)):
> >          # Look for things like "var = \", "var := \" or "var += \"
> >          # to start the sorted list.
> > -        var = re.search(r'^([a-zA-Z0-9-]*) [\+:]?\= \\$', lines[i])
> > +        var = re.search(r'^([-_a-zA-Z0-9]*) [\+:]?\= \\$', lines[i])
>
> Yes, exactly, I agree with Florian here you want to show that the literal '-' and '_'
> are not part of a range in the character class. So either start or end is fine with
> me.
>
> Looking forward to v2. Thanks for fixing this!

Done.

Thanks.

> >          if var:
> >              # Remember the index and the name.
> >              startmarks.append((i, var.group(1)))
> >
> >> Thanks,
> >> Florian
> >>
> >
> >
>
> --
> Cheers,
> Carlos.
>


-- 
H.J.