Skip to contentSkip to conversation

Document from line in patch format

PATCH Superseded Paul Jackson 3 participants 7 messages Oct 2, 2005 – Oct 4, 2005 Documentation

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 Document from line in patch format

Document more details of patch format such as the "from" line used to specify the patch author, and provide more references for patch guidelines.

Documentation/SubmittingPatches +47 -1
Index: 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>
 
 
 -----------------------------------
Signature · 3 lines
                          I won't rest till it's the best ...
                          Programmer, Linux Scalability
                          Paul Jackson <[email protected]> 1.650.933.1373
#1 lore raw
Quoted 4 lines · Paul Jackson

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

#2 lore raw
Quoted 2 lines · Linus

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.

Signature · 3 lines
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <[email protected]> 1.925.600.0401
#3 lore raw
In reply to Linus Torvalds
Quoted 6 lines · Linus Torvalds
Quoted 4 lines · Paul Jackson

Document more details of patch format such as the "from" line used to specify the patch author, and provide more references for patch guidelines.

Quoted 8 lines

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

#4 lore raw
Quoted 2 lines · Magnus

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.

Quoted 1 line

Relying on "diff -" or "Index: " seems wrong.

I don't think anyone is relying on those patterns.

Signature · 3 lines
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <[email protected]> 1.925.600.0401
#5 lore raw
Quoted 6 lines · Paul Jackson
Quoted 2 lines · Magnus

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.

Quoted 3 lines
Quoted 1 line

Relying on "diff -" or "Index: " seems wrong.

I don't think anyone is relying on those patterns.

Good. Thanks.

/ magnus

#6 lore raw
In reply to Magnus Damm
Quoted 3 lines · Magnus Damm

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.

Quoted 4 lines

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