sort-makefile-lines.py: Allow '_' in name and "^# name"
Checks
Context |
Check |
Description |
redhat-pt-bot/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
redhat-pt-bot/TryBot-32bit |
success
|
Build for i686
|
linaro-tcwg-bot/tcwg_glibc_build--master-arm |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_glibc_check--master-arm |
fail
|
Testing failed
|
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 |
fail
|
Testing failed
|
Commit Message
'_' 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(-)
Comments
* 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
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
>
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
>>
>
>
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.
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.
>
@@ -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).