User:Veremit/Patch format

From Gentoo Wiki
Jump to:navigation Jump to:search

Clean Patch HOWTO

Note
This article was originally published by Mike Frysinger (vapier) and is also published in his devspace.

When I say "clean patch", I am not referring to the patch itself (as in the changes it makes to the source code), I am referring to all the metadata that exists in the patch to make it "maintainable".

Why?

You might be thinking "wow, this looks like effort". well you best shut your brain hole and do it anyways. Seriously though ...

This may take more effort "up front", but the amount of effort that it saves for everyone else in the future more than makes up for it. i refer to other distributions or upstream maintainers that read the patch, or future Gentoo maintainers / developers. Too many hours have I spent staring at a patch (whether it be long or tiny) with no documentation and no references as to why the changes it is making to a package exist at all. By keeping all patches "clean", people can quickly and easily assess a patch without searching through a metric butt ton of other files.

So make your patch clean in the first place and stop screwing others in ways they do not enjoy. Stick with the pleasant methods please.

File Naming

This is quick to do, so let's get it out of the way. your patch name should be short and to the point. When doing a file listing (e.g. `ls files/`), it's a lot easier to be able to scan for relevant patches when they have good keywords in their file names.

It should also include the package name and the version it was written against. This way people searching for patches or who happen to just stumble across the file itself have a clue as to what it's for. Stripping out the $PN (and to a lesser extent, the $PV) makes the filename significantly less useful. The fact the files are typically stored in $CATEGORY/$PN/files/ is irrelevant. We're trying to think beyond the Gentoo box here.

It's also more consistent. Consistency matters as minor/pointless deviations only serve to slow people down.

How?

Here's a check list of things to keep in the patch header:

  • external references
    • upstream mailing archives
    • upstream bug reports
    • upstream commit links
    • upstream changelog entries
    • Gentoo bug reports
  • short / medium explanation
    • why is the patch needed ?
    • what is it fixing ?
    • why is it fixing it the way it is ?
    • proposal for better fixes in the future ?
    • is it a stop gap measure (workaround) ?
    • how was it regression tested ?
    • examples of before / after behavior
      • how to reproduce bug w/out patch
      • how to show bug is fixed after patch
      • maybe upstream fixed it in a different way, so this test can be used to show that the patch is no longer needed w/newer versions
  • status
    • was it merged/rejected/postponed/etc... upstream ?
    • is it distribution-specific ?
  • attribution
    • who found the bug ?
    • who fixed the bug ?
    • who wrote the patch ?
    • who tested the patch ?
    • who gave advice on the patch ?

All this information should be *in the patch itself*. It should never ever be found in something like the ebuild. If you really really want to put a comment next to a patch in an ebuild, then this is about the only thing that is OK: 

epatch "${FILESDIR}"/${P}-fatty-cow.patch #12345

