Re: [PATCH 2/2] gdb: fix bugs in gdb/copyright.py, make it use glob patterns

[Guinevere Larsen <guinevere@redhat.com>]

On 4/11/25 11:14 AM, Simon Marchi wrote:
> On 4/11/25 9:52 AM, Guinevere Larsen wrote:
>> On 4/9/25 11:49 AM, Simon Marchi wrote:
>>> gdb/copyright.py currently changes some files that it shouldn't:
>>>
>>>    - despite having a `gnulib/import` entry in EXCLUDE_LIST, it does
>>>      change the files under that directory
>>>    - it is missing `sim/Makefile.in`
>>>
>>> Change the exclude list logic to use glob patterns.  This makes it
>>> easier to specify exclusions of full directories or files by basename,
>>> while simplifying the code.
>>>
>>> Merge EXCLUDE_LIST and NOT_FSF_LIST, since there's no fundamental reason
>>> to keep them separate (they are treated identically).  I kept the
>>> comment that explains that some files are excluded due to not being
>>> FSF-licensed.
>>>
>>> Merge EXCLUDE_ALL_LIST in EXCLUDE_LIST, converting the entries to glob
>>> patterns that match everywhere in the tree (e.g. `**/configure`).
>>>
>>> Tested by running the script on the parent commit of d01e823438c7
>>> ("Update copyright dates to include 2025") and diff'ing the result with
>>> d01e823438c7.  The only differences are:
>>>
>>>    - the files that we don't want to modify (gnulib/import and
>>>      sim/Makefile.in)
>>>    - the files that need to be modified by hand
>>>
>>> Running the script on latest master produces no diff.
>> Is there any reason why you turned all globals into lists instead of tuples?
>>
>>  From what I can see in the script (and my understanding of it), there's no reason why the arrays should be mutable, so using the immutable tuple type would be better IMO.
> BY_HAND being an empty tuple messes up the typing, so I made it a
> list[str] to say that if it contained something, it would be strings.
> And then because I'm adding EXCLUDE_LIST and BY_HAND, it's easier if
> EXCLUDE_LIST is also a list[str].  But I can work around that one by
> doing:
>
>      full_exclude_list = list(EXCLUDE_LIST) + BY_HAND

would flipping that expression to

     full_exclude_list = EXCLUDE_LIST + tuple(BY_HAND)

also mess up the typing?

If that's the case, I'm fine with keeping them as lists, it was a minor 
nit anyway

>
> That all could be avoided if we just removed the BY_HAND thing, since
> it's unused at the moment.  But I thought it would be cheap to keep it
> around just it case we need it in the future.  Not sure which is better.
Yeah, I think recognizing that some files may need to be changed by 
hand, and keeping that easy is a good idea.
>
> Simon
>

-- 
Cheers,
Guinevere Larsen
She/Her/Hers