[bug#57598] [PATCH] doc: Update contribution guidelines on patches, etc.

[Maxime Devos]

* doc/contributing.texi (Modifying Sources): Replaced with ...
("Modifying Sources"): ... this.  List more use cases and some principles.

This patch incorporates some tet written by Liliana Marie Prikler.  The
structure is based on a proposal by Julien Lepiller.  The text has been
revised based on reviews by David Larsson and blake.
---
 doc/contributing.texi | 141 +++++++++++++++++++++++++++++++++++++-----
 doc/guix.texi         |   2 +-
 2 files changed, 127 insertions(+), 16 deletions(-)

diff --git a/doc/contributing.texi b/doc/contributing.texi
index 02c7c5ae59..f6bdb8a441 100644
--- a/doc/contributing.texi
+++ b/doc/contributing.texi
@@ -441,7 +441,7 @@ needed is to review and apply the patch.
 * Package Naming::              What's in a name?
 * Version Numbers::             When the name is not enough.
 * Synopses and Descriptions::   Helping users find the right package.
-* Snippets versus Phases::      Whether to use a snippet, or a build phase.
+* Modifying Sources::           Whether to use a snippet, or a build phase.
 * Emacs Packages::              Your Elisp fix.
 * Python Modules::              A touch of British comedy.
 * Perl Modules::                Little pearls.
@@ -698,20 +698,131 @@ Gettext}):
 for the X11 resize-and-rotate (RandR) extension. @dots{}")
 @end lisp
 
