Document more details of patch format such as the "from" line
used to specify the patch author, and provide more references
for patch guidelines.
Signed-off-by: Paul Jackson <[email protected]>
Index: 2.6.14-rc2-mm2/Documentation/SubmittingPatches
===================================================================
--- 2.6.14-rc2-mm2.orig/Documentation/SubmittingPatches
+++ 2.6.14-rc2-mm2/Documentation/SubmittingPatches
@@ -301,8 +301,46 @@ now, but you can do this to mark interna
point out some special detail about the sign-off.
+12) The canonical patch format
-12) More references for submitting patches
+The canonical patch subject line is:
+
+ Subject: [PATCH 001/123] [<area>:] <explanation>
+
+The canonical patch message body contains the following:
+
+ The first line of the body contains a "from" line specifying
+ the author of the patch:
+
+ From: Original Author <[email protected]>
+
+ If the "from" line is missing, then the author of the patch will
+ be recorded in the source code revision history as whomever sent
+ Linus the email (the email message "From:" header.)
+
+ The "from" line is followed by an empty line and then the body
+ of the explanation.
+
+ After the body of the explanation comes the "Signed-off-by:"
+ lines, and then a simple "---" line, and below that comes the
+ diffstat of the patch and then the patch itself. The "---" line
+ and diffstat are optional, but helpful to readers of non-trivial
+ patches.
+
+The Subject line format makes it very easy to sort the emails
+alphabetically by subject line - pretty much any email reader will
+support that - since because the sequence number is zero-padded,
+the numerical and alphabetic sort is the same.
+
+See further details on how to phrase the "<explanation>" in
+the "Subject:" line in Andrew Morton's "The perfect patch",
+referenced below.
+
+See more details on the proper patch format in the following
+references.
+
+
+13) More references for submitting patches
Andrew Morton, "The perfect patch" (tpp).
<http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt>
@@ -310,6 +348,14 @@ Andrew Morton, "The perfect patch" (tpp)
Jeff Garzik, "Linux kernel patch submission format."
<http://linux.yyz.us/patch-format.html>
+Greg KH, "How to piss off a kernel subsystem maintainer"
+ <http://www.kroah.com/log/2005/03/31/>
+
+Kernel Documentation/CodingStyle
+ <http://sosdg.org/~coywolf/lxr/source/>
+
+Linus Torvald's mail on the canonical patch format:
+ <http://lkml.org/lkml/2005/4/7/183>
-----------------------------------
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.650.933.1373
On Sun, 2 Oct 2005, Paul Jackson wrote:
>
> Document more details of patch format such as the "from" line
> used to specify the patch author, and provide more references
> for patch guidelines.
One more issue: I'd really prefer that the "---" not be documented as
"optional".
Yes, my tools will also notice "diff -" and "Index: " at the start of the
line as being markers for where the real patch starts, but that's a hack
because people haven't been following the "---" rule. I'd much rather make
it clear that the "---" is supposed to be there, to mark where the end of
the comments are.
Note that after the "---" you can have any amount of explanations that are
not to be committed as comments on why. Not just a "diffstat", but in
general it's an area for explaining why I should commit it or random notes
on the patch. Things that don't necessarily make sense once the patch _is_
committed.
For example, a submitter might want to note that he's working on a better
patch, but that this patch is the minimal one before a release. Things he
wants to tell the maintainer, but that don't necessarily make sense to
anybody else.
So the _diffstat_ is optional (although much preferred, especially for
bigger patches), but the marker of "this is the end of the comments for
the changelog" is not.
Linus
Linus wrote:
> One more issue: I'd really prefer that the "---" not be documented as
> "optional".
Makes sense. Thanks for taking the time to spell it out
PATCHv3 coming up soon.
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.925.600.0401
On 10/3/05, Linus Torvalds <[email protected]> wrote:
> On Sun, 2 Oct 2005, Paul Jackson wrote:
> >
> > Document more details of patch format such as the "from" line
> > used to specify the patch author, and provide more references
> > for patch guidelines.
>
> One more issue: I'd really prefer that the "---" not be documented as
> "optional".
>
> Yes, my tools will also notice "diff -" and "Index: " at the start of the
> line as being markers for where the real patch starts, but that's a hack
> because people haven't been following the "---" rule. I'd much rather make
> it clear that the "---" is supposed to be there, to mark where the end of
> the comments are.
Huh, I thought that the first line in a unified patch started with
"---", and that the lines above were treated as garbage. Relying on
"diff -" or "Index: " seems wrong. Try diffing two files by "diff -u
file1 file2" and look at the output - the first line is "---"... This
extra "---" you are proposing seems like a workaround to me.
/ magnus
Magnus wrote:
> Huh, I thought that the first line in a unified patch started with
> "---",
Yes, it does. But then the next character is a space, and there
is at least one more space-separated field on the line.
That is, Linus's marker marches
/^---$/
and the first line of a unified patch matches:
/^--- .*[^ ].*$/
It ain't fancy, but it works. These patterns
are easily distinguished.
> Relying on "diff -" or "Index: " seems wrong.
I don't think anyone is relying on those patterns.
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.925.600.0401
On 10/4/05, Paul Jackson <[email protected]> wrote:
> Magnus wrote:
> > Huh, I thought that the first line in a unified patch started with
> > "---",
>
> Yes, it does. But then the next character is a space, and there
> is at least one more space-separated field on the line.
Nitpicking, but with "diff -L" only one space exists, and that is
between "---" and the label. So it is possible to create patches
without time stamp information.
> > Relying on "diff -" or "Index: " seems wrong.
>
> I don't think anyone is relying on those patterns.
Good. Thanks.
/ magnus
On Tue, 4 Oct 2005, Magnus Damm wrote:
>
> Huh, I thought that the first line in a unified patch started with
> "---", and that the lines above were treated as garbage.
Indeed, that's true of traditional unified diffs. That's why my tools
started using "---" in the first place. Cutting off at the "---" means
that it's _guaranteed_ that we never include any valid patch in the
description by mistake rather than applying it.
And I _used_ to just hand-edit emails so that things like "Please apply"
and "Thanks", and "diff -urN" didn't show up.
However, note that "git diffs" can actually contain renames, so a git diff
may have relevant stuff before the "---". For example:
diff --git a/arch/v850/kernel/asm-consts.c b/arch/v850/kernel/asm-offsets.c
similarity index 100%
rename from arch/v850/kernel/asm-consts.c
rename to arch/v850/kernel/asm-offsets.c
diff --git a/arch/v850/kernel/entry.S b/arch/v850/kernel/entry.S
--- a/arch/v850/kernel/entry.S
+++ b/arch/v850/kernel/entry.S
@@ -22,7 +22,7 @@
#include <asm/irq.h>
#include <asm/errno.h>
-#include <asm/asm-consts.h>
+#include <asm/asm-offsets.h>
/* Make a slightly more convenient alias for C_SYMBOL_NAME. */
is a real diff as far as git is concerned, and is obviously much more
readable (and much more compact) than the traditional broken "delete file
and re-create it under another name" kind of diff.
So "^diff --git " is actually the real marker for where a git diff starts.
> Relying on "diff -" or "Index: " seems wrong. Try diffing two files by
> "diff -u file1 file2" and look at the output - the first line is
> "---"... This extra "---" you are proposing seems like a workaround to
> me.
With traditional unified patches nothing _relies_ of "diff -" or "Index:",
exactly because "---" is _always_ seen as the beginning of the patch. But
even then the patch applicator often wants to see the "diff -" or "Index:"
line, because it can use them to disambiguate file names.
So there are real _technical_ reasons so make sure that we know where the
diff starts, and "---" is a good marker, because it's a marker that is
guaranteed to be there, regardless.
But even apart from those technical reasons, I don't want the first "diff
-urN ..." or "Index:" line to show up as a commit message. So the tools
will _also_ break the commit messages at those markers, but that's a hack.
So the _real_ rule is that we break at "---". No ifs, buts, or maybes. And
no, it's not "extra" or "optional" or "unnecessary".
And it also allows people to put extra commentary, like a diffstat, or a
private message to me about why I should apply the patch.
Linus