bug#57978: [bug#58017] [PATCH 2/2] substitute: Retry downloading when a nar is unavailable.

[Maxime Devos]

On 24-09-2022 18:22, Ludovic Courtès wrote:

Hi Maxime,

Maxime Devos <maximede...@telenet.be> skribis:

+(test-equal "substitute, first URL has narinfo but nar is 404, both URLs 
authorized"
+  "Substitutable data."
+  (with-narinfo*
+      (string-append %narinfo "Signature: "
+                     (signature-field %narinfo))
+      %main-substitute-directory
+
+    (with-http-server `((200 ,(string-append %narinfo "Signature: "
+                                             (signature-field %narinfo)))
+                        (404 "Sorry, nar is missing!"))
+      (dynamic-wind
+        (const #t)
+        (lambda ()
+          (parameterize ((substitute-urls
+                          (list (%local-url)
+                                (string-append "file://"
+                                               %main-substitute-directory))))
+            (request-substitution (string-append (%store-prefix)
+                                                 
"/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-foo")
+                                  "substitute-retrieved"))
+          (call-with-input-file "substitute-retrieved" get-string-all))
+        (lambda ()
+          (false-if-exception (delete-file "substitute-retrieved")))))))

Shouldn't it only ignore 'file not found' (ENOENT?) exceptions?

By “it”, do you mean ‘dynamic-wind’ should be replaced by a ‘catch’
form?

 (false-if-exception (delete-file "substitute-retrieved"))

-- the catch can stay, AFAIK.

No, I'm not referring to the dynamic-wind as a whole, rather 'it' = the following code:

We could discuss it, but note that this patch just keeps with the style
of existing tests.

Greetings,
Maxime.

For the reasons given, I don't think this style should be continued, though I suppose all of them can be done at once in a separate patch.

OpenPGP_0x49E3EE22191725EE.asc
Description: OpenPGP public key

OpenPGP_signature
Description: OpenPGP digital signature