(where 12345 is the Gentoo bug #)

when documenting these things, it might be useful to use RFC822/git style tags to format the metadata. so when documenting the author, use: 

From: Nice Person <foo@cow.com>

or when documenting relevant urls, use something like: 

Project-Bug-URL: http://upstream.tracker.com/12345
Gentoo-Bug-URL: http://bugs.gentoo.org/9889

and if you want to note your approval, slap on a s-o-b tag: 

Signed-off-by: Diligent Developer <funky-cow-butt@gentoo.org>

Finally, your patch should be clear of useless cruft. if it was not taken straight from an upstream SCM (`git format-patch` or `svn diff -r #` or `cvs diff -r 1.123 -r 1.124`), then the metadata is useless, so delete it. I'm referring to things like the diff command used to produce the patch, or the timestamps on the files, local revision info, or other similar spam. note that the context info (the stuff that comes after the @@) should be left as that can be invaluable when applying patches to later versions. for example: 

@@ -80,6 +82,7 @@ case $sys in
                  ^^^^^^^^^^^^ keep this part

Extra points if you make the filename in the ---/+++ section consistent and sane. i.e. remove different leading backup/paths/ and .orig/.new suffixes. Extra extra points if your patch is in the -p1 format. This tends to be much more standard than any other -p#. A good suggestion is to use the package name / version as the leading portion that gets stripped.

Also note that while `patch` uses the timestamp info in order to remove empty files automatically, in Gentoo, we apply all patches with -E, so the timestamp info does not matter. If you really want to keep an empty file around though, just replace the file with a comment or an empty line or ...

If deleting these things yourself sounds like the kind of fun that involves nipple clamps and electricity, try this:

FILE some-patch-you-found.patch
scrub_patch() {
	sed -i \
		-e '/^index /d' \
		-e '/^new file mode /d' \
		-e '/^Index:/d' \
		-e '/^=========/d' \
		-e '/^RCS file:/d' \
		-e '/^retrieving/d' \
		-e '/^diff/d' \
		-e '/^Files .* differ$/d' \
		-e '/^Only in /d' \
		-e '/^Common subdirectories/d' \
		-e '/^deleted file mode [0-9]*$/d' \
		-e '/^+++/s:\t.*::' \
		-e '/^---/s:\t.*::' \
		"$@"
}

Some more info can be found here: http://devmanual.gentoo.org/ebuild-writing/misc-files/patches/index.html

Example

Here we see a simple explanation and a URL for more info (this patch could do with some attribution however ...). no metadata exists from running the `diff` command (timestamps, etc...). 

<<<<<<<<<<<<<<<<<<<<<<<<<<<<< man-1.6d-fbsd.patch >>>>>>>>>>>>>>>>>>>>>>>>>>>>>
Fixes compilation in FreeBSD

http://bugs.gentoo.org/138123

--- man-1.6d/gencat/genlib.c
+++ man-1.6d/gencat/genlib.c
@@ -54,7 +54,7 @@
 #include <unistd.h>
 #endif

-#ifndef __linux__
+#if !defined(__linux__) && !defined(__FreeBSD__)
 #include <memory.h>
 static int bcopy(src, dst, length)
 char *src, *dst;

<<<<<<<<<<<<<<<<<<<<<<<<<<<<< man-1.6d-fbsd.patch >>>>>>>>>>>>>>>>>>>>>>>>>>>>>


<<<<<<<<<<<<<<<<<<<<<< util-linux-2.12q-dont-umask.patch >>>>>>>>>>>>>>>>>>>>>>
Don't force umask to 022 or the -o umask option doesn't work.

Patch by Daniel Drake.

http://bugs.gentoo.org/93671

--- mount/mount.c
+++ mount/mount.c
@@ -1491,8 +1491,6 @@ main(int argc, char *argv[]) {
    if ((p = strrchr(progname, '/')) != NULL)
        progname = p+1;

-   umask(022);
-
    /* People report that a mount called from init without console
       writes error messages to /etc/mtab
       Let us try to avoid getting fd's 0,1,2 */
<<<<<<<<<<<<<<<<<<<<<< util-linux-2.12q-dont-umask.patch >>>>>>>>>>>>>>>>>>>>>>


<<<<<<<<<<<<<<<<<<<< iproute2-2.6.25.20080417-build.patch >>>>>>>>>>>>>>>>>>>>>
dont let target flags bleed into build flags

fix by Bertrand Jacquin

http://bugs.gentoo.org/226035

--- netem/Makefile
+++ netem/Makefile
@@ -2,6 +2,7 @@ DISTGEN = maketable normal pareto paretonormal
 DISTDATA = normal.dist pareto.dist paretonormal.dist experimental.dist

 HOSTCC ?= $(CC)
+CCOPTS  = $(CBUILD_CFLAGS)
 LDLIBS += -lm

 all: $(DISTGEN) $(DISTDATA)
<<<<<<<<<<<<<<<<<<<< iproute2-2.6.25.20080417-build.patch >>>>>>>>>>>>>>>>>>>>>
Retrieved from "https://wiki.gentoo.org/index.php?title=User:Veremit/Patch_format&oldid=884358"