From debbugs-submit-bounces@debbugs.gnu.org Fri Sep 09 08:31:05 2022 Received: (at 57598) by debbugs.gnu.org; 9 Sep 2022 12:31:05 +0000 Received: from localhost ([127.0.0.1]:60998 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oWdA0-0006xj-7B for submit@debbugs.gnu.org; Fri, 09 Sep 2022 08:31:05 -0400 Received: from albert.telenet-ops.be ([195.130.137.90]:52404) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oWd9w-0006xI-KE for 57598@debbugs.gnu.org; Fri, 09 Sep 2022 08:31:03 -0400 Received: from localhost.localdomain ([IPv6:2a02:1811:8c09:9d00:5dba:d409:33f7:a16]) by albert.telenet-ops.be with bizsmtp id HoWy2800N20ykKC06oWyqS; Fri, 09 Sep 2022 14:30:58 +0200 From: Maxime Devos To: 57598@debbugs.gnu.org Subject: [PATCH v2] doc: Update contribution guidelines on patches, etc. Date: Fri, 9 Sep 2022 14:30:51 +0200 Message-Id: <20220909123051.15747-1-maximedevos@telenet.be> X-Mailer: git-send-email 2.37.2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=telenet.be; s=r22; t=1662726659; bh=ShuyIi5oL2KLu5raMueIVayMxJvraM6D4TAkISgfEBU=; h=From:To:Cc:Subject:Date; b=WhNwaFDfVGb1lt3/LKr8fbyZWF1n1GX/h40LURo36dX5Gf81MjofENZePz+wp6/0j 4ovILLK/DSRrY0ab0eftwP1dwT7rNgIeYbfp11QQoY915TgzvH/uUhr0bkdO4iK2MQ x4/7qK8iA8sHzcVcO84nh5y1g5/B7EJiob4L9A8AeGm0kcu0NFf+CcE+HnOYVd7H2k /tzJ0Y2K+ZDIc2WOA6DbP2z3seyXX2N8CtBn0maIvZWUaq5OFvBBZBanLrkHmbk5Re +EeAbRdGW/RhksxuddT2rYzUpVMB/8wpsOdNi+OPuIHeQjzsw6yWGgi92WwAsSFBI2 hs+j5qc+tf9dQ== X-Spam-Score: -0.0 (/) X-Debbugs-Envelope-To: 57598 Cc: Maxime Devos X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) * doc/contributing.texi (Modifying Sources): Replaced with ... ("Modifying Sources"): ... this. List more use cases and some principles. This patch incorporates some text 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. (! remove following text before committing !) Unless someone finds some more typo's or such, this is the final version for me, as it has become a time sink with diminishing returns. Changes since previous version: - typofix: tree -> three - third and fourth principle are merged - corrected @subsection -> @subsubsection Not changed: - no additional information on bugfixes even though initially proposed, I'm done with the patch, if you'd like such information please write a follow-up patch. - I didn't add 'no naming the file names of non-free things in the snippets’, as I don't think this is currently policy and it seems rather inconvenient to work with (e.g.: then we wouldn't be able to keep records of what parts still need to be replaced with something free in the comments) - going from ‘problem -> solution’ to ‘solution -> problem’ -- would partially defeats the point of the patch. - going through the two different patches and take the most concise phrasing (other people in 57598@debbugs.gnu.org didn't agree to my proposal, also, I'm about done with the patch - ‘subsubsections -> item / table’: I haven't read a convincing explanation on why item / table is better here. --- doc/contributing.texi | 140 +++++++++++++++++++++++++++++++++++++----- doc/guix.texi | 2 +- 2 files changed, 126 insertions(+), 16 deletions(-) diff --git a/doc/contributing.texi b/doc/contributing.texi index 02c7c5ae59..bd2ae8d9b6 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,130 @@ 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 three, +there are a few guiding principles 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 +The source needs to actually work, not only on your Guix system but also +on other Guix systems. It preferably should also work on non-Guix +systems if supported by upstream. 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. + +@subsubsection 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}. + +@subsubsection 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: a44e08337d15b3f254a35d0311663c2bbd501852 prerequisite-patch-id: 0caac311875ee39cb48573657ebb960e90da6dfb prerequisite-patch-id: 418285493d89ebf102175902d9b09a0174e88190 prerequisite-patch-id: 3c39eb839d9d3ff3fca6cd98621a5d5c411b7af4 prerequisite-patch-id: 8d5662e874c469f5ee496ef5181cf2d0a30ad1d8 prerequisite-patch-id: 26513c3b3b86963df718ee41d14a25d1cc6a8f3f prerequisite-patch-id: 2b2497e2edec0afc48ebadd6f09f0c661c466127 prerequisite-patch-id: 2712efb97bf33985fd0658e4dd8e936dc08be5fe prerequisite-patch-id: 9d2409b480a8bff0fef029b4b095922d4957e06f -- 2.37.2