From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp0 ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms11 with LMTPS id uC2OGNPST2B7eAAA0tVLHw (envelope-from ) for ; Mon, 15 Mar 2021 21:34:11 +0000 Received: from aspmx1.migadu.com ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp0 with LMTPS id cO9eFNPST2CwRwAA1q6Kng (envelope-from ) for ; Mon, 15 Mar 2021 21:34:11 +0000 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id D722D1D60B for ; Mon, 15 Mar 2021 22:34:10 +0100 (CET) Received: from localhost ([::1]:50006 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lLuqn-0004XQ-Vr for larch@yhetil.org; Mon, 15 Mar 2021 17:34:10 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:37100) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lLuqg-0004Vx-EK for guix-patches@gnu.org; Mon, 15 Mar 2021 17:34:02 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:54092) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1lLuqg-0000Uk-6J for guix-patches@gnu.org; Mon, 15 Mar 2021 17:34:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1lLuqg-0001mz-2C for guix-patches@gnu.org; Mon, 15 Mar 2021 17:34:02 -0400 X-Loop: help-debbugs@gnu.org Subject: [bug#47160] [PATCH] scripts: substitute: Add back some error handling. Resent-From: Christopher Baines Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Mon, 15 Mar 2021 21:34:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 47160 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Ludovic =?UTF-8?Q?Court=C3=A8s?= Cc: 47160@debbugs.gnu.org Received: via spool by 47160-submit@debbugs.gnu.org id=B47160.16158440196844 (code B ref 47160); Mon, 15 Mar 2021 21:34:02 +0000 Received: (at 47160) by debbugs.gnu.org; 15 Mar 2021 21:33:39 +0000 Received: from localhost ([127.0.0.1]:37405 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lLuqJ-0001mJ-0n for submit@debbugs.gnu.org; Mon, 15 Mar 2021 17:33:39 -0400 Received: from mira.cbaines.net ([212.71.252.8]:37376) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lLuqG-0001mA-U0 for 47160@debbugs.gnu.org; Mon, 15 Mar 2021 17:33:37 -0400 Received: from localhost (unknown [IPv6:2a02:8010:68c1:0:8ac0:b4c7:f5c8:7caa]) by mira.cbaines.net (Postfix) with ESMTPSA id B873427BC52; Mon, 15 Mar 2021 21:33:35 +0000 (GMT) Received: from capella (localhost [127.0.0.1]) by localhost (OpenSMTPD) with ESMTP id 82238ee9; Mon, 15 Mar 2021 21:33:34 +0000 (UTC) References: <20210315151133.16282-1-mail@cbaines.net> <8735wwh29g.fsf@gnu.org> <874khcfl6j.fsf@cbaines.net> <87pn00b0p5.fsf_-_@gnu.org> User-agent: mu4e 1.4.15; emacs 27.1 From: Christopher Baines In-reply-to: <87pn00b0p5.fsf_-_@gnu.org> Date: Mon, 15 Mar 2021 21:33:31 +0000 Message-ID: <87k0q8drv8.fsf@cbaines.net> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: guix-patches@gnu.org List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+larch=yhetil.org@gnu.org Sender: "Guix-patches" X-Migadu-Flow: FLOW_IN ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1615844051; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:resent-cc:resent-from:resent-sender: resent-message-id:in-reply-to:in-reply-to:references:references: list-id:list-help:list-unsubscribe:list-subscribe:list-post; bh=5u3deUAtBnbSs4sc+XpZKxt+hSPaxvJKMXXXK3oUSW4=; b=SX+wmGUC6ApElr7FEcrzmXap4dasO0nNMkwI4Yf6MlrasoE4yGbbWYnFgkkRa1swDTNuDh Yq2oxZZ9fbvYcLUoFw6pKIyqmHTMLj6ARWW+FFMGBorCq5FWG1y4xwIEPKk4OZm9ikpn7I 0Jc+ys7MFx1i9n/OGVrr/UMYSETTnB9PADcae0cjf6utCxRk0tAsLuoXLPApM+CNo1IYyk wDxMOe+8QdiFBfq0XVqRbC8g8RcLk5EApqENK3tZFZqe778ifDkfgMFGaeiZYVLfxap7N7 Z871+bo3oH1g/N0v/XkPbRtnObddX/98R4dKRWJWV/kkBwZIpqYJ1H9/nMmxzw== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1615844051; a=rsa-sha256; cv=none; b=LNPofIvjBnoXfifzZcO9vtIEPs5JPAX1Ziyqul/6EsjWdDMOzYSMPlxe+wuUWTR59QVR/7 Ghx3gNIL4uWYaHAKLVGYxQEPoDzdCMiOPUtcIDmUNfpGafElf0ozJnpWC8PSPDq4XmDHXl vz3t85A9u4WvpQiZRLQJh6aJ8lCG4S82F32hLs/UMF+KsTwgG4L+Ug8fLmE3F/o3FeRBKP /bwdQ+VlRowbDumhyf29HMGXjdMDDxStdoXzPBQSfjwodqDbvC/inJDwQxvOZaJesE0WR+ QLUELDJbHXAXS6w72nJtPaa9vLTuIPrjQbb/dita98hPB6VWOCTIbO/DYU4mFA== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=none; dmarc=none; spf=pass (aspmx1.migadu.com: domain of guix-patches-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=guix-patches-bounces@gnu.org X-Migadu-Spam-Score: -4.50 Authentication-Results: aspmx1.migadu.com; dkim=none; dmarc=none; spf=pass (aspmx1.migadu.com: domain of guix-patches-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=guix-patches-bounces@gnu.org X-Migadu-Queue-Id: D722D1D60B X-Spam-Score: -4.50 X-Migadu-Scanner: scn0.migadu.com X-TUID: q/ljXW78TXhp --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Ludovic Court=C3=A8s writes: > Christopher Baines skribis: > >> Ludovic Court=C3=A8s writes: > > [...] > >>> Regarding , I would lean towards >>> perhaps reverting the connection/error-handling patch series and >>> starting anew from that =E2=80=9Cknown state=E2=80=9D. >>> >>> This area is unfortunately quite tedious to test and to get right so I= =E2=80=99d >>> err on the path of conservative, incremental changes. >>> >>> Thought? >> >> My preference is still to try and move forward and to make the error >> handling easier to see in the code. >> >> Particularly with this change, I think the problem was introduced in >> this commit [1], but I think it's hard to tell from the diff, since the >> error handling and retrying is within with-cached-connection. >> >> 1: https://git.savannah.gnu.org/cgit/guix.git/commit/?id=3Df50f5751fff4c= fc6d5abba9681054569694b7a5c >> >> That commit was one of the commits where I was making small incremental >> changes prior to actually getting to the changes I was looking at >> making, but a breakage was still introduced. >> >> What I was thinking about with this patch was how to make the error >> handling being added back here easier to see, and thus harder to >> break/remove. > > OK. > > Though I=E2=80=99m still unsure what the patch series starting at > 7b812f7c84c43455cdd68a0e51b6ded018afcc8e was about. What was the end > goal? So that was part of the creation of the (guix substitutes) module, unpicking the code in the script to separate out some of the connection caching was a prerequisite (discussion starts here https://issues.guix.gnu.org/45409#5 ). I think separating out that module is still a good thing. It's allowed for improvements in guix, the weather script doesn't now call in to the substitute script code for example. I'd also like the separation for things like the Guix Build Coordinator, which currently attempts to use the substitute code from Guix. > I also wonder if it introduced other issues. For > example, 7b812f7c84c43455cdd68a0e51b6ded018afcc8e replaced a reference > to =E2=80=98open-connection-for-uri/cached=E2=80=99 by one to > =E2=80=98open-connection-for-uri/maybe=E2=80=99. Are we still using cach= ed > connections? At least on that commit, open-connection-for-uri/maybe calls open-connection-for-uri/cached, so yes, still using cached connections. > Commit f50f5751fff4cfc6d5abba9681054569694b7a5c no longer passes the > #:port parameter to =E2=80=98http-fetch=E2=80=99. Yeah, that change is sort of fine if you're just looking at how the port/connection is handled, but that area is being fixed up here, and because closing the port is something that happens, it's better to also pass the port in. > Commit 20c08a8a45d0f137ead7c05e720456b2aea44402 does other things but at > first sight I=E2=80=99m not sure what the effect is. So, open-connection-for-uri/maybe is like open-connection-for-uri/cached, but it catches a couple of exceptions relating to not being able to connect to a substitute server, it also remembers about showing the messages. The second commit here is changing that slightly, to not apply to process-substitution, however I do think that code might have applied in the past (as open-connection-for-uri/maybe was used I believe). But I think you're right in saying there's probably some overlap between the error handling here and done by with-networking. > If you=E2=80=99re confident we can move forward to fix the bug, that=E2= =80=99s great > (though we=E2=80=99ll need a good deal of testing), but I=E2=80=99d still= like to > clarify these points later on. Well, the changes I'm suggesting here seem reasonable to me. As for testing, checking things basically work is easy enough, but I don't currently have many ideas for how to test for when fetching things doesn't go to plan (which can of course happen). --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQKlBAEBCgCPFiEEPonu50WOcg2XVOCyXiijOwuE9XcFAmBP0qtfFIAAAAAALgAo aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNF ODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcRHG1haWxAY2Jh aW5lcy5uZXQACgkQXiijOwuE9Xddlg//TR2Bk4rpGZXVPBe9lGJVksjCMUgQi4+k 8gnyLLQj5NxVTlHUe5amThLwqi3AES6VC8r3I18X8ya+zJRSflb6yLawteil3bu7 YQ8J1fOE20EPUK9+A1zSxnBRCA5Mtf6AqmbkxUNyABL08sbEYgJinbjP/TLQuvOt ZQAvEVND/Mdm7UCdSruNQX/G/h2epG6c+6MSAUIb1DTrpPJ7w1sydROTWZzS1lqs Pp6R+B04NndxLoidPfrbFHzM4VAuVgIjynzSUk/EvpdzTqwPLvlG99NMrFQjEmML 4nryWXLEBbNL9w5/lGRhgMTOupMm4Sd4A2c7MO5v41Wzp46y3juIZwGl5AuiJDQp 2YYdDk9ximEymj4RTrE6eFwdd4sJo5NU2DfH82DRQ11Ng7m0VVAG0QvcHzT1UtXi 3btZ+74QIOS1+8+CfvCbIXVC8OoeOh5c3QEBLz/fawPn9iUgi1QHhrxac3XSIZ1T cWPuff+1ykuw0+hSQFi8uBixX9PJiEDRDV650sXff6i/0ZkYw2/DQz+AnoMs4Z/7 xLHco+UFNUMrG0KSipXTmgmwLdGaCtXfOsy5faTWSMZZCD9VjGZqccK+QcnKgGNR s/DfnJL/MVSHFfdWeTArgw38dH2PGOrz/C345hhhACpZND7haw0dm3dW8oxE9OPT 7v99lkZfjhg= =NEX4 -----END PGP SIGNATURE----- --=-=-=--