[gentoo-dev] [RFC] Establishing Gentoo patch policy to keep our patches consistent and clean

[gentoo-dev] [RFC] Establishing Gentoo patch policy to keep our patches consistent and clean

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 4215 bytes --]

Hello,

As far as I'm aware, we don't really have much of a patch maintenance
policy in Gentoo. There a few loose rules like «don't put awfully big
files into FILESDIR» or the common sense «use unified diff», but no
complete and clear policy.

Especially considering the late discussion related to the needless
and semi-broken functionality in epatch, I'd like to propose
setting the following rules for patches in tree and in Gentoo-sourced
patchsets:

1. Patches have to be either in unified or context diff format. Unified
diff is preferred.

2. Patches have to apply to the top directory of the source tree with
'patch -p1'. If patches are applied to sub-directories, necessary '-p'
argument shall be passed to 'epatch' explicitly. Developers are
encouraged to create patches which are compatible with 'git am'.

3. Patches have to end with either '.patch' or '.diff' suffix.

4. If possible, patches shall be named in a way allowing them to be
applied in lexical order. However, this one isn't necessary if patches
from an older ebuild are applied to a newer one.

5. The patch name shall shortly summarize the changes done by it.

6. Patch files shall start with a brief description of what the patch
does. Developers are encouraged to use git-style tags like 'Fixes:' to
point to the relevant bug URIs.

7. Patch combining is discouraged. Developers shall prefer multiple
patches following either the upstream commits or a logical commit
sequence (if changes are not committed upstream).

The above-listed policy will apply to the patches kept in the gx86 tree
(in FILESDIRs) and patch archives created by Gentoo developers. They
will not apply to the patch archives created upstream.


RATIONALE
---------

The main reason for the whole policy is to let Gentoo supply users with
consistent and friendly patches. That is, patches which can be used
directly on a source tree or submitted upstream without any additional
actions from user.

(1) lists the most common patch formats. The formats shall be generally
both readable and reusable whenever possible. I wanted just 'unified
diff' but ulm suggested that we should also support those upstreams who
use 'context diffs'.


(2) is likely to be a bikeshed point here. Long story short, epatch has
this fragile patchlevel guessing, users don't have it. If we keep our
patches consistent to a single patchlevel, we gain:

* ability for users to apply the patches without having them try all
  patchlevels by hand.

* clean error output if patch stops to apply for some reason.

* no risk that patch will get applied to the wrong file if patch stops
  to apply at expected patchlevel and starts to apply on another.

Also, by creating git-format patches, we gain the ability of rebasing
and updating the patches easily. Even with non-git upstreams, we can
do:

  git init; git add -A; git commit -m 1; git am ...


(3) should be mostly obvious. The main idea is that if we apply a whole
patchdir, we should be able to easily tell between patches
and auxiliary files like 'README' or Debian's 'series'.

I have never seen a patch file named other than '*.patch' or '*.diff'.
Therefore, I think that it's better to just require those rather than
trying to provide a sane list of excludes.


(4) is mostly about friendliness (again). Since shell does filename
expansion in lexical order, it's just great for user to be able apply
patches like:

  git am /usr/portage/foo/bar/files/198-*.patch

The other sentence is not to enforce this rule e.g. when the same patch
is applied to different versions of the same package. Although with
a fair of trickery that could be gotten working, I don't think it will
be user-friendly anymore :).


(5) makes finding a particular patch of interest easier, while (6)
makes sure that the purpose of the patch can be read from patch alone.
In both cases, having described patches is much better than having to
look into ebuilds for explanations.


(7) is because merged patches are usually hard to read and completely
not suitable for submitting anywhere.


What are your thoughts?

-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 966 bytes --]

Re: [gentoo-dev] [RFC] Establishing Gentoo patch policy to keep our patches consistent and clean

[Tony "Chainsaw" Vroon]

On Sat, 2013-04-06 at 20:08 +0200, Michał Górny wrote:
> What are your thoughts?

Sensible document. Can we have it on the agenda for the council meeting
please. It looks suitable for a yes/no vote, and I expect some guidance
from the wider developer community in how they respond on the list.

Regards,
Tony V.

Re: [gentoo-dev] [RFC] Establishing Gentoo patch policy to keep our patches consistent and clean

[Markos Chandras]

On 6 April 2013 19:08, Michał Górny <mgorny@gentoo.org> wrote:
> Hello,
>
> ...
> What are your thoughts?

Maybe it is time to setup a patch tracking system like Debian[1]?

Sometimes it is really hard to understand what patches are applied by
an ebuild (especially when all the
build process is handled by an eclass) and/or when people keep a
separate .tar.* with all the patches. Debian
makes is so much easier to see what patches each package contains.

[1]http://patch-tracker.debian.org/