-@node Snippets versus Phases
-@subsection Snippets versus Phases
-
-@cindex snippets, when to use
-The boundary between using an origin snippet versus a build phase to
-modify the sources of a package can be elusive.  Origin snippets are
-typically used to remove unwanted files such as bundled libraries,
-nonfree sources, or to apply simple substitutions.  The source derived
-from an origin should produce a source that can be used to build the
-package on any system that the upstream package supports (i.e., act as
-the corresponding source).  In particular, origin snippets must not
-embed store items in the sources; such patching should rather be done
-using build phases.  Refer to the @code{origin} record documentation for
-more information (@pxref{origin Reference}).
+@node Modifying Sources
+@subsection Modifying Sources
+
+Guix has tree main ways of modifying the source code of a package, that
+you as a packager may use.  These are patches, snippets and phases.
+Each one has its strengths and drawbacks.  To decide between the tree,
+there are a few guiding principles that to satisfy simultanuously where
+possible:
+
+@itemize
+@item
+In principle, Guix only has free software; when the upstream source
+contains some non-free software, it has to be removed such that
+@command{guix build --source} returns the ‘freed’ source code rather than
+the unmodified upstream source (@pxref{Software Freedom}).
+@item
+The source of the package needs to correspond to what is actually built
+(i.e., act as the corresponding source), to fulfill our ethical and
+legal obligations.
+@item
+It is convenient for the source derived from an origin to build on any
+system that the upstream package supports.
+@item
+The source needs to actually work, not only on your Guix system but also
+for other systems; this requires some care for substitutions involving
+store items and other architecture-specific changes.
+@item
+When there is more than one way to do something, choose whichever method
+is the simplest.  Sometimes this is subjective, which is also fine.
+What matters is that you use techniques that are common within the
+community (i.e., patterns that appear throughout @code{gnu/packages/...})
+and are thus clearly legible for reviewers.
+@end itemize
+
+To make things more concrete and to resolve conflicts between the
+principles, a few cases have been worked out:
+
+@subsubsection Removing non-free software
+Non-free software has to be removed in snippets; the reason is that
+patches or phases will not work.
+
+For patches, the problem is that a patch removing a non-free file
+automatically contains the non-free file@footnote{It has been noted that
+git patches support removing files without including the file in the
+patch in
+@url{https://yhetil.org/guix/8b13e899-eb60-490b-a268-639249698c81@@www.fastmail.com/}.
 If
+it is verified that the @command{patch} utility supports such patches,
+this method can be used and this policy adjusted appropriately.}, and we
+do not want anything non-free in Guix even if only in its patches.
+
+For phases, the problem is that phases do not influence the result of
+@command{guix build --source}.
+
+@subsubsection Removing bundled libraries
+Bundled libraries should not be removed with patches, because then the
+patch would contain the full bundled library, which can be large. They
+can be removed either in snippets or phases, often using the procedure
+@code{delete-file-recursively}. There are a few benefits for snippets here:
+
+When using snippets, the bundled library does not occur in the source
+returned by @code{guix build --source}, so users and reviewers do not
+have to worry about whether the bundled library contains malware,
+whether it is non-free, if it contains pre-compiled binaries ... There
+are also less licensing concerns: if the bundled libraries are removed,
+it becomes less likely that the licensing conditions apply to people
+sharing the source returned by @command{guix build --source}, especially if
+the bundled library is not actually used on Guix systems.@footnote{This
+is @emph{not} a claim that you can simply ignore the licenses of
+libraries when they are unbundled and replaced by Guix packages -- there
+are less concerns, not none.}
+
+As such, snippets are recommended here.
+
+@subsubsection Fixing technical issues (compilation errors, test failures, 
other bugs ...)
+Usually, a bug fix comes in the form of a patch copied from upstream or
+another distribution.  In that case, simply adding the patch to the
+@code{patches} field is the most convenient and usually does not cause
+any problems; there is no need to rewrite it as a snippet or a phase.
+
+If no ready-made patch already exists, then choosing between a patch or
+a snippet is a matter of convenience. However, there are two things to
+keep in mind:
+
+First, when the fix is not Guix-specific, upstreaming the fix is
+strongly desired to avoid the additional maintenance cost to Guix.  As
+upstreams cannot accept snippets, writing a patch can be a more
+efficient use of time.  Secondly, if the fix of a technical issue embeds
+a store file name, then it has to be a phase.  Otherwise, if the store
+file name were embedded in the source, the result of @command{guix build
+--source} would be unusable on non-Guix systems and also likely unusable
+on Guix systems of another architecture.
+
+@subsubsection Adding new functionality
+To add new functionality, a patch is almost always the most convenient
+choice of the three -- patches are usually multi-line changes, which are
+convenient to do with patches and inconvenient to do with phases or
+snippets.  This choice is usually also compatible with all the guiding
+principles.  As such, patches are preferred.  However, as with bug
+fixes, upstreaming the new functionality is desired.
+
+@subsection How to add patches
+Refer to the @code{origin} record documentation (particularly the fields
+@code{patches}, @code{patch-inputs}, and @code{patch-flags}) for
+information on how to use patches (@pxref{origin Reference}).  When
+adding a patch, do not forget to also list it in @code{dist_patch_DATA}
+of @file{gnu/local.mk}.
+
+@subsection How to add files
+New source files can be added in phases or snippets, by using
+@b{auxiliarry files}.  Auxiliary files are stored in the
+@file{gnu/packages/aux-files} directory and can be retrieved (in a
+snippet or a phase) with @code{search-auxiliary-file}.  When adding an
+auxiliary file, do not forget to also list it in @code{AUX_FILES} of
+@file{Makefile.am}.
+
+Another option for adding new files, is to use procedures such as
+@code{display}, @code{format} and @code{call-with-output-file}.  As a
+matter of principle, auxiliary files ought to be preferred over an
+equivalent @code{call-with-output-file} when creating non-trivil files,
+as that makes them easier to edit.  The exact threshold for a
+non-trivial file might be subjective, though it should lie somewhere
+between 10--20 lines.
+
+Currently, there is no policy on deciding between phase and snippets for
+adding new files, except for the guiding principles.
 
 @node Emacs Packages
 @subsection Emacs Packages
diff --git a/doc/guix.texi b/doc/guix.texi
index 7bce8a567c..042ab3bd8a 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -70,7 +70,7 @@ Copyright @copyright{} 2019 Jakob L. Kreuze@*
 Copyright @copyright{} 2019 Kyle Andrews@*
 Copyright @copyright{} 2019 Alex Griffin@*
 Copyright @copyright{} 2019, 2020, 2021, 2022 Guillaume Le Vaillant@*
-Copyright @copyright{} 2020 Liliana Marie Prikler@*
+Copyright @copyright{} 2020, 2022 Liliana Marie Prikler@*
 Copyright @copyright{} 2019, 2020, 2021, 2022 Simon Tournier@*
 Copyright @copyright{} 2020 Wiktor Żelazny@*
 Copyright @copyright{} 2020 Damien Cassou@*

base-commit: 57f8f69562e942557e3331bb81c7e4acd973d189
prerequisite-patch-id: 78c06b38a189109a5108a157d39ffe7eab8be109
prerequisite-patch-id: aaf0731113d36df901ed2186975e3bb872ec22c0
prerequisite-patch-id: 28e8223cfd59adf84007db9ceefd8a78c41fd10d
prerequisite-patch-id: fb73228d99c36f50e2959c2303c7c707460fd147
prerequisite-patch-id: 7626f1464f4926416fb13daf3d846176aa93f51b
prerequisite-patch-id: 445c6f624e99627959f2e54a6ee97337c44d9ea6
prerequisite-patch-id: 7a16c500faec9d58700a2b50b26bded079e9c3ac
prerequisite-patch-id: f7d406c61e069c04c3b7da453192f51c04763db1
prerequisite-patch-id: 4674bf40052d97215f837c9dfd4e7e1ae999492d
prerequisite-patch-id: 6259468375bfa157277521b17fdd97d6ab0748b7
prerequisite-patch-id: 9d14b38a33b68883c43d6b26dcdbdf7c28e417e7
prerequisite-patch-id: f0e3faffe768e9c660b0a9340042acfa0f790308
prerequisite-patch-id: 550485506255a67c0a1cb9ede7778d4d538b6e2a
prerequisite-patch-id: 9282e95ff076cc2c742be8d2fede83ac14006f6f
prerequisite-patch-id: 1503aa5c698f72ee47b7a987a95c0919efb203c4
prerequisite-patch-id: 24297940086a3780fd7e2e7fa345f262b12efb6f
prerequisite-patch-id: c5f647b5472465666328b123f0f314a6138d6293
prerequisite-patch-id: 56386c4df9130221cad664ec161d1ad9713f4dc3
prerequisite-patch-id: f09ccfb7e53bc7934326af603a197344f4ef53f3
prerequisite-patch-id: 0c18c83d1f2da4639b43861103a028706a147022
prerequisite-patch-id: 066bfca8bf0c3d3bc57a14b48aa1e241555c1e86
prerequisite-patch-id: 13d9ac7b0fadc92b9351409df26b41443497a964
prerequisite-patch-id: ad831a04543475288aba1c938078dcc5ad05870e
prerequisite-patch-id: ed9ec2d0bea23c2c2dbfa4c62290893f1a938f7f
prerequisite-patch-id: 335ce9dbbb2b36b960203a79fdc8f6033ebda2fb
prerequisite-patch-id: f2ca362056369913d0b8319187a8f46ea78b6dc7
prerequisite-patch-id: c02a17479ad4e01837fc307cf6defe0ae92e2435
prerequisite-patch-id: 3a794307f3bbd3641023d978f4b359eb2f5a46e4
prerequisite-patch-id: c545007535db365a29dc3a86e10866f5eef7b7d5
prerequisite-patch-id: 8b8fd18762282129e2d034f7cdceb368f53295d6
prerequisite-patch-id: d435d42bafa65f14049740a3d6cf1a163f855f97
prerequisite-patch-id: 27da1b857019b217b25b6795b84577fdc992a84c
prerequisite-patch-id: aef95e76144ae5e92a41f81b11e84ddc7ececd91
prerequisite-patch-id: 95aa6b45d93026e4375b53a471f0f96e2016914e
prerequisite-patch-id: 715d2bb93fe711e72388458846a0afde6babdc97
prerequisite-patch-id: 63422b710539c3eeac249bf0201107914a215d16
prerequisite-patch-id: 53c9d2236538f9a9009b5b7b2455ef4875d0e31a
prerequisite-patch-id: 37df0e9658435e0a5e2b49fe54a69b91385fb596
prerequisite-patch-id: d2ecaf3439d153de1d53f608e7f5815c73d7b93c
-- 
2.37.2