avoiding rejects

avoiding rejects

[Andrew Morton]

There are many situations where patching the kernel involves adding a new
item to a list, such as:

- adding a makefile line
- adding a new #include
- adding a new Kconfig entry
- adding a new PCI ID
- adding a record to feature-removal.txt
- adding a new sysctl table entry
- etc

Of course, everyone just sticks the new entry at the end of the existing
entries.  This strategy carefully maximises the opportunity for patch
rejects and leads to unhappiness.

Most of these lists are unordered anyway, so inserting the new item at a
randomly-chosen position is a better approach than just appending it.

Re: avoiding rejects

[Dan Aloni]

On Tue, Jul 10, 2007 at 12:44:22PM -0700, Andrew Morton wrote:
> 
> There are many situations where patching the kernel involves adding a new
> item to a list, such as:
> 
> - adding a makefile line
> - adding a new #include
> - adding a new Kconfig entry
> - adding a new PCI ID
> - adding a record to feature-removal.txt
> - adding a new sysctl table entry
> - etc
> 
> Of course, everyone just sticks the new entry at the end of the existing
> entries.  This strategy carefully maximises the opportunity for patch
> rejects and leads to unhappiness.
> 
> Most of these lists are unordered anyway, so inserting the new item at a
> randomly-chosen position is a better approach than just appending it.

AFAIK some Kconfig entries do have some importance to order (take 
kernel/Kconfig.hz for example, though it's not a good one, not long 
enough).

I think that random insertions can hurt on the readability side. So, here's
a list of more ways to discuss:

1. How about adding a context-aware logic to the merging process? (i.e, 
project-specific "plugins") That is, beside the regular 3-line context, 
the merging logic can be a aware of particular file formats, whether it's a 
Kconfig tree-like structure or list of lines all starting with a prefix of 
'#include'. 

2. Alternatively, these special one-liner diff hunks can be generated with
smaller context space (2, 1, or even no lines). This means instead of 
adding context-awareness to the merger process, add context-awareness
to the diff generator. The other 'more complex' code modifications hunks 
must remain with a context of 3 lines.

3. Another option is to come up with file formats that have '3 space 
lines' dilimeters between items (it wouldn't make sense for #include, but 
for PCI IDs it might be worth it).

-- 
Dan Aloni
XIV LTD, http://www.xivstorage.com
da-x (at) monatomic.org, dan (at) xiv.co.il

Re: avoiding rejects

[Geert Uytterhoeven]

On Tue, 10 Jul 2007, Andrew Morton wrote:
> There are many situations where patching the kernel involves adding a new
> item to a list, such as:
> 
> - adding a makefile line
> - adding a new #include
> - adding a new Kconfig entry
> - adding a new PCI ID
> - adding a record to feature-removal.txt
> - adding a new sysctl table entry
> - etc
> 
> Of course, everyone just sticks the new entry at the end of the existing
> entries.  This strategy carefully maximises the opportunity for patch
> rejects and leads to unhappiness.
> 
> Most of these lists are unordered anyway, so inserting the new item at a
> randomly-chosen position is a better approach than just appending it.

Really?

#includes should be sorted alphabetically
Lots of other stuff should be sorted numerically/alphabetically

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: avoiding rejects

[Arnd Bergmann]

On Tuesday 10 July 2007, Geert Uytterhoeven wrote:
> 
> > Most of these lists are unordered anyway, so inserting the new item at a
> > randomly-chosen position is a better approach than just appending it.
> 
> Really?
> 
> #includes should be sorted alphabetically
> Lots of other stuff should be sorted numerically/alphabetically
> 

I would also argue that adding in an ordered way is better than
at random positions, that is if the list is ordered to start with.
As long as everyone follows the same order, you will get (desired)
conflicts in case two people try to add the same line, and when
the added lines are different, it's as good as random ordering
to avoid undesired merge conflicts.

	Arnd <><

Re: avoiding rejects

[Boaz Harrosh]

Andrew Morton wrote:
> 
> Most of these lists are unordered anyway, so inserting the new item at a
> randomly-chosen position is a better approach than just appending it.
> -
But you have lost the order you have just stated. 
"Order by time of addition" ascending ;)

Sometimes what I do is a diff with only post sync lines. 
With -p and the end of the list it mostly works.

consider this file
<list1>
BEGIN LIST
	I1
	I2
	I3
END LIST
..
...
</list1>

and consider this patch

<list2.patch>
--- list1	2007-07-11 10:33:26.000000000 +0300
+++ list2	2007-07-11 10:22:16.000000000 +0300
@@ -5,2 +5,3 @@ BEGIN LIST
+	add1
 END LIST
 ..
</list2.patch>

Now this will work. In fact you can apply this patch 
multiple times and it will keep adding Items. "patch"
will not warn about reverse patching, but ...
It will add Items from the top. Actually it will add
the Item at an absolute line in the file. At -# line 
stated at hunk header, like "-5" above.

Free Life
Boaz

Re: avoiding rejects

[Jan Engelhardt]

On Jul 10 2007 12:44, Andrew Morton wrote:
>
>There are many situations where patching the kernel involves adding a new
>item to a list, such as:
>
>- adding a makefile line
>- adding a new #include
>- adding a new Kconfig entry
>- adding a new PCI ID
>- adding a record to feature-removal.txt
>- adding a new sysctl table entry
>- etc
>
>Of course, everyone just sticks the new entry at the end of the existing
>entries.  This strategy carefully maximises the opportunity for patch
>rejects and leads to unhappiness.
>
>Most of these lists are unordered anyway, so inserting the new item at a
>randomly-chosen position is a better approach than just appending it.

As for Makefiles: Sort the list lexicographically. Let the submitter put
it into the right spot.
Example: net/netfilter/Makefile.
There are lots of obj-$(CONFIG_NETFILTER_XT_MATCH_xyz) += xt_xyz.o for example,
which can be sorted without disturbing required linking order or whatnot
(for some directories this seems important).
Benefit: Less rejects of course (not as much as a truly random insert,
but still less than appending) _and_ a sorted list.


	Jan
--