--
Regards,
Markos Chandras - Gentoo Linux Developer
http://dev.gentoo.org/~hwoarang

[gentoo-dev] Re: [RFC] Establishing Gentoo patch policy to keep our patches consistent and clean

[Michael Palimaka]

On 7/04/2013 04:22, Markos Chandras wrote:
> On 6 April 2013 19:08, Michał Górny <mgorny@gentoo.org> wrote:
>> Hello,
>>
>> ...
>> What are your thoughts?
>
> Maybe it is time to setup a patch tracking system like Debian[1]?
>
> Sometimes it is really hard to understand what patches are applied by
> an ebuild (especially when all the
> build process is handled by an eclass) and/or when people keep a
> separate .tar.* with all the patches. Debian
> makes is so much easier to see what patches each package contains.
>
> [1]http://patch-tracker.debian.org/
>
> --
> Regards,
> Markos Chandras - Gentoo Linux Developer
> http://dev.gentoo.org/~hwoarang
>
>

I have always found Debian's patch tracker very useful, I would 
definitely support us implementing something similar.

Re: [gentoo-dev] [RFC] Establishing Gentoo patch policy to keep our patches consistent and clean

[Alexandre Rostovtsev]

On Sat, 2013-04-06 at 20:08 +0200, Michał Górny wrote:
> 2. Patches have to apply to the top directory of the source tree with
> 'patch -p1'. If patches are applied to sub-directories, necessary '-p'
> argument shall be passed to 'epatch' explicitly. Developers are
> encouraged to create patches which are compatible with 'git am'.

Please don't make -p1 into a hard requirement :/

There are upstreams who have different directory layouts in their scm
tree and in their source tarballs. If I clone an upstream git repository
to obtain a patch or to write a new patch that I will submit upstream, I
want to be able to apply that patch in an ebuild without having to
manually sed it to change the -p level.

Specific examples of this among packages that I maintain: app-cdr/cdemu,
app-cdr/cdemu-daemon, sys-fs/vhba. These all use the same git tree, and
source tarballs for individual packages are created from different
subdirectories of that git tree.

Re: [gentoo-dev] [RFC] Establishing Gentoo patch policy to keep our patches consistent and clean

[Michael Mol]

[-- Attachment #1: Type: text/plain, Size: 1166 bytes --]

On Apr 6, 2013 2:36 PM, "Alexandre Rostovtsev" <tetromino@gentoo.org> wrote:
>
> On Sat, 2013-04-06 at 20:08 +0200, Michał Górny wrote:
> > 2. Patches have to apply to the top directory of the source tree with
> > 'patch -p1'. If patches are applied to sub-directories, necessary '-p'
> > argument shall be passed to 'epatch' explicitly. Developers are
> > encouraged to create patches which are compatible with 'git am'.
>
> Please don't make -p1 into a hard requirement :/
>
> There are upstreams who have different directory layouts in their scm
> tree and in their source tarballs. If I clone an upstream git repository
> to obtain a patch or to write a new patch that I will submit upstream, I
> want to be able to apply that patch in an ebuild without having to
> manually sed it to change the -p level.
>
> Specific examples of this among packages that I maintain: app-cdr/cdemu,
> app-cdr/cdemu-daemon, sys-fs/vhba. These all use the same git tree, and
> source tarballs for individual packages are created from different
> subdirectories of that git tree.
>
>

It wouldn't be; you pass the p level in as an argument, if necessary.

[-- Attachment #2: Type: text/html, Size: 1461 bytes --]

Re: [gentoo-dev] [RFC] Establishing Gentoo patch policy to keep our patches consistent and clean

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 1337 bytes --]

On Sat, 06 Apr 2013 14:35:47 -0400
Alexandre Rostovtsev <tetromino@gentoo.org> wrote:

> On Sat, 2013-04-06 at 20:08 +0200, Michał Górny wrote:
> > 2. Patches have to apply to the top directory of the source tree with
> > 'patch -p1'. If patches are applied to sub-directories, necessary '-p'
> > argument shall be passed to 'epatch' explicitly. Developers are
> > encouraged to create patches which are compatible with 'git am'.
> 
> Please don't make -p1 into a hard requirement :/
> 
> There are upstreams who have different directory layouts in their scm
> tree and in their source tarballs. If I clone an upstream git repository
> to obtain a patch or to write a new patch that I will submit upstream, I
> want to be able to apply that patch in an ebuild without having to
> manually sed it to change the -p level.

You will need to supply '-p2' in the *worst* case.

> Specific examples of this among packages that I maintain: app-cdr/cdemu,
> app-cdr/cdemu-daemon, sys-fs/vhba. These all use the same git tree, and
> source tarballs for individual packages are created from different
> subdirectories of that git tree.

In the context of the entry, 'top directory' would mean the common git
root. So the patches suitable for 'git am' on that repo will be correct.

-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 966 bytes --]

