unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
From: Attila Lendvai <attila@lendvai.name>
To: 54836@debbugs.gnu.org
Cc: Attila Lendvai <attila@lendvai.name>
Subject: [bug#54836] [PATCH v4 1/2] http-client: Factor out open-connection*, rename variables.
Date: Fri,  6 Jan 2023 19:46:54 +0100	[thread overview]
Message-ID: <20230106184654.28037-1-attila@lendvai.name> (raw)
In-Reply-To: <20220410133431.30058-1-attila@lendvai.name>

This is an idempotent refactor.

* guix/http-client.scm (http-fetch): Introduce open-connection*. Rename some
variables to turn programmer mistakes into compile time errors.
---

i'm (re)sending the two commits that are sitting in my local branch.

i freshly rebased them.

the conlusion was to not add the accept-all-response-codes? argument,
and accordingly i have added a duplicate of http-fetch into the go
importer. my fix was applied by Ludo in his own commit. this is why
the 3 commits got reduced to one.

i'm only proposing to push the first patch. what the second one does
was rejected by Ludo.

ratinale for the first patch: it renames variables in a way that is
less confusing for the programmer, and hence helps avoiding mistakes
that i have fixed in my first original commit. an uri and an uri*
variable in the same lexical scope is simply calling for mistakes...

 guix/http-client.scm | 66 ++++++++++++++++++++++----------------------
 1 file changed, 33 insertions(+), 33 deletions(-)

diff --git a/guix/http-client.scm b/guix/http-client.scm
index 9138a627ac..2d48a882e1 100644
--- a/guix/http-client.scm
+++ b/guix/http-client.scm
@@ -100,15 +100,17 @@ (define* (http-fetch uri #:key port (text? #f) (buffered? #t)
 Write information about redirects to LOG-PORT.
 
 Raise an '&http-get-error' condition if downloading fails."
-  (define uri*
+  (define parsed-initial-uri
     (if (string? uri) (string->uri uri) uri))
 
-  (let loop ((uri uri*)
-             (port (or port (open-connection uri*
-                                             #:verify-certificate?
-                                             verify-certificate?
-                                             #:timeout timeout))))
-    (let ((headers (match (uri-userinfo uri)
+  (define (open-connection* uri)
+    (open-connection uri
+                     #:verify-certificate? verify-certificate?
+                     #:timeout timeout))
+
+  (let loop ((current-uri parsed-initial-uri)
+             (current-port (or port (open-connection parsed-initial-uri))))
+    (let ((headers (match (uri-userinfo current-uri)
                      ((? string? str)
                       (cons (cons 'Authorization
                                   (string-append "Basic "
@@ -116,10 +118,10 @@ (define uri*
                                                   (string->utf8 str))))
                             headers))
                      (_ headers))))
-      (unless (or buffered? (not (file-port? port)))
-        (setvbuf port 'none))
+      (unless (or buffered? (not (file-port? current-port)))
+        (setvbuf current-port 'none))
       (let*-values (((resp data)
-                     (http-get uri #:streaming? #t #:port port
+                     (http-get current-uri #:streaming? #t #:port current-port
                                #:keep-alive? keep-alive?
                                #:headers headers))
                     ((code)
@@ -132,36 +134,34 @@ (define uri*
             303                                   ; see other
             307                                   ; temporary redirection
             308)                                  ; permanent redirection
-           (let ((host (uri-host uri))
-                 (uri  (resolve-uri-reference (response-location resp) uri)))
+           (let ((host (uri-host current-uri))
+                 (new-uri (resolve-uri-reference (response-location resp)
+                                                 current-uri)))
              (if keep-alive?
                  (dump-port data (%make-void-port "w0")
                             (response-content-length resp))
-                 (close-port port))
+                 (close-port current-port))
              (format log-port (G_ "following redirection to `~a'...~%")
-                     (uri->string uri))
-             (loop uri
+                     (uri->string new-uri))
+             (loop new-uri
                    (or (and keep-alive?
-                            (or (not (uri-host uri))
-                                (string=? host (uri-host uri)))
-                            port)
-                       (open-connection uri
-                                        #:verify-certificate?
-                                        verify-certificate?
-                                        #:timeout timeout)))))
+                            (or (not (uri-host new-uri))
+                                (string=? host (uri-host new-uri)))
+                            current-port)
+                       (open-connection* new-uri)))))
           (else
            (raise (condition (&http-get-error
-                              (uri uri)
-                              (code code)
-                              (reason (response-reason-phrase resp))
-                              (headers (response-headers resp)))
-                             (&message
-                              (message
-                               (format
-                                #f
-                                (G_ "~a: HTTP download failed: ~a (~s)")
-                                (uri->string uri) code
-                                (response-reason-phrase resp))))))))))))
+                                  (uri current-uri)
+                                  (code code)
+                                  (reason (response-reason-phrase resp))
+                                  (headers (response-headers resp)))
+                                 (&message
+                                  (message
+                                   (format
+                                    #f
+                                    (G_ "~a: HTTP download failed: ~a (~s)")
+                                    (uri->string current-uri) code
+                                    (response-reason-phrase resp))))))))))))
 
 (define-syntax-rule (false-if-networking-error exp)
   "Return #f if EXP triggers a network related exception as can occur when
-- 
2.35.1





  parent reply	other threads:[~2023-01-06 18:56 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-10 13:34 [bug#54836] [PATCH 1/2] http-client: Fix redirection Attila Lendvai
2022-04-10 13:35 ` [bug#54836] [PATCH 2/2] http-client: Factor out open-connection*, rename variables Attila Lendvai
2022-04-10 13:41 ` [bug#54836] [PATCH v2 1/3] http-client: Added accept-all-response-codes? argument Attila Lendvai
2022-04-10 13:41   ` [bug#54836] [PATCH 2/3] http-client: Fix redirection Attila Lendvai
2022-04-11 12:44     ` [bug#54836] [PATCH 1/2] " Ludovic Courtès
2022-04-10 13:41   ` [bug#54836] [PATCH 3/3] http-client: Factor out open-connection*, rename variables Attila Lendvai
2022-04-11 12:45   ` [bug#54836] [PATCH 1/2] http-client: Fix redirection Ludovic Courtès
2022-04-12  7:28     ` Attila Lendvai
2022-04-27 16:37       ` Attila Lendvai
2022-04-27 20:53       ` Ludovic Courtès
2023-01-03 22:29         ` Maxim Cournoyer
2022-04-28 10:22 ` [bug#54836] [PATCH v3] http-client: Factor out open-connection*, rename variables Attila Lendvai
2023-01-06 18:46 ` Attila Lendvai [this message]
2023-01-06 18:46   ` [bug#54836] [PATCH v4 2/2] http-client: Added accept-all-response-codes? argument Attila Lendvai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230106184654.28037-1-attila@lendvai.name \
    --to=attila@lendvai.name \
    --cc=54836@debbugs.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).