From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp11.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms5.migadu.com with LMTPS id yL9gNRtGF2PrVgEAbAwnHQ (envelope-from ) for ; Tue, 06 Sep 2022 15:07:39 +0200 Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp11.migadu.com with LMTPS id GI9fNRtGF2O+hwAA9RJhRA (envelope-from ) for ; Tue, 06 Sep 2022 15:07:39 +0200 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 54F202B960 for ; Tue, 6 Sep 2022 15:07:39 +0200 (CEST) Received: from localhost ([::1]:35674 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oVYIk-00027Z-60 for larch@yhetil.org; Tue, 06 Sep 2022 09:07:38 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48002) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oVWr9-00072z-Dm for guix-patches@gnu.org; Tue, 06 Sep 2022 07:35:04 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:32960) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1oVWr9-0002u5-4f for guix-patches@gnu.org; Tue, 06 Sep 2022 07:35:03 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1oVWr9-00031N-1F for guix-patches@gnu.org; Tue, 06 Sep 2022 07:35:03 -0400 X-Loop: help-debbugs@gnu.org Subject: [bug#57598] [PATCH] doc: Update contribution guidelines on patches, etc. Resent-From: Liliana Marie Prikler Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Tue, 06 Sep 2022 11:35:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 57598 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Maxime Devos , 57598@debbugs.gnu.org Received: via spool by 57598-submit@debbugs.gnu.org id=B57598.166246405211485 (code B ref 57598); Tue, 06 Sep 2022 11:35:02 +0000 Received: (at 57598) by debbugs.gnu.org; 6 Sep 2022 11:34:12 +0000 Received: from localhost ([127.0.0.1]:49881 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oVWq5-0002yB-2G for submit@debbugs.gnu.org; Tue, 06 Sep 2022 07:34:12 -0400 Received: from mailrelay.tugraz.at ([129.27.2.202]:15843) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oVWq2-0002y1-Io for 57598@debbugs.gnu.org; Tue, 06 Sep 2022 07:33:56 -0400 Received: from lprikler-laptop.ist.intra (gw.ist.tugraz.at [129.27.202.101]) by mailrelay.tugraz.at (Postfix) with ESMTPSA id 4MMNZ60QrVz3wk1; Tue, 6 Sep 2022 13:33:49 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tugraz.at; s=mailrelay; t=1662464030; bh=LAYm5HIuwF/zB9QMru6igLB+LSuMQyJA6hrE4W57i1Y=; h=Subject:From:To:Date:In-Reply-To:References; b=A5YvNcq+VVxbui4lxT6Sf+/g0ZLZbcIjHtLcJ/oRhTUsohdj1SLuTqiorwkrD7GyL XUNSd6loyGr6BWuZOPUa0hnGd4scYzvx8bV7cCgc7QOojdKZRBRSUhtZYYtK4i+MOq cWaxhg9GQ8IxU1GU1iUQPpbc2VmzHH+L6xfZJXr0= Message-ID: <0f78953d57d8f66a934baf518b7c9e3247b580fd.camel@ist.tugraz.at> From: Liliana Marie Prikler Date: Tue, 06 Sep 2022 13:33:49 +0200 In-Reply-To: <20220905160048.18173-1-maximedevos@telenet.be> References: <20220905160048.18173-1-maximedevos@telenet.be> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.42.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-TUG-Backscatter-control: waObeELIUl4ypBWmcn/8wQ X-Scanned-By: MIMEDefang 2.74 on 129.27.10.116 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+larch=yhetil.org@gnu.org Sender: "Guix-patches" X-Migadu-Flow: FLOW_IN X-Migadu-To: larch@yhetil.org X-Migadu-Country: US ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1662469659; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding:resent-cc: resent-from:resent-sender:resent-message-id:in-reply-to:in-reply-to: references:references:list-id:list-help:list-unsubscribe: list-subscribe:list-post:dkim-signature; bh=LAYm5HIuwF/zB9QMru6igLB+LSuMQyJA6hrE4W57i1Y=; b=NEQJ7xfpKUmN6qhSo16TQaSyMf04PsShIKPGkz8Nm6FM3oIW1u7uhZFYvCTiiso3fCGdt5 Q0gmYfCHUVWKau/fw+jOjaSHztnKKNeaw/c6VrH+KrLgcR8IvxpeXiZEVju4hi+1EHGF1J lDlXAbKzWrfRWP4TKD2C//de91y//4fcMi9uIRlsYNqksp/owc6HAWxkDeVsiVXUj/BtGr Nh/ae/nOHZJubTo8XCcAg/qpTxQIUqx6udWStSaz3RdsHNAGW2kLBHSunBUM/fcpB3gH/N JdZLYJAJ+rEwB/+DoJQGwaA5S/yqD/czceubU9xkcV5qovviZ7B9eAu0zcIGHw== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1662469659; a=rsa-sha256; cv=none; b=jzR5MSEmT3xOtBTI6Ha0XTmALwViKzRvw/r0Yg7FfsNyC+h6itoWWYD6Nx47O9TCQGpFkp WIVLjlEONwSKOiDqdryg2w6yAnlKHYJ+X2Ll+mpgBxaWCaipfAQiRyB1bPgP3Awr7mvXTJ CkBhxd/7qCuG5ctnt+K1b8E3euJUvJMtGR1erOY9WqAenne6h1wCb5DFiympw3JOyPHOxG zQkXpJcYqSoNLWi6puyWg9F+o+B/4W0OSi3auklks4/V7x1GFMkl1DBS7NkQsF5sYJJVi3 HUZLe/++qINIIMf/zIApFz0ryQ0jby262HI5lMm9ZshGXff7HF0uaG1GnoOeGQ== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=tugraz.at header.s=mailrelay header.b=A5YvNcq+; dmarc=fail reason="SPF not aligned (relaxed)" header.from=tugraz.at (policy=none); spf=pass (aspmx1.migadu.com: domain of "guix-patches-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="guix-patches-bounces+larch=yhetil.org@gnu.org" X-Migadu-Spam-Score: 6.73 Authentication-Results: aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=tugraz.at header.s=mailrelay header.b=A5YvNcq+; dmarc=fail reason="SPF not aligned (relaxed)" header.from=tugraz.at (policy=none); spf=pass (aspmx1.migadu.com: domain of "guix-patches-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="guix-patches-bounces+larch=yhetil.org@gnu.org" X-Migadu-Queue-Id: 54F202B960 X-Spam-Score: 6.73 X-Migadu-Scanner: scn0.migadu.com X-TUID: z8iGStTBT1Dh Am Montag, dem 05.09.2022 um 18:00 +0200 schrieb Maxime Devos: > +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, three > +there are a few guiding principles that to satisfy simultanuously > where > +possible: "there are a few guiding principles to satisfy simultaneously", "there are some guiding principles, of which as many as possible should be satisfied". > + > +@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}). If the latter wording above is chosen, this is not a "guiding principle", because it is non-negotiable. > +@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. If you embed store items, it won't even work on Guix System 😛️ Also, this appears to be a rather convenient rewording of the previous item, does it not? > +@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. We also avoid spelling out the non-free filename where possible, preferring keep lists over remove lists, which this kind of patches would be. > +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.   I am pretty sure that most of these are *not* done in snippets, but rather phases, if they only affect Guix. In particular, grep for failing-tests and you will find a few phases disabling them. In fact, as far as files that will not be installed are concerned, I think phases ought to be preferred, because they're easier to take away if an actual fix is made. For the store path embedding, that's a rather roundabout way of saying that contributers *ought to* embed store paths of certaing things, such as commands invoked via exec et al. > 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. Why are you repeating a guiding principle? > +@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. Uhm, what? Patches are the preferred form of patches? >   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}. I don't think this should be a subsection. > +@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. This should probably be a subsubsection after "Adding new functionality" or explained within "Adding new functionality". Overall, I'm not convinced that we have enough guiding principles to call them that, which (along with its sheer length) is my main complaint with the way you've phrased things. Going down to subsubsections just to find out where patches are appropriate, is imho overkill. Cheers