Re: [gentoo-dev] [RFC] Establishing Gentoo patch policy to keep our patches consistent and clean

["Paweł Hajdan, Jr."]

[-- Attachment #1: Type: text/plain, Size: 2009 bytes --]

On 4/6/13 11:08 AM, Michał Górny wrote:
> 1. Patches have to be either in unified or context diff format. Unified
> diff is preferred.

Are there any other formats than unified and context diff? If not, it'd
be like another "for indoor or outdoor use only" or "home or office use"
- i.e. no need to explicitly list all possible options.

> 5. The patch name shall shortly summarize the changes done by it.

Common sense again. :) And I agree that patches should do that, the
question is just whether we put common sense into the policy.

> 6. Patch files shall start with a brief description of what the patch
> does. Developers are encouraged to use git-style tags like 'Fixes:' to
> point to the relevant bug URIs.

That could be helpful - could this be made more precise though? Is there
any existing convention (going beyond git style, but specifically
designed for distro or similar patches) we could follow?

> 7. Patch combining is discouraged. Developers shall prefer multiple
> patches following either the upstream commits or a logical commit
> sequence (if changes are not committed upstream).

Common sense as well.

> (2) is likely to be a bikeshed point here. Long story short, epatch has
> this fragile patchlevel guessing, users don't have it. If we keep our
> patches consistent to a single patchlevel, we gain:
> 
> * ability for users to apply the patches without having them try all
>   patchlevels by hand.
> 
> * clean error output if patch stops to apply for some reason.
> 
> * no risk that patch will get applied to the wrong file if patch stops
>   to apply at expected patchlevel and starts to apply on another.

The above two points are convincing for me. However, I do acknowledge
that it some cases the guessing behavior can be useful for some people
(e.g. different layout of git repo vs. release tarballs).

How about having a one guessing and one non-guessing variant of epatch,
and encouraging the non-guessing one?

Paweł


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 203 bytes --]

Re: [gentoo-dev] [RFC] Establishing Gentoo patch policy to keep our patches consistent and clean

[Alex Xu]

[-- Attachment #1: Type: text/plain, Size: 864 bytes --]

On 06/04/13 03:02 PM, "Paweł Hajdan, Jr." wrote:
> Are there any other formats than unified and context diff? If not, it'd
> be like another "for indoor or outdoor use only" or "home or office use"
> - i.e. no need to explicitly list all possible options.

From the man page:

> -c, -C NUM, --context[=NUM]
>   output NUM (default 3) lines of copied context
> 
> -u, -U NUM, --unified[=NUM]
>   output NUM (default 3) lines of unified context
> 
> -e, --ed
>   output an ed script
> 
> -n, --rcs
>   output an RCS format diff
> 
> -y, --side-by-side
>   output in two columns

Plus the default.

On 06/04/13 03:02 PM, "Paweł Hajdan, Jr." wrote:
> How about having a one guessing and one non-guessing variant of epatch,
> and encouraging the non-guessing one?

User1: how do i put a patch in an ebuild
User2: use epatch_guesslevel


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [gentoo-dev] [RFC] Establishing Gentoo patch policy to keep our patches consistent and clean

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 2767 bytes --]

On Sat, 06 Apr 2013 12:02:28 -0700
""Paweł Hajdan, Jr."" <phajdan.jr@gentoo.org> wrote:

> On 4/6/13 11:08 AM, Michał Górny wrote:
> > 5. The patch name shall shortly summarize the changes done by it.
> 
> Common sense again. :) And I agree that patches should do that, the
> question is just whether we put common sense into the policy.

Well, I think it's more like pointing out the few people who rather use
very short and ambiguous names. Although the git naming is bit verbose
here, I prefer that than 'lm.patch'.

find /usr/portage -name '*.patch' \
  | awk -F/ '{ print length($NF) " " $NF }' | sort -k1 -n > /tmp/lol

> > 6. Patch files shall start with a brief description of what the patch
> > does. Developers are encouraged to use git-style tags like 'Fixes:' to
> > point to the relevant bug URIs.
> 
> That could be helpful - could this be made more precise though? Is there
> any existing convention (going beyond git style, but specifically
> designed for distro or similar patches) we could follow?

I would honestly just go for the git style. It's the first thing that
really succeeded in standardizing patches. Inventing something new is
not really necessary, I believe.

> > 7. Patch combining is discouraged. Developers shall prefer multiple
> > patches following either the upstream commits or a logical commit
> > sequence (if changes are not committed upstream).
> 
> Common sense as well.

Tell that to the people who believe in space savings :).

