Re: [workflow] Automatically close bug report when a patch is committed
On 2023-09-14, Giovanni Biscuolo wrote: > Maxim Cournoyer writes: >> I like the 'Closes: ' trailer idea; it's simple. However, it'd need to >> be something added locally, either the user typing it out (unlikely for >> most contributors) or via some mumi wizardry (it's unlikely that all >> users will use mumi), which means its usage (and value) would depend on >> how motivated individuals are to learn these new tricks. > > I agree: the ratio, or better usecase, of my /trivial/ (in design, not > in implementation) initial proposal [1] was to try to help committers > closing bugs "in one go" by adding proper information to the commit > message, e.g. "Closes: #N" To be really, really clear, I am honestly pretty much fine with anything except: Closes:.*#N or Closes:.*N These are already widely used in Debian, and their use was well established before guix or even nix were even an inkling of an idea wriggling around in any human brains. Staying away from anything that uses any permutation of "close" would be most appreciated. :) (There may be some slightly more complicated variants; I think someone posted a link to the regexp used earlier in the thread) Since I push the entire relevent portions of upstream guix git history when pushing changes for guix packaging in Debian, it would be a significant bother for me if guix started using the same syntax, or similar enough that a trivial typo might lead to something acting on the wrong issue tracker... This is why I think it is important for projects to have some sort of namespacing for this sort of thing; I am not really opinionated on the exact details, other than it not conflicting with Debian's terribly generic entirely un-namespaced historical "Closes:" syntax. :) live well, vagrant signature.asc Description: PGP signature
Re: [workflow] Automatically close bug report when a patch is committed
On 2023-09-15, Liliana Marie Prikler wrote: > Am Donnerstag, dem 14.09.2023 um 15:51 -0700 schrieb Vagrant Cascadian: >> On 2023-09-10, Liliana Marie Prikler wrote: >> > Am Donnerstag, dem 07.09.2023 um 09:12 -0700 schrieb Vagrant >> > Cascadian: >> > > I am much more comfortable with the "Fixes" convention of: >> > > >> > > Fixes: https://issues.guix.gnu.org/NNN >> > I like the idea, but we should also consider the bugs.gnu.org >> > address >> > here as well as the convention of putting it into angular >> > brackets. In >> > fact, I might even prefer it if the convention was >> > Fixes: Bug description >> > where bug description is a (possibly empty) name for the bug such >> > as "Emacs hangs when I press a key" or something. >> > >> > >> > As for when to send it, remember that we already send a bunch of >> > mails to guix-comm...@gnu.org as our commit hook? I think it >> > shouldn't be too hard to search for the fixes line and send it to >> > debbugs control. >> >> Well, the complication gets to be ... which branch did it land in? in >> master, it's fairly obvious... you can just mark it as >> done/closed/etc. I guess with other branches it makes sense to mark >> it with the "pending" or maybe some more specific usertag >> "pending-in-BRANCH"? > I don't think such a distinction is needed in most cases. In fact, if > it's about regular bugs, then a graft should likely hit master in case > that an actual update is needed on another branch. Other than that, > it'd be silly to mark bugs specifically for e.g. "emacs-team" as still > pending on the account of them not having hit master yet. I guess I do not consider anything done until it lands in the master branch, but obviously if it is committed in some branch, it is nice to flag that somehow. "pending" seems appropriate up until it lands in master. Maybe marking by team or branch or whatnot is overkill, sure. Though it would allow you could see at a glance which branch to look in without diving into the whole history of the issue... Of course, I will not make terrible loud noises if folks decide otherwise. :) live well, vagrant signature.asc Description: PGP signature
Re: The already complicated (complex?) process for contributing.
Hi Giovanni, On Fri, 15 Sep 2023 at 16:37, Giovanni Biscuolo wrote: > maybe we are drifting... again? ;-) Yes we are. :-) > Is it /complex/ or /complicated/? Change-Id is not really complex – especially if the complexity is hidden behind some pre-commit hook – and it is not complex compared to setup git-send-email for example. IMHO, it is complicated because it adds yet another point of potential friction. Well, let focus the discussion on practical implementation details instead of some abstract considerations. It is only once all is implemented that this discussion about “complicated” enters, IMHO. On a side note, documentation is very fine but I do not read (or study!?) the documentation of my oven, instead I am just cooking stuff for fun. Other said, the first principle is the principle of the least astonishment (POLA). If a project needs really lengthy documentation for just contributing, either it is a more-than-thousand contributors project as the Linux kernel, either something is wrong. Maybe both. ;-) Cheers, simon
Re: Implementing the guix-dameon in Guile
My old university email address started demanding a phone number to "verify the security of my account", which was pretty funny considering it never had a phone number to begin with, so I'm locked out of that. Same GPG public key, though. Christopher Baines writes: > Hey! > > I think this has been talked about for a while [1], but I want to make it > happen. Currently the guix-daemon is still similar to the nix-daemon > that it was forked from, and is implemented in C++. I think that a Guile > implementation of the guix-daemon will simplify Guix and better support > hacking on and around the daemon to add new features and move Guix > forward. I'd like to help with this if at all possible. > Still though, I'd like to hear what people think about which direction > the implementation should go, and what features they'd like to see. Even > if those are not essential to make the Guile implementation viable, it > still might inform the direction to take. Okay, brain dump time: I think that using fibers has a lot of potential, but there are obstacles that need to be worked around. In the single-threaded case, we risk a big slowdown if multiple clients are active at once, since we're doing what used to be done by n processes with one single thread. It would be especially noticeable during big disk reads and writes, since those basically ignore O_NONBLOCK, and most procedures that act on entire files at once would therefore probably not hit many yield points. The worst situation would be where multiple worker fibers are attempting to do reference scanning at the same time. Posix asynchronous disk IO could be used, but glibc currently implements it... using a thread pool. There is the RWF_NOWAIT flag to preadv2, though it's only available on newer linuxes and has bugs in 5.9 and 5.10. Additionally, being single-threaded means use of F_SETLKW is a no-go, so you're stuck with polling there. Granted, that's not such a big issue if in 99% of use cases there is only one process doing the locking, so it can all be managed internally. Speaking of file locks, the risk of accidental clobbering of locks jumps way up once it's all moved in to one process, and IIRC we already have bugs with accidental clobbering of locks. You can get a half-decent interface by doing what sqlite does, which is a combination of intra-process locking and holding on to open file descriptors until all locks on the underlying file are released. There are some subtle pathological cases there that are a lot more likely in the guix daemon than in sqlite, though. For example, suppose you open a file twice to get ports p1 and p2, acquire read locks on both of them, then close p1, then open the file again to get p3, acquire a read lock on it, close p2, get p4, acquire a read lock on it, close p3, get p5... and so on. This will cause unbounded file descriptor usage, and eventually you'll run out. There is no workaround in this model other than "hope that usage pattern doesn't come up much". Additionally, you need to ensure that every close of a potentially-locked file goes through a special close-wrapper. I'm actually in the middle of working on a solution for this that involves a separate locker process that gets passed file descriptors to lock via a unix socket. Speaking of file descriptors, running the entire daemon in one process is going to mean much higher pressure on file descriptor resource usage. IIRC, while building a derivation, the closure of its inputs needs to be locked, and that means a file descriptor for each and every store item in its input closure, simultaneously. The separate locker process would make it possible to retain those locks while not having them open in the main process. Another issue that will need to be addressed, whether single-threaded or not, is the use of memoization caches in various places. These aren't weak hash tables, so they are both not-thread-safe and will retain strong references to both the cached results and the arguments used to obtain them for as long as the procedure it is based on remains. In a long-running server process, this is less than ideal. One approach could be to put a bound on how large they can grow, with some eviction policy for deciding what gets discarded first. If memoization is used to ensure pointer equality as a matter of correctness, though, that probably won't work well. The simplest solution would probably be to change them to use weak hash tables, though perhaps with an option available to bring back non-weak hash tables on the client side. In the multithreaded case, fork() and clone() become concerns, since they can no longer be safely run from guile. One way around this would be to use posix_spawn to produce a single-threaded guile process, then have that do the fork or clone as necessary. The fork case shouldn't actually be necessary, though, as the child process can just exec directly. In the clone case, CLONE_PARENT can be used to make the
Re: branch master updated: gnu: qemu: Reinstate the iothreads-commit-active test.
Christopher Baines writes: > [[PGP Signed Part:Good signature from 5E28A33B0B84F577 Christopher Baines > (trust ultimate) created at 2023-09-15T12:40:53+0100 using > RSA]] > > guix-comm...@gnu.org writes: > >> commit 076b3384dfa29ae118d0375d516376a7fe98a197 >> Author: Maxim Cournoyer >> AuthorDate: Tue Sep 12 09:29:22 2023 -0400 >> >> gnu: qemu: Reinstate the iothreads-commit-active test. >> >> * gnu/packages/virtualization.scm (qemu) [arguments]: Add set-SOCK_DIR >> phase. >> (qemu-minimal) [arguments]: Delete the disable-extra-tests phase. >> --- >> gnu/packages/virtualization.scm | 17 + >> 1 file changed, 9 insertions(+), 8 deletions(-) > > I cannot seem to get qemu-minimal to build for x86_64-linux with this > change [1]. > > 1: > https://data.guix.gnu.org/gnu/store/zn390q1p6alvklsymfjs7kk286dg0jmn-qemu-minimal-8.1.0.drv > > It could be that it just happened to build previously, but this is the > kind of issue that using QA should help to catch (and ironically, now I > can't update QA because qemu-minimal fails to build). 12th times the charm apparently. signature.asc Description: PGP signature
The already complicated (complex?) process for contributing.
Hi Simon, maybe we are drifting... again? ;-) Simon Tournier writes: [...] >> If this is stil not properly documented it will be fixed. > > Maybe… and it will be another item in the already very long list of > steps to complete before contributing. This is one of my concern: add > yet another thing to an already complicated process for contributing. [...] > Yes, yet another thing to an already complicated process for > contributing. [...] > Yes, yet another thing to an already complicated process for > contributing. [...] While I agree that if some-"thing" (or the lack of, OK?) is /complicating/ the contributing process, that "thing" should be addressed, I disagree that _adding_ the **requirement** for contributors to properly configure git to use git hooks provided by Guix and understand the purpose of and pay attention to the 'Change-Id' field is another "thing" that adds /complication/. Talking in general: if you mean that contributing to Guix is /complex/ I agree, but /complex/ does not imply /complication/; also, /complexity/ is common to every DCVS based project with significant dimensions that I know of. So yes, contributing /in general/ is a complex process and I guess we all would like it to be less complicated as possible; proposals in this thread are trying to go in this direction: adding a little help in «integrating a proposed change» with no complications (useless by design) for _all_ involved parties. Looking at other project development processes, take as an example **one** of the activities in the Linux kernel development process: «posting patches» [1]. You also need to know: - «Submitting patches: the essential guide to getting your code into the kernel» [2] - «Linux Kernel patch submission checklist» [3] - «Linux kernel coding style» [4] - «Email clients info for Linux» [5]... just to mention one of the cited MUAs, it states: «Gmail (Web GUI). Does not work for sending patches..». Probably Guix should copy/paste that. Is it /complex/ or /complicated/? To begin with, it's quite a lot of documentation, quite challenging to study /just/ to be able to send a useful patch to the Linux kernel... or /just/ to understand how and _why_ the process is designed that way. I hear you Someone™ reader: I cannot summarise, sorry! :-D ...anyway it's a very interesting reading, I'd suggest it. (I did not read all.) To have an overall picture of the /complexity/ of the whole development process of the Linux kernel, take a look at «the index» [6]. :-O Could it be simpified without making it /complicated/ for Someone™? ...maybe. Is Guix development process comparable to the Linux kernel one? ...who knows :-D Thanks! Gio' [1] https://docs.kernel.org/process/5.Posting.html [2] https://www.kernel.org/doc/html/latest/process/submitting-patches.html [3] https://www.kernel.org/doc/html/latest/process/submit-checklist.html [4] https://www.kernel.org/doc/html/latest/process/coding-style.html [5] https://docs.kernel.org/process/email-clients.html "Run away from it.": ROTFL! [6] https://docs.kernel.org/process/index.html -- Giovanni Biscuolo Xelera IT Infrastructures signature.asc Description: PGP signature
Re: branch master updated: gnu: qemu: Reinstate the iothreads-commit-active test.
Hi Chris, On Fri, Sep 15, 2023 at 4:41 AM Christopher Baines wrote: > > It could be that it just happened to build previously I recently tried to build the full qemu (8.1.0) but it failed six times before succeeding. Maybe your issue is intermittent also? Kind regards Felix
Re: branch master updated: gnu: qemu: Reinstate the iothreads-commit-active test.
guix-comm...@gnu.org writes: > commit 076b3384dfa29ae118d0375d516376a7fe98a197 > Author: Maxim Cournoyer > AuthorDate: Tue Sep 12 09:29:22 2023 -0400 > > gnu: qemu: Reinstate the iothreads-commit-active test. > > * gnu/packages/virtualization.scm (qemu) [arguments]: Add set-SOCK_DIR > phase. > (qemu-minimal) [arguments]: Delete the disable-extra-tests phase. > --- > gnu/packages/virtualization.scm | 17 + > 1 file changed, 9 insertions(+), 8 deletions(-) I cannot seem to get qemu-minimal to build for x86_64-linux with this change [1]. 1: https://data.guix.gnu.org/gnu/store/zn390q1p6alvklsymfjs7kk286dg0jmn-qemu-minimal-8.1.0.drv It could be that it just happened to build previously, but this is the kind of issue that using QA should help to catch (and ironically, now I can't update QA because qemu-minimal fails to build). signature.asc Description: PGP signature
Re: [workflow] Automatically close bug report when a patch is committed
Hi Giovanni, Before commenting, let me repeat. ;-) That’s said, I am not against the proposal. I just have mixed feelings and before deploying I strongly suggest to review if the proposal covers the intent. On Fri, 15 Sep 2023 at 09:16, Giovanni Biscuolo wrote: > Before commenting, just let me repeat that we are _copying_ the > 'Change-Id' idea (and related possible implementation issues) from > Gerrit: > > https://gerrit-review.googlesource.com/Documentation/user-changeid.html > > This means that Somewhere™ in our documentation we should start > explaining that: > > --8<---cut here---start->8--- > > Our code review system needs to identify commits that belong to the same > review. For instance, when a proposed patch needs to be modified to > address the comments of code reviewers, a second version of that patch > can be sent to guix-patc...@gnu.org. Our code review system allows > attaching those 2 commits to the same change, and relies upon a > Change-Id line at the bottom of a commit message to do so. With this > Change-Id, our code review system can automatically associate a new > version of a patch back to its original review, even across cherry-picks > and rebases. > > --8<---cut here---end--->8--- > > In other words, 'Change-Id' is /just/ metadata automatically added to > help in code review **tracking**, specificcally helping "across > cherry-picks and rebases" [1] > > Sorry if I'm repeating things probably already understood! All this does not address my concern. :-) >> 1. The hook must be installed. [...] > If this is stil not properly documented it will be fixed. Maybe… and it will be another item in the already very long list of steps to complete before contributing. This is one of my concern: add yet another thing to an already complicated process for contributing. >> 2. The hook must not be in conflict with user configuration. > > I guess you mean not in conflict with other locally installed git hooks: > since AFAIU we **already** have a locally installed git hook, this is > already a requirement and this is something the user (contributor) > should be aware of. AFAIK, we have a pre-push hook. This hook is only useful for the small set of people who push the code. And we can assume that this set of people is skilled and are able to invest if something is unexpected. Being Guix committer implies such commitment, IMHO. Here, we are speaking for a Git hook applied to all. That’s a different situation, IMHO. As an user and simple contributor, if I already have a global pre-commit hook and then when I want to contribute to Guix, I potentially hit some unexpected behaviour or even conflict, then as an user, either a. I give up, drop and move my interest to something else than Guix, either b. I spend some time for fixing this annoyance and that is pure friction (no value for me and no value for the project). > If this is stil not properly documented it will be fixed. Yes, yet another thing to an already complicated process for contributing. >> 3. The generated Change-Id string can be mangled by some user unexpected >>action. > > Contributors and committers should not delete or change and already > existing 'Change-Id', this will be documented. Yes, yet another thing to an already complicated process for contributing. >> Many things can rail out on user side. For an example, base-commit is >> almost appearing systematically in submitted patches almost 3 years >> later. > > I don't understand how this could impact the addition of the > patch-tracking metadata named 'Change-Id' > >> The patches of some submissions are badly formatted. Etc. > > I don't understand what is the problem of having a 'Change-Id' (in > commit messages) in badly formatted patch submissions. In these both example, I just pointed that we already have some potential frictions. And here, another one is added. What I know from my experience at work and from the small experience backed by my reading of help-guix or some bug reports is that: robustness is hard in an environment you do not control. More action we add in that uncontrolled environment and weaker the robustness becomes; and weak robustness means friction. Again, I am not saying the proposal is not worth – I am following the discussion with interest. :-) I am just trying to ask if this proposal will really fix an concrete annoyance compared to the relative complexity it adds, and if it is the right balance between the part and the counter-part. Cheers, simon
Re: Can we provide another UI for patches than just email ?
Hi Edouard, On Thu, 14 Sep 2023 at 20:57, Edouard Klein wrote: > Second, I don't know what the general feeling is here towards debbugs, but > if moving away from it is something that may happen, my suggestion is not > to use anything with Pull Requests, as Simon you seem to have > understood. People are discussing about hosting Sourcehut for managing the patches. But it is still under discussion and AFAIK nothing concrete has emerged. >What I had in mind was a > (semi-)public-writable repo: code is pushed by developers, pulled and > reviewed by maintainers, and if satisfactory, rebased on to master. Well, I do not think what you are describing is pragmatically doable. IMHO, the only option would be to use “git request-pull”. Similarly as you wrote in your first message, the contributor does: git remote add guix-patches WHATEVER #only once git push -u guix-patches master:some-unique-name Instead of clicking on a button some web-forge requesting a pull, then you send the kind of “pull request” by email. First, you prepare the cover-letter: git request-pull origin WHATEVER -p > cover-letter.txt If you want, you can edit this cover-letter and add more comments if needed. Last, you send it – it would be nice to be able to send what git-request-pull generates using git-send-email but I do not know how. And one Debbugs issue will be open containing the patches and where to pull. And that’s all. Hum, I do not know… Cheers, simon
Re: How can we decrease the cognitive overhead for contributors?
Hi, On Thu, 14 Sep 2023 at 23:19, Sarthak Shah wrote: > I believe that it should not be very hard to create a user interface (web, > ncurses or GUI) that searches for a package's location (the package record > has a field that accommodates this) and then displays it, which the user > can then edit. A part already exists: https://toys.whereis.みんな/ https://toys.whereis.みんな/?search=hello Then click to location leads to: https://git.savannah.gnu.org/cgit/guix.git/tree/gnu/packages/base.scm?id=master#n90 and note it also works for packages in some channels: https://gitlab.inria.fr/guix-hpc/guix-hpc/-/blob/master/inria/mpi.scm#L26 However, that’s not possible to edit. It would appears to me hard to implement. Because I do not see how it would be possible to implement some edition features without a login – some login management is always painful from my tiny experience. Therefore, if you think “it should not be very hard to create a user interface” that fulfills your description, then I am sure many of us will appreciate to run your implementation. :-p Cheers, simon
Re: [workflow] Automatically close bug report when a patch is committed
Hello Simon, thank you for havig listed possible troubles. Before commenting, just let me repeat that we are _copying_ the 'Change-Id' idea (and related possible implementation issues) from Gerrit: https://gerrit-review.googlesource.com/Documentation/user-changeid.html This means that Somewhere™ in our documentation we should start explaining that: --8<---cut here---start->8--- Our code review system needs to identify commits that belong to the same review. For instance, when a proposed patch needs to be modified to address the comments of code reviewers, a second version of that patch can be sent to guix-patc...@gnu.org. Our code review system allows attaching those 2 commits to the same change, and relies upon a Change-Id line at the bottom of a commit message to do so. With this Change-Id, our code review system can automatically associate a new version of a patch back to its original review, even across cherry-picks and rebases. --8<---cut here---end--->8--- In other words, 'Change-Id' is /just/ metadata automatically added to help in code review **tracking**, specificcally helping "across cherry-picks and rebases" [1] Sorry if I'm repeating things probably already understood! Simon Tournier writes: [...] >> Please can you expand what troubles do you see in automatically adding >> 'Change-Id:' using a hook-commit-msg like >> https://gerrit-review.googlesource.com/Documentation/cmd-hook-commit-msg.html >> ? > > 1. The hook must be installed. AFAIU a hook is already installed when configuring for contribution. If this is stil not properly documented it will be fixed. > 2. The hook must not be in conflict with user configuration. I guess you mean not in conflict with other locally installed git hooks: since AFAIU we **already** have a locally installed git hook, this is already a requirement and this is something the user (contributor) should be aware of. If this is stil not properly documented it will be fixed. > 3. The generated Change-Id string can be mangled by some user unexpected >action. Contributors and committers should not delete or change and already existing 'Change-Id', this will be documented. > Many things can rail out on user side. For an example, base-commit is > almost appearing systematically in submitted patches almost 3 years > later. I don't understand how this could impact the addition of the patch-tracking metadata named 'Change-Id' > The patches of some submissions are badly formatted. Etc. I don't understand what is the problem of having a 'Change-Id' (in commit messages) in badly formatted patch submissions. > Whatever the implementation, I am not convinced that the effort is worth > the benefits. OK, I'm sorry > And I am not convinced it will help in closing the submissions when > the patches have already been applied. OK, I'm sorry > That’s said, I am not against the proposal. I just have mixed feelings > and before deploying I strongly suggest to review if the proposal covers > the intent. OK, thank you for your suggestion. Happy hacking! Gio' [1] AFAIU 'Change-Id' can even track different versions of patches (that by design are from commits in the same branch, properly rebased as needed) sent by mistake via **different bug reports**, this also means that different bug reports containing the same 'Change-Id' are _surely_ linked togheter. -- Giovanni Biscuolo Xelera IT Infrastructures signature.asc Description: PGP signature
