Re: [PATCH] Add support for http_proxy and https_proxy environment variables

On 09/12/2012 07:52 PM, Josh Triplett wrote:
> The attached patch implements support for the http_proxy and https_proxy
> environment variables. This allows subversion to automatically make use
> of system-wide proxy configuration, rather than requiring
> subversion-specific proxy configuration; environment variables also make
> it easier to have network-specific configuration on machines that use
> more than one network. Subversion-specific configuration (in
> ~/.subversion/servers) will override the environment.
>
> Written after observing some ChromeOS build system documentation that
> showed how to configure proxy environment variables, and then listed
> subversion as the one special case requiring its own configuration.
>
> - Josh Triplett

Josh,

Thanks for the patch. This missing feature in Subversion has been observed
several times in the past[1]. I'm in favor of rectifying it -- where
longstanding and useful conventions exist, it just makes sense to honor them.

Some thoughts on your patch:

- Do the environment variables "http_proxy" and "https_proxy" mean anything
on non-*nix systems? If so, do they mean exactly the same thing as in the
*nix case? Do we need to avoid honoring those on, say, Windows platforms?

- Should the code ensure that the scheme of the proxy URL matches the scheme
of the Subversion repository URL?

- You return an error ("Invalid proxy URL '%s'") when the proxy URL fails to
parse cleanly. I'd suggested indicating in that error message that we got
the URL from the environment (so users know where to look in order to make
corrections). But stepping back a second: what is the convention employed
by other programs when the environment variable value is malformed? Do they
error, or simply ignore the variable? If we're going to track a convention
here, we might as well track it completely.

- While your approach was the super simple one to take, how do we feel about
the fact that this would cause all programs which linked against
Subversion's libs to start noticing those environment variables which are
really kind meant to be specific to just the command-line clients. In the
past, I (and others) have gone on record against such things. Perhaps a
better approach would be for 'svn' -- or one of the common svn_cmdline_*
functions in libsvn_subr -- to parse the env variables and use the results
to override the parsed runtime configuration values in the svn_config_t
structures that get passed all over the place. I dunno.

-- C-Mike

[1] http://subversion.tigris.org/issues/show_bug.cgi?id=1326,
    http://subversion.tigris.org/issues/show_bug.cgi?id=1327,
    http://subversion.tigris.org/issues/show_bug.cgi?id=2718,
    http://www.google.com/search?q=site%3asvn.haxx.se+http_proxy

-- 
C. Michael Pilato <cmpilato_at_collab.net>
CollabNet   <>   www.collab.net   <>   Enterprise Cloud Development