> > (2) is likely to be a bikeshed point here. Long story short, epatch has
> > this fragile patchlevel guessing, users don't have it. If we keep our
> > patches consistent to a single patchlevel, we gain:
> > 
> > * ability for users to apply the patches without having them try all
> >   patchlevels by hand.
> > 
> > * clean error output if patch stops to apply for some reason.
> > 
> > * no risk that patch will get applied to the wrong file if patch stops
> >   to apply at expected patchlevel and starts to apply on another.
> 
> The above two points are convincing for me. However, I do acknowledge
> that it some cases the guessing behavior can be useful for some people
> (e.g. different layout of git repo vs. release tarballs).
> 
> How about having a one guessing and one non-guessing variant of epatch,
> and encouraging the non-guessing one?

In the end we will probably have a simple patch applying method
from PMS, and we will keep the epatch eclass monster -- at least
for some time.

To be honest, I'd rather prefer to find a good way to help people add
correct '-p'. We can -- for example -- make portage try various
patchlevels and suggest one if applying patch failed.

-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 966 bytes --]

Re: [gentoo-dev] [RFC] Establishing Gentoo patch policy to keep our patches consistent and clean

["Paweł Hajdan, Jr."]

[-- Attachment #1: Type: text/plain, Size: 789 bytes --]

On 4/6/13 12:41 PM, Michał Górny wrote:
>>> 6. Patch files shall start with a brief description of what the patch
>>> does. Developers are encouraged to use git-style tags like 'Fixes:' to
>>> point to the relevant bug URIs.
>>
>> That could be helpful - could this be made more precise though? Is there
>> any existing convention (going beyond git style, but specifically
>> designed for distro or similar patches) we could follow?
> 
> I would honestly just go for the git style. It's the first thing that
> really succeeded in standardizing patches. Inventing something new is
> not really necessary, I believe.

Oh, I didn't mean inventing anything new.

Is there some URL or documentation I could read to familiarize myself
with all of the "git style"?

Paweł


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 203 bytes --]

Re: [gentoo-dev] [RFC] Establishing Gentoo patch policy to keep our patches consistent and clean

[Michał Górny]

[-- Attachment #1: Type: text/plain, Size: 2683 bytes --]

On Sat, 06 Apr 2013 13:00:35 -0700
""Paweł Hajdan, Jr."" <phajdan.jr@gentoo.org> wrote:

> On 4/6/13 12:41 PM, Michał Górny wrote:
> > I would honestly just go for the git style. It's the first thing that
> > really succeeded in standardizing patches. Inventing something new is
> > not really necessary, I believe.
> 
> Oh, I didn't mean inventing anything new.
> 
> Is there some URL or documentation I could read to familiarize myself
> with all of the "git style"?

Hmm, it seems that there is nothing really common, just various things
that were used and started to be expected by random tools. Except for
bits of notes in git manpages, there's some info in the 'submitting
patches' doc [1].

The general idea is similar to the e-mail concept. Shortly saying, it's:

  One line of topic

  Paragraph of text, usually wrapped at 72 chars.......................
  Second line of the same paragraph, etcetera.

  Second paragraph. Then we'll have a list:

  - item 1

  - item 2

  Some-tags: ...
  Other-tag: ...

(indent added to mail to make it clear, no indent in patch file :))

Although git itself rather preserves newlines, some tool concatenate
successive lines, so you separate paragraphs (and items) with empty
lines.

First paragraph (some tools may read one line only!) goes as a short
summary, that is shown in 'git log --oneline'. The following paragraphs
(optional) should explain better what's happening.

Optionally, after the last paragraph you can add a few lines with tags
in form of 'Tag: value'. AFAICS git itself uses only
'Signed-off-by' but you can find more tags in various 'submitting
patches' docs.

'Signed-off-by' lists the one responsible for the patch. Some upstreams
require that line, some take over authorship of patches if you don't
add it (like X.org), some just hate it :).

The X.org wiki lists also 'Fixes' for bugtracker URL [2] and a few tags
for reviewing patches [3] (those are probably not useful for us).

Some upstreams also use different bugtracker tag formats [4,5] but I
feel like they're not proper for our patches since they don't mention
the bugtracker URI. That said, I'd probably ping them about it.

[1]:http://git.kernel.org/cgit/git/git.git/tree/Documentation/SubmittingPatches
[2]:http://www.x.org/wiki/Development/Documentation/SubmittingPatches#Commit_message_format
[3]:http://www.x.org/wiki/Development/Documentation/SubmittingPatches#Signing_off_and_reviewing
[4]:http://www.mediawiki.org/wiki/Gerrit/Commit_message_guidelines
[5]:https://wiki.openstack.org/wiki/GitCommitMessages#Including_external_references

-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 966 bytes --]

