[Mesa-dev] [PATCH 1/5] utils: automake: remove uncommon $()

[Matt Turner]

On Thu, Aug 13, 2015 at 4:31 AM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> On 7 August 2015 at 19:22, Matt Turner <mattst88 at gmail.com> wrote:
>> On Fri, Aug 7, 2015 at 11:05 AM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>>> On 21 July 2015 at 13:12, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>>>> On 20/07/15 21:25, Chad Versace wrote:
>>>>> On Fri 17 Jul 2015, Emil Velikov wrote:
>>>>>> On 17 July 2015 at 19:11, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>>>>>>> On Fri, Jul 17, 2015 at 2:11 PM, Eric Anholt <eric at anholt.net> wrote:
>>>>>>>> Matt Turner <mattst88 at gmail.com> writes:
>>>>>>>>
>>>>>>>>> On Fri, Jul 17, 2015 at 10:17 AM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>>>>>>>>>> Cc: Eric Anholt <eric at anholt.net>
>>>>>>>>>> Signed-off-by: Emil Velikov <emil.l.velikov at gmail.com>
>>>>>>>>>> ---
>>>>>>>>>>  src/util/tests/hash_table/Makefile.am | 3 +--
>>>>>>>>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/src/util/tests/hash_table/Makefile.am b/src/util/tests/hash_table/Makefile.am
>>>>>>>>>> index 04a77e3..0c99e7b 100644
>>>>>>>>>> --- a/src/util/tests/hash_table/Makefile.am
>>>>>>>>>> +++ b/src/util/tests/hash_table/Makefile.am
>>>>>>>>>> @@ -38,7 +38,6 @@ TESTS = \
>>>>>>>>>>         null_destroy \
>>>>>>>>>>         random_entry \
>>>>>>>>>>         remove_null \
>>>>>>>>>> -       replacement \
>>>>>>>>>> -       $()
>>>>>>>>>> +       replacement
>>>>>>>>>
>>>>>>>>> To get the benefit of $() without some unknown incompatibility, pixman
>>>>>>>>> uses $(NULL) which of course relies on not having a variable named
>>>>>>>>> NULL.
>>>>>>>>>
>>>>>>>>> I might suggest that instead of removing them, but I'm not much
>>>>>>>>> opposed to removing them either.
>>>>>>>>
>>>>>>>> I do really like having a terminator on these lists.  I find that
>>>>>>>> without them, I'll end up copy-and-pasting the wrong thing and missing
>>>>>>>> the trailing backslash on a line.
>>>>>>>
>>>>>>> Also makes diffs easier to read since you don't have spurious changes
>>>>>>> which just add a \ .
>>>>>
>>>>> I second Anholt. I prefer the sentinel too, but don't feel too strongly
>>>>> about it.
>>>>>
>>>>> I find that a sentinel helps me avoid making mistakes when adding new
>>>>> list members or when sorting the lists in $EDITOR.
>>>>>
>>>> In case it's not (too) obvious I'm nuking these for consistency sake, as
>>>> there are five of these in over 80 makefiles. If people like/prefer them
>>>> can we have a volunteer that adds them everywhere ?
>>>>
>>> Can we have some volunteers, please ?
>>
>> I don't think we should require consistency here.
>>
> Genuine question - what does this case makes it different from any other ?

I think your question is "why shouldn't we require consistency?"

There are many things in the code base that don't match with our
current style. We fix them when we modify the code, but otherwise
don't force changes just for the sake of style. For instance, we don't
use tabs for indentation but we haven't removed all the tabs either. I
think this situation is the same.

> We do config/build check before pushing which easily catches the
> problem of missing \. Do we not ?

That's only partially the point of $(NULL). It's purpose is to avoid
mistakes, but it also allows you to sort the list very easily, and
using it means that the tail of the list isn't changed when new items
are appended (reduces git blame noise).