| Message ID | e2047d5738d30969bc766ef85ea65715954a6927.1632855961.git.public@yoctocell.xyz |
|---|---|
| State | New |
| Headers | show |
| Series | [bug#50874] lint: Check if HTTPS version of HTTP URL exists. | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | success | View Laminar job |
| cbaines/issue | success | View issue |
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | success | View Laminar job |
| cbaines/issue | success | View issue |
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | success | View Laminar job |
| cbaines/issue | success | View issue |
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | success | View Laminar job |
| cbaines/issue | success | View issue |
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | success | View Laminar job |
| cbaines/issue | success | View issue |
Hi, Xinglu Chen <public@yoctocell.xyz> skribis: > * guix/lint.scm (check-if-https-uri-exists?): New procedure. > (check-home-page, check-source): Use it. Applied, thanks! > I don’t really know how to test this while making it future-proof, any > suggestions? I don’t know either, since we don’t have an easy way to spin up an HTTPS server. I think it’s okay to leave it as is, for lack of a better idea. However, this version of the patch leads to test failures in tests/lint.scm (“Connection refused”). Ludo’.
On Sat, Oct 02 2021, Ludovic Courtès wrote: > Hi, > > Xinglu Chen <public@yoctocell.xyz> skribis: > >> * guix/lint.scm (check-if-https-uri-exists?): New procedure. >> (check-home-page, check-source): Use it. > > Applied, thanks! Was it? I don’t see it in the log. >> I don’t really know how to test this while making it future-proof, any >> suggestions? > > I don’t know either, since we don’t have an easy way to spin up an HTTPS > server. I think it’s okay to leave it as is, for lack of a better idea. > > However, this version of the patch leads to test failures in > tests/lint.scm (“Connection refused”). Thanks for catching this; I will look into it. Good that you didn’t apply it then. :-)
On Sat, Oct 09 2021, Xinglu Chen wrote: > On Sat, Oct 02 2021, Ludovic Courtès wrote: > >> Hi, >> >> Xinglu Chen <public@yoctocell.xyz> skribis: >> >>> * guix/lint.scm (check-if-https-uri-exists?): New procedure. >>> (check-home-page, check-source): Use it. >> >> Applied, thanks! > > Was it? I don’t see it in the log. > >>> I don’t really know how to test this while making it future-proof, any >>> suggestions? >> >> I don’t know either, since we don’t have an easy way to spin up an HTTPS >> server. I think it’s okay to leave it as is, for lack of a better idea. >> >> However, this version of the patch leads to test failures in >> tests/lint.scm (“Connection refused”). > > Thanks for catching this; I will look into it. Good that you didn’t > apply it then. :-) Hm, I am not able to reproduce this. All the tests in ‘tests/lint.scm’ pass for me.
Hi! Xinglu Chen <public@yoctocell.xyz> skribis: > On Sat, Oct 02 2021, Ludovic Courtès wrote: > >> Hi, >> >> Xinglu Chen <public@yoctocell.xyz> skribis: >> >>> * guix/lint.scm (check-if-https-uri-exists?): New procedure. >>> (check-home-page, check-source): Use it. >> >> Applied, thanks! > > Was it? I don’t see it in the log. Actually no. :-) I initially applied it, started replying, noticed there was a problem, and then sent that inconsistent reply. >>> I don’t really know how to test this while making it future-proof, any >>> suggestions? >> >> I don’t know either, since we don’t have an easy way to spin up an HTTPS >> server. I think it’s okay to leave it as is, for lack of a better idea. >> >> However, this version of the patch leads to test failures in >> tests/lint.scm (“Connection refused”). > > Thanks for catching this; I will look into it. Good that you didn’t > apply it then. :-) Here’s what I see (among others): --8<---------------cut here---------------start------------->8--- test-name: home-page: 200 location: /home/ludo/src/guix/tests/lint.scm:650 source: + (test-equal + "home-page: 200" + '() + (with-http-server + `((200 ,%long-string)) + (let ((pkg (package + (inherit (dummy-package "x")) + (home-page (%local-url))))) + (check-home-page pkg)))) expected-value: () actual-value: #f actual-error: + (system-error + connect* + "~A" + ("Connection refused") + (111)) result: FAIL […] test-name: source: 200 location: /home/ludo/src/guix/tests/lint.scm:917 source: + (test-equal + "source: 200" + '() + (with-http-server + `((200 ,%long-string)) + (let ((pkg (package + (inherit (dummy-package "x")) + (source + (origin + (method url-fetch) + (uri (%local-url)) + (sha256 %null-sha256)))))) + (check-source pkg)))) expected-value: () actual-value: #f actual-error: + (system-error + connect* + "~A" + ("Connection refused") + (111)) result: FAIL --8<---------------cut here---------------end--------------->8--- I believe that’s because, in addition to (%local-url), ‘check-home-page’ & co. now try to connect on port 443 of the same host, and there’s nothing listening to that port on my machine. Ludo’.
diff --git a/guix/lint.scm b/guix/lint.scm index 527fda165a..246a5ab9c8 100644 --- a/guix/lint.scm +++ b/guix/lint.scm @@ -875,17 +875,44 @@ (define (validate-uri uri package field) (else (error "internal linter error" status))))) +(define (check-if-https-uri-exists? uri field package) + "Given a URI that uses HTTP, check whether a HTTPS version exists." + (guard (c ((http-get-error? c) + #f)) + (catch #t + (lambda () + (let* ((url (uri->string uri)) + (https-url (string-append + "https" (string-drop url (string-length "http")))) + (https-uri (string->uri https-url))) + (when (http-fetch/cached https-uri) + (make-warning package + (G_ "HTTPS version is available for: ~a") + (list url) + #:field field)))) + (match-lambda* + ((or ('gnutls-error _ ...) ('tls-certificate-error _ ...)) + #f) + (args + (apply throw args)))))) + (define (check-home-page package) "Emit a warning if PACKAGE has an invalid 'home-page' field, or if that 'home-page' is not reachable." - (let ((uri (and=> (package-home-page package) string->uri))) + (let* ((home-page (package-home-page package)) + (uri (and=> home-page string->uri))) (cond ((uri? uri) (match (validate-uri uri package 'home-page) ((and (? lint-warning? warning) warning) (list warning)) - (_ '()))) - ((not (package-home-page package)) + (_ (if (eq? (uri-scheme uri) 'http) + (match (check-if-https-uri-exists? uri 'home-page package) + ((? lint-warning? warning) + (list warning)) + (_ '())) + '())))) + ((not home-page) (if (or (string-contains (package-name package) "bootstrap") (string=? (package-name package) "ld-wrapper")) '() @@ -1079,8 +1106,12 @@ (define (warnings-for-uris uris) ((uri rest ...) (match (validate-uri uri package 'source) (#t - ;; We found a working URL, so stop right away. - '()) + (if (eq? (uri-scheme uri) 'http) + (match (check-if-https-uri-exists? uri 'source package) + ((? lint-warning? warning) + (list warning)) + (_ '())) + '())) (#f ;; Unsupported URL or other error, skip. (loop rest warnings))