Message ID | 20220905160048.18173-1-maximedevos@telenet.be |
---|---|
State | New |
Headers | show |
Series | [bug#57598] doc: Update contribution guidelines on patches, etc. | expand |
Context | Check | Description |
---|---|---|
cbaines/comparison | success | View comparision |
cbaines/git-branch | success | View Git branch |
cbaines/applying patch | success | View Laminar job |
cbaines/issue | success | View issue |
See the thread <https://yhetil.org/guix/03a28262-316d-20fc-ea91-5379c74362b1@telenet.be/>. There is also another patch <https://yhetil.org/guix/1dcca575580624ddcc208883ee62d8c75f908139.camel@gmail.com/>, which documents the same thing, but differently. Greetings, Maxime
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
On 06-09-2022 13:33, Liliana Marie Prikler wrote: > 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 Right. >> +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. I'll go for first option, "there are a few guiding principles to satisfy simultaneously where possible", dropping the misplaced "to". >> +@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 😛️ It does, though? Conditional on no --system and no --target. Though given the third @item, doesn't matter. > Also, this appears to be a rather convenient rewording of the previous > item, does it not? I think that with the first, I referred to systems in the sense of --system / --target, and that with the second I meant distributions, though yes. I'll look into unifying the two. >> +@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. Should we? I'm not seeing the point of that. I have not experienced any such avoidance myself, see e.g. 'tennix', 'neverball' and 'shogun'. It is, to my knowledge, not forbidden to mention non-free software by name in code, as long as its not a recommendation (explicit or implied). >> +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. I do not think that ignoring a test counts as a bug fix. I'll add it to this subsubsection, at cost of some additional length. > 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. I do not see a difference in hardness/easyness between removing a phase and removing a snippet (both are just a matter of opening an editor, pointing it at gnu/packages/... and removing a few lines), though I do consider removing a patch to be slightly harder (because gnu/local.mk is easy to forget). > 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. It's not? It's kind of implied, yes, but the purpose isn't being a 'you should embed store paths' (subsub)section, but rather, 'if you go embedding store paths (at least for fixing a technical issue), do it in a phase'. I'm not following what the complaint is, I suppose a section could be added somewhere to properly document the 'embedding store file names' practice, and insert a cross-reference, but that wasn't the purpose of the patch and going by later responses, you seem opposed to making things longer. The alternative would be to remove this information, but then valuable information would be lost (there had been some cases where store file names were embedded in origin). >> 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? I'm showing why, in this case, a phase must be used, by noting that not doing so would be contrary to one of the principles. If not repeating the principle is desired, I could perhaps number them, and refer to the principles by number instead of restating them? Would reduce the length a little. >> +@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? No, I meant that patches are (usually) the preferred method for adding new functionality, and that multi-line changes are convenient to do with patches. ‘which’ refers to the ‘multi-line changes’ here, not ‘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. Right, should have been a subsubsection. >> +@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, I don't think there's any lower limit on how many guiding principles to have, except for perhaps 2 (because otherwise it should have been singular or there aren't any). At how few guiding principles stop the guiding principles from being guiding principles for you, and why? For example, on <https://www.gnu.org/philosophy/free-sw.html>, four guiding principles are mentioned (which are named 'essential freedoms' there), and <https://en.wikipedia.org/wiki/Guiding_Principles> has 5 ‘Guiding Principles’. > which (along with its sheer length) is my main > complaint with the way you've phrased things. (I'm assuming "its = the patch as a whole" here) I could remove another section of the manual to compensate for the additional length, but I doubt that's what you intended. I do not see the problem with the sheer length -- we have a bit of a documentation problem in Guix, there is lots of useful information that is currently undocumented. I do not think there have been any complaints about the manual being too long, if anything, it's too short. I've written some documentation, it was originally a bit hard to follow so in a next version I've restructured it a bit and explained more, this restructuring and explanation entailed some additional length. There had been some proposals for additional cases to document, so they were added, increasing the length. You have added new information is your patch, it was considered useful so I've integrated some of it in my patch, increasing the length. (I didn't integrate all of the new parts, if I did, it would increase even further. (If desired, in can integrate the rest, at cost of some time.)). I do not see what the problem is with additional length as long as this additional length comes with additional useful information and the manual is well-structured (e.g. with (sub)(sub)sections, chapters and indices) -- we do not have a page limit. At worst, perhaps the same information could perhaps be encoded with fewer words? I could compare the two patches to see which one formulates certain information in the fewest words, and choose the least verbose of the two for each piece of information that is present in both? Also, comparing the two patches, my patch has 40 more lines, but about 25 of them are for noting the guiding principles (which are absent in your patch). Compensating for that, the patches are about the same length, so I do not think that 'sheer length' is accurate here. > Going down to > subsubsections just to find out where patches are appropriate, is imho > overkill. The 'going down to subsubsection' is the case for your patch too, though? In my case, it's a subsubsection, in your case it's a table entry inside a subsection, both are the same level of nesting. Also, it's a matter of different structure -- in my v2 and v3 patch, I have a 'problem -> solution' structure -- the idea is that the packages has a problem, they look at the section, they read the subsubsection names, select the subsubsection that matches their problem and read the solution -- in short, the idea is to provide a solution to the problem. Your structure is the other way around -- for solutions (patches, snippets, phases), it gives the permitted problems to apply it to. So yes, your patch is more convenient for finding out where patches are appropriate. I do not see the benefit of that though -- a new contributor packaging a thing wouldn't know in advance which solutions could be appropriate for them (your 'solution -> problem' patch?), rather, they start with a problem and are searching for an appropriate solution (my problem->solution patch). Greetings, Maxime.
Am Dienstag, dem 06.09.2022 um 22:21 +0200 schrieb Maxime Devos: > > > We also avoid spelling out the non-free filename where possible, > > preferring keep lists over remove lists, which this kind of patches > > would be. > Should we? I'm not seeing the point of that. I have not experienced > any such avoidance myself, see e.g. 'tennix', 'neverball' and > 'shogun'. It is, to my knowledge, not forbidden to mention non-free > software by name in code, as long as its not a recommendation > (explicit or implied). Indeed, there is no hard rule, hence "avoid" rather than "forbid". > > > > > +@subsubsection Fixing technical issues (compilation errors, test > > > failures, other bugs ...) > > > [...] > > 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. > I do not think that ignoring a test counts as a bug fix. I'll add it > to this subsubsection, at cost of some additional length. I do think it counts as "fixing technical issues such as test failures". > > 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. > I do not see a difference in hardness/easyness between removing a > phase and removing a snippet (both are just a matter of opening > an editor, pointing it at gnu/packages/... and removing a few lines), > though I do consider removing a patch to be slightly harder (because > gnu/local.mk is easy to forget). There still is the difference that phases are clearly delimited while snippets are a block of code that shouldn't get too large. > > 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. > > It's not? It's kind of implied, yes, but the purpose isn't being a > 'you should embed store paths' (subsub)section, but rather, 'if you > go embedding store paths (at least for fixing a technical issue), do > it in a phase'. > > I'm not following what the complaint is, I suppose a section could be > added somewhere to properly document the 'embedding store file names' > practice, and insert a cross-reference, but that wasn't the purpose > of the patch and going by later responses, you seem opposed to making > things longer. > > The alternative would be to remove this information, but then > valuable information would be lost (there had been some cases where > store file names were embedded in origin). I think my version at least hinted at this practice in a more concise way, so it's not impossible to mention. Not embedding store paths *is* a technical issue, because it'll cause the installed program to fail and most people new to Guix will then just go "oh, let's propagate gcc- toolchain". > > > 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? > I'm showing why, in this case, a phase must be used, by noting that > not doing so would be contrary to one of the principles. > > If not repeating the principle is desired, I could perhaps number > them, and refer to the principles by number instead of restating > them? Would reduce the length a little. I think calling back to a guiding principle in and of itself shows that the section has grown too long to remember it by the point you come to this example, and I think that's more problematic than merely the callback. If you didn't need to divide this into subsubsections, you could introduce the guiding principles in a way that feels more natural. > > > +@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? > > No, I meant that patches are (usually) the preferred method for > adding new functionality, and that multi-line changes are convenient > to do with patches. ‘which’ refers to the ‘multi-line changes’ here, > not ‘patches’. I still find this wording very confusing. Perhaps "To add new functionality, a patch is almost always the best choice. For one, it is likely that the new functionality requires changing multiple lines of source code, which is more convenient to do with a patch than with a snippet. Further, patches can be taken from and submitted to upstreams more easily. If your patch has not been submitted to upstream, consider doing so." > > > > [...] > > Overall, I'm not convinced that we have enough guiding principles > > to call them that, > > I don't think there's any lower limit on how many guiding principles > to have, except for perhaps 2 (because otherwise it should have been > singular or there aren't any). At how few guiding principles stop > the guiding principles from being guiding principles for you, and > why? > > For example, on <https://www.gnu.org/philosophy/free-sw.html>, four > guiding principles are mentioned (which are named 'essential > freedoms' there), and > <https://en.wikipedia.org/wiki/Guiding_Principles> has 5 ‘Guiding > Principles’. An enumeration ought to have at least three elements (otherwise it's just a pair), and I think if we do proper counting and omit no- brainers, such as the "only free software" part that has already been mentioned, we come very close to skirting that line. > > which (along with its sheer length) is my main > > complaint with the way you've phrased things. > > (I'm assuming "its = the patch as a whole" here) > > I could remove another section of the manual to compensate for the > additional length, but I doubt that's what you intended. I do not > see the problem with the sheer length -- we have a bit of a > documentation problem in Guix, there is lots of useful information > that is currently undocumented. > I do not think there have been any complaints about the manual being > too long, if anything, it's too short. I personally tend towards "less verbose", hence my complaint of describing something with many words that could be described with fewer. A section can still be too long while the chapter around it is too short. > I've written some documentation, it was originally a bit hard to > follow so in a next version I've restructured it a bit and explained > more, this restructuring and explanation entailed some additional > length. > > There had been some proposals for additional cases to document, so > they were added, increasing the length. You have added new > information is your patch, it was considered useful so I've > integrated some of it in my patch, increasing the length. (I didn't > integrate all of the new parts, if I did, it would increase even > further. (If desired, in can integrate the rest, at cost of some > time.)). My patch did not just state some things you missed, it also omitted things that I think are either not necessary or probably better documented elsewhere. > I do not see what the problem is with additional length as long as > this additional length comes with additional useful information and > the manual is well-structured (e.g. with (sub)(sub)sections, chapters > and indices) -- we do not have a page limit. > > At worst, perhaps the same information could perhaps be encoded with > fewer words? I could compare the two patches to see which one > formulates certain information in the fewest words, and choose the > least verbose of the two for each piece of information that is > present in both? > > Also, comparing the two patches, my patch has 40 more lines, but > about 25 of them are for noting the guiding principles (which are > absent in your patch). > Compensating for that, the patches are about the same length, so I do > not think that 'sheer length' is accurate here. 25 lines calling back to earlier information are, imho, an indicator that the section is too long. Imagine you'd have twenty-five function calls to guiding_principles(n) in your program – at some point, you'd try to cache those. > > Going down to subsubsections just to find out where patches are > > appropriate, is imho overkill. > > The 'going down to subsubsection' is the case for your patch too, > though? In my case, it's a subsubsection, in your case it's a table > entry inside a subsection, both are the same level of nesting. These are still two very different kinds of nesting. A table fits onto a (virtual) page more easily than several subsections. > Also, it's a matter of different structure -- in my v2 and v3 patch, > I have a 'problem -> solution' structure -- the idea is that the > packages has a problem, they look at the section, they read the > subsubsection names, select the > subsubsection that matches their problem and read the solution -- in > short, the idea is to provide a solution to the problem. > > Your structure is the other way around -- for solutions (patches, > snippets, phases), it gives the permitted problems to apply it to. > > So yes, your patch is more convenient for finding out where patches > are appropriate. I do not see the benefit of that though -- a new > contributor packaging a thing wouldn't know in advance which > solutions could be appropriate for them (your 'solution -> problem' > patch?), rather, they start with a problem and are searching for an > appropriate solution (my problem->solution patch). I think this idea can be debunked pretty easily. If I give you a hammer and I tell you "this is a hammer, you can use it to put nails into a wall", and you have a nail and you want to put it into a wall, you won't go "oh no, however will I put this nail into a wall?" – you will simply use the hammer to do so. Of course, for this to work I also have to tell you *how* to use a hammer to put nails into a wall, but that's exactly what I did in my patch by inserting the right notes into the Guix manual. My solution->problem approach has the benefit, that folks can just go over all the solutions, check if their problem fits, and apply the one that says "here, use this". And if they don't find anything, they see the handy little line at the bottom saying "use whatever you think is convenient". I also expand a little on the benefits and drawbacks of these approaches as you would when describing design patterns. Your problem->solution approach instead leaves people wondering when their particular use case has not been described. It gives them a solution rather than the tools to build solutions with. Cheers
On 07-09-2022 10:09, Liliana Marie Prikler wrote: > Am Dienstag, dem 06.09.2022 um 22:21 +0200 schrieb Maxime Devos: >>> We also avoid spelling out the non-free filename where possible, >>> preferring keep lists over remove lists, which this kind of patches >>> would be. >> Should we? I'm not seeing the point of that. I have not experienced >> any such avoidance myself, see e.g. 'tennix', 'neverball' and >> 'shogun'. It is, to my knowledge, not forbidden to mention non-free >> software by name in code, as long as its not a recommendation >> (explicit or implied). > Indeed, there is no hard rule, hence "avoid" rather than "forbid". What I also meant is, that to my knowledge there is no soft rule either. Again, why should we avoid this, what's the point of that? >>>> +@subsubsection Fixing technical issues (compilation errors, test >>>> failures, other bugs ...) >>>> [...] >>> 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. >> I do not think that ignoring a test counts as a bug fix. I'll add it >> to this subsubsection, at cost of some additional length. > I do think it counts as "fixing technical issues such as test > failures". How does ignoring a test fix the technical issue identified by the test (sometimes, the technical issue being a bug in the test itself)? >>> 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. >> I do not see a difference in hardness/easyness between removing a >> phase and removing a snippet (both are just a matter of opening >> an editor, pointing it at gnu/packages/... and removing a few lines), >> though I do consider removing a patch to be slightly harder (because >> gnu/local.mk is easy to forget). > There still is the difference that phases are clearly delimited while > snippets are a block of code that shouldn't get too large. Snippets are delimited clearly as well, though, with the 'snippet' field? And the limitations of snippet length and phases length are the same -- no limits, though conciseness is appreciate as always. >>> 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. >> It's not? It's kind of implied, yes, but the purpose isn't being a >> 'you should embed store paths' (subsub)section, but rather, 'if you >> go embedding store paths (at least for fixing a technical issue), do >> it in a phase'. >> >> I'm not following what the complaint is, I suppose a section could be >> added somewhere to properly document the 'embedding store file names' >> practice, and insert a cross-reference, but that wasn't the purpose >> of the patch and going by later responses, you seem opposed to making >> things longer. >> >> The alternative would be to remove this information, but then >> valuable information would be lost (there had been some cases where >> store file names were embedded in origin). > I think my version at least hinted at this practice in a more concise > way, so it's not impossible to mention. [...] I agree it's possible -- as I replied previously: > I suppose a section could be added somewhere to properly document the > 'embedding store file names' practice, and insert a cross-reference, I don't think documenting the how of the practice should be done in this section, properly explaining 'search-input-file' / 'search-input-directory', 'inputs / native-inputs', 'bash' being an implicit input but you still have to add it to 'inputs' in some cases because of cross-compilation, this-package-input and this-package-native-input ... would make the subsubsection a bit too long I think, distracting from other situations, hence the proposal for a cross-reference. How about leaving the 'how to embed store file names' for a separate documentation patch and section, adding a cross-reference later? >>>> 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? >> I'm showing why, in this case, a phase must be used, by noting that >> not doing so would be contrary to one of the principles. >> >> If not repeating the principle is desired, I could perhaps number >> them, and refer to the principles by number instead of restating >> them? Would reduce the length a little. > I think calling back to a guiding principle in and of itself shows that > the section has grown too long to remember it by the point you come to > this example, This has nothing to do with length and remembering, but rather with explaining why a phase must be used -- to explain that, I state which principle applies (as mentioned previously). If I removed the explanations, I would just be stating how to do things, without giving a logical reasoning on the 'why'. > and I think that's more problematic than merely the > callback. If you didn't need to divide this into subsubsections, you > could introduce the guiding principles in a way that feels more > natural. I consider it more natural to have the 'guiding principles' _before_ the concrete cases, as they are meant to be 'guiding' and 'principles'. It's like 'starting from first principles', there introducing the first principles as you go is ad-hoc. The guiding principles also need to be outside the examples, in case one of the examples doesn't apply to the packager's use case, such that they can fall-back to the guiding principles. Also, in your patch you are dividing things in subsubsections as well, just under a different name and different representation (table entries in a subsection), as mentioned previously. >>>> +@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? >> No, I meant that patches are (usually) the preferred method for >> adding new functionality, and that multi-line changes are convenient >> to do with patches. ‘which’ refers to the ‘multi-line changes’ here, >> not ‘patches’. > I still find this wording very confusing. Perhaps "To add new > functionality, a patch is almost always the best choice. For one, it > is likely that the new functionality requires changing multiple lines > of source code, which is more convenient to do with a patch than with a > snippet. Further, patches can be taken from and submitted to upstreams > more easily. If your patch has not been submitted to upstream, > consider doing so." It loses some information (that patches are preferred) and (after re-adding the conclusion) is more verbose, which appears to be considered very important. >>> [...] >>> Overall, I'm not convinced that we have enough guiding principles >>> to call them that, >> I don't think there's any lower limit on how many guiding principles >> to have, except for perhaps 2 (because otherwise it should have been >> singular or there aren't any). At how few guiding principles stop >> the guiding principles from being guiding principles for you, and >> why? >> >> For example, on<https://www.gnu.org/philosophy/free-sw.html>, four >> guiding principles are mentioned (which are named 'essential >> freedoms' there), and >> <https://en.wikipedia.org/wiki/Guiding_Principles> has 5 ‘Guiding >> Principles’. > An enumeration ought to have at least three elements (otherwise it's > just a pair), and I think if we do proper counting and omit no- > brainers, such as the "only free software" part that has already been > mentioned, we come very close to skirting that line. The "only free software" is mentioned elsewhere, yes, but it is one of the principles for deciding between snippets, phases and patches. While you call it a no-brainer, it is sometimes neglected, so it sounds important to me to explicitly list it. Merging the 3th and 4th @item, I count 4 principles, so it fits with an enumeration. Also, I'm not following your point here -- your complaint was that they aren't guiding principles (based on the number of them), but your response is that they might not form an enumeration? They are named the guiding principles, not the guiding enumeration. What have enumerations to do with anything? >>> which (along with its sheer length) is my main >>> complaint with the way you've phrased things. >> (I'm assuming "its = the patch as a whole" here) >> >> I could remove another section of the manual to compensate for the >> additional length, but I doubt that's what you intended. I do not >> see the problem with the sheer length -- we have a bit of a >> documentation problem in Guix, there is lots of useful information >> that is currently undocumented. >> I do not think there have been any complaints about the manual being >> too long, if anything, it's too short. > I personally tend towards "less verbose", hence my complaint of > describing something with many words that could be described with > fewer. A section can still be too long while the chapter around it is > too short. Do you have anything in particular in mind? >> I've written some documentation, it was originally a bit hard to >> follow so in a next version I've restructured it a bit and explained >> more, this restructuring and explanation entailed some additional >> length. >> >> There had been some proposals for additional cases to document, so >> they were added, increasing the length. You have added new >> information is your patch, it was considered useful so I've >> integrated some of it in my patch, increasing the length. (I didn't >> integrate all of the new parts, if I did, it would increase even >> further. (If desired, in can integrate the rest, at cost of some >> time.)). > My patch did not just state some things you missed, it also omitted > things that I think are either not necessary or probably better > documented elsewhere. What particular things do you have in mind, and where do you think they should better be documented? I can move things around a bit and add cross-references. >> I do not see what the problem is with additional length as long as >> this additional length comes with additional useful information and >> the manual is well-structured (e.g. with (sub)(sub)sections, chapters >> and indices) -- we do not have a page limit. >> >> At worst, perhaps the same information could perhaps be encoded with >> fewer words? I could compare the two patches to see which one >> formulates certain information in the fewest words, and choose the >> least verbose of the two for each piece of information that is >> present in both? >> >> Also, comparing the two patches, my patch has 40 more lines, but >> about 25 of them are for noting the guiding principles (which are >> absent in your patch). >> Compensating for that, the patches are about the same length, so I do >> not think that 'sheer length' is accurate here. > 25 lines calling back to earlier information are, imho, an indicator > that the section is too long. Imagine you'd have twenty-five function > calls to guiding_principles(n) in your program – at some point, you'd > try to cache those. (define cached-guiding-principles (delay (list (guiding-principle-0) [...] (guiding-principle-24)))) Caching the guiding principles does not reduce the length. I don't see the problem with calling back to earlier information. Also, it isn't earlier information, there is no nice list of guiding principles anywhere else. >>> Going down to subsubsections just to find out where patches are >>> appropriate, is imho overkill. >> The 'going down to subsubsection' is the case for your patch too, >> though? In my case, it's a subsubsection, in your case it's a table >> entry inside a subsection, both are the same level of nesting. > These are still two very different kinds of nesting. A table fits onto > a (virtual) page more easily than several subsections. I suppose table items might take two less line or so less than subsubsections, but I don't think that's sufficient reason to step away from a nice section structure. >> Also, it's a matter of different structure -- in my v2 and v3 patch, >> I have a 'problem -> solution' structure -- the idea is that the >> packages has a problem, they look at the section, they read the >> subsubsection names, select the >> subsubsection that matches their problem and read the solution -- in >> short, the idea is to provide a solution to the problem. >> >> Your structure is the other way around -- for solutions (patches, >> snippets, phases), it gives the permitted problems to apply it to. >> >> So yes, your patch is more convenient for finding out where patches >> are appropriate. I do not see the benefit of that though -- a new >> contributor packaging a thing wouldn't know in advance which >> solutions could be appropriate for them (your 'solution -> problem' >> patch?), rather, they start with a problem and are searching for an >> appropriate solution (my problem->solution patch). > I think this idea can be debunked pretty easily. If I give you a > hammer and I tell you "this is a hammer, you can use it to put nails > into a wall", and you have a nail and you want to put it into a wall, > you won't go "oh no, however will I put this nail into a wall?" – you > will simply use the hammer to do so. The patch does this, currently. It already proposes a number of hammers (patches, snippets and phases) and purposes (adding new functionality, fixing technical issues, unbundling, ...). Also, the scenario "oh no, however will I put this nail into a wall" actually happened -- see the Shepherd discussion, where there was a lot of disagreement on how nails (= small work-around in the Makefile) should be put in walls (= patches, snippet, phase?). It was the whole reason to start writing a documentation patch. > Of course, for this to work I > also have to tell you *how* to use a hammer to put nails into a wall, > but that's exactly what I did in my patch by inserting the right notes > into the Guix manual. Also already the case. > My solution->problem approach has the benefit, that folks can just go > over all the solutions, check if their problem fits, and apply the one > that says "here, use this". A problem->solution structure is useful for that too? And it already lists all the solutions (snippets, phases and patches) and information to decide whether the solution fits their problem (the guiding principles, and the worked-out cases). > And if they don't find anything, they see > the handy little line at the bottom saying "use whatever you think is > convenient". Nowhere did the patch imply that the listed cases were all cases. In fact, in two places in the introduction it is implied that the examples are not exhaustive, and that they can choose according to convenience: * ‘To make things more concrete and to resolve conflicts between the principles, a _few_ cases have been worked out:’ (Emphasis on _few_ added) * ‘When there is more than one way to do something, choose whichever method is the simplest’ (It says ‘simplest’ instead of ‘most convenient’, but whatever.) > I also expand a little on the benefits and drawbacks of > these approaches as you would when describing design patterns. This is also done in my patch. E.g., * ‘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.’ * ‘For phases, the problem is that phases do not influence the result of @command{guix build --source}.’ > Your problem->solution approach instead leaves people wondering when > their particular use case has not been described. See my reply to ‘And if they don't find anything, they see the handy little line at the bottom saying "use whatever you think is convenient’. > It gives them a solution rather than the tools to build solutions with. It does give the tools: snippets, patches and phases. And as tools for deciding between the three for not-yet-documented cases, there are the guiding principles. As a demonstration on how to use these guiding principles, various cases have been worked out based on the guiding principles. Summarised, it gives both the tools _and_ the solutions. Also, "giving the tools to build solutions with" does not help with the problem that I aimed to solve -- there was disagreement on what the appropriate tools are (see: Shepherd), so it not just needs to give the tools, but also the solutions. Greetings, Maxime
Am Donnerstag, dem 08.09.2022 um 13:12 +0200 schrieb Maxime Devos: > On 07-09-2022 10:09, Liliana Marie Prikler wrote: > > Am Dienstag, dem 06.09.2022 um 22:21 +0200 schrieb Maxime Devos: > > > It is, to my knowledge, not forbidden to mention non-free > > > software by name in code, as long as its not a recommendation > > > (explicit or implied). > > Indeed, there is no hard rule, hence "avoid" rather than "forbid". > What I also meant is, that to my knowledge there is no soft rule > either. Again, why should we avoid this, what's the point of that? In descriptions, it is wise to do so because it helps software stand on its own merits rather than just "being a clone of this thing you aren't allowed to have" (this is real criticism pointed at us from the proprietary software embracers). See for instance minetest, whichisy > How does ignoring a test fix the technical issue identified by the > test (sometimes, the technical issue being a bug in the test itself)? It fixes the technical issue that an otherwise functional package (w.r.t. the N tests that don't fail) builds. This is a particularly useful distinction with tests that require a network connection and thus have to fail in a build container, or are known flaky upstream and thus cause reproducibility issues. > > There still is the difference that phases are clearly delimited > > while snippets are a block of code that shouldn't get too large. > Snippets are delimited clearly as well, though, with the 'snippet' > field? > And the limitations of snippet length and phases length are the same > -- no limits, though conciseness is appreciate as always. True, but phases can be made to do just one thing, whereas snippets have to fix everything that's wrong in more or less one (begin ...). This is a noticable distinction. > > > I think my version at least hinted at this practice in a more > > concise way, so it's not impossible to mention. [...] > I agree it's possible -- as I replied previously: > > I suppose a section could be added somewhere to properly document > > the 'embedding store file names' practice, and insert a > > cross-reference, > I don't think documenting the how of the practice should be done > in this section, properly explaining 'search-input-file' / 'search- > input-directory', 'inputs / native-inputs', 'bash' being an implicit > input but you still have to add it to 'inputs' in some cases because > of cross-compilation, this-package-input and this-package-native- > input ... would make the > subsubsection a bit too long I think, distracting from other > situations, hence the proposal for a cross-reference. > How about leaving the 'how to embed store file names' for a separate > documentation patch and section, adding a cross-reference later? See above, "I suppose a section could be added..." means I'm somewhat indifferent to whether it's done now or later; I would however very much prefer a wording that points people toward this practice existing, even if they have to go look in the code for examples. Alternatively, a footnote for the most common case (search-input-file inputs "bin/command") ought to suffice for the time being. > > > > I think calling back to a guiding principle in and of itself shows > > that the section has grown too long to remember it by the point you > > come to this example, > This has nothing to do with length and remembering, but rather with > explaining why a phase must be used -- to explain that, I state which > principle applies (as mentioned previously). If I removed the > explanations, I would just be stating how to do things, without > giving a logical reasoning on the 'why'. IMHO, I think a reader who remembers the guiding principles should see that it applies. > > and I think that's more problematic than merely the > > callback. If you didn't need to divide this into subsubsections, > > you could introduce the guiding principles in a way that feels more > > natural. > I consider it more natural to have the 'guiding principles' _before_ > the concrete cases, as they are meant to be 'guiding' and > 'principles'. > It's like 'starting from first principles', there introducing the > first principles as you go is ad-hoc. > The guiding principles also need to be outside the examples, in case > one of the examples doesn't apply to the packager's use case, such > that they can fall-back to the guiding principles. > Also, in your patch you are dividing things in subsubsections as > well, just under a different name and different representation (table > entries in a subsection), as mentioned previously. A table entry is not a subsection, as much as you want it to be that. Also, your guiding principles are (with one exception) really just invariants that ought to hold for the source field of a package. As such, I think it would be easier to state "A package's source should be the smallest corresponding source in terms of uncompressed file size. This corresponding source must consist only of free software (note Free Software) and should build on all platforms supported by upstream." Note how smallest naturally implies unbundling bundled sources. > > I still find this wording very confusing. Perhaps "To add new > > functionality, a patch is almost always the best choice. For one, > > it is likely that the new functionality requires changing multiple > > lines of source code, which is more convenient to do with a patch > > than with a snippet. Further, patches can be taken from and > > submitted to upstreams more easily. If your patch has not been > > submitted to upstream, consider doing so." > It loses some information (that patches are preferred) and > (after re-adding the conclusion) is more verbose, which appears > to be considered very important. Which conclusion is there to re-add? The conclusion is already stated in the beginning: patches are almost always the best choice. Then two reasons as for why. The part w.r.t. upstreaming changes has also been addressed. > > An enumeration ought to have at least three elements (otherwise > > it's just a pair), and I think if we do proper counting and omit > > no-brainers, such as the "only free software" part that has already > > been mentioned, we come very close to skirting that line. > The "only free software" is mentioned elsewhere, yes, but it is one > of the principles for deciding between snippets, phases and patches. > While you call it a no-brainer, it is sometimes neglected, so it > sounds important to me to explicitly list it. > Merging the 3th and 4th @item, I count 4 principles, so it fits with > an enumeration. > Also, I'm not following your point here -- your complaint was that > they aren't guiding principles (based on the number of them), but > your response is that they might not form an enumeration? They are > named the guiding principles, not the guiding enumeration. What have > enumerations to do with anything? I'm using enumeration as a super term here, which can be ((sub)sub)sections, chapters, list elements, whatever, and my claim is that we barely have enough principles to allow the use of a plural. Adding to that, now that I think of it, I also doubt their usefulness in guiding. "Use whatever feels convenient, but note that that might be subjective" is more useful at the end of the section when a user didn't find what they were looking for than at the start. > > > > which (along with its sheer length) is my main > > > > complaint with the way you've phrased things. > > > (I'm assuming "its = the patch as a whole" here) > > > > > > I could remove another section of the manual to compensate for > > > the additional length, but I doubt that's what you intended. I > > > do not see the problem with the sheer length -- we have a bit of > > > a documentation problem in Guix, there is lots of useful > > > information that is currently undocumented. > > > I do not think there have been any complaints about the manual > > > being too long, if anything, it's too short. > > I personally tend towards "less verbose", hence my complaint of > > describing something with many words that could be described with > > fewer. A section can still be too long while the chapter around it > > is too short. > Do you have anything in particular in mind? This is more of a broad statement that applies to the patch as a whole than to any of its constituent parts in particular. However, in some cases where I think it's particularly noticable, I'll try to point out shorter formulations. > > > I've written some documentation, it was originally a bit hard to > > > follow so in a next version I've restructured it a bit and > > > explained more, this restructuring and explanation entailed some > > > additional length. > > > > > > There had been some proposals for additional cases to document, > > > so they were added, increasing the length. You have added new > > > information is your patch, it was considered useful so I've > > > integrated some of it in my patch, increasing the length. (I > > > didn't integrate all of the new parts, if I did, it would > > > increase even further. (If desired, in can integrate the rest, > > > at cost of some time.)). > > My patch did not just state some things you missed, it also omitted > > things that I think are either not necessary or probably better > > documented elsewhere. > What particular things do you have in mind, and where do you > think they should better be documented? I can move things > around a bit and add cross-references. > > > > > I do not see what the problem is with additional length as long > > > as this additional length comes with additional useful > > > information and the manual is well-structured (e.g. with > > > (sub)(sub)sections, chapters and indices) -- we do not have a > > > page limit. > > > > > > At worst, perhaps the same information could perhaps be encoded > > > with fewer words? I could compare the two patches to see which > > > one formulates certain information in the fewest words, and > > > choose the least verbose of the two for each piece of information > > > that is present in both? > > > > > > Also, comparing the two patches, my patch has 40 more lines, but > > > about 25 of them are for noting the guiding principles (which are > > > absent in your patch). > > > Compensating for that, the patches are about the same length, so > > > I do not think that 'sheer length' is accurate here. > > 25 lines calling back to earlier information are, imho, an > > indicator that the section is too long. Imagine you'd have twenty- > > five function calls to guiding_principles(n) in your program – at > > some point, you'd try to cache those. > (define cached-guiding-principles > (delay (list (guiding-principle-0) > [...] > (guiding-principle-24)))) > Caching the guiding principles does not reduce the length. > I don't see the problem with calling back to earlier information. > Also, it isn't earlier information, there is no nice list of guiding > principles anywhere else. At the risk of responding jokingly to what was meant serious: I didn't know we suddenly gained 20 guiding principles. > > > > > Going down to subsubsections just to find out where patches are > > > > appropriate, is imho overkill. > > > The 'going down to subsubsection' is the case for your patch too, > > > though? In my case, it's a subsubsection, in your case it's a > > > table > > > entry inside a subsection, both are the same level of nesting. > > These are still two very different kinds of nesting. A table fits > > onto a (virtual) page more easily than several subsections. > I suppose table items might take two less line or so less than > subsubsections, but I don't think that's sufficient reason to step > away from a nice section structure. Another reason is that you can end a table in the middle of a section, which you can't do with subsections. Hence why I'm able to put a remark at the bottom, which you have to clumsily fit into the top. > > > Also, it's a matter of different structure -- in my v2 and v3 > > > patch, I have a 'problem -> solution' structure -- the idea is > > > that the packages has a problem, they look at the section, they > > > read the subsubsection names, select the > > > subsubsection that matches their problem and read the solution -- > > > in short, the idea is to provide a solution to the problem. > > > > > > Your structure is the other way around -- for solutions (patches, > > > snippets, phases), it gives the permitted problems to apply it > > > to. > > > > > > So yes, your patch is more convenient for finding out where > > > patches are appropriate. I do not see the benefit of that though > > > -- a new contributor packaging a thing wouldn't know in advance > > > which solutions could be appropriate for them (your 'solution -> > > > problem' patch?), rather, they start with a problem and are > > > searching for an appropriate solution (my problem->solution > > > patch). > > I think this idea can be debunked pretty easily. If I give you a > > hammer and I tell you "this is a hammer, you can use it to put > > nails into a wall", and you have a nail and you want to put it into > > a wall, you won't go "oh no, however will I put this nail into a > > wall?" – you will simply use the hammer to do so. > The patch does this, currently. It already proposes a number of > hammers (patches, snippets and phases) and purposes (adding new > functionality, fixing technical issues, unbundling, ...). > Also, the scenario "oh no, however will I put this nail into a wall" > actually happened -- see the Shepherd discussion, where there was > a lot of disagreement on how nails (= small work-around in the > Makefile) should be put in walls (= patches, snippet, phase?). It > was the whole reason to start writing a documentation patch. You might want to add a link here if it supports your argument, but without looking at the discussion this rather sounds like "oh no, I have three hammers, which one do I pick?" – which, fair enough, is still a problem, but one that neither of our patches would cause imho. > > Of course, for this to work I also have to tell you *how* to use a > > hammer to put nails into a wall, but that's exactly what I did in > > my patch by inserting the right notes into the Guix manual. > Also already the case. > > My solution->problem approach has the benefit, that folks can just > > go over all the solutions, check if their problem fits, and apply > > the one that says "here, use this". > A problem->solution structure is useful for that too? > And it already lists all the solutions (snippets, phases and patches) > and information to decide whether the solution fits their problem > (the guiding principles, and the worked-out cases). Again, I believe you're overselling the guiding principles. > > And if they don't find anything, they see the handy little line at > > the bottom saying "use whatever you think is convenient". > Nowhere did the patch imply that the listed cases were all cases. In > fact, in two places in the introduction it is implied that the > examples are not exhaustive, and that they can choose according to > convenience [...] Emphasis on handy little line rather than needing to be told twice (particularly if people have no idea what to expect due to not having looked at the worked-out cases yet). > > I also expand a little on the benefits and drawbacks of > > these approaches as you would when describing design patterns. > This is also done in my patch. [...] This is not about the contained information, but the structure of the contained information. My solution->problem style follows roughly this style: 1. solution 2. problems it is known to solve 3. how to use 4. properties, caveats, etc. Your problem->solution style roughly has the following: 1. problem 2. (set of) solution(s) 3. if more than one solution, maybe a help to select This makes it so that people might have to go to a different subsection than the one they read for their solution to find out about potential caveats, e.g. not embedding store paths in a snippet. > > Your problem->solution approach instead leaves people wondering > > when their particular use case has not been described. > See my reply to ‘And if they don't find anything, they see the handy > little line at the bottom saying "use whatever you think is > convenient’. > > It gives them a solution rather than the tools to build solutions > > with. > It does give the tools: snippets, patches and phases. As far as I read, it describes none of those. It puts out guiding principles and some already worked-out cases. > Also, "giving the tools to build solutions with" does not help with > the problem that I aimed to solve -- there was disagreement on what > the appropriate tools are (see: Shepherd), so it not just needs to > give the tools, but also the solutions. I don't see how my patch lacks this information however. In particular, for review purposes, mine should be easier to work with. For instance, the reviewer sees a hash embedded in a snippet, sees in the snippet entry that you shouldn't do that, and can thus say "nope, don't do a snippet here". Regardless of which patch we pick, it should not mandate a particular solution in potentially contentious cases, and also point towards the right solution. See our discussions on the individual solutions on points in which I believe you've errored. Cheers
On 09-09-2022 10:04, Liliana Marie Prikler wrote: > Am Donnerstag, dem 08.09.2022 um 13:12 +0200 schrieb Maxime Devos: >> On 07-09-2022 10:09, Liliana Marie Prikler wrote: >>> Am Dienstag, dem 06.09.2022 um 22:21 +0200 schrieb Maxime Devos: >>>> It is, to my knowledge, not forbidden to mention non-free >>>> software by name in code, as long as its not a recommendation >>>> (explicit or implied). >>> Indeed, there is no hard rule, hence "avoid" rather than "forbid". >> What I also meant is, that to my knowledge there is no soft rule >> either. Again, why should we avoid this, what's the point of that? > In descriptions, it is wise to do so because it helps software stand on > its own merits rather than just "being a clone of this thing you aren't > allowed to have" (this is real criticism pointed at us from the > proprietary software embracers). See for instance minetest, whichisy Sentence might have been truncated? Also, this is the package source field, not the description, I don't see how the "helps software stand on its own merits" applies to snippets of the package source. > >> How does ignoring a test fix the technical issue identified by the >> test (sometimes, the technical issue being a bug in the test itself)? > It fixes the technical issue that an otherwise functional package > (w.r.t. the N tests that don't fail) builds. This is a particularly > useful distinction with tests that require a network connection and > thus have to fail in a build container, or are known flaky upstream and > thus cause reproducibility issues. Network test: right (though preferably those would support a --no-network-tests test option or such). For flaky tests: those sound like bugs to me, ignoring them doesn't remove the flakyness. >>> There still is the difference that phases are clearly delimited >>> while snippets are a block of code that shouldn't get too large. >> Snippets are delimited clearly as well, though, with the 'snippet' >> field? >> And the limitations of snippet length and phases length are the same >> -- no limits, though conciseness is appreciate as always. > True, but phases can be made to do just one thing, whereas snippets > have to fix everything that's wrong in more or less one (begin ...). > This is a noticable distinction.> You can do that in snippets too, with comments: (snippet #~(begin ;; Do the foo thing (foo) (foo-2 [...]) [...] ;; Do the bar thing (bar) (bar-2 [...]) [...])) >> >>> I think my version at least hinted at this practice in a more >>> concise way, so it's not impossible to mention. [...] >> I agree it's possible -- as I replied previously: >>> I suppose a section could be added somewhere to properly document >>> the 'embedding store file names' practice, and insert a >>> cross-reference, >> I don't think documenting the how of the practice should be done >> in this section, properly explaining 'search-input-file' / 'search- >> input-directory', 'inputs / native-inputs', 'bash' being an implicit >> input but you still have to add it to 'inputs' in some cases because >> of cross-compilation, this-package-input and this-package-native- >> input ... would make the >> subsubsection a bit too long I think, distracting from other >> situations, hence the proposal for a cross-reference. >> How about leaving the 'how to embed store file names' for a separate >> documentation patch and section, adding a cross-reference later? > See above, "I suppose a section could be added..." means I'm somewhat > indifferent to whether it's done now or later; Nitpick: you are quoting some text I wrote, so 'I' refers to me here, not you. I would however very > much prefer a wording that points people toward this practice existing, > even if they have to go look in the code for examples. Alternatively, > a footnote for the most common case (search-input-file inputs > "bin/command") ought to suffice for the time being. Aside from the typo's and a few rephrasings, I'm done with the documentation. If someone wants to extend the section with such information, they can always do so later. >> >>> I think calling back to a guiding principle in and of itself shows >>> that the section has grown too long to remember it by the point you >>> come to this example, >> This has nothing to do with length and remembering, but rather with >> explaining why a phase must be used -- to explain that, I state which >> principle applies (as mentioned previously). If I removed the >> explanations, I would just be stating how to do things, without >> giving a logical reasoning on the 'why'. > IMHO, I think a reader who remembers the guiding principles should see > that it applies. Likely. But removing the explicit mention of the guiding principle still makes the logical reasoning incomplete, remembering has nothing to do with it. As I've written previously: ‘This has nothing do do with [...] and remembering, but rather with [...]’. >>> and I think that's more problematic than merely the >>> callback. If you didn't need to divide this into subsubsections, >>> you could introduce the guiding principles in a way that feels more >>> natural. >> I consider it more natural to have the 'guiding principles' _before_ >> the concrete cases, as they are meant to be 'guiding' and >> 'principles'. >> It's like 'starting from first principles', there introducing the >> first principles as you go is ad-hoc. >> The guiding principles also need to be outside the examples, in case >> one of the examples doesn't apply to the packager's use case, such >> that they can fall-back to the guiding principles. >> Also, in your patch you are dividing things in subsubsections as >> well, just under a different name and different representation (table >> entries in a subsection), as mentioned previously. > A table entry is not a subsection, as much as you want it to be that. It indeed is not, rather they are equivalent here in terms of structure, nesting and problematicness. > > Also, your guiding principles are (with one exception) really just > invariants that ought to hold for the source field of a package. I don't know about the exceptions (I haven't counted them), but yes, indeed. I do not see the problem of this. > As such, I think it would be easier to state "A package's source should > be the smallest corresponding source in terms of uncompressed file > size. This corresponding source must consist only of free software > (note Free Software) and should build on all platforms supported by > upstream." Note how smallest naturally implies unbundling bundled > sources. This criterium on overly small sources. Often, a package's source contains things that is not used for the build or outputs and hence is not part of the corresponding source. Examples: * the source contains documentation that could be built and installed, but Guix doesn't do so yet. --> should be kept (unless non-free) * documentation that isn't meant to be built or installed (e.g. HACKING, PACKAGERS, ...) --> useful, shouldn't be removed. * .gitignore, .github, ... --> nothing wrong with removing those, but pointless, let's not waste our time with looking for those and removing them, even though doing so would make it smaller. * source files for platforms the upstream does not support yet/anymore (but with some volunteer effort (e.g. bugfixes), it might become a supported system again) --> removing them (e.g. as part of an overly-broad (delete-file-recursively "this-dir-has-bundling-albeit-not-all-of-it-is-bundling")) can be OK-ish, but removing them for 'minimality' is pointless. >>> I still find this wording very confusing. Perhaps "To add new >>> functionality, a patch is almost always the best choice. For one, >>> it is likely that the new functionality requires changing multiple >>> lines of source code, which is more convenient to do with a patch >>> than with a snippet. Further, patches can be taken from and >>> submitted to upstreams more easily. If your patch has not been >>> submitted to upstream, consider doing so." >> It loses some information (that patches are preferred) and >> (after re-adding the conclusion) is more verbose, which appears >> to be considered very important. > Which conclusion is there to re-add? "patches are preferred" The conclusion is already stated > in the beginning: patches are almost always the best choice. Then two > reasons as for why. The part w.r.t. upstreaming changes has also been > addressed. While I consider that policies should have "best choices" coinciding with "preferred" and that not doing so would be illogical, people, projects, decisions ... are far from always logical. Because of this, people cannot assume that the 'best choices' are 'preferred', so it needs to be mentioned explicitly that these 'best choices' are actually 'preferred'. >>> An enumeration ought to have at least three elements (otherwise >>> it's just a pair), and I think if we do proper counting and omit >>> no-brainers, such as the "only free software" part that has already >>> been mentioned, we come very close to skirting that line. >> The "only free software" is mentioned elsewhere, yes, but it is one >> of the principles for deciding between snippets, phases and patches. >> While you call it a no-brainer, it is sometimes neglected, so it >> sounds important to me to explicitly list it. >> Merging the 3th and 4th @item, I count 4 principles, so it fits with >> an enumeration. >> Also, I'm not following your point here -- your complaint was that >> they aren't guiding principles (based on the number of them), but >> your response is that they might not form an enumeration? They are >> named the guiding principles, not the guiding enumeration. What have >> enumerations to do with anything? > I'm using enumeration as a super term here, which can be > ((sub)sub)sections, chapters, list elements, whatever, and my claim is > that we barely have enough principles to allow the use of a plural. In English, things are either plural or singular. Everything >= 2 is plural. There number of principles, however we count them, is, at least, 2. Consequently, the principles are plural, not singular. Treating the principles as singular is simply grammatically incorrect. Maybe it is barely allowed to be plural, but English grammar doesn't care about that -- it is definitively disallowed to be singular, only plural remains. > Adding to that, now that I think of it, I also doubt their usefulness > in guiding. "Use whatever feels convenient, but note that that might > be subjective" is more useful at the end of the section when a user > didn't find what they were looking for than at the start. The introduction has a set of guiding principles, from with concrete cases are built. By adding another principle at the end, the cases cannot make use of the "use [...] convenient" principle. Additionally, now the user has to look at _two_ places to find the guiding principles -- at the beginning, and at the end. Worse, the beginning does not have a cross-reference to the end, so since the set at the beginning is presented as exhaustive, the user might not know there is another guiding principle. And even if they did read through the whole section (even though they should only have to read the introduction and the relevant worked-out case), assuming they read through it in a linear fashion, due to the new guiding principle that wasn't alluded to at the beginning, they have to redo their mental model of "Modifying Sources" as this principle could invalidate things. >>>> I do not see what the problem is with additional length as long >>>> as this additional length comes with additional useful >>>> information and the manual is well-structured (e.g. with >>>> (sub)(sub)sections, chapters and indices) -- we do not have a >>>> page limit. >>>> >>>> At worst, perhaps the same information could perhaps be encoded >>>> with fewer words? I could compare the two patches to see which >>>> one formulates certain information in the fewest words, and >>>> choose the least verbose of the two for each piece of information >>>> that is present in both? >>>> >>>> Also, comparing the two patches, my patch has 40 more lines, but >>>> about 25 of them are for noting the guiding principles (which are >>>> absent in your patch). >>>> Compensating for that, the patches are about the same length, so >>>> I do not think that 'sheer length' is accurate here. >>> 25 lines calling back to earlier information are, imho, an >>> indicator that the section is too long. Imagine you'd have twenty- >>> five function calls to guiding_principles(n) in your program – at >>> some point, you'd try to cache those. >> (define cached-guiding-principles >> (delay (list (guiding-principle-0) >> [...] >> (guiding-principle-24)))) >> Caching the guiding principles does not reduce the length. >> I don't see the problem with calling back to earlier information. >> Also, it isn't earlier information, there is no nice list of guiding >> principles anywhere else. > At the risk of responding jokingly to what was meant serious: I didn't > know we suddenly gained 20 guiding principles. 25 lines are for the guiding principles (sometimes referring to a principle of elsewhere in Guix, sometimes a new principle for "Modifying Sources"). You proposed to 'cache' them somehow. In Guile, to cache something, you can use 'delay/force'. But this increases the amount of code (due to the additional use of 'delay'), instead of decreasing. The documentation equivalent (whatever that might be, I am not seeing one myself) would then also be at least as long, maybe even a little longer due to the use of 'delay'. >>>>> Going down to subsubsections just to find out where patches are >>>>> appropriate, is imho overkill. >>>> The 'going down to subsubsection' is the case for your patch too, >>>> though? In my case, it's a subsubsection, in your case it's a >>>> table >>>> entry inside a subsection, both are the same level of nesting. >>> These are still two very different kinds of nesting. A table fits >>> onto a (virtual) page more easily than several subsections. >> I suppose table items might take two less line or so less than >> subsubsections, but I don't think that's sufficient reason to step >> away from a nice section structure. > Another reason is that you can end a table in the middle of a section, > which you can't do with subsections. Hence why I'm able to put a > remark at the bottom, which you have to clumsily fit into the top. I can, indeed, not put the 'convenience principle' at the bottom, except perhaps by adding a new subsubsection, and for tables adding such a remark at the bottom is indeed more convenient. However, I don't see the 'have to clumsily' here -- as explained elsewhere, I believe that the 'convenience principle' shouldn't be separated from the other principles and that it fits nicely next to the the principles --- there is no 'have to', because I choose for this, and I am not seeing any 'clumsiness' here. >>>> Also, it's a matter of different structure -- in my v2 and v3 >>>> patch, I have a 'problem -> solution' structure -- the idea is >>>> that the packages has a problem, they look at the section, they >>>> read the subsubsection names, select the >>>> subsubsection that matches their problem and read the solution -- >>>> in short, the idea is to provide a solution to the problem. >>>> >>>> Your structure is the other way around -- for solutions (patches, >>>> snippets, phases), it gives the permitted problems to apply it >>>> to. >>>> >>>> So yes, your patch is more convenient for finding out where >>>> patches are appropriate. I do not see the benefit of that though >>>> -- a new contributor packaging a thing wouldn't know in advance >>>> which solutions could be appropriate for them (your 'solution -> >>>> problem' patch?), rather, they start with a problem and are >>>> searching for an appropriate solution (my problem->solution >>>> patch). >>> I think this idea can be debunked pretty easily. If I give you a >>> hammer and I tell you "this is a hammer, you can use it to put >>> nails into a wall", and you have a nail and you want to put it into >>> a wall, you won't go "oh no, however will I put this nail into a >>> wall?" – you will simply use the hammer to do so. >> The patch does this, currently. It already proposes a number of >> hammers (patches, snippets and phases) and purposes (adding new >> functionality, fixing technical issues, unbundling, ...). >> Also, the scenario "oh no, however will I put this nail into a wall" >> actually happened -- see the Shepherd discussion, where there was >> a lot of disagreement on how nails (= small work-around in the >> Makefile) should be put in walls (= patches, snippet, phase?). It >> was the whole reason to start writing a documentation patch. > You might want to add a link here if it supports your argument, but > without looking at the discussion this rather sounds like "oh no, I > have three hammers, which one do I pick?" – which, fair enough, is > still a problem, but one that neither of our patches would cause imho. As I think I've written previously, the whole point was to solve that problem. For the discussion, see: * https://issues.guix.gnu.org/54216 * https://yhetil.org/guix/c4c0a071-2e03-d4d6-6718-05424d21d146@telenet.be/ * https://yhetil.org/guix-devel/84e13ef7d437062df5cca51a12e6da54929e0176.camel@telenet.be/ Not solving the problem defeats the whole point, as the purpose is to solve that problem. >>> Of course, for this to work I also have to tell you *how* to use a >>> hammer to put nails into a wall, but that's exactly what I did in >>> my patch by inserting the right notes into the Guix manual. >> Also already the case. >>> My solution->problem approach has the benefit, that folks can just >>> go over all the solutions, check if their problem fits, and apply >>> the one that says "here, use this". >> A problem->solution structure is useful for that too? >> And it already lists all the solutions (snippets, phases and patches) >> and information to decide whether the solution fits their problem >> (the guiding principles, and the worked-out cases). > Again, I believe you're overselling the guiding principles. I never claimed they were super great, just that they are convenient and solved a number of problems. I'm not seeing any overselling here. >>> And if they don't find anything, they see the handy little line at >>> the bottom saying "use whatever you think is convenient". >> Nowhere did the patch imply that the listed cases were all cases. In >> fact, in two places in the introduction it is implied that the >> examples are not exhaustive, and that they can choose according to >> convenience [...] > Emphasis on handy little line rather than needing to be told twice > (particularly if people have no idea what to expect due to not having > looked at the worked-out cases yet). They don't need to be told twice. Also, my patch also has that handy little line (albeit differently worded), see the 'guiding principles'. Also, on the 'no idea what to expect' -- this is exactly the same for your patch too? In both patches, if the user only reads the introduction and conclusion (if any) and doesn't read the actual (relevant examples)/(explanation of patches, snippets, phases), then that's their problem. >>> I also expand a little on the benefits and drawbacks of >>> these approaches as you would when describing design patterns. >> This is also done in my patch. [...] > This is not about the contained information, but the structure of the > contained information. > > My solution->problem style follows roughly this style: > 1. solution > 2. problems it is known to solve > 3. how to use > 4. properties, caveats, etc. > > Your problem->solution style roughly has the following: > 1. problem > 2. (set of) solution(s) > 3. if more than one solution, maybe a help to select Also, in no particular order 4.: how to use 5.: explanation why it is preferred (properties, caveats?) > > This makes it so that people might have to go to a different subsection > than the one they read for their solution to find out about potential > caveats, e.g. not embedding store paths in a snippet. I assume their problem was "X doesn't find Y". This being a technical issue, they go to "Fixing technical issues". In that subsubsection, there are the words: > 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. so they actually do know of the caveat 'don't embed store paths in a snippet, do it in a phase instead', and they did not need to go to a different subsubsection. >>> Your problem->solution approach instead leaves people wondering >>> when their particular use case has not been described. >> See my reply to ‘And if they don't find anything, they see the handy >> little line at the bottom saying "use whatever you think is >> convenient’. >>> It gives them a solution rather than the tools to build solutions >>> with. >> It does give the tools: snippets, patches and phases. > As far as I read, it describes none of those. It puts out guiding > principles and some already worked-out cases. Here are the descriptions: > 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 Once the user knows _which_ of the three they should use (by consulting the following subsubsections), they can then search for 'patch', 'snippet' and 'phases' in the manual for more information, no need to duplicate that information. >> Also, "giving the tools to build solutions with" does not help with >> the problem that I aimed to solve -- there was disagreement on what >> the appropriate tools are (see: Shepherd), so it not just needs to >> give the tools, but also the solutions. > I don't see how my patch lacks this information however. IIUC, the raw information should usually be indeed all there, but the user has to consult _all_ of the sections to determine which option (patch, snippet, phase) is appropriate, having to assemble all the information is (a) a waste of time and (b) can lead to different interpretations and conclusions (see: Shepherd). More concretely, I cannot use your patch to decide between phases, snippets and patches for the Shepherd issue: * none of three appear to be forbidden by your documentation * there is no recommendation for 'patches' (IIRC it wasn't accepted upstream and there was no intent to submit it upstream, it being really a Guile bug, not a Shepherd bug) * there is no recommendation for snippets (it's all free, no bundling) * build phases are 'to be avoided' (but not forbidden), as it resulted in observably different runtime behaviour (being a bug fix) -- three (or at best, two, if build phases are discounted) options remain. Myself, I would then consider 'snippets' to be the most convenient, but some others (see: Shepherd, IIRC) would really want a patch instead, because 'patches can be reverted' or something like that. As such, you are giving the tools (snippets / patches / phases, some downsides and upsides), but different people can construct different solutions from those tools even in the same situation, leading to conflicts. If I use my patch instead, I go to "fixing technical issues". This section tells me to use a patch or a snippet. As the fix is not Guix-specific, it recommends a patch to save time when upstreaming. Conclusion: write a patch. It was a Guile bug, so the fix was a patch to Guile. But that would take time and upstream took the responsibility for a fix, so the 'efficient time thing' doesn't really apply and a small work-around (setting optimisation flags) suffices. In this situation, the subsubsection doesn't care at all if you go for a snippet, so apply the last guiding principle: go for the simplest thing: a snippet. (Unless you already have a patch, then a patch is simplest.) Does someone else have a different idea on what's simplest? Doesn't really matter, ‘Sometimes this is subjective, which is also fine’. > In > particular, for review purposes, mine should be easier to work with. > For instance, the reviewer sees a hash embedded in a snippet, sees in > the snippet entry that you shouldn't do that, and can thus say "nope, > don't do a snippet here". I think we should optimise for packagers before reviewers > > Regardless of which patch we pick, it should not mandate a particular > solution in potentially contentious cases, Actually the whole point was to mandate a particular solution for the contentious cases, see Shepherd. > and also point towards the > right solution. See our discussions on the individual solutions on > points in which I believe you've errored. These are: * the typo's * including "skipping tests indicating failure under ‘Fixing technical issues’" * "don't mention file names of non-free things in patches" Did I miss any? Greetings, Maxime
Am Freitag, dem 09.09.2022 um 13:14 +0200 schrieb Maxime Devos: > > > In descriptions, it is wise to do so because it helps software > > stand on its own merits rather than just "being a clone of this > > thing you aren't allowed to have" (this is real criticism pointed > > at us from the proprietary software embracers). See for instance > > minetest, which is described in terms of its features rather than > > "being a Minecraft clone". > > Sentence might have been truncated? Corrected above. > Also, this is the package source field, not the description, I don't > see how the "helps software stand on its own merits" applies to > snippets of the package source. Point taken, but imho it still makes sense to prefer keep lists over remove lists. The GNU project encourages people to read the sources after all. > > > How does ignoring a test fix the technical issue identified by > > > the test (sometimes, the technical issue being a bug in the test > > > itself)? > > It fixes the technical issue that an otherwise functional package > > (w.r.t. the N tests that don't fail) builds. This is a > > particularly > > useful distinction with tests that require a network connection and > > thus have to fail in a build container, or are known flaky upstream > > and thus cause reproducibility issues. > > Network test: right (though preferably those would support a > --no-network-tests test option or such). > > For flaky tests: those sound like bugs to me, ignoring them doesn't > remove the flakyness. Call it a bug or whatever, it is a technical issue that we deal with at build time and not in the origin. > > > > > > > There still is the difference that phases are clearly delimited > > > > while snippets are a block of code that shouldn't get too > > > > large. > > > Snippets are delimited clearly as well, though, with the > > > 'snippet' field? > > > And the limitations of snippet length and phases length are the > > > same -- no limits, though conciseness is appreciate as always. > > True, but phases can be made to do just one thing, whereas snippets > > have to fix everything that's wrong in more or less one (begin > > ...). > > This is a noticable distinction.> > > You can do that in snippets too, with comments: > > (snippet > #~(begin > ;; Do the foo thing > (foo) > (foo-2 [...]) > [...] > ;; Do the bar thing > (bar) > (bar-2 [...]) > [...])) You can, but it's still wiser to just keep the snippet short enough so you don't have to. > > > > > > I think my version at least hinted at this practice in a more > > > > concise way, so it's not impossible to mention. [...] > > > I agree it's possible -- as I replied previously: > > > > I suppose a section could be added somewhere to properly > > > > document > > > > the 'embedding store file names' practice, and insert a > > > > cross-reference, > > > I don't think documenting the how of the practice should be done > > > in this section, properly explaining 'search-input-file' / > > > 'search- > > > input-directory', 'inputs / native-inputs', 'bash' being an > > > implicit > > > input but you still have to add it to 'inputs' in some cases > > > because > > > of cross-compilation, this-package-input and this-package-native- > > > input ... would make the > > > subsubsection a bit too long I think, distracting from other > > > situations, hence the proposal for a cross-reference. > > > How about leaving the 'how to embed store file names' for a > > > separate > > > documentation patch and section, adding a cross-reference > > > later? > > See above, "I suppose a section could be added..." means I'm > > somewhat indifferent to whether it's done now or later; > > Nitpick: you are quoting some text I wrote, so 'I' refers to me here, > not you. Ahh, my bad, the nesting level confused me. > I would however very > > much prefer a wording that points people toward this practice > > existing, > > even if they have to go look in the code for examples. > > Alternatively, > > a footnote for the most common case (search-input-file inputs > > "bin/command") ought to suffice for the time being. > > Aside from the typo's and a few rephrasings, I'm done with the > documentation. If someone wants to extend the section with such > information, they can always do so later. I haven't seen a v2 though. Am I correct in assuming that none of the points we discussed in the last few mails are going to be addressed? > > > > IMHO, I think a reader who remembers the guiding principles should > > see that it applies. > > Likely. But removing the explicit mention of the guiding principle > still makes the logical reasoning incomplete, remembering has nothing > to do with it. As I've written previously: ‘This has nothing do do > with [...] and remembering, but rather with [...]’. In that case, let me rephrase my criticism: "It passes the guidelines" should not be part of the logical reasoning here. Otherwise, why not have a guideline checklist at the end of each subsection (which would be silly, obviously, but that's the point). > > > > > Also, your guiding principles are (with one exception) really just > > invariants that ought to hold for the source field of a package. > > I don't know about the exceptions (I haven't counted them), but yes, > indeed. I do not see the problem of this. For the record, the exception is the "most convenient" bit you've copied from my patch :) > > As such, I think it would be easier to state "A package's source > > should > > be the smallest corresponding source in terms of uncompressed file > > size. This corresponding source must consist only of free software > > (note Free Software) and should build on all platforms supported by > > upstream." Note how smallest naturally implies unbundling bundled > > sources. > > This criterium on overly small sources. Often, a package's source > contains things that is not used for the build or outputs and hence > is not part of the corresponding source. Examples: Note "should" rather than "shall" or "must", meaning that slightly larger tarballs are still acceptable. > * the source contains documentation that could be built and > installed, > but Guix doesn't do so yet. --> should be kept (unless non-free) > * documentation that isn't meant to be built or installed > (e.g. HACKING, PACKAGERS, ...) --> useful, shouldn't be removed. > * .gitignore, .github, ... --> nothing wrong with removing those, > but pointless, let's not waste our time with looking for those > and removing them, even though doing so would make it smaller. > * source files for platforms the upstream does not support > yet/anymore > (but with some volunteer effort (e.g. bugfixes), it might become > a supported system again) > --> removing them (e.g. as part of an overly-broad > (delete-file-recursively > "this-dir-has-bundling-albeit-not-all-of-it-is-bundling")) > can be OK-ish, but removing them for 'minimality' is > pointless. I see you're nitpicking for the sake of the argument, but none of the examples you provide (safe for the bundling one) add much cost to either packing or unpacking. Thus, I'm pretty sure we can ignore them for practical purposes. That being said, if you have a better formulation, please do tell. > > > > > I still find this wording very confusing. Perhaps "To add new > > > > functionality, a patch is almost always the best choice. For > > > > one, > > > > it is likely that the new functionality requires changing > > > > multiple > > > > lines of source code, which is more convenient to do with a > > > > patch > > > > than with a snippet. Further, patches can be taken from and > > > > submitted to upstreams more easily. If your patch has not been > > > > submitted to upstream, consider doing so." > > > It loses some information (that patches are preferred) and > > > (after re-adding the conclusion) is more verbose, which appears > > > to be considered very important. > > Which conclusion is there to re-add? > > "patches are preferred" > > The conclusion is already stated > > in the beginning: patches are almost always the best choice. Then > > two > > reasons as for why. The part w.r.t. upstreaming changes has also > > been > > addressed. > > While I consider that policies should have "best choices" coinciding > with "preferred" and that not doing so would be illogical, people, > projects, decisions ... are far from always logical. > > Because of this, people cannot assume that the 'best choices' are > 'preferred', so it needs to be mentioned explicitly that these 'best > choices' are actually 'preferred'. In that case, simply write preferred choice? > > I'm using enumeration as a super term here, which can be > > ((sub)sub)sections, chapters, list elements, whatever, and my claim > > is that we barely have enough principles to allow the use of a > > plural. > > In English, things are either plural or singular. Everything >= 2 is > plural. There number of principles, however we count them, is, at > least, 2. Consequently, the principles are plural, not singular. > Treating the principles as singular is simply grammatically > incorrect. Correction: Everything that isn't exactly 1 is plural in English, including 0, with perhaps the exception of -1 also using singular. > Maybe it is barely allowed to be plural, but English grammar doesn't > care about that -- it is definitively disallowed to be singular, only > plural remains. Note that my argument isn't about English grammar, it uses English grammar. > > Adding to that, now that I think of it, I also doubt their > > usefulness in guiding. "Use whatever feels convenient, but note > > that that might be subjective" is more useful at the end of the > > section when a user didn't find what they were looking for than at > > the start. > > The introduction has a set of guiding principles, from with concrete > cases are built. By adding another principle at the end, the cases > cannot make use of the "use [...] convenient" principle. > > Additionally, now the user has to look at _two_ places to find the > guiding principles -- at the beginning, and at the end. Worse, > the beginning does not have a cross-reference to the end, so since > the set at the beginning is presented as exhaustive, the user might > not know there is another guiding principle. > > And even if they did read through the whole section (even though they > should only have to read the introduction and the relevant worked-out > case), assuming they read through it in a linear fashion, due to the > new guiding principle that wasn't alluded to at the beginning, they > have to redo their mental model of "Modifying Sources" as this > principle could invalidate things. This shows both a problem with your structure, but more importantly, a failure to understand what I meant when I wrote "use whatever is convenient" in my own patch. This principle *can* only work for cases in which there is *no clearly established practice*, thus putting it at the end *after having enumerated all practices* is the logical choice. With your structure, you have to bend it sideways to shoehorn it in with the other principles. > > At the risk of responding jokingly to what was meant serious: I > > didn't know we suddenly gained 20 guiding principles. > > 25 lines are for the guiding principles (sometimes referring to a > principle of elsewhere in Guix, sometimes a new principle for > "Modifying Sources"). You proposed to 'cache' them somehow. In > Guile, to cache something, you can use 'delay/force'. But this > increases the amount of code (due to the additional use of 'delay'), > instead of decreasing. > > The documentation equivalent (whatever that might be, I am not seeing > one myself) would then also be at least as long, maybe even a little > longer due to the use of 'delay'. Okay, so I did misunderstand those 25 lines as 25 lines within the text calling back to those guiding principles rather than 25 lines for the guiding principles themselves. Still, 25 lines are a little much, especially given that the bulk of it explains the semantics of the packages' source field. > > > > > I suppose table items might take two less line or so less than > > > subsubsections, but I don't think that's sufficient reason to > > > step away from a nice section structure. > > Another reason is that you can end a table in the middle of a > > section, which you can't do with subsections. Hence why I'm able > > to put a remark at the bottom, which you have to clumsily fit into > > the top. > > I can, indeed, not put the 'convenience principle' at the bottom, > except perhaps by adding a new subsubsection, and for tables adding > such a remark at the bottom is indeed more convenient. > > However, I don't see the 'have to clumsily' here -- as explained > elsewhere, I believe that the 'convenience principle' shouldn't be > separated from the other principles and that it fits nicely next to > the the principles --- there is no 'have to', because I choose for > this, and I am not seeing any 'clumsiness' here. I think we'd have to debate choice vs. force here which would be a rather long aside, but to make my argument short, your choice of how to structure this section (table vs subsections) directly influences your "choice" where to place particular information in a way that might inhibit natural information flow. > > > > > > > > > The patch does this, currently. It already proposes a number of > > > hammers (patches, snippets and phases) and purposes (adding new > > > functionality, fixing technical issues, unbundling, ...). > > > Also, the scenario "oh no, however will I put this nail into a > > > wall" > > > actually happened -- see the Shepherd discussion, where there was > > > a lot of disagreement on how nails (= small work-around in the > > > Makefile) should be put in walls (= patches, snippet, phase?). > > > It > > > was the whole reason to start writing a documentation patch. > > You might want to add a link here if it supports your argument, but > > without looking at the discussion this rather sounds like "oh no, I > > have three hammers, which one do I pick?" – which, fair enough, is > > still a problem, but one that neither of our patches would cause > > imho. > > As I think I've written previously, the whole point was to solve that > problem. For the discussion, see: > > * https://issues.guix.gnu.org/54216 > * > https://yhetil.org/guix/c4c0a071-2e03-d4d6-6718-05424d21d146@telenet.be/ > * > https://yhetil.org/guix-devel/84e13ef7d437062df5cca51a12e6da54929e0176.camel@telenet.be/ > > Not solving the problem defeats the whole point, as the purpose is to > solve that problem. I think we might be disagreeing about what constitutes "solving the problem" here. Correct me if I'm wrong, but to me it seems any scenario that is not covered by whatever patch is applied counts as "not having solved the problem". I personally find this line of reasoning questionable, as there are perfectly valid reasons why multiple tools could apply. Take the problem of embedding a store path. You could for instance replace all occurences of "sh" with /gnu/store/.../bin/sh, or you could first replace them in a patch with "@sh@" and then replace that with /gnu/store/.../bin/sh. Of course, you rarely have to do the latter and the former is simpler, hence why you will often see the former, but that doesn't mean the latter is invalid. > > > > > > > > > And if they don't find anything, they see the handy little line > > > > at the bottom saying "use whatever you think is convenient". > > > Nowhere did the patch imply that the listed cases were all cases. > > > In fact, in two places in the introduction it is implied that the > > > examples are not exhaustive, and that they can choose according > > > to convenience [...] > > Emphasis on handy little line rather than needing to be told twice > > (particularly if people have no idea what to expect due to not > > having looked at the worked-out cases yet). > > They don't need to be told twice. Also, my patch also has that handy > little line (albeit differently worded), see the 'guiding > principles'. > > Also, on the 'no idea what to expect' -- this is exactly the same for > your patch too? In both patches, if the user only reads the > introduction and conclusion (if any) and doesn't read the actual > (relevant examples)/(explanation of patches, snippets, phases), then > that's their problem. I do think a table in the middle of the section is hard to ignore, but suppose for the sake of argument that they do, then in my case they will have learned exactly nothing. In your case, they will know the "guiding principles" and then interpret them to mean whatever they think it means. I'm not convinced mine is worse here. > > > > > > > I also expand a little on the benefits and drawbacks of > > > > these approaches as you would when describing design patterns. > > > This is also done in my patch. [...] > > This is not about the contained information, but the structure of > > the contained information. > > > > My solution->problem style follows roughly this style: > > 1. solution > > 2. problems it is known to solve > > 3. how to use > > 4. properties, caveats, etc. > > > > Your problem->solution style roughly has the following: > > 1. problem > > 2. (set of) solution(s) > > 3. if more than one solution, maybe a help to select > > Also, in no particular order > > 4.: how to use > 5.: explanation why it is preferred (properties, caveats?) No particular order is problematic, but more importantly if a technology appears more than once (guaranteed by the pigeonhole principle), then either its properties get repeated (not good for length) or scattered (not good for the flow you want). Again, a structural problem. > > This makes it so that people might have to go to a different > > subsection than the one they read for their solution to find out > > about potential caveats, e.g. not embedding store paths in a > > snippet. > > I assume their problem was "X doesn't find Y". This being a > technical issue, they go to "Fixing technical issues". In that > subsubsection, there are the words: > > > 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. > > so they actually do know of the caveat 'don't embed store paths in a > snippet, do it in a phase instead', and they did not need to go to a > different subsubsection. Typical happy path. > > > > > > > Your problem->solution approach instead leaves people wondering > > > > when their particular use case has not been described. > > > See my reply to ‘And if they don't find anything, they see the > > > handy little line at the bottom saying "use whatever you think is > > > convenient’. > > > > It gives them a solution rather than the tools to build > > > > solutions with. > > > It does give the tools: snippets, patches and phases. > > As far as I read, it describes none of those. It puts out guiding > > principles and some already worked-out cases. > Here are the descriptions: > > > 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 > > Once the user knows _which_ of the three they should use (by > consulting the following subsubsections), they can then search for > 'patch', 'snippet' and 'phases' in the manual for more information, > no need to duplicate that information. Point taken, but imho cross-references are nice for those who just learned "okay, I now know I need to solve my technical problem with a phase, how do I do that?" > > > > Also, "giving the tools to build solutions with" does not help > > > with the problem that I aimed to solve -- there was disagreement > > > on what the appropriate tools are (see: Shepherd), so it not just > > > needs to give the tools, but also the solutions. > > I don't see how my patch lacks this information however. > > IIUC, the raw information should usually be indeed all there, but the > user has to consult _all_ of the sections to determine which option > (patch, snippet, phase) is appropriate, having to assemble all the > information is (a) a waste of time and (b) can lead to different > interpretations and conclusions (see: Shepherd). > > More concretely, I cannot use your patch to decide between phases, > snippets and patches for the Shepherd issue: > > * none of three appear to be forbidden by your documentation > * there is no recommendation for 'patches' (IIRC it wasn't accepted > upstream and there was no intent to submit it upstream, it being > really a Guile bug, not a Shepherd bug) > * there is no recommendation for snippets (it's all free, no > bundling) > * build phases are 'to be avoided' (but not forbidden), as it > resulted > in observably different runtime behaviour (being a bug fix) > > -- three (or at best, two, if build phases are discounted) options > remain. Myself, I would then consider 'snippets' to be the most > convenient, but some others (see: Shepherd, IIRC) would really want a > patch instead, because 'patches can be reverted' or something like > that. > > As such, you are giving the tools (snippets / patches / phases, some > downsides and upsides), but different people can construct different > solutions from those tools even in the same situation, leading to > conflicts. > > If I use my patch instead, I go to "fixing technical issues". This > section tells me to use a patch or a snippet. As the fix is not > Guix-specific, it recommends a patch to save time when upstreaming. > Conclusion: write a patch. It was a Guile bug, so the fix was a > patch to Guile. But that would take time and upstream took the > responsibility for a fix, so the 'efficient time thing' doesn't > really apply and a small work-around (setting optimisation flags) > suffices. > > In this situation, the subsubsection doesn't care at all if you go > for a snippet, so apply the last guiding principle: go for the > simplest thing: a snippet. (Unless you already have a patch, then a > patch is simplest.) > Does someone else have a different idea on what's simplest? Doesn't > really matter, ‘Sometimes this is subjective, which is also fine’. I do get the impression that this patch simply codifies what you feel is right based on the shepherd situation rather than thinking about the general case, which is why my patch makes weaker statements here. If this issue could have been avoided with a #:make-flag, we would have used that. More importantly, I fail to see the superiority of your patch here. IIUC neither patch offers a unique solution to the shepherd thing, so the room for debate remains. You could claim that my patch offers more room, which fair enough, it does, but on the same token it allows people to see that the guidelines have been followed and the patch can be applied. > > In particular, for review purposes, mine should be easier to work > > with. For instance, the reviewer sees a hash embedded in a > > snippet, sees in the snippet entry that you shouldn't do that, and > > can thus say "nope, don't do a snippet here". > > I think we should optimise for packagers before reviewers Code is more often read than written, so optimising for the reader is optimizing for the packager as well. > > Regardless of which patch we pick, it should not mandate a > > particular solution in potentially contentious cases, > > Actually the whole point was to mandate a particular solution for the > contentious cases, see Shepherd. But I disagree. Memes aside, please refrain from the "I'm right, do this" style (which is another problem with the "problem-centric" approach), but rather focus on writing a patch that makes reviewers not discard a patch due to formalities. If the shepherd thing had needed an upstream patch, even with a snippet that patch could have easily been created by converting that snippet to a sed, so more time was wasted debating than overworking. > > and also point towards the right solution. See our discussions on > > the individual solutions on points in which I believe you've > > errored. > > These are: > > * the typo's > * including "skipping tests indicating failure under ‘Fixing > technical issues’" > * "don't mention file names of non-free things in patches" > > Did I miss any? I'm pretty sure there's also a discussion on store path embeddings at the very least. Forgive me if I too missed something. Cheers
On 09-09-2022 15:32, Liliana Marie Prikler wrote: >> Also, this is the package source field, not the description, I don't >> see how the "helps software stand on its own merits" applies to >> snippets of the package source. > Point taken, but imho it still makes sense to prefer keep lists over > remove lists. The GNU project encourages people to read the sources > after all. I do not see how the former follows from the latter, I would expect the opposite. GNU project encourages reading source code, we put useful information in the source code (e.g. records on what non-free parts haven't been replaced yet). >>>> >>>>> There still is the difference that phases are clearly delimited >>>>> while snippets are a block of code that shouldn't get too >>>>> large. >>>> Snippets are delimited clearly as well, though, with the >>>> 'snippet' field? >>>> And the limitations of snippet length and phases length are the >>>> same -- no limits, though conciseness is appreciate as always. >>> True, but phases can be made to do just one thing, whereas snippets >>> have to fix everything that's wrong in more or less one (begin >>> ...). >>> This is a noticable distinction.> >> >> You can do that in snippets too, with comments: >> >> (snippet >> #~(begin >> ;; Do the foo thing >> (foo) >> (foo-2 [...]) >> [...] >> ;; Do the bar thing >> (bar) >> (bar-2 [...]) >> [...])) > You can, but it's still wiser to just keep the snippet short enough so > you don't have to. That's also the case for phases, though? Conciseness is both good for phases and snippets. >> I would however very >>> much prefer a wording that points people toward this practice >>> existing, >>> even if they have to go look in the code for examples. >>> Alternatively, >>> a footnote for the most common case (search-input-file inputs >>> "bin/command") ought to suffice for the time being. >> >> Aside from the typo's and a few rephrasings, I'm done with the >> documentation. If someone wants to extend the section with such >> information, they can always do so later. > I haven't seen a v2 though. Am I correct in assuming that none of the > points we discussed in the last few mails are going to be addressed? I did sent a v2: <https://issues.guix.gnu.org/57598#8>. And most were addressed, even if only as me not considering it an issue. >>> >>> IMHO, I think a reader who remembers the guiding principles should >>> see that it applies. >> >> Likely. But removing the explicit mention of the guiding principle >> still makes the logical reasoning incomplete, remembering has nothing >> to do with it. As I've written previously: ‘This has nothing do do >> with [...] and remembering, but rather with [...]’. > In that case, let me rephrase my criticism: "It passes the guidelines" > should not be part of the logical reasoning here. Otherwise, why not > have a guideline checklist at the end of each subsection (which would > be silly, obviously, but that's the point). Because, as you note, that's a silly structure, putting the premises (= guiding principles) after the conclusions (= worked-out cases) is an illogical structure, whereas putting the premises _before_ the conclusions is logical >>>> >>> Also, your guiding principles are (with one exception) really just >>> invariants that ought to hold for the source field of a package. >> >> I don't know about the exceptions (I haven't counted them), but yes, >> indeed. I do not see the problem of this. > For the record, the exception is the "most convenient" bit you've > copied from my patch :) That guideline actually predates your patch, it's part of the original proposal (of which the wording changed later): > https://yhetil.org/guix/c4c0a071-2e03-d4d6-6718-05424d21d146@telenet.be/ > Sometimes, there remains more than one acceptable way to accomplish the > goal. In that case, choose whatever appears to be most convenient. >>> As such, I think it would be easier to state "A package's source >>> should >>> be the smallest corresponding source in terms of uncompressed file >>> size. This corresponding source must consist only of free software >>> (note Free Software) and should build on all platforms supported by >>> upstream." Note how smallest naturally implies unbundling bundled >>> sources. >> >> This criterium on overly small sources. Often, a package's source >> contains things that is not used for the build or outputs and hence >> is not part of the corresponding source. Examples: > Note "should" rather than "shall" or "must", meaning that slightly > larger tarballs are still acceptable. OK. The criterium remains overly tight though -- by that criterium, slightly larger tarballs are considered undesirable, even when they have useful (and hence, desired) things: > >> * the source contains documentation that could be built and >> installed, >> but Guix doesn't do so yet. --> should be kept (unless non-free) >> * documentation that isn't meant to be built or installed >> (e.g. HACKING, PACKAGERS, ...) --> useful, shouldn't be removed. >> * .gitignore, .github, ... --> nothing wrong with removing those, >> but pointless, let's not waste our time with looking for those >> and removing them, even though doing so would make it smaller. >> * source files for platforms the upstream does not support >> yet/anymore >> (but with some volunteer effort (e.g. bugfixes), it might become >> a supported system again) >> --> removing them (e.g. as part of an overly-broad >> (delete-file-recursively >> "this-dir-has-bundling-albeit-not-all-of-it-is-bundling")) >> can be OK-ish, but removing them for 'minimality' is >> pointless. > I see you're nitpicking for the sake of the argument, I'm not. It's nitpicking for the sake of the four * examples above. > but none of the > examples you provide (safe for the bundling one) add much cost to > either packing or unpacking. Not the point, the point is that having to treat those examples seriously is a waste of time and even somewhat harmful (removing useful stuff). > Thus, I'm pretty sure we can ignore them > for practical purposes. As I've explained, this guideline is inconsistent with what is actually recommended. Here are some practical purposes: * minimal dissonance between 'appreciated by policy' and 'appreciated by reviewers, packagers, users'. * not having to review and accept pointless patches to Guix for deleting .github, .gitignore, uninstalled documentation that were written for the sake of minimality * not having to (in the sense of: it would be appreciated if you'd do that) write such patches That being said, if you have a better > formulation, please do tell. The four guiding principles. > >> >>>>> I still find this wording very confusing. Perhaps "To add new >>>>> functionality, a patch is almost always the best choice. For >>>>> one, >>>>> it is likely that the new functionality requires changing >>>>> multiple >>>>> lines of source code, which is more convenient to do with a >>>>> patch >>>>> than with a snippet. Further, patches can be taken from and >>>>> submitted to upstreams more easily. If your patch has not been >>>>> submitted to upstream, consider doing so." >>>> It loses some information (that patches are preferred) and >>>> (after re-adding the conclusion) is more verbose, which appears >>>> to be considered very important. >>> Which conclusion is there to re-add? >> >> "patches are preferred" >> >> The conclusion is already stated >>> in the beginning: patches are almost always the best choice. Then >>> two >>> reasons as for why. The part w.r.t. upstreaming changes has also >>> been >>> addressed. >> >> While I consider that policies should have "best choices" coinciding >> with "preferred" and that not doing so would be illogical, people, >> projects, decisions ... are far from always logical. >> >> Because of this, people cannot assume that the 'best choices' are >> 'preferred', so it needs to be mentioned explicitly that these 'best >> choices' are actually 'preferred'. > In that case, simply write preferred choice? It is already mentioned that they are preferred: > ‘As such, patches are preferred’ By removing the information that 'patches are the most convenient choice' (by replacing it with ‘patches are the preferred choice’), the logical structure becomes weaker; a part of the explanation on the ‘why’ becomes missing. > >>> I'm using enumeration as a super term here, which can be >>> ((sub)sub)sections, chapters, list elements, whatever, and my claim >>> is that we barely have enough principles to allow the use of a >>> plural. >> >> In English, things are either plural or singular. Everything >= 2 is >> plural. There number of principles, however we count them, is, at >> least, 2. Consequently, the principles are plural, not singular. >> Treating the principles as singular is simply grammatically >> incorrect. > Correction: Everything that isn't exactly 1 is plural in English, > including 0, with perhaps the exception of -1 also using singular. > >> Maybe it is barely allowed to be plural, but English grammar doesn't >> care about that -- it is definitively disallowed to be singular, only >> plural remains. > Note that my argument isn't about English grammar, it uses English > grammar. Your argument is that ‘there aren't a sufficient amount of principles to allow the use of a plural’. 'Plural' is a concept of English grammar. As such, you have written an argument about English grammar. You appear to have meant some different argument, but all the arguments on ‘are they guiding principles or not’ I've read are based on the number of principles or on what 'guiding principle' means, which are issues of grammar and vocabulary respectively. I'm still not seeing how they aren't guiding principles. Maybe you have a different meaning of ‘guiding principles’ in mind? Or maybe you would like to name them 'guidelines' or 'general recommendations' instead. >>> Adding to that, now that I think of it, I also doubt their >>> usefulness in guiding. "Use whatever feels convenient, but note >>> that that might be subjective" is more useful at the end of the >>> section when a user didn't find what they were looking for than at >>> the start. >> >> The introduction has a set of guiding principles, from with concrete >> cases are built. By adding another principle at the end, the cases >> cannot make use of the "use [...] convenient" principle. >> >> Additionally, now the user has to look at _two_ places to find the >> guiding principles -- at the beginning, and at the end. Worse, >> the beginning does not have a cross-reference to the end, so since >> the set at the beginning is presented as exhaustive, the user might >> not know there is another guiding principle. >> >> And even if they did read through the whole section (even though they >> should only have to read the introduction and the relevant worked-out >> case), assuming they read through it in a linear fashion, due to the >> new guiding principle that wasn't alluded to at the beginning, they >> have to redo their mental model of "Modifying Sources" as this >> principle could invalidate things. > This shows both a problem with your structure, I'm not seeing the problem. How does this show a problem? > but more importantly, a > failure to understand what I meant when I wrote "use whatever is > convenient" in my own patch. This principle *can* only work for cases > in which there is *no clearly established practice*, thus putting it at > the end *after having enumerated all practices* is the logical choice. > With your structure, you have to bend it sideways to shoehorn it in > with the other principles. You appear to have meant it as a kind of ‘fallback principle’, in which case putting it at the end does indeed make sense. In my patch, the principle works differently, it is also used for explaining already established practices, for the cases where there are multiple technically allowed principles but still often a single _recommended_ principle. E.g.: > 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 ...] and > To add new functionality, a patch is almost always the most convenient > choice of the three . To go from the principle as used in your patch to the principle as used in my patch, perhaps in the process it is bent sideways and shoehorned. But after all the bending and shoehorning, it seems to fit well now in the beginning (and not well at all the end), so I'm not seeing any problems here. > >>> At the risk of responding jokingly to what was meant serious: I >>> didn't know we suddenly gained 20 guiding principles. >> >> 25 lines are for the guiding principles (sometimes referring to a >> principle of elsewhere in Guix, sometimes a new principle for >> "Modifying Sources"). You proposed to 'cache' them somehow. In >> Guile, to cache something, you can use 'delay/force'. But this >> increases the amount of code (due to the additional use of 'delay'), >> instead of decreasing. >> >> The documentation equivalent (whatever that might be, I am not seeing >> one myself) would then also be at least as long, maybe even a little >> longer due to the use of 'delay'. > Okay, so I did misunderstand those 25 lines as 25 lines within the text > calling back to those guiding principles rather than 25 lines for the > guiding principles themselves. Still, 25 lines are a little much, > especially given that the bulk of it explains the semantics of the > packages' source field. I think we'll have to agree to disagree on that. > >>>> I suppose table items might take two less line or so less than >>>> subsubsections, but I don't think that's sufficient reason to >>>> step away from a nice section structure. >>> Another reason is that you can end a table in the middle of a >>> section, which you can't do with subsections. Hence why I'm able >>> to put a remark at the bottom, which you have to clumsily fit into >>> the top. >> >> I can, indeed, not put the 'convenience principle' at the bottom, >> except perhaps by adding a new subsubsection, and for tables adding >> such a remark at the bottom is indeed more convenient. >> >> However, I don't see the 'have to clumsily' here -- as explained >> elsewhere, I believe that the 'convenience principle' shouldn't be >> separated from the other principles and that it fits nicely next to >> the the principles --- there is no 'have to', because I choose for >> this, and I am not seeing any 'clumsiness' here. > I think we'd have to debate choice vs. force here which would be a > rather long aside, but to make my argument short, your choice of how to > structure this section (table vs subsections) directly influences your > "choice" where to place particular information Sure. > in a way that might > inhibit natural information flow. I'm not seeing it. As I've explained previously, putting the premises (= guiding principles) before the conclusions (= worked out cases / solutions) is a logical (hence, natural) information flow, and introducing them on-the-way or implicitly is ad-hoc (unnatural). >>>> The patch does this, currently. It already proposes a number of >>>> hammers (patches, snippets and phases) and purposes (adding new >>>> functionality, fixing technical issues, unbundling, ...). >>>> Also, the scenario "oh no, however will I put this nail into a >>>> wall" >>>> actually happened -- see the Shepherd discussion, where there was >>>> a lot of disagreement on how nails (= small work-around in the >>>> Makefile) should be put in walls (= patches, snippet, phase?). >>>> It >>>> was the whole reason to start writing a documentation patch. >>> You might want to add a link here if it supports your argument, but >>> without looking at the discussion this rather sounds like "oh no, I >>> have three hammers, which one do I pick?" – which, fair enough, is >>> still a problem, but one that neither of our patches would cause >>> imho. >> >> As I think I've written previously, the whole point was to solve that >> problem. For the discussion, see: >> >> * https://issues.guix.gnu.org/54216 >> * >> https://yhetil.org/guix/c4c0a071-2e03-d4d6-6718-05424d21d146@telenet.be/ >> * >> https://yhetil.org/guix-devel/84e13ef7d437062df5cca51a12e6da54929e0176.camel@telenet.be/ >> >> Not solving the problem defeats the whole point, as the purpose is to >> solve that problem. > I think we might be disagreeing about what constitutes "solving the > problem" here. Correct me if I'm wrong, but to me it seems any > scenario that is not covered by whatever patch is applied counts as > "not having solved the problem". It doesn't -- I consider the Shepherd problem to be solved by my patch (albeit rather weakly, see later), and the wider problem of unclear guidelines and policy on snippet/phase/patch to be, perhaps not really solved, but still much improved (this time not weakly). > I personally find this line of > reasoning questionable, as there are perfectly valid reasons why > multiple tools could apply. See previous paragraph. > Take the problem of embedding a store path. You could for instance > replace all occurences of "sh" with /gnu/store/.../bin/sh, or you could > first replace them in a patch with "@sh@" and then replace that with > /gnu/store/.../bin/sh. Of course, you rarely have to do the latter and > the former is simpler, hence why you will often see the former, but > that doesn't mean the latter is invalid. It's contrary to 'It preferably should also work on non-Guix systems’ -- not invalid, but still usually (*) against (proposed) policy because a direct substitute* is at the same time more convenient and follows all the the principles. (*) sometimes, in case of shell scripts, a substitute* is fragile (substituting too much), in which case a "sh" -> "@sh@" becomes more convenient and hence the violation of the 'non-Guix system' rules becomes acceptable. >>>>> And if they don't find anything, they see the handy little line >>>>> at the bottom saying "use whatever you think is convenient". >>>> Nowhere did the patch imply that the listed cases were all cases. >>>> In fact, in two places in the introduction it is implied that the >>>> examples are not exhaustive, and that they can choose according >>>> to convenience [...] >>> Emphasis on handy little line rather than needing to be told twice >>> (particularly if people have no idea what to expect due to not >>> having looked at the worked-out cases yet). >> >> They don't need to be told twice. Also, my patch also has that handy >> little line (albeit differently worded), see the 'guiding >> principles'. >> >> Also, on the 'no idea what to expect' -- this is exactly the same for >> your patch too? In both patches, if the user only reads the >> introduction and conclusion (if any) and doesn't read the actual >> (relevant examples)/(explanation of patches, snippets, phases), then >> that's their problem. > I do think a table in the middle of the section is hard to ignore, I think so too, hence my previous response. > but suppose for the sake of argument that they do, [...] Given this response, I assume I've misinterpreted what you meant with ‘not having looked at the worked-out cases yet’. >>>> >>>>> I also expand a little on the benefits and drawbacks of >>>>> these approaches as you would when describing design patterns. >>>> This is also done in my patch. [...] >>> This is not about the contained information, but the structure of >>> the contained information. >>> >>> My solution->problem style follows roughly this style: >>> 1. solution >>> 2. problems it is known to solve >>> 3. how to use >>> 4. properties, caveats, etc. >>> >>> Your problem->solution style roughly has the following: >>> 1. problem >>> 2. (set of) solution(s) >>> 3. if more than one solution, maybe a help to select >> >> Also, in no particular order >> >> 4.: how to use >> 5.: explanation why it is preferred (properties, caveats?) > No particular order is problematic, I'm not seeing why. but more importantly if a > technology appears more than once (guaranteed by the pigeonhole > principle), then either its properties get repeated (not good for > length) or scattered (not good for the flow you want). Again, a > structural problem. That's equally applies to your patch too, though? (Switching "technology" with "problem".) Also, I've re-read my patch, and I didn't find any repeated properties or scattering, except for a single property (patches are upstreamable): > First, when the fix is not Guix-specific, upstreaming the fix is > strongly desired to avoid the additional maintenance cost to Guix. > [...] > As such, patches are preferred. However, as with bug > fixes, upstreaming the new functionality is desired. As such, I'm not following your point here. >>> This makes it so that people might have to go to a different >>> subsection than the one they read for their solution to find out >>> about potential caveats, e.g. not embedding store paths in a >>> snippet. >> >> I assume their problem was "X doesn't find Y". This being a >> technical issue, they go to "Fixing technical issues". In that >> subsubsection, there are the words: >> >>> 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. >> >> so they actually do know of the caveat 'don't embed store paths in a >> snippet, do it in a phase instead', and they did not need to go to a >> different subsubsection. > Typical happy path. Indeed, it's a happy path! >>>> >>>>> Your problem->solution approach instead leaves people wondering >>>>> when their particular use case has not been described. >>>> See my reply to ‘And if they don't find anything, they see the >>>> handy little line at the bottom saying "use whatever you think is >>>> convenient’. >>>>> It gives them a solution rather than the tools to build >>>>> solutions with. >>>> It does give the tools: snippets, patches and phases. >>> As far as I read, it describes none of those. It puts out guiding >>> principles and some already worked-out cases. >> Here are the descriptions: >> >>> 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 >> >> Once the user knows _which_ of the three they should use (by >> consulting the following subsubsections), they can then search for >> 'patch', 'snippet' and 'phases' in the manual for more information, >> no need to duplicate that information. > Point taken, but imho cross-references are nice for those who just > learned "okay, I now know I need to solve my technical problem with a > phase, how do I do that?" They are indeed. But I'm not following what you mean with "But" here. I didn't claim anything about cross-references there? Do you want me to add a few cross-references (for 'patch', 'snippet' and 'phases')? >>>> Also, "giving the tools to build solutions with" does not help >>>> with the problem that I aimed to solve -- there was disagreement >>>> on what the appropriate tools are (see: Shepherd), so it not just >>>> needs to give the tools, but also the solutions. >>> I don't see how my patch lacks this information however. >> >> IIUC, the raw information should usually be indeed all there, but the >> user has to consult _all_ of the sections to determine which option >> (patch, snippet, phase) is appropriate, having to assemble all the >> information is (a) a waste of time and (b) can lead to different >> interpretations and conclusions (see: Shepherd). >> >> More concretely, I cannot use your patch to decide between phases, >> snippets and patches for the Shepherd issue: >> >> * none of three appear to be forbidden by your documentation >> * there is no recommendation for 'patches' (IIRC it wasn't accepted >> upstream and there was no intent to submit it upstream, it being >> really a Guile bug, not a Shepherd bug) >> * there is no recommendation for snippets (it's all free, no >> bundling) >> * build phases are 'to be avoided' (but not forbidden), as it >> resulted >> in observably different runtime behaviour (being a bug fix) >> >> -- three (or at best, two, if build phases are discounted) options >> remain. Myself, I would then consider 'snippets' to be the most >> convenient, but some others (see: Shepherd, IIRC) would really want a >> patch instead, because 'patches can be reverted' or something like >> that. >> >> As such, you are giving the tools (snippets / patches / phases, some >> downsides and upsides), but different people can construct different >> solutions from those tools even in the same situation, leading to >> conflicts. >> >> If I use my patch instead, I go to "fixing technical issues". This >> section tells me to use a patch or a snippet. As the fix is not >> Guix-specific, it recommends a patch to save time when upstreaming. >> Conclusion: write a patch. It was a Guile bug, so the fix was a >> patch to Guile. But that would take time and upstream took the >> responsibility for a fix, so the 'efficient time thing' doesn't >> really apply and a small work-around (setting optimisation flags) >> suffices. >> >> In this situation, the subsubsection doesn't care at all if you go >> for a snippet, so apply the last guiding principle: go for the >> simplest thing: a snippet. (Unless you already have a patch, then a >> patch is simplest.) >> Does someone else have a different idea on what's simplest? Doesn't >> really matter, ‘Sometimes this is subjective, which is also fine’. > I do get the impression that this patch simply codifies what you feel > is right based on the shepherd situation True, except for the 'simply', as I also considered several other situations (Removing bundled libraries, Removing non-free software, Adding new functionality). I do not see the problem with this -- the proposed policy was considered workable even if some would like it to be a little bit different, and if others feel strongly about they could, I don't, maybe set up a vote system or such. > rather than thinking about the general case, See: several other cases that should cover most situations encountered in practice, and for the parts of the general case that aren't worked-out yet, there are the guiding principles. > which is why my patch makes weaker statements here. If > this issue could have been avoided with a #:make-flag, we would have > used that. > > More importantly, I fail to see the superiority of your patch here > IIUC neither patch offers a unique solution to the shepherd thing, so > the room for debate remains In my patch, multiple options remain, but there is no real room for debate. More concretely: > 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. it is considered subjective, having multiple options if fine! To correct a packaging following the other option (to help with following the policies), you don't have to debate on what's the proper option to get a fix in, as both are considered acceptable. In many cases, no need for debate, just pick one and move on. (Also the case for your patch IIUC.) Things are also phrased reconciliatory, e;g.: > To make things more concrete and to resolve conflicts between the > principles, a few cases have been worked out: However, in your patch, there is more opportunity for conflicts. E.g.: > Each one has its strengths and drawbacks, along with intended andhistorically derived use cases. By this line, arguments like 'X was historically the intended purpose of Y, using Y for Z is incorrect' become reasonable, which makes disagreements more difficult to resolve (as you know need to consult history and because history is fixed, so no improvements would be possible anymore in areas where there is a historically intended use case). > You could claim that my patch offers more > room, which fair enough, it does, but on the same token it allows > people to see that the guidelines have been followed and the patch can > be applied. > >>> In particular, for review purposes, mine should be easier to work >>> with. For instance, the reviewer sees a hash embedded in a >>> snippet, sees in the snippet entry that you shouldn't do that, and >>> can thus say "nope, don't do a snippet here". That is also the case for problem->solution? A "sh" -> #$(file-append bash-minimal "/bin/sh") substitution can be recognised on sight as not possibly being anything else than a technical fix, and then the subsubsection on technical fixes mentions that those must be done in phases. Or, another explanation: reviewers usually have seen a lot of more packages than a new packager. Because of that, and because they have familiarised theirselves with policy, they have over time automatically built a mental 'reverse index' of solutions->problems. >> I think we should optimise for packagers before reviewers > Code is more often read than written, so optimising for the reader is > optimizing for the packager as well. While all reviewers are readers, not all readers are reviewers, and reviewing usually happens only once, maybe twice per patch. >>> Regardless of which patch we pick, it should not mandate a >>> particular solution in potentially contentious cases, >> >> Actually the whole point was to mandate a particular solution for the >> contentious cases, see Shepherd. > But I disagree. > > Memes aside, please refrain from the "I'm right, do this" style (which > is another problem with the "problem-centric" approach), Is this referring to the style of the documentation patch? If so: it does explain why the "do this" is right, so I don't see the problem. Do you have a particular point of the documentation in mind you consider wrong (*)? (*) points I know of: * keep lists / remove lists (for the footnote about not-yet-policy on removing things in patches) * the guiding principles aren't guiding principles > but rather > focus on writing a patch that makes reviewers not discard a patch due > to formalities. I think the patch (v1 and v2) satisfies that. > If the shepherd thing had needed an upstream patch, > even with a snippet that patch could have easily been created by > converting that snippet to a sed, so more time was wasted debating than > overworking. I do not see your point here -- #57598 is about documentating/making policy, not 'should we spent time on correcting the shepherd package to match policy / convincing the other party that the status quo matches policy'. >>> and also point towards the right solution. See our discussions on >>> the individual solutions on points in which I believe you've >>> errored. >> >> These are: >> >> * the typo's >> * including "skipping tests indicating failure under ‘Fixing >> technical issues’" >> * "don't mention file names of non-free things in patches" >> >> Did I miss any? > I'm pretty sure there's also a discussion on store path embeddings at > the very least. Forgive me if I too missed something. There was a discussion on store file name embeddings. There was no mention of any errors in the documentation of store file names. IIRC, we agree on how store file name embeddings must be done. There were, however, proposals to extend the documentation to cover ‘how to embed’ in more detail (substitute* + search-input-file ...), but no errors. Greetings, Maxime.
Am Freitag, dem 09.09.2022 um 20:44 +0200 schrieb Maxime Devos: > GNU project encourages reading source code, we put useful > information in the source code (e.g. records on what non-free parts > haven't been replaced yet). You mean bundled parts not unbundled, right? IIUC, there should be no non-free parts in a free software distribution. > > You can, but it's still wiser to just keep the snippet short enough > > so you don't have to. > > That's also the case for phases, though? Conciseness is both good > for phases and snippets. Intentionally or otherwise, you're talking past me. > > > I would however very > > > > much prefer a wording that points people toward this practice > > > > existing, > > > > even if they have to go look in the code for examples. > > > > Alternatively, > > > > a footnote for the most common case (search-input-file inputs > > > > "bin/command") ought to suffice for the time being. > > > > > > Aside from the typo's and a few rephrasings, I'm done with the > > > documentation. If someone wants to extend the section with such > > > information, they can always do so later. > > I haven't seen a v2 though. Am I correct in assuming that none of > > the > > points we discussed in the last few mails are going to be > > addressed? > > I did sent a v2: <https://issues.guix.gnu.org/57598#8>. > And most were addressed, even if only as me not considering it an > issue. Hmm, that was while I was still composing that message. > > In that case, let me rephrase my criticism: "It passes the > > guidelines" should not be part of the logical reasoning here. > > Otherwise, why not have a guideline checklist at the end of each > > subsection (which would be silly, obviously, but that's the point). > > Because, as you note, that's a silly structure, putting the premises > (= guiding principles) after the conclusions (= worked-out cases) is > an illogical structure, whereas putting the premises _before_ the > conclusions is logical And yet you're putting a premise after a conclusion here. > > For the record, the exception is the "most convenient" bit you've > > copied from my patch :) > > That guideline actually predates your patch, it's part of the > original proposal (of which the wording changed later): > > > > https://yhetil.org/guix/c4c0a071-2e03-d4d6-6718-05424d21d146@telenet.be/ > > Sometimes, there remains more than one acceptable way to accomplish > > the goal. In that case, choose whatever appears to be most > > convenient. Fair enough. > > > Note "should" rather than "shall" or "must", meaning that slightly > > larger tarballs are still acceptable. > OK. The criterium remains overly tight though -- by that criterium, > slightly larger tarballs are considered undesirable, even when they > have useful (and hence, desired) things: Point taken, but... > > > > > * the source contains documentation that could be built and > > > installed, > > > but Guix doesn't do so yet. --> should be kept (unless non- > > > free) > > > * documentation that isn't meant to be built or installed > > > (e.g. HACKING, PACKAGERS, ...) --> useful, shouldn't be > > > removed. > > > * .gitignore, .github, ... --> nothing wrong with removing those, > > > but pointless, let's not waste our time with looking for > > > those > > > and removing them, even though doing so would make it > > > smaller. > > > * source files for platforms the upstream does not support > > > yet/anymore > > > (but with some volunteer effort (e.g. bugfixes), it might > > > become > > > a supported system again) > > > --> removing them (e.g. as part of an overly-broad > > > (delete-file-recursively > > > "this-dir-has-bundling-albeit-not-all-of-it-is-bundling")) > > > can be OK-ish, but removing them for 'minimality' is > > > pointless. > > > I see you're nitpicking for the sake of the argument, > > I'm not. It's nitpicking for the sake of the four * examples above. > > > but none of the > > examples you provide (safe for the bundling one) add much cost to > > either packing or unpacking. > > Not the point, the point is that having to treat those examples > seriously is a waste of time and even somewhat harmful (removing > useful stuff). > > > Thus, I'm pretty sure we can ignore them > > for practical purposes. > > As I've explained, this guideline is inconsistent with what is > actually recommended. Here are some practical purposes: > > * minimal dissonance between 'appreciated by policy' and 'appreciated > by > reviewers, packagers, users'. > * not having to review and accept pointless patches to Guix for > deleting > .github, .gitignore, uninstalled documentation that were written > for the sake of minimality > * not having to (in the sense of: it would be appreciated if you'd do > that) write such patches > > That being said, if you have a better > > formulation, please do tell. > > The four guiding principles. That's an awful lot to write rather than "the package's source should be the corresponding source minus the bits we don't like". Ludo’ already commented on the length of my patch (and yes, I'm aware that both of our patches blow up the section by a factor of like 3 at minimum), so I don't think you're doing anyone a service here by expanding something into four guidelines that would take, like, a simple sentence instead if carefully worded. Let's take the time to write the shorter letter, shall we? > By removing the information that 'patches are the most convenient > choice' (by replacing it with ‘patches are the preferred choice’), > the logical structure becomes weaker; a part of the explanation on > the ‘why’ becomes missing. I'm going to sound like a broken record, but conciseness is key: "Patches are preferred when adding functionality to a package. They can more easily be shared with upstreams and are easier to work with when making multi-line changes. Next item." > > > > [W]e barely have enough principles to allow the use of a > > > > plural. > Your argument is that ‘there aren't a sufficient amount of principles > to allow the use of a plural’. Don't put words into my mouth. > 'Plural' is a concept of English grammar. The concept of a plural is not unique to English grammar. > As such, you have written an argument about English grammar. You're failing to read between the lines and it's getting rather exhausting. > You appear to have meant some different argument, but all the > arguments on ‘are they guiding principles or not’ I've read are based > on the number of principles or on what 'guiding principle' means, > which are issues of grammar and vocabulary respectively. > > I'm still not seeing how they aren't guiding principles. Maybe you > have a different meaning of ‘guiding principles’ in mind? Or maybe > you would like to name them 'guidelines' or 'general recommendations' > instead. Call it whatever you want, my point is twofold: 1. By proper counting and simplification, you will have at most two. 2. At least one of those two doesn't feel very "guiding" to me. Further, I think you're doing a bad job of splitting mandatory and optional "recommendations" by lumping them together. > > > > > > The introduction has a set of guiding principles, from with > > > concrete cases are built. By adding another principle at the > > > end, the cases cannot make use of the "use [...] convenient" > > > principle. > > > > > > Additionally, now the user has to look at _two_ places to find > > > the guiding principles -- at the beginning, and at the end. > > > Worse, the beginning does not have a cross-reference to the end, > > > so since the set at the beginning is presented as exhaustive, the > > > user might not know there is another guiding principle. > > > > > > And even if they did read through the whole section (even though > > > they should only have to read the introduction and the relevant > > > worked-out case), assuming they read through it in a linear > > > fashion, due to the new guiding principle that wasn't alluded to > > > at the beginning, they have to redo their mental model of > > > "Modifying Sources" as this principle could invalidate things. > > This shows both a problem with your structure, > > I'm not seeing the problem. How does this show a problem? You're writing three paragraphs to lament how necessary the principle of "use whatever is convenient" – which mind you I believe should never be used outside of resolving conflicts between otherwise equally applicable choices – is to your reasoning. > > but more importantly, a failure to understand what I meant when I > > wrote "use whatever is convenient" in my own patch. This principle > > *can* only work for cases in which there is *no clearly established > > practice*, thus putting it at the end *after having enumerated all > > practices* is the logical choice. > > With your structure, you have to bend it sideways to shoehorn it in > > with the other principles. > > You appear to have meant it as a kind of ‘fallback principle’, in > which case putting it at the end does indeed make sense. > > In my patch, the principle works differently, it is also used for > explaining already established practices, for the cases where there > are multiple technically allowed [technique] but still often a single > _recommended_ [technique]. E.g.: > > > 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 ...] > > and > > > To add new functionality, a patch is almost always the most > > convenient choice of the three > > . > > To go from the principle as used in your patch to the principle as > used in my patch, perhaps in the process it is bent sideways and > shoehorned. > But after all the bending and shoehorning, it seems to fit well now > in the beginning (and not well at all the end), so I'm not seeing any > problems here. And here I must disagree with the nature of your patch, because elevating this into a guiding principle makes what feels most convenient to you (or any other writer at the time of editing the manual) most convenient to everyone. My patch allows for deliberation by both the packager and reviewer, which might lead to some conflicts of opinion, but unless either solution can be shown clearly superior by appeal to *another reason*, which mind you is lacking here, both solutions should be accepted. > > > in a way that might > > inhibit natural information flow. > > I'm not seeing it. As I've explained previously, putting the > premises (= guiding principles) before the conclusions (= worked out > cases / solutions) is a logical (hence, natural) information flow, > and introducing them on-the-way or implicitly is ad-hoc (unnatural). > > I think we might be disagreeing about what constitutes "solving the > > problem" here. Correct me if I'm wrong, but to me it seems any > > scenario that is not covered by whatever patch is applied counts as > > "not having solved the problem". > > It doesn't -- I consider the Shepherd problem to be solved by my > patch (albeit rather weakly, see later), and the wider problem of > unclear guidelines and policy on snippet/phase/patch to be, perhaps > not really solved, but still much improved (this time not weakly). > > > I personally find this line of reasoning questionable, as there are > > perfectly valid reasons why multiple tools could apply. > > See previous paragraph. > > Take the problem of embedding a store path. You could for instance > > replace all occurences of "sh" with /gnu/store/.../bin/sh, or you > > could first replace them in a patch with "@sh@" and then replace > > that with /gnu/store/.../bin/sh. Of course, you rarely have to do > > the latter and the former is simpler, hence why you will often see > > the former, but that doesn't mean the latter is invalid. > > It's contrary to 'It preferably should also work on non-Guix systems’ > -- > not invalid, but still usually (*) against (proposed) policy because > a direct substitute* is at the same time more convenient and follows > all the the principles. > > (*) sometimes, in case of shell scripts, a substitute* is fragile > (substituting too much), in which case a "sh" -> "@sh@" becomes more > convenient and hence the violation of the 'non-Guix system' rules > becomes acceptable. Fair enough, guess I'll have to update configure.ac in the same patch to honour the guiding principles. > > > > > > > > In both patches, if the user only reads the > > > introduction and conclusion (if any) and doesn't read the actual > > > (relevant examples)/(explanation of patches, snippets, phases), > > > then that's their problem. > > I do think a table in the middle of the section is hard to ignore, > > I think so too, hence my previous response. > > > but suppose for the sake of argument that they do, [...] > > Given this response, I assume I've misinterpreted what you meant with > ‘not having looked at the worked-out cases yet’. I think you understood it elsewhere, see the fallback guiding principle. > > No particular order is problematic, > > I'm not seeing why. Why have a structure, when you won't have a structure? Sounds a little counterintuitive, does it not? > but more importantly if a > > technology appears more than once (guaranteed by the pigeonhole > > principle), then either its properties get repeated (not good for > > length) or scattered (not good for the flow you want). Again, a > > structural problem. > > That's equally applies to your patch too, though? (Switching > "technology" with "problem".) Not really, since most problems that are to be uniquely solved with a given tool can be uniquely named under that tool. I do expand a little on using patches and phases in combination for a more complicated job, but that use case is notably missing from your patch. I also repeat shared characteristics of patches and snippets, which I could condense to save some space, but in comparison to your patch, space is no issue. > Also, I've re-read my patch, and I didn't find any repeated > properties or scattering, except for a single property (patches are > upstreamable): > > > First, when the fix is not Guix-specific, upstreaming the fix is > > strongly desired to avoid the additional maintenance cost to Guix. > > [...] > > As such, patches are preferred. However, as with bug > > fixes, upstreaming the new functionality is desired. > > As such, I'm not following your point here. The trend of you not seeing continues. Look at your first two subsubsections (both v1 and v2). > > > Point taken, but imho cross-references are nice for those who just > > learned "okay, I now know I need to solve my technical problem with > > a phase, how do I do that?" > > They are indeed. But I'm not following what you mean with "But" > here. I didn't claim anything about cross-references there? > > Do you want me to add a few cross-references (for 'patch', 'snippet' > and 'phases')? I don't think you have a useful location to put them though, owing once again to structure :) > > > I do get the impression that this patch simply codifies what you > > feel is right based on the shepherd situation > > True, except for the 'simply' Okay, I'll correct myself: This patch complicatedly codifies what you feel is right based on the shepherd situation. Jokes aside, > I also considered several other situations (Removing bundled > libraries, Removing non-free software, Adding new functionality). I > do not see the problem with this -- the proposed policy was > considered workable even if some would like it to be a little bit > different, and if others feel strongly about they could, I don't, > maybe set up a vote system or such. I think this is not the place for the kind of democracy where you put everything barely the half of people can agree is the worst tradeoff into "law". Even worse if we had an anonymous online vote, because those aren't susceptible to manipulation at all. I think we should rather exercise caution and document what if not the entire community then at least the reviewers can agree on. > > rather than thinking about the general case, > > See: several other cases that should cover most situations > encountered in practice, and for the parts of the general case that > aren't worked-out yet, there are the guiding principles. "guiding" principle"s" > > which is why my patch makes weaker statements here. If > > this issue could have been avoided with a #:make-flag, we would > > have used that. > > > > More importantly, I fail to see the superiority of your patch here > > > IIUC neither patch offers a unique solution to the shepherd > > thing, so > > the room for debate remains > > In my patch, multiple options remain, but there is no real room for > debate. More concretely: > > > 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. > > it is considered subjective, having multiple options if fine! To > correct a packaging following the other option (to help with > following the policies), you don't have to debate on what's the > proper option to get a fix in, as both are considered acceptable. In > many cases, no need for debate, just pick one and move on. (Also the > case for your patch IIUC.) I do think elevating this into a guiding principle somewhat undermines what you're stating here though. > Things are also phrased reconciliatory, e;g.: > > > To make things more concrete and to resolve conflicts between the > > principles, a few cases have been worked out: > > However, in your patch, there is more opportunity for conflicts. > E.g.: > > > Each one has its strengths and drawbacks, along with intended > > andhistorically derived use cases. > > By this line, arguments like 'X was historically the intended purpose > of Y, using Y for Z is incorrect' become reasonable, which makes > disagreements more difficult to resolve (as you know need to consult > history and because history is fixed, so no improvements would be > possible anymore in areas where there is a historically intended use > case). The intended and historically derived use cases are those that ought to be explicitly named in the following. If it's not documented, there is no historic precedent. > > > > In particular, for review purposes, mine should be easier to > > > > work with. For instance, the reviewer sees a hash embedded in > > > > a snippet, sees in the snippet entry that you shouldn't do > > > > that, and can thus say "nope, don't do a snippet here". > > That is also the case for problem->solution? A "sh" -> #$(file- > append bash-minimal "/bin/sh") substitution can be recognised on > sight as not possibly being anything else than a technical fix, and > then the subsubsection on technical fixes mentions that those must be > done in phases. > > Or, another explanation: reviewers usually have seen a lot of more > packages than a new packager. Because of that, and because they have > familiarised theirselves with policy, they have over time > automatically built a mental 'reverse index' of solutions->problems. I wouldn't put too much value in this experience. It can also blind you to the point of producing old-style inputs, for example. > > > > > I think we should optimise for packagers before reviewers > > Code is more often read than written, so optimising for the reader > > is optimizing for the packager as well. > > While all reviewers are readers, not all readers are reviewers, and > reviewing usually happens only once, maybe twice per patch. That's not a problem, though? You can take my guidelines, apply it to any random package in the Guix source and see whether things (still) fit. That's more troublesome with the problem->solution style. In other words, that the solution passes is apparent from the written form rather than requiring the process of writing it. > > > > Regardless of which patch we pick, it should not mandate a > > > > particular solution in potentially contentious cases, > > > > > > Actually the whole point was to mandate a particular solution for > > > the > > > contentious cases, see Shepherd. > > But I disagree. > > > > Memes aside, please refrain from the "I'm right, do this" style > > (which is another problem with the "problem-centric" approach), > > Is this referring to the style of the documentation patch? If so: it > does explain why the "do this" is right, so I don't see the problem. > Do you have a particular point of the documentation in mind you > consider wrong (*)? > > (*) points I know of: > * keep lists / remove lists (for the footnote about not-yet-policy on > removing things in patches) > * the guiding principles aren't guiding principles For one, you seriously erred on the test cases thing. I also wonder if we're drawing the wrong conclusions for Makefiles from the Shepherd incident. Historically, patching build files in a phase has been very fine. > > but rather focus on writing a patch that makes reviewers not > > discard a patch due to formalities. > > I think the patch (v1 and v2) satisfies that. I'm getting slightly different vibes, but sure. Cheers
>> +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. > ^ > “may use: patches, snippets, and build phases.” OK, for the next version of the patch I won't write. >> +Each one has its strengths and drawbacks. To decide between the three, > > s/decide between the three/choose among these/ Why? > Toggle quote (7 lines) >> +there are a few guiding principles to satisfy simultanuously where >> +possible: >> + >> +@itemize >> +@item >> +In principle, Guix only has free software; when the upstream source > > s/In principle, Guix only has free software/Every package in Guix is > free software/g This new sentence is most likely false. There have been packages in Guix that were non-free and have been non-free, quite likely there will be non-free packages in Guix in the future. I would rather have that Guix doesn't lie about itself. >> +community (i.e., patterns that appear throughout @code{gnu/packages/...}) > > Normally such parenthetical expressions go between em dashes: > > community---i.e., patterns that appear throughout > @file{gnu/packages}---and … Err, source? Normally parenthetical expressions go between parentheses. Also, normality is irrelevant. >> +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. > > You can’t have a colon between the section heading. Instead, you can > write “a few cases have been worked out and will be illustrated in the > following sections.” You definitely can have a colon between the section heading, I just did so. Also, that new sentence has a different meaning. > Section titles should be capitalized. The letter R is capitalized. I disagree that non-first words should be capitalized, but I suppose that's a style change that, if done, should be separate from this patch. > Leave a blank line after the section title. Why? > Instead of “will not work”, which is vague, I’d suggest more explicit > wording: “would not be appropriate because they would expose the > offending code.” To be more precise: s/would not be appropriate/would be inappropriate ‘expose code’ and ‘offending code’ seems rather vague to me. ‘will not work’ might be slightly vague, but it's clear from context (i.e., the words before the semicolon). Also, didn't you have complaints about denseness? >> +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. > > I’d drop the footnote, it’s already dense enough. I would rather keep the footnote -- there are no file size concerns for info files, it contains useful information, and it's a _footnote_. One of the main benefits of footnotes is that they are out of the way and you can just skip them if you don't want to read them? > s/snippets here:/snippets here./ Err, no? Directly after this sentence I'm saying which the benefits are, so this should be ':', not '.' -- ':' is strictly better here. > s/When using snippets/First, when using snippets/ The next sentences don't do ‘Second, ...’, ‘Third, ...’, so no. > Toggle quote (2 lines) >> +As such, snippets are recommended here. > > s/are recommended here/are the recommended way to delete non-free material/ No. Please see see the subsubsection title: > +@subsubsection Removing bundled libraries It's not merely about non-free material in in bundled libraries, it is about all bundled libraries. It is not about all non-free material, but only about material in bundled libraries (unbundled non-free material are treated in the previous subsubsection). As such, the proposed new sentence does not fit the subsubsection. > Toggle quote (2 lines) >> +@subsubsection Fixing technical issues (compilation errors, test failures, other bugs ...) > > In addition to capitalizing, please remove the parenthetical bit. No. ‘Technical issues’ is vague, the parenthetical bit clarifies things. Besides, in another comment for something else you wanted things to be less vague by adding additional text. > Could you send an updated version? No. The continued misunderstanding elsewhere in the thread and the inconsistency and occasional vagueness in arguments is tiresome. Best regards, Maxime Devos.
> > 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. (Note: this may need to be changed a bit, in the sense of "guix build --source=transitive,all" instead of "guix build --source".) > This also seems worded too strongly -- I don't think Guix is bound to > the concept of the "corresponding source", although it is a nice > property. As long as copyright law exists in something sufficiently close to its current form, as long as Guix contains GPL software and as long as Guix intends to be legal, Guix is bound to the concept of corresponding source. If Guix decides it isn't bound by the concept of corresponding source, there is some software in Guix I am a copyright holder of and the GPL has a section ‘8. Termination’. IIRC, last time I checked, "guix build --sources=transitive,all scheme-gnunet guile-fibers" still works, but if that functionality is removed without a replacement ... Best regards, Maxime Devos
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@*