One test per line, one source per line, etc. etc. etc.

One test per line, one source per line, etc. etc. etc.

[Carlos O'Donell]

Do we have consensus for someone to just cleanup all the makefiles
to have one source per line, one test per line etc.?

The more reviews I do, the more this causes my to stumble and have
to work through merge issues.

If I want to get to 100 patches reviewed per day I think this is going
to block me.

I'd like to see consensus so that anyone who wants to can just expand
the current lists into one-per-line lists.

Thoughts?

-- 
Cheers,
Carlos.

Re: One test per line, one source per line, etc. etc. etc.

[Florian Weimer]

* Carlos O'Donell via Libc-alpha:

> Do we have consensus for someone to just cleanup all the makefiles
> to have one source per line, one test per line etc.?

I think there was some opposition to trailing \ in lists in Makefiles.
This:

routines += \
  read \
  write \

vs:

routines += \
  read \
  write

> The more reviews I do, the more this causes my to stumble and have
> to work through merge issues.
>
> If I want to get to 100 patches reviewed per day I think this is going
> to block me.

Avoid merge conflicts also needs a custom merger (easier if the data is
in separate files with a more regular file).

Sorting the lists lexicographically avoids conflicts only
stochastically, for sufficiently long lists.

Thanks,
Florian

Re: One test per line, one source per line, etc. etc. etc.

[Carlos O'Donell]

On 6/27/21 4:14 PM, Florian Weimer wrote:
> * Carlos O'Donell via Libc-alpha:
> 
>> Do we have consensus for someone to just cleanup all the makefiles
>> to have one source per line, one test per line etc.?
> 
> I think there was some opposition to trailing \ in lists in Makefiles.

In this thread:
https://sourceware.org/pipermail/libc-alpha/2020-December/120667.html

There was no objection, but it was a concrete change, not the general
discussion.

Ah, this is the thread I proposed it:
https://sourceware.org/pipermail/libc-alpha/2020-December/120693.html

DJ was in favor.
Joseph appears to favor LC_COLLATE=C.
Siddhesh suggests tests could be renamed to get more logical orderings.
Carlos clarifies, yes, LC_COLLATE=C, and yes test renaming would help.

There doesn't seem to be any opposition.

I think we have consensus to change all the files in a semi-automatic
refactoring that would help reduce conflicts.

> This:
> 
> routines += \
>   read \
>   write \

This. To reduce conflicts.

> 
> vs:
> 
> routines += \
>   read \
>   write

No objections noted for this that I can find.

> 
>> The more reviews I do, the more this causes my to stumble and have
>> to work through merge issues.
>>
>> If I want to get to 100 patches reviewed per day I think this is going
>> to block me.
> 
> Avoid merge conflicts also needs a custom merger (easier if the data is
> in separate files with a more regular file).

Avoiding *all* conflicts needs a custom merge driver.
 
> Sorting the lists lexicographically avoids conflicts only
> stochastically, for sufficiently long lists.

And we have such lists already so the refactor would reduce conflicts
without much actual work required.

In summary:
- I see no objections to a refactoring across libc to do this.
- I see no objections to trailing \.

The patches would obviously need to go through review, but it seems like
all we need to do is do the work.

-- 
Cheers,
Carlos.

Re: One test per line, one source per line, etc. etc. etc.

[Andreas Schwab]

On Jun 27 2021, Carlos O'Donell via Libc-alpha wrote:

>> This:
>> 
>> routines += \
>>   read \
>>   write \
>
> This. To reduce conflicts.

The trailing backslash will gobble the next line which is bad.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: One test per line, one source per line, etc. etc. etc.

[Carlos O'Donell]

On 6/28/21 3:24 AM, Andreas Schwab wrote:
> On Jun 27 2021, Carlos O'Donell via Libc-alpha wrote:
> 
>>> This:
>>>
>>> routines += \
>>>   read \
>>>   write \
>>
>> This. To reduce conflicts.
> 
> The trailing backslash will gobble the next line which is bad.

If you feel there is risk in that, then I'm happy to avoid the
trialing slash since we would still gain from the reduced probability
of conflict with named functions and tests.

Would you be OK if we proceed without the trailing slash?

-- 
Cheers,
Carlos.

Re: One test per line, one source per line, etc. etc. etc.

[Dmitry V. Levin]

On Sun, Jun 27, 2021 at 10:14:09PM +0200, Florian Weimer via Libc-alpha wrote:
> * Carlos O'Donell via Libc-alpha:
> 
> > Do we have consensus for someone to just cleanup all the makefiles
> > to have one source per line, one test per line etc.?
> 
> I think there was some opposition to trailing \ in lists in Makefiles.
> This:
> 
> routines += \
>   read \
>   write \
> 
> vs:
> 
> routines += \
>   read \
>   write

I'd suggest this instead:

routines += \
  read \
  write \
  #


-- 
ldv

Re: One test per line, one source per line, etc. etc. etc.

[Florian Weimer]

* Dmitry V. Levin:

> On Sun, Jun 27, 2021 at 10:14:09PM +0200, Florian Weimer via Libc-alpha wrote:
>> * Carlos O'Donell via Libc-alpha:
>> 
>> > Do we have consensus for someone to just cleanup all the makefiles
>> > to have one source per line, one test per line etc.?
>> 
>> I think there was some opposition to trailing \ in lists in Makefiles.
>> This:
>> 
>> routines += \
>>   read \
>>   write \
>> 
>> vs:
>> 
>> routines += \
>>   read \
>>   write
>
> I'd suggest this instead:
>
> routines += \
>   read \
>   write \
>   #

Or even:

routines += \
  read \
  write \
  # routines

It seems to work:

“
routines += \
  read \
  write \
  # routines
stop = 1

all: print-routines print-stop

print-%:
	@echo "$* = $($*)"
”

This prints:

“
routines = read write 
stop = 1
”

Thanks,
Florian

Re: One test per line, one source per line, etc. etc. etc.

[Carlos O'Donell]

On 6/28/21 6:05 AM, Florian Weimer via Libc-alpha wrote:
> routines += \
>   read \
>   write \
>   # routines

Floian, Dmitry,

This is my *favourite* because it increases observability
of the list you're editing. Thanks for exploring the options.

Andreas,

Any objection to the above? As Florian noted it works and
subsequent variable setting after "# routines" works as
expected.

-- 
Cheers,
Carlos.

Re: One test per line, one source per line, etc. etc. etc.

[DJ Delorie]

"Carlos O'Donell via Libc-alpha" <libc-alpha@sourceware.org> writes:
> Do we have consensus for someone to just cleanup all the makefiles
> to have one source per line, one test per line etc.?

Yes (again) :-)

I have two comments:

1. I use some sort of end-of-list tag, like this:

FILES = \
	a.c \
	b.c \
	$(ENDOFLIST)

or just $E or anything that expands to nothing.  Remnants of an old job
where adding the trailing \ to the previous line caused 100% patch
conflicts.  I'm not tied to any particular method, but I'm very against
the omit-last-slash format.

2. I recommend the files NOT be listed in strict alphabetical order,
when there's a large variance in build times.  List the files that take
longest, first.  Comment it as such, of course.  Leftovers from
libiberty's builds, where a parallel make spent most of its time waiting
for the ONE file that took forver to build, which was last in the
alphabetical list, so was always built by itself.