From patchwork Mon Sep 5 16:00:48 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: M X-Patchwork-Id: 42226 Return-Path: X-Original-To: patchwork@mira.cbaines.net Delivered-To: patchwork@mira.cbaines.net Received: by mira.cbaines.net (Postfix, from userid 113) id 2B21027BBEA; Mon, 5 Sep 2022 17:08:35 +0100 (BST) X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on mira.cbaines.net X-Spam-Level: X-Spam-Status: No, score=-2.7 required=5.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,FREEMAIL_FROM,MAILING_LIST_MULTI,SPF_HELO_PASS, URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.6 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mira.cbaines.net (Postfix) with ESMTPS id D75F727BBE9 for ; Mon, 5 Sep 2022 17:08:33 +0100 (BST) Received: from localhost ([::1]:40948 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oVEeG-0001k7-Vo for patchwork@mira.cbaines.net; Mon, 05 Sep 2022 12:08:33 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:52366) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oVEXy-0003oO-SD for guix-patches@gnu.org; Mon, 05 Sep 2022 12:02:02 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:59973) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1oVEXy-0005w0-Hi for guix-patches@gnu.org; Mon, 05 Sep 2022 12:02:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1oVEXy-0002vZ-5j for guix-patches@gnu.org; Mon, 05 Sep 2022 12:02:02 -0400 X-Loop: help-debbugs@gnu.org Subject: [bug#57598] [PATCH] doc: Update contribution guidelines on patches, etc. Resent-From: Maxime Devos Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Mon, 05 Sep 2022 16:02:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: report 57598 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: 57598@debbugs.gnu.org Cc: Maxime Devos X-Debbugs-Original-To: guix-patches@gnu.org Received: via spool by submit@debbugs.gnu.org id=B.166239366711180 (code B ref -1); Mon, 05 Sep 2022 16:02:02 +0000 Received: (at submit) by debbugs.gnu.org; 5 Sep 2022 16:01:07 +0000 Received: from localhost ([127.0.0.1]:48672 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oVEX4-0002uF-JG for submit@debbugs.gnu.org; Mon, 05 Sep 2022 12:01:07 -0400 Received: from lists.gnu.org ([209.51.188.17]:47192) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oVEX2-0002u7-Dp for submit@debbugs.gnu.org; Mon, 05 Sep 2022 12:01:05 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:54508) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oVEX1-0002eu-O8 for guix-patches@gnu.org; Mon, 05 Sep 2022 12:01:04 -0400 Received: from laurent.telenet-ops.be ([2a02:1800:110:4::f00:19]:56432) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1oVEWu-0005oG-R3 for guix-patches@gnu.org; Mon, 05 Sep 2022 12:01:02 -0400 Received: from localhost.localdomain ([IPv6:2a02:1811:8c09:9d00:5dba:d409:33f7:a16]) by laurent.telenet-ops.be with bizsmtp id GG0q2800B20ykKC01G0qud; Mon, 05 Sep 2022 18:00:50 +0200 From: Maxime Devos Date: Mon, 5 Sep 2022 18:00:48 +0200 Message-Id: <20220905160048.18173-1-maximedevos@telenet.be> X-Mailer: git-send-email 2.37.2 MIME-Version: 1.0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=telenet.be; s=r22; t=1662393650; bh=KPB1SsoMTS46gQyNDNiuGhWEicwgcbuMCxn15uCYdD4=; h=From:To:Cc:Subject:Date; b=butgFw7JPLgjy5LI96zN20DCLhjISBaU/bRi7uzkf6vFVF7iqVecfClD0r6QPmNtR M2JmduTUU62kuyfWKIEIwYkHt6q+2OlPqaxYeoF1etX+cTa6F2CNtgOPBtqj0VWipU rkfNyc/ZlBrHnq9rxPsPvrSWhuAu4x8xQYoeTw3QrxsFCDyTRnijV5OUOx9eqy9KKe Ht3Engzo5E3KQiC6hTMYYahtF1Tcud9zlYnHeHlF1B4c1c30jhrbGzVVfQKjq3XGUo RSPk4fwm4Vrqap57rxV7FPphkj695ccZVb4iGzGUe+tRdzF6OCR87nAWuctQbB1Bdw TBkakAJTsWzVw== Received-SPF: pass client-ip=2a02:1800:110:4::f00:19; envelope-from=maximedevos@telenet.be; helo=laurent.telenet-ops.be X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_LOW=-0.7, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: guix-patches@gnu.org List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+patchwork=mira.cbaines.net@gnu.org Sender: "Guix-patches" X-getmail-retrieved-from-mailbox: Patches * 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(-) 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 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@*