Re: [gentoo-dev] [RFC] Establishing Gentoo patch policy to keep our patches consistent and clean

[Donnie Berkholz]

[-- Attachment #1: Type: text/plain, Size: 933 bytes --]

On 23:41 Sat 06 Apr     , Michał Górny wrote:
> Optionally, after the last paragraph you can add a few lines with tags
> in form of 'Tag: value'. AFAICS git itself uses only
> 'Signed-off-by' but you can find more tags in various 'submitting
> patches' docs.
> 
> 'Signed-off-by' lists the one responsible for the patch. Some upstreams
> require that line, some take over authorship of patches if you don't
> add it (like X.org), some just hate it :).
> 
> The X.org wiki lists also 'Fixes' for bugtracker URL [2] and a few tags
> for reviewing patches [3] (those are probably not useful for us).

This is the part that would be most useful to document more. Suggesting 
which tags would be valuable to include. Fixes, Reviewed-by, 
Signed-off-by, etc.

-- 
Thanks,
Donnie

Donnie Berkholz
Council Member / Sr. Developer, Gentoo Linux <http://dberkholz.com>
Analyst, RedMonk <http://redmonk.com/dberkholz/>

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

[gentoo-dev] Re: [RFC] Establishing Gentoo patch policy to keep our patches consistent and clean

[Paul Arthur]

On 2013-04-06, Paweł Hajdan, Jr. <phajdan.jr@gentoo.org> wrote:

> On 4/6/13 11:08 AM, Micha=C5=82 G=C3=B3rny wrote:
>
>> 1. Patches have to be either in unified or context diff format.
>> Unified diff is preferred.
>
> Are there any other formats than unified and context diff? If not,
> it'd be like another "for indoor or outdoor use only" or "home or
> office use" - i.e. no need to explicitly list all possible options.

Yes.

Unified:
--- test-       2013-04-06 16:27:07.000000000 -0400
+++ test        2013-04-06 16:27:22.000000000 -0400
@@ -1,6 +1,6 @@
 0
 1
 2
-3
+2
 4
 5

Context:
*** test-       Sat Apr  6 16:27:07 2013  
--- test        Sat Apr  6 16:27:22 2013  
***************  
*** 1,6 ****  
  0  
  1  
  2  
! 3  
  4  
  5  
--- 1,6 ----  
  0  
  1  
  2  
! 2  
  4  
  5  

Normal:
4c4  
< 3  
---  
> 2  

ed script:
4c
2
.

RCS format:
d4 1
a4 1
2

-- 
[T]he only virtue it had was that the CD-ROM did not generally on physical
contact explode to drive high-velocity metallic shards laden with radioactive
weaponized anthrax spores through the local postal code.
    --Joseph Nebus in RASFW, on SimCity 3000

Re: [gentoo-dev] [RFC] Establishing Gentoo patch policy to keep our patches consistent and clean

[Robin H. Johnson]

On Sat, Apr 06, 2013 at 08:08:43PM +0200, Michał Górny wrote:
> The above-listed policy will apply to the patches kept in the gx86 tree
> (in FILESDIRs) and patch archives created by Gentoo developers. They
> will not apply to the patch archives created upstream.
What about patches created by upstream, but stored in the FILESDIR (eg
from upstream mailing lists, bugfixes before the next release).

We want to keep them intact and not respin them.

-- 
Robin Hugh Johnson
Gentoo Linux: Developer, Trustee & Infrastructure Lead
E-Mail     : robbat2@gentoo.org
GnuPG FP   : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85

[gentoo-dev] Re: [RFC] Establishing Gentoo patch policy to keep our patches consistent and clean

[Michael Palimaka]

On 7/04/2013 07:01, Robin H. Johnson wrote:
> On Sat, Apr 06, 2013 at 08:08:43PM +0200, Michał Górny wrote:
>> The above-listed policy will apply to the patches kept in the gx86 tree
>> (in FILESDIRs) and patch archives created by Gentoo developers. They
>> will not apply to the patch archives created upstream.
> What about patches created by upstream, but stored in the FILESDIR (eg
> from upstream mailing lists, bugfixes before the next release).
>
> We want to keep them intact and not respin them.
>

What sort of respinning are you concerned about? It seems that each 
point could be addressed trivially with a text editor.

Re: [gentoo-dev] Re: [RFC] Establishing Gentoo patch policy to keep our patches consistent and clean

[Robin H. Johnson]

