Clean Patch Howto

From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lists.gentoo.org (pigeon.gentoo.org [208.92.234.80]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by finch.gentoo.org (Postfix) with ESMTPS id 2851D1382C5 for ; Sat, 20 Feb 2021 08:01:00 +0000 (UTC) Received: from pigeon.gentoo.org (localhost [127.0.0.1]) by pigeon.gentoo.org (Postfix) with SMTP id 0E972E0823; Sat, 20 Feb 2021 08:00:59 +0000 (UTC) Received: from smtp.gentoo.org (mail.gentoo.org [IPv6:2001:470:ea4a:1:5054:ff:fec7:86e4]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by pigeon.gentoo.org (Postfix) with ESMTPS id DAAD2E0823 for ; Sat, 20 Feb 2021 08:00:58 +0000 (UTC) Received: from oystercatcher.gentoo.org (oystercatcher.gentoo.org [148.251.78.52]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.gentoo.org (Postfix) with ESMTPS id 74438340FB1 for ; Sat, 20 Feb 2021 08:00:57 +0000 (UTC) Received: from localhost.localdomain (localhost [IPv6:::1]) by oystercatcher.gentoo.org (Postfix) with ESMTP id B88AE4C6 for ; Sat, 20 Feb 2021 08:00:55 +0000 (UTC) From: "Ulrich Müller" To: gentoo-commits@lists.gentoo.org Content-Transfer-Encoding: 8bit Content-type: text/plain; charset=UTF-8 Reply-To: gentoo-dev@lists.gentoo.org, "Ulrich Müller" Message-ID: <1613807642.8ffb4ecb220c9126a9a57ac3d2591a2604cdd438.ulm@gentoo> Subject: [gentoo-commits] proj/devmanual:master commit in: ebuild-writing/misc-files/patches/ X-VCS-Repository: proj/devmanual X-VCS-Files: ebuild-writing/misc-files/patches/text.xml X-VCS-Directories: ebuild-writing/misc-files/patches/ X-VCS-Committer: ulm X-VCS-Committer-Name: Ulrich Müller X-VCS-Revision: 8ffb4ecb220c9126a9a57ac3d2591a2604cdd438 X-VCS-Branch: master Date: Sat, 20 Feb 2021 08:00:55 +0000 (UTC) Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-Id: Gentoo Linux mail X-BeenThere: gentoo-commits@lists.gentoo.org X-Auto-Response-Suppress: DR, RN, NRN, OOF, AutoReply X-Archives-Salt: 340de210-2acd-44f8-abaf-e9e1a0e02ddd X-Archives-Hash: 6a6d6df16cf0de262fd4251f8d5a7fbb commit: 8ffb4ecb220c9126a9a57ac3d2591a2604cdd438 Author: Ulrich Müller gentoo org> AuthorDate: Tue Feb 16 19:31:37 2021 +0000 Commit: Ulrich Müller gentoo org> CommitDate: Sat Feb 20 07:54:02 2021 +0000 URL: https://gitweb.gentoo.org/proj/devmanual.git/commit/?id=8ffb4ecb ebuild-writing/misc-files/patches: Convert to DevBook XML markup - Start sentences with uppercase letters - Change some sentences from first-person to third-person - Less informal/colloquial style - Modernise the text a little (e.g., eapply instead of epatch) - Be more in line with GLEP 66 (e.g., "Bug" instead of "Gentoo-Bug-URL") Closes: https://bugs.gentoo.org/247266 Closes: https://github.com/gentoo/devmanual/pull/192 Signed-off-by: Ulrich Müller gentoo.org> ebuild-writing/misc-files/patches/text.xml | 337 +++++++++++++++++------------ 1 file changed, 200 insertions(+), 137 deletions(-) diff --git a/ebuild-writing/misc-files/patches/text.xml b/ebuild-writing/misc-files/patches/text.xml index 21e6ef8..62b8a04 100644 --- a/ebuild-writing/misc-files/patches/text.xml +++ b/ebuild-writing/misc-files/patches/text.xml @@ -114,123 +114,189 @@ from the vim patch tarball: Clean Patch Howto +

+"Clean patch" does not refer to the patch itself (as in the changes it makes to +the source code). It refers to all the metadata that exists in the patch to +make it "maintainable". +

+ + + + +Why + + +

+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. This refers to other +distributions or upstream maintainers who read the patch, or future Gentoo +maintainers. By keeping all patches "clean", people can quickly and easily +assess a patch without searching through many other files. +

+ + + + + +File Naming + + +

+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, because the patch may be used +outside Gentoo. +

+ + + + + +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 behaviour +
  - How to reproduce bug without 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 with 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 be +found in something like the ebuild. If you really want to put a comment next +to a patch in an ebuild, then this is about the only thing that is OK +(where 93671 is the Gentoo bug number): +

+ + +PATCHES=( + "${FILESDIR}"/${P}-dont-umask.patch #93671 +) + + +

+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.example.com>
+

+ +

+Or when documenting relevant URLs, use something like: +

+ +

+Bug: https://upstream.example.com/12345
+Bug: https://bugs.gentoo.org/9889
+

+ +

+And if you want to note your copyright signoff, slap on a Signed-off-by tag: +

+ +

+Signed-off-by: Diligent Developer <larry@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. This refers 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: +

----------------------
-  CLEAN PATCH HOWTO
----------------------
-
-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:
+

+ +

+Extra points if you make the filename in the ---/+++ section +consistent and sane. That is, remove different leading backup/paths/ +and .orig/.new suffixes. Your patch should be in the -p1 +format because this tends to be much more standard than any other -p#. +It is also what eapply understands by default. A good suggestion is to +use either a/ and b/ (as in git format-patch) or the +package name/version as the leading portion that gets stripped. +

+ +

+Also note that patch uses the timestamp info in order to remove empty +files automatically. Alternatively, you can specify the -E option with +eapply if you want to remove an empty file. +

+ +

+The following function (for your interactive shell, not for the ebuild) will +help deleting these things: +

+ + scrub_patch() { sed -i \ -e '/^index /d' \ @@ -248,22 +314,26 @@ scrub_patch() { -e '/^---/s:\t.*::' \ "$@" } + scrub_patch some-patch-you-found.patch + -some more info can be found here: -http://devmanual.gentoo.org/ebuild-writing/misc-files/patches/index.html + + ------------ - EXAMPLE ------------ + +Examples + -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. +

+This shows 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.). +

-http://bugs.gentoo.org/138123 +


+Fixes compilation in FreeBSD.
+https://bugs.gentoo.org/138123
 
 --- man-1.6d/gencat/genlib.c
 +++ man-1.6d/gencat/genlib.c
@@ -276,15 +346,11 @@ http://bugs.gentoo.org/138123
  #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.
-
 https://bugs.gentoo.org/93671
 
 --- mount/mount.c
@@ -298,14 +364,11 @@ https://bugs.gentoo.org/93671
     /* 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 >>>>>>>>>>>>>>>>>>>>> +


 Don't let target flags bleed into build flags.
-
 Fix by Bertrand Jacquin.
-
 https://bugs.gentoo.org/226035
 
 --- netem/Makefile
@@ -318,10 +381,10 @@ https://bugs.gentoo.org/226035
  LDLIBS += -lm
 
  all: $(DISTGEN) $(DISTDATA)
-<<<<<<<<<<<<<<<<<<<< iproute2-2.6.25.20080417-build.patch >>>>>>>>>>>>>>>>>>>>>