From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:470:142:3::10]:50519) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hoZ8Y-0004M6-Od for guix-patches@gnu.org; Fri, 19 Jul 2019 16:05:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hoZ8W-0003ab-IK for guix-patches@gnu.org; Fri, 19 Jul 2019 16:05:50 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:47742) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1hoZ8S-0003Yw-8c for guix-patches@gnu.org; Fri, 19 Jul 2019 16:05:48 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1hoYjZ-00040t-Kk for guix-patches@gnu.org; Fri, 19 Jul 2019 15:40:01 -0400 Subject: [bug#36555] [PATCH v3 0/3] Refactor out common behavior for system reconfiguration. Resent-Message-ID: Received: from eggs.gnu.org ([2001:470:142:3::10]:43162) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hoYj6-0008MM-6W for guix-patches@gnu.org; Fri, 19 Jul 2019 15:39:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hoYiz-0001B3-Q2 for guix-patches@gnu.org; Fri, 19 Jul 2019 15:39:28 -0400 References: <87imsci9sj.fsf@sdf.lonestar.org> <87ef30i9fl.fsf@sdf.lonestar.org> <87y3129qsn.fsf@gnu.org> <87sgr9bziq.fsf@sdf.lonestar.org> <87pnmc7nt1.fsf@gnu.org> <8736j7nwcb.fsf@sdf.lonestar.org> <87muhfjm14.fsf@gnu.org> <87ftn63l7d.fsf@sdf.lonestar.org> <87v9w1zgon.fsf_-_@sdf.lonestar.org> <87y30v3qke.fsf@sdf.lonestar.org> From: Christopher Lemmer Webber In-reply-to: <87y30v3qke.fsf@sdf.lonestar.org> Date: Fri, 19 Jul 2019 15:36:20 -0400 Message-ID: <87a7d924wb.fsf@dustycloud.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org Sender: "Guix-patches" To: 36555@debbugs.gnu.org Jakob L. Kreuze writes: > Hello to anyone reviewing this patch, > > I probably should've held off on sending this reroll out. After taking > some more time to experiment with possible solutions, I was able to > figure most of this out. Comments would still be appreciated, but the > points I specifically asked for comments on no longer need special > treatment. Also, if you haven't already started reviewing this, v4 will > likely hit the mailing list tomorrow; everything's there, it just needs > to be cleaned up. > > zerodaysfordays@sdf.lonestar.org (Jakob L. Kreuze) writes: > >> I still need to handle failed deployments in 'guix deploy'. I suspect >> that, for now, it would make sense to implement remote roll-backs and >> just roll-back the system on failure, at least until we've have some >> dialog about the proper way to do atomic deployments. > > Well, except for this. I'll submit a separate patch series addressing > this. I think that's fine to do in a separate series, and a good idea too. >> My biggest concern at the moment is error handling reporting in the >> new 'guix system reconfigure'. I'd like to emulate what was done with >> the previous version, but I'm at somewhat of a loss for how I'd go >> about that, since the error reporting was mixed with the >> reconfiguration code. So I'd like to ask for some suggestions: is the >> best way to catch errors in '%store-monad' to do what >> 'with-shepherd-error-handling' does, and then 'leave' on failure? >> >> Ludovic suggested guarding against 'message-condition' and having the >> expression I send to 'remote-eval' return either ('error message) or >> ('success). Would it make sense to just do this in all of the >> reconfiguration procedures? Or is raising exceptions in the >> reconfiguration procedures and catching them in the scripts' code the >> way to go? > > Comments, if anyone has them, would be appreciated, but I feel that I'm > in a good spot in terms of error handling now. Or even: ('error "error message here") (I suppose in case of success, a value would never be returned?) >> There's also a slight bug in the new 'guix system reconfigure' that >> I'll need to figure out. At the moment, it installs a bootloader entry >> for all but the newest generation. > > It wasn't actually a bug, I was misinterpreting the intended behavior of > 'guix system reconfigure'. :) Heh :) >> Oh, how na=C3=AFve I was four days ago. This reroll doesn't address this. >> Having the procedures "parameterized by an evaluation procedure" can >> be done in so many ways, and I think it would be best I put some >> serious thought into which of those ways would be the best. A >> 'local-eval' would clearly be much better than what I'm doing at the >> present in 'system.scm', but the solution I came up with today >> involved three layers of 'primitive-load', which I doubt is the way to >> go about it. I had the idea to parameterize on a procedure that takes >> a '' rather than a G-Expression as I was making dinner >> tonight, which seems to me like a sound idea, but we'll see if it >> works tomorrow when I try to implement it. > > Actually, a more generalized 'eval' (taking a G-Expression) was the > better way to go: it allowed me to simplify the interface to the > reconfiguration procedures even further. And, thanks to Ludovic's recent > patches with 'lower-gexp', I was able to collapse the Russian nesting > doll of 'primitive-load' calls. Yay! Generalization! >> Also, it hit me today that the safety checks done in 'guix system >> reconfigure' -- 'check-mapped-devices', >> 'check-file-system-availability', and 'check-initrd-modules' -- should >> also be done in 'guix deploy'. It might make sense for me to submit that >> change as a separate patch series so the code review for this doesn't >> get too complicated, but since we're on the topic of unifying the code >> between 'guix deploy' and 'guix system reconfigure', should I perhaps >> reimplement those procedures as '' objects like everything >> else in '(guix scripts system reconfigure)'? They aren't really >> effectful, but they concern system reconfiguration. > > Again, separate patch series. Yes, please do. My main worry is that such a patch series may be forgotten. Would it be inappropriate to make a "stub" patch issue for both of the followup patch series, since both seem important and we don't want to forget them? >> And, on the same note, should I go ahead and refactor the rest of the >> reconfiguration code in 'system.scm' out into '(guix scripts system >> reconfigure)'? I mean, this will probably be a separate patch series for >> the same reason that the safety checks would be a separate patch series, >> and I'll likely do this _after_ I come up with a decent way to >> parameterize on an evaluation procedure, but I'd like to know if it's a >> good idea or not before going ahead and ripping apart 'system.scm'. > > I'd still like comments on this, though. I guess see above. But I think we shouldn't wait, since I'd like to keep the energy up and get this merged in. - Chris