On Mon, Apr 08, 2013 at 12:36:36AM +1000, Michael Palimaka wrote:
> On 7/04/2013 07:01, Robin H. Johnson wrote:
> > On Sat, Apr 06, 2013 at 08:08:43PM +0200, Michał Górny wrote:
> >> The above-listed policy will apply to the patches kept in the gx86 tree
> >> (in FILESDIRs) and patch archives created by Gentoo developers. They
> >> will not apply to the patch archives created upstream.
> > What about patches created by upstream, but stored in the FILESDIR (eg
> > from upstream mailing lists, bugfixes before the next release).
> >
> > We want to keep them intact and not respin them.
> What sort of respinning are you concerned about? It seems that each 
> point could be addressed trivially with a text editor.
The -p1 directive primarily.

-- 
Robin Hugh Johnson
Gentoo Linux: Developer, Trustee & Infrastructure Lead
E-Mail     : robbat2@gentoo.org
GnuPG FP   : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85

Re: [gentoo-dev] [RFC] Establishing Gentoo patch policy to keep our patches consistent and clean

[Kacper Kowalik]

[-- Attachment #1: Type: text/plain, Size: 2284 bytes --]

On 06.04.2013 20:08, Michał Górny wrote:
> Hello,
> 
> As far as I'm aware, we don't really have much of a patch maintenance
> policy in Gentoo. There a few loose rules like «don't put awfully big
> files into FILESDIR» or the common sense «use unified diff», but no
> complete and clear policy.
> 
> Especially considering the late discussion related to the needless
> and semi-broken functionality in epatch, I'd like to propose
> setting the following rules for patches in tree and in Gentoo-sourced
> patchsets:
> 
> 1. Patches have to be either in unified or context diff format. Unified
> diff is preferred.
> 
> 2. Patches have to apply to the top directory of the source tree with
> 'patch -p1'. If patches are applied to sub-directories, necessary '-p'
> argument shall be passed to 'epatch' explicitly. Developers are
> encouraged to create patches which are compatible with 'git am'.
> 
> 3. Patches have to end with either '.patch' or '.diff' suffix.
> 
> 4. If possible, patches shall be named in a way allowing them to be
> applied in lexical order. However, this one isn't necessary if patches
> from an older ebuild are applied to a newer one.
> 
> 5. The patch name shall shortly summarize the changes done by it.
> 
> 6. Patch files shall start with a brief description of what the patch
> does. Developers are encouraged to use git-style tags like 'Fixes:' to
> point to the relevant bug URIs.
> 
> 7. Patch combining is discouraged. Developers shall prefer multiple
> patches following either the upstream commits or a logical commit
> sequence (if changes are not committed upstream).
> 
> The above-listed policy will apply to the patches kept in the gx86 tree
> (in FILESDIRs) and patch archives created by Gentoo developers. They
> will not apply to the patch archives created upstream.
> 

Hi,
there's at least one guideline written by the Ancient Ones that I know
[1] It roughly follows the ideas that you've described. I think it'd be
enough if people read it and used as a suggestion not a strict ruling.
Imposing things like lexical order or git-style heading is a bit too
much for me.

Do we really need rules for everything?

Cheers,
Kacper

[1] http://dev.gentoo.org/~vapier/clean-patches


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

[gentoo-dev] Re: [RFC] Establishing Gentoo patch policy to keep our patches consistent and clean

[Michael Palimaka]

On 7/04/2013 16:53, Kacper Kowalik wrote:
> On 06.04.2013 20:08, Michał Górny wrote:
>> Hello,
>>
>> As far as I'm aware, we don't really have much of a patch maintenance
>> policy in Gentoo. There a few loose rules like «don't put awfully big
>> files into FILESDIR» or the common sense «use unified diff», but no
>> complete and clear policy.
>>
>> Especially considering the late discussion related to the needless
>> and semi-broken functionality in epatch, I'd like to propose
>> setting the following rules for patches in tree and in Gentoo-sourced
>> patchsets:
>>
>> 1. Patches have to be either in unified or context diff format. Unified
>> diff is preferred.
>>
>> 2. Patches have to apply to the top directory of the source tree with
>> 'patch -p1'. If patches are applied to sub-directories, necessary '-p'
>> argument shall be passed to 'epatch' explicitly. Developers are
>> encouraged to create patches which are compatible with 'git am'.
>>
>> 3. Patches have to end with either '.patch' or '.diff' suffix.
>>
>> 4. If possible, patches shall be named in a way allowing them to be
>> applied in lexical order. However, this one isn't necessary if patches
>> from an older ebuild are applied to a newer one.
>>
>> 5. The patch name shall shortly summarize the changes done by it.
>>
>> 6. Patch files shall start with a brief description of what the patch
>> does. Developers are encouraged to use git-style tags like 'Fixes:' to
>> point to the relevant bug URIs.
>>
>> 7. Patch combining is discouraged. Developers shall prefer multiple
>> patches following either the upstream commits or a logical commit
>> sequence (if changes are not committed upstream).
>>
>> The above-listed policy will apply to the patches kept in the gx86 tree
>> (in FILESDIRs) and patch archives created by Gentoo developers. They
>> will not apply to the patch archives created upstream.
>>
>
> Hi,
> there's at least one guideline written by the Ancient Ones that I know
> [1] It roughly follows the ideas that you've described. I think it'd be
> enough if people read it and used as a suggestion not a strict ruling.
> Imposing things like lexical order or git-style heading is a bit too
> much for me
>
> Do we really need rules for everything?
>
> Cheers,
> Kacper
>
> [1] http://dev.gentoo.org/~vapier/clean-patches
>

We already have policy regarding patches[1], this just expands and 
formalises it a bit more.

Regarding lexical order and git-style headings, my interpretation is 
that this is a recommendation only. I don't think the intention is to 
make you rebase complex patches needlessly.

vapier's clean-patches document is an informative read, but I don't 
think devspace is a good method of disseminating of information that may 
not necessarily reflect the reality of the project.

[1]: 
http://devmanual.gentoo.org/ebuild-writing/misc-files/patches/index.html

[gentoo-dev] Re: [RFC] Establishing Gentoo patch policy to keep our patches consistent and clean

[Steven J. Long]

On Mon, Apr 08, 2013 at 12:30:08AM +1000, Michael Palimaka wrote:
> On 7/04/2013 16:53, Kacper Kowalik wrote:
> > On 06.04.2013 20:08, Michał Górny wrote:
> >> As far as I'm aware, we don't really have much of a patch maintenance
> >> policy in Gentoo. There a few loose rules like «don't put awfully big
> >> files into FILESDIR» or the common sense «use unified diff», but no
> >> complete and clear policy.

As soon as I saw this I thought of the same clean-patches doc as Kacper.

 1-4 are basically about making patches work better in the automated ebuild
framework, and I heartily agree (when it does not entail modifying 'upstream'
or rather imported patches; shared with other distros in particular.) Your
other post made a lot of sense (with the glob restriction.)

> >> 5. The patch name shall shortly summarize the changes done by it.
> >>
> >> 6. Patch files shall start with a brief description of what the patch
> >> does. Developers are encouraged to use git-style tags like 'Fixes:' to
> >> point to the relevant bug URIs.
> >>
> >> 7. Patch combining is discouraged. Developers shall prefer multiple
> >> patches following either the upstream commits or a logical commit
> >> sequence (if changes are not committed upstream).
> >>
> >> The above-listed policy will apply to the patches kept in the gx86 tree
> >> (in FILESDIRs) and patch archives created by Gentoo developers. They
> >> will not apply to the patch archives created upstream.
> >>

 Patches in FILESDIR are often those imported from and shared with other
distros.

> > there's at least one guideline written by the Ancient Ones that I know
> > [1] It roughly follows the ideas that you've described. I think it'd be
> > enough if people read it and used as a suggestion not a strict ruling.
> > Imposing things like lexical order or git-style heading is a bit too
> > much for me
> >
> > Do we really need rules for everything?
> >
> > Cheers,
> > Kacper
> >
> > [1] http://dev.gentoo.org/~vapier/clean-patches
> >
> 
> We already have policy regarding patches[1], this just expands and 
> formalises it a bit more.

 Trouble is it's got a lot less meat than vapier's doc. If you're "expanding
and formalising" it's a good opportunity to add more relevant info, as well
as the formalities that will make EAPI-6 patching cleaner.
 
> vapier's clean-patches document is an informative read, but I don't 
> think devspace is a good method of disseminating of information that may 
> not necessarily reflect the reality of the project.

So why not just merge vapiers work into the devmanual, along with updated
info to deal with newer git style tags? ATM the devmanual is woefully thin
in comparison; it should be more prescriptive, along with the reasons, just
like vapier wrote it the first time around (I actually link people to
vapier's doc on IRC, but I'd never link them to the current devmanual as it
has little tutorial content for a beginner, just some basic info mostly
Gentoo-specific.)

Sure, you can make the language a bit nicer, but really before you push policy
there needs to be best practice info in place first (and that usually means
content with wider development context, like the clean-patches doc.)

Regards,
steveL. 
-- 
#friendly-coders -- Where everybody knows your nick ;-)

[gentoo-dev] Re: [RFC] Establishing Gentoo patch policy to keep our patches consistent and clean

[Ryan Hill]

[-- Attachment #1: Type: text/plain, Size: 5697 bytes --]

On Sat, 6 Apr 2013 20:08:43 +0200
Michał Górny <mgorny@gentoo.org> wrote:

> Hello,
> 
> As far as I'm aware, we don't really have much of a patch maintenance
> policy in Gentoo. There a few loose rules like «don't put awfully big
> files into FILESDIR» or the common sense «use unified diff», but no
> complete and clear policy.
> 
> Especially considering the late discussion related to the needless
> and semi-broken functionality in epatch, I'd like to propose
> setting the following rules for patches in tree and in Gentoo-sourced
> patchsets:
> 
> 1. Patches have to be either in unified or context diff format. Unified
> diff is preferred.

Yes.

> 2. Patches have to apply to the top directory of the source tree with
> 'patch -p1'. If patches are applied to sub-directories, necessary '-p'
> argument shall be passed to 'epatch' explicitly. Developers are
> encouraged to create patches which are compatible with 'git am'.

Bikeshedding below.

> 3. Patches have to end with either '.patch' or '.diff' suffix.

Yes.

> 4. If possible, patches shall be named in a way allowing them to be
> applied in lexical order. However, this one isn't necessary if patches
> from an older ebuild are applied to a newer one.

Even more below.

> 5. The patch name shall shortly summarize the changes done by it.

Yes.

> 6. Patch files shall start with a brief description of what the patch
> does. Developers are encouraged to use git-style tags like 'Fixes:' to
> point to the relevant bug URIs.

Yes.

> 7. Patch combining is discouraged. Developers shall prefer multiple
> patches following either the upstream commits or a logical commit
> sequence (if changes are not committed upstream).

Kinda.

> 
> The above-listed policy will apply to the patches kept in the gx86 tree
> (in FILESDIRs) and patch archives created by Gentoo developers. They
> will not apply to the patch archives created upstream.
> 
> 
> RATIONALE
> ---------
> 
> The main reason for the whole policy is to let Gentoo supply users with
> consistent and friendly patches. That is, patches which can be used
> directly on a source tree or submitted upstream without any additional
> actions from user.
> 
> (1) lists the most common patch formats. The formats shall be generally
> both readable and reusable whenever possible. I wanted just 'unified
> diff' but ulm suggested that we should also support those upstreams who
> use 'context diffs'.
> 
> 
> (2) is likely to be a bikeshed point here. Long story short, epatch has
> this fragile patchlevel guessing, users don't have it. If we keep our
> patches consistent to a single patchlevel, we gain:
> 
> * ability for users to apply the patches without having them try all
>   patchlevels by hand.
> 
> * clean error output if patch stops to apply for some reason.
> 
> * no risk that patch will get applied to the wrong file if patch stops
>   to apply at expected patchlevel and starts to apply on another.

If I can't drop patches from an upstream mailing list with a different repo
structure than source, or just a random user who doesn't know better, into my
patchdir I'm going to be very grumpy.

Our patches should be -p1, I agree.  I respin any patches I find that aren't.

The first two points are nice, but not worth it to me in particular.  The third
I don't see the risk.  What are the chances of that patch actually applying
successfully?  It'll fail, and you root it out.  And since all our own patches
will be -p1 this becomes a tiny corner case.


> Also, by creating git-format patches, we gain the ability of rebasing
> and updating the patches easily. Even with non-git upstreams, we can
> do:
> 
>   git init; git add -A; git commit -m 1; git am ...
>
> (3) should be mostly obvious. The main idea is that if we apply a whole
> patchdir, we should be able to easily tell between patches
> and auxiliary files like 'README' or Debian's 'series'.
> 
> I have never seen a patch file named other than '*.patch' or '*.diff'.
> Therefore, I think that it's better to just require those rather than
> trying to provide a sane list of excludes.
> 
> 
> (4) is mostly about friendliness (again). Since shell does filename
> expansion in lexical order, it's just great for user to be able apply
> patches like:
> 
>   git am /usr/portage/foo/bar/files/198-*.patch
> 
> The other sentence is not to enforce this rule e.g. when the same patch
> is applied to different versions of the same package. Although with
> a fair of trickery that could be gotten working, I don't think it will
> be user-friendly anymore :).

That's useful but ${PN}-${PV}-desc.patch is the currently accepted convention.
What you're suggesting here is a suggested workflow.  I don't think it belongs
in a policy doc, at least as it's own section, or at least until it becomes
more commonplace.

> (5) makes finding a particular patch of interest easier, while (6)
> makes sure that the purpose of the patch can be read from patch alone.
> In both cases, having described patches is much better than having to
> look into ebuilds for explanations.
> 
> 
> (7) is because merged patches are usually hard to read and completely
> not suitable for submitting anywhere.

Well, patches should do one thing, agreed.  But I'm not adding 4 patches to
respect LDFLAGS, use CFLAGS while linking, install into /usr, and fix the
version number in a 30 line Makefile.  I'm adding foo-0.01-makefile.patch which
does one thing (fixes crappy-ass Makefile).  I can live with "discouraged" though.


-- 
gcc-porting
toolchain, wxwidgets
@ gentoo.org

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 490 bytes --]