* [PATCH 1/2] svn-download: Respect current-http-proxy when downloading.
@ 2016-02-16 12:37 Jookia
2016-02-28 16:44 ` Ludovic Courtès
0 siblings, 1 reply; 9+ messages in thread
From: Jookia @ 2016-02-16 12:37 UTC (permalink / raw)
To: guix-devel
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 4680 bytes --]
When downloading a repository through SVN over HTTP, do it using a proxy if
possible. This is especially useful for people who use Tor to do all their
downloading. This doesn't work with svn:// repositories to my knowledge.
* guix/build/svn.scm (svn-fetch): Pass the "servers:global:http-proxy-host"
and "servers:global:http-proxy-port" configuration options to SVN if
current-http-proxy is set. Bail if unable to parse the proxy to avoid leaks.
* guix/svn-download.scm (svn-fetch): Leak the http_proxy environment variable.
---
guix/build/svn.scm | 48 +++++++++++++++++++++++++++++++++++-------------
guix/svn-download.scm | 2 ++
2 files changed, 37 insertions(+), 13 deletions(-)
diff --git a/guix/build/svn.scm b/guix/build/svn.scm
index 74fe084..2de5abc 100644
--- a/guix/build/svn.scm
+++ b/guix/build/svn.scm
@@ -1,6 +1,7 @@
;;; GNU Guix --- Functional package management for GNU
;;; Copyright © 2014 Ludovic Courtès <ludo@gnu.org>
;;; Copyright © 2014 Sree Harsha Totakura <sreeharsha@totakura.in>
+;;; Copyright © 2016 Jookia <166291@gmail.com>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -19,6 +20,10 @@
(define-module (guix build svn)
#:use-module (guix build utils)
+ #:use-module (ice-9 format)
+ #:use-module (srfi srfi-2)
+ #:use-module (web uri)
+ #:use-module (web client)
#:export (svn-fetch))
;;; Commentary:
@@ -32,18 +37,35 @@
#:key (svn-command "svn"))
"Fetch REVISION from URL into DIRECTORY. REVISION must be an integer, and a
valid Subversion revision. Return #t on success, #f otherwise."
- (and (zero? (system* svn-command "checkout" "--non-interactive"
- ;; Trust the server certificate. This is OK as we
- ;; verify the checksum later. This can be removed when
- ;; ca-certificates package is added.
- "--trust-server-cert" "-r" (number->string revision)
- url directory))
- (with-directory-excursion directory
- (begin
- ;; The contents of '.svn' vary as a function of the current status
- ;; of the repo. Since we want a fixed output, this directory needs
- ;; to be taken out.
- (delete-file-recursively ".svn")
- #t))))
+ (define proxy-config
+ (if (current-http-proxy)
+ (and-let* ((proxy-uri (string->uri (current-http-proxy)))
+ (proxy-host (uri-host proxy-uri))
+ (proxy-port (number->string (uri-port proxy-uri)))
+ (config-host "servers:global:http-proxy-host=")
+ (config-port "servers:global:http-proxy-port="))
+ `("--config-option" ,(string-append config-host proxy-host)
+ "--config-option" ,(string-append config-port proxy-port)))
+ '()))
+
+ (if proxy-config
+ (and (zero? (apply system* (append
+ `(,svn-command "checkout")
+ proxy-config
+ `("--non-interactive"
+ ;; Trust the server certificate. This is OK as we
+ ;; verify the checksum later. This can be removed when
+ ;; ca-certificates package is added.
+ "--trust-server-cert" "-r" ,(number->string revision)
+ ,url ,directory))))
+ (with-directory-excursion directory
+ (begin
+ ;; The contents of '.svn' vary as a function of the current status
+ ;; of the repo. Since we want a fixed output, this directory needs
+ ;; to be taken out.
+ (delete-file-recursively ".svn")
+ #t)))
+ (format (current-error-port)
+ "Unable to parse current-http-proxy: ~s~%" (current-http-proxy))))
;;; svn.scm ends here
diff --git a/guix/svn-download.scm b/guix/svn-download.scm
index d6853ca..fbc96df 100644
--- a/guix/svn-download.scm
+++ b/guix/svn-download.scm
@@ -1,6 +1,7 @@
;;; GNU Guix --- Functional package management for GNU
;;; Copyright © 2014, 2015 Ludovic Courtès <ludo@gnu.org>
;;; Copyright © 2014 Sree Harsha Totakura <sreeharsha@totakura.in>
+;;; Copyright © 2016 Jookia <166291@gmail.com>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -72,6 +73,7 @@ HASH-ALGO (a symbol). Use NAME as the file name, or a generic name if #f."
#:recursive? #t
#:modules '((guix build svn)
(guix build utils))
+ #:leaked-env-vars '("http_proxy")
#:guile-for-build guile
#:local-build? #t)))
--
2.7.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] svn-download: Respect current-http-proxy when downloading.
2016-02-16 12:37 [PATCH 1/2] svn-download: Respect current-http-proxy when downloading Jookia
@ 2016-02-28 16:44 ` Ludovic Courtès
2016-02-28 21:45 ` Jookia
2016-03-03 4:32 ` Jookia
0 siblings, 2 replies; 9+ messages in thread
From: Ludovic Courtès @ 2016-02-28 16:44 UTC (permalink / raw)
To: Jookia; +Cc: guix-devel
Jookia <166291@gmail.com> skribis:
> When downloading a repository through SVN over HTTP, do it using a proxy if
> possible. This is especially useful for people who use Tor to do all their
> downloading. This doesn't work with svn:// repositories to my knowledge.
>
> * guix/build/svn.scm (svn-fetch): Pass the "servers:global:http-proxy-host"
> and "servers:global:http-proxy-port" configuration options to SVN if
> current-http-proxy is set. Bail if unable to parse the proxy to avoid leaks.
> * guix/svn-download.scm (svn-fetch): Leak the http_proxy environment variable.
Sorry for the delay, and thanks for the useful patch!
I gather ‘svn’ doesn’t honor ‘http_proxy’ in the first place, right?
That would have simplified things.
Also, what about ‘https_proxy’? (This could be left for a subsequent
patch, if you prefer.)
[...]
> + (define proxy-config
> + (if (current-http-proxy)
> + (and-let* ((proxy-uri (string->uri (current-http-proxy)))
> + (proxy-host (uri-host proxy-uri))
> + (proxy-port (number->string (uri-port proxy-uri)))
> + (config-host "servers:global:http-proxy-host=")
> + (config-port "servers:global:http-proxy-port="))
> + `("--config-option" ,(string-append config-host proxy-host)
> + "--config-option" ,(string-append config-port proxy-port)))
> + '()))
I would suggest using an explicit (getenv "http_proxy") instead of
(current-http-proxy) since the latter is specific to the (web …)
modules, which are not involved here.
Also, I’m not a fan of SRFI-2. :-) So I’d write it as:
(match (getenv "http_proxy")
(#f '())
(uri-string
(let* ((uri (string->uri uri-string))
(host (uri-host uri))
(port (uri-port uri)))
(list (string-append
"--config-option=servers:global:http-proxy-host="
host)
(string-append
"--config-option=servers:global:http-proxy-port="
(number->string port))))))
Probably with something like:
(unless uri
(format (current-error-port) "invalid HTTP proxy URI: ~s~%"
uri-string)
(exit 1))
Could you send an updated patch?
Thanks!
Ludo’.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] svn-download: Respect current-http-proxy when downloading.
2016-02-28 16:44 ` Ludovic Courtès
@ 2016-02-28 21:45 ` Jookia
2020-03-15 11:22 ` levenson
2016-03-03 4:32 ` Jookia
1 sibling, 1 reply; 9+ messages in thread
From: Jookia @ 2016-02-28 21:45 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: guix-devel
On Sun, Feb 28, 2016 at 05:44:18PM +0100, Ludovic Courtès wrote:
> Sorry for the delay, and thanks for the useful patch!
No problem. :)
> I gather ‘svn’ doesn’t honor ‘http_proxy’ in the first place, right?
> That would have simplified things.
It really would, but for some reason that's not a feature. Well, it was for one
or two releases but that was unintentional and fixed.
> Also, what about ‘https_proxy’? (This could be left for a subsequent
> patch, if you prefer.)
I'd have to parse the subversion URL and figure out if its scheme is HTTPS as I
don't think Subverion has a https-proxy-* set of variables.
> > + (define proxy-config
> > + (if (current-http-proxy)
> > + (and-let* ((proxy-uri (string->uri (current-http-proxy)))
> > + (proxy-host (uri-host proxy-uri))
> > + (proxy-port (number->string (uri-port proxy-uri)))
> > + (config-host "servers:global:http-proxy-host=")
> > + (config-port "servers:global:http-proxy-port="))
> > + `("--config-option" ,(string-append config-host proxy-host)
> > + "--config-option" ,(string-append config-port proxy-port)))
> > + '()))
>
> I would suggest using an explicit (getenv "http_proxy") instead of
> (current-http-proxy) since the latter is specific to the (web …)
> modules, which are not involved here.
>
> Also, I’m not a fan of SRFI-2. :-) So I’d write it as:
>
> (match (getenv "http_proxy")
> (#f '())
> (uri-string
> (let* ((uri (string->uri uri-string))
> (host (uri-host uri))
> (port (uri-port uri)))
> (list (string-append
> "--config-option=servers:global:http-proxy-host="
> host)
> (string-append
> "--config-option=servers:global:http-proxy-port="
> (number->string port))))))
>
> Probably with something like:
>
> (unless uri
> (format (current-error-port) "invalid HTTP proxy URI: ~s~%"
> uri-string)
> (exit 1))
Interesting!
> Could you send an updated patch?
Sure, I'll do it hopefully this week.
> Thanks!
>
> Ludo’.
Cheers,
Jookia.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] svn-download: Respect current-http-proxy when downloading.
2016-02-28 16:44 ` Ludovic Courtès
2016-02-28 21:45 ` Jookia
@ 2016-03-03 4:32 ` Jookia
2016-03-03 16:39 ` Ludovic Courtès
1 sibling, 1 reply; 9+ messages in thread
From: Jookia @ 2016-03-03 4:32 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: guix-devel
On Sun, Feb 28, 2016 at 05:44:18PM +0100, Ludovic Courtès wrote:
> > + (define proxy-config
> > + (if (current-http-proxy)
> > + (and-let* ((proxy-uri (string->uri (current-http-proxy)))
> > + (proxy-host (uri-host proxy-uri))
> > + (proxy-port (number->string (uri-port proxy-uri)))
> > + (config-host "servers:global:http-proxy-host=")
> > + (config-port "servers:global:http-proxy-port="))
> > + `("--config-option" ,(string-append config-host proxy-host)
> > + "--config-option" ,(string-append config-port proxy-port)))
> > + '()))
>
> I would suggest using an explicit (getenv "http_proxy") instead of
> (current-http-proxy) since the latter is specific to the (web …)
> modules, which are not involved here.
Sorry for the reply, now that I have an RFC for the coreboot patches I've looked
back at this. The URI module is also in the web module, and I'm not sure how I
feel about rewriting a URI parser to get the host and port out of http_proxy.
> Thanks!
>
> Ludo’.
Thoughts?
Jookia.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] svn-download: Respect current-http-proxy when downloading.
2016-03-03 4:32 ` Jookia
@ 2016-03-03 16:39 ` Ludovic Courtès
2016-03-03 16:44 ` Jookia
0 siblings, 1 reply; 9+ messages in thread
From: Ludovic Courtès @ 2016-03-03 16:39 UTC (permalink / raw)
To: Jookia; +Cc: guix-devel
Jookia <166291@gmail.com> skribis:
> On Sun, Feb 28, 2016 at 05:44:18PM +0100, Ludovic Courtès wrote:
>> > + (define proxy-config
>> > + (if (current-http-proxy)
>> > + (and-let* ((proxy-uri (string->uri (current-http-proxy)))
>> > + (proxy-host (uri-host proxy-uri))
>> > + (proxy-port (number->string (uri-port proxy-uri)))
>> > + (config-host "servers:global:http-proxy-host=")
>> > + (config-port "servers:global:http-proxy-port="))
>> > + `("--config-option" ,(string-append config-host proxy-host)
>> > + "--config-option" ,(string-append config-port proxy-port)))
>> > + '()))
>>
>> I would suggest using an explicit (getenv "http_proxy") instead of
>> (current-http-proxy) since the latter is specific to the (web …)
>> modules, which are not involved here.
>
> Sorry for the reply, now that I have an RFC for the coreboot patches I've looked
> back at this. The URI module is also in the web module, and I'm not sure how I
> feel about rewriting a URI parser to get the host and port out of http_proxy.
Of course you should keep using ‘string->uri’, no problem with that.
I’m just saying that the code should explicitly check for the
‘http_proxy’ environment variable instead of using the
‘current-http-proxy’ SRFI-9 parameter.
WDYT?
Ludo’.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] svn-download: Respect current-http-proxy when downloading.
2016-03-03 16:39 ` Ludovic Courtès
@ 2016-03-03 16:44 ` Jookia
0 siblings, 0 replies; 9+ messages in thread
From: Jookia @ 2016-03-03 16:44 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: guix-devel
On Thu, Mar 03, 2016 at 05:39:21PM +0100, Ludovic Courtès wrote:
> Of course you should keep using ‘string->uri’, no problem with that.
>
> I’m just saying that the code should explicitly check for the
> ‘http_proxy’ environment variable instead of using the
> ‘current-http-proxy’ SRFI-9 parameter.
>
> WDYT?
I think I know what you mean, I'll do a patch up when I can. :)
> Ludo’.
Jookia.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] svn-download: Respect current-http-proxy when downloading.
2016-02-28 21:45 ` Jookia
@ 2020-03-15 11:22 ` levenson
2020-03-15 12:45 ` Jookia
2020-03-15 18:04 ` Leo Famulari
0 siblings, 2 replies; 9+ messages in thread
From: levenson @ 2020-03-15 11:22 UTC (permalink / raw)
To: Jookia; +Cc: guix-devel
Hi Jookia, Ludo
Was this patch merged? I haven't found it in guix-patches and commits =(. I came across the same problem today by just doing guix pull. In addition, a package (texlive-hyph-utf8) uses svn:// scheme which I am not sure does respect those http-proxy variables.
It would be also nice to have a http/https fallback in svn-download for such cases.
WDYT?
--
Alexey
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] svn-download: Respect current-http-proxy when downloading.
2020-03-15 11:22 ` levenson
@ 2020-03-15 12:45 ` Jookia
2020-03-15 18:04 ` Leo Famulari
1 sibling, 0 replies; 9+ messages in thread
From: Jookia @ 2020-03-15 12:45 UTC (permalink / raw)
To: levenson; +Cc: Jookia, guix-devel
Hi Alexey,
I don't contribute to Guix and abandoned this patch.
Please untag me,
Jookia.
On Sun, Mar 15, 2020 at 12:22:08PM +0100, levenson@mmer.org wrote:
> Hi Jookia, Ludo
>
> Was this patch merged? I haven't found it in guix-patches and commits =(. I came across the same problem today by just doing guix pull. In addition, a package (texlive-hyph-utf8) uses svn:// scheme which I am not sure does respect those http-proxy variables.
>
> It would be also nice to have a http/https fallback in svn-download for such cases.
>
> WDYT?
>
> --
> Alexey
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] svn-download: Respect current-http-proxy when downloading.
2020-03-15 11:22 ` levenson
2020-03-15 12:45 ` Jookia
@ 2020-03-15 18:04 ` Leo Famulari
1 sibling, 0 replies; 9+ messages in thread
From: Leo Famulari @ 2020-03-15 18:04 UTC (permalink / raw)
To: levenson; +Cc: Jookia, guix-devel
On Sun, Mar 15, 2020 at 12:22:08PM +0100, levenson@mmer.org wrote:
> Was this patch merged? I haven't found it in guix-patches and commits
> =(. I came across the same problem today by just doing guix pull. In
> addition, a package (texlive-hyph-utf8) uses svn:// scheme which I am
> not sure does respect those http-proxy variables.
>
> It would be also nice to have a http/https fallback in svn-download
> for such cases.
If you can't find it in the commit log, then I doubt the problem was
ever addressed, since SVN and HTTP proxy usage are both somewhat rare.
If you are having this issue, then we need someone to finish the patch,
addressing Ludo's feedback.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-03-15 18:04 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-16 12:37 [PATCH 1/2] svn-download: Respect current-http-proxy when downloading Jookia
2016-02-28 16:44 ` Ludovic Courtès
2016-02-28 21:45 ` Jookia
2020-03-15 11:22 ` levenson
2020-03-15 12:45 ` Jookia
2020-03-15 18:04 ` Leo Famulari
2016-03-03 4:32 ` Jookia
2016-03-03 16:39 ` Ludovic Courtès
2016-03-03 16:44 ` Jookia
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).