{"thread":{"id":"29822","subject":"[PATCH] http.proxy: also mention https_proxy and all_proxy","startedAt":"2012-03-03T14:50:53Z","lastAt":"2012-03-05T05:11:17Z","messageCount":8,"participants":["Clemens Buchacher","Jeff King","Jakub Narebski","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"185993","messageId":"20120303145053.GA29948@ecki","threadId":"29822","inReplyTo":null,"subject":"[PATCH] http.proxy: also mention https_proxy and all_proxy","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2012-03-03T14:50:53Z","receivedAt":"2012-03-03T14:50:53Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"The current wording of the http.proxy documentation suggests that\nhttp_proxy is somehow equivalent to http.proxy. However, while\nhttp.proxy (by the means of curl's CURLOPT_PROXY option) overrides the\nproxy for both HTTP and HTTPS protocols, the http_proxy environment\nvariable is used only for HTTP. But since the docs mention only\nhttp_proxy, a user might expect it to apply to all HTTP-like protocols.\n\nAvoid any such misunderstanding by explicitly mentioning https_proxy and\nall_proxy as well.\n\nSigned-off-by: Clemens Buchacher \n---\n\nAs you can maybe guess from the commit message, I was bitten by this\nmyself. I also found a couple of questions online where users were\nexpecting the http_proxy environment variable to apply to HTTPS as well.\nHopefully, this will prevent some confusion in the future.\n\nClemens\n\n Documentation/config.txt | 7 ++++---\n 1 files changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex abeb82b..7d197bb 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1258,9 +1258,10 @@ help.autocorrect::\n \tThis is the default.\n \n http.proxy::\n-\tOverride the HTTP proxy, normally configured using the 'http_proxy'\n-\tenvironment variable (see linkgit:curl[1]). This can be overridden\n-\ton a per-remote basis; see remote..proxy\n+\tOverride the HTTP proxy, normally configured using the 'http_proxy',\n+\t'https_proxy', and 'all_proxy' environment variables (see\n+\tlinkgit:curl[1]). This can be overridden on a per-remote basis; see\n+\tremote..proxy\n \n http.cookiefile::\n \tFile containing previously stored cookie lines which should be used\n-- \n1.7.9.1\n"},{"id":"185998","messageId":"20120303174252.GC28602@sigill.intra.peff.net","threadId":"29822","inReplyTo":"20120303145053.GA29948@ecki","subject":"Re: [PATCH] http.proxy: also mention https_proxy and all_proxy","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-03-03T17:42:52Z","receivedAt":"2012-03-03T17:42:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Mar 03, 2012 at 03:50:53PM +0100, Clemens Buchacher wrote:\n\n> The current wording of the http.proxy documentation suggests that\n> http_proxy is somehow equivalent to http.proxy. However, while\n> http.proxy (by the means of curl's CURLOPT_PROXY option) overrides the\n> proxy for both HTTP and HTTPS protocols, the http_proxy environment\n> variable is used only for HTTP. But since the docs mention only\n> http_proxy, a user might expect it to apply to all HTTP-like protocols.\n\nHmm, I didn't know that. This certainly adds an interesting twist to the\npatch in a nearby thread to start reading http_proxy ourselves.\n\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index abeb82b..7d197bb 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -1258,9 +1258,10 @@ help.autocorrect::\n> \tThis is the default.\n> \n> http.proxy::\n> -\tOverride the HTTP proxy, normally configured using the 'http_proxy'\n> -\tenvironment variable (see linkgit:curl[1]). This can be overridden\n> -\ton a per-remote basis; see remote..proxy\n> +\tOverride the HTTP proxy, normally configured using the 'http_proxy',\n> +\t'https_proxy', and 'all_proxy' environment variables (see\n> +\tlinkgit:curl[1]). This can be overridden on a per-remote basis; see\n> +\tremote..proxy\n\nText looks OK. I think this linkgit:curl is wrong, though. In the\nmanpages, it formats as simply curl(1), but in the HTML pages, it\ncreates a link to curl.html, which does not exist. This is not a problem\nintroduced by your patch, obviously, but maybe it is worth cleaning up.\nI think just using:\n\n `curl(1)`\n\nmight be sufficient.\n\n-Peff\n"},{"id":"186007","messageId":"m31up9v8jo.fsf_-_@localhost.localdomain","threadId":"29822","inReplyTo":"20120303174252.GC28602@sigill.intra.peff.net","subject":"[RFH] 'man:' macro for asciidoc","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2012-03-03T18:33:36Z","receivedAt":"2012-03-03T18:33:36Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Jeff King writes:\n> On Sat, Mar 03, 2012 at 03:50:53PM +0100, Clemens Buchacher wrote:\n\n> > http.proxy::\n> > -\tOverride the HTTP proxy, normally configured using the 'http_proxy'\n> > -\tenvironment variable (see linkgit:curl[1]). This can be overridden\n> > -\ton a per-remote basis; see remote..proxy\n> > +\tOverride the HTTP proxy, normally configured using the 'http_proxy',\n> > +\t'https_proxy', and 'all_proxy' environment variables (see\n> > +\tlinkgit:curl[1]). This can be overridden on a per-remote basis; see\n> > +\tremote..proxy\n> \n> Text looks OK. I think this linkgit:curl is wrong, though. In the\n> manpages, it formats as simply curl(1), but in the HTML pages, it\n> creates a link to curl.html, which does not exist. This is not a problem\n> introduced by your patch, obviously, but maybe it is worth cleaning up.\n> I think just using:\n> \n> `curl(1)`\n> \n> might be sufficient.\n\nBTW it wouldreally be nice be able to use simply man:curl[1] instead\nof worrying if it shoudl be `curl(1)`, or *curl*(1), or curl(1)\n\n\n-- \nJakub Narebski\n"},{"id":"186018","messageId":"7v399p72a8.fsf@alter.siamese.dyndns.org","threadId":"29822","inReplyTo":"20120303174252.GC28602@sigill.intra.peff.net","subject":"Re: [PATCH] http.proxy: also mention https_proxy and all_proxy","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-03T22:22:39Z","receivedAt":"2012-03-03T22:22:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Sat, Mar 03, 2012 at 03:50:53PM +0100, Clemens Buchacher wrote:\n>\n>> The current wording of the http.proxy documentation suggests that\n>> http_proxy is somehow equivalent to http.proxy. However, while\n>> http.proxy (by the means of curl's CURLOPT_PROXY option) overrides the\n>> proxy for both HTTP and HTTPS protocols, the http_proxy environment\n>> variable is used only for HTTP. But since the docs mention only\n>> http_proxy, a user might expect it to apply to all HTTP-like protocols.\n>\n> Hmm, I didn't know that. This certainly adds an interesting twist to the\n> patch in a nearby thread to start reading http_proxy ourselves.\n\nYeah, I too had an \"Ahhhh\" moment.\n\nI recall that I was frustrated more than 10 years ago when I had to point\nat our company proxy with many *_proxy variables in my .login and ended up\nwriting a shell script (and placed it under source control) that generates\ndot files from a single source (which of course is under source control).\n\nThe manual page for curl seems to say that they have separate HTTPS_PROXY\nand friends all in caps, and http_proxy is left lowercase for historical\nreasons [*1*].\n\nAs to the way forward, I suspect that http.proxy was a mistake to begin\nwith, considering the structure of namespace our configuration variables\nfit in. Shouldn't they be proxy.http, proxy.https, etc.?\n\nAnd if we were to map curl's HTTPS_PROXY to proxy.https to fix this\nnamespace gotha, we could treat http.proxy as a historical mistake, and\nintroduce proxy.http as what matches curl's http_proxy.\n\nThen the change proposed in the nearby thread can notice http_proxy and\nbehave as if proxy.http is set, which would only affect http but not\nhttps. When http.proxy is set, we still let it affect anything curl\nhandles.\n\n\n[Footnote]\n\n*1* Come to think of it, the alleged Mac OS X breakage I mentined may\nmerely be a second-hand rumor of a misdiagnosed user error---the user\nmight have been trying to reach https:// resource with only http_proxy\nset.\n"},{"id":"186028","messageId":"20120304124627.GA1596@ecki","threadId":"29822","inReplyTo":"7v399p72a8.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] http.proxy: also mention https_proxy and all_proxy","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2012-03-04T12:46:28Z","receivedAt":"2012-03-04T12:46:28Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"On Sat, Mar 03, 2012 at 02:22:39PM -0800, Junio C Hamano wrote:\n> \n> As to the way forward, I suspect that http.proxy was a mistake to begin\n> with, considering the structure of namespace our configuration variables\n> fit in. Shouldn't they be proxy.http, proxy.https, etc.?\n\nI actually prefer the current behavior, which is to configure only one\nproxy for all protocols. I have not seen a setup where HTTP and HTTPS\nare routed through different proxies before. But if this is really\nneeded, one has the option to use the environment variable, or\nremote..proxy.\n\nI suggest instead that we map curl's CURLOPT_PROXY to core.proxy. That\nwould also fit well with the remote..proxy scheme.\n"},{"id":"186042","messageId":"20120304165043.GA4677@ecki","threadId":"29822","inReplyTo":"20120303174252.GC28602@sigill.intra.peff.net","subject":"[PATCH v2] http.proxy: also mention https_proxy and all_proxy","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2012-03-04T16:50:43Z","receivedAt":"2012-03-04T16:50:43Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"The current wording of the http.proxy documentation suggests that\nhttp_proxy is somehow equivalent to http.proxy. However, while\nhttp.proxy (by the means of curl's CURLOPT_PROXY option) overrides the\nproxy for both HTTP and HTTPS protocols, the http_proxy environment\nvariable is used only for HTTP. But since the docs mention only\nhttp_proxy, a user might expect it to apply to all HTTP-like protocols.\n\nAvoid any such misunderstanding by explicitly mentioning https_proxy and\nall_proxy as well.\n\nAlso replace linkgit:curl[1] with a literal 'curl(1)', because the\nformer gets translated to a dead link in the HTML pages.\n\nSigned-off-by: Clemens Buchacher \n---\n\nOn Sat, Mar 03, 2012 at 12:42:52PM -0500, Jeff King wrote:\n>\n> I think just using: `curl(1)` might be sufficient.\n\nDone.\n\n Documentation/config.txt | 7 ++++---\n 1 file changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex abeb82b..f0ab288 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1258,9 +1258,10 @@ help.autocorrect::\n \tThis is the default.\n \n http.proxy::\n-\tOverride the HTTP proxy, normally configured using the 'http_proxy'\n-\tenvironment variable (see linkgit:curl[1]). This can be overridden\n-\ton a per-remote basis; see remote..proxy\n+\tOverride the HTTP proxy, normally configured using the 'http_proxy',\n+\t'https_proxy', and 'all_proxy' environment variables (see\n+\t`curl(1)`). This can be overridden on a per-remote basis; see\n+\tremote..proxy\n \n http.cookiefile::\n \tFile containing previously stored cookie lines which should be used\n-- \n1.7.9.2\n"},{"id":"186054","messageId":"7vhay455is.fsf@alter.siamese.dyndns.org","threadId":"29822","inReplyTo":"20120304124627.GA1596@ecki","subject":"Re: [PATCH] http.proxy: also mention https_proxy and all_proxy","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-04T23:07:55Z","receivedAt":"2012-03-04T23:07:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher writes:\n\n> On Sat, Mar 03, 2012 at 02:22:39PM -0800, Junio C Hamano wrote:\n>> \n>> As to the way forward, I suspect that http.proxy was a mistake to begin\n>> with, considering the structure of namespace our configuration variables\n>> fit in. Shouldn't they be proxy.http, proxy.https, etc.?\n>\n> I actually prefer the current behavior, which is to configure only one\n> proxy for all protocols. I have not seen a setup where HTTP and HTTPS\n> are routed through different proxies before. But if this is really\n> needed, one has the option to use the environment variable, or\n> remote..proxy.\n>\n> I suggest instead that we map curl's CURLOPT_PROXY to core.proxy. That\n> would also fit well with the remote..proxy scheme.\n\nContaminating core.* namespace would not solve anything.\n\nPeople are already relying on http.proxy to apply to any cURL transport,\nso we will keep supporting it anyway. There is no justification to teach\nnew people to learn that both exists, and the transport is not core anyway; \nit is still cURL specific.\n\nI forgot about remote..proxy that already can let you use different\nproxies for http/https, and the use of different ones per protocol would\nbe rare to begin with, so I agree that we would not have to worry about\nintroducing proxy.{http,https,...}.\n\nThanks.\n"},{"id":"186081","messageId":"7vk42z639m.fsf@alter.siamese.dyndns.org","threadId":"29822","inReplyTo":"20120304165043.GA4677@ecki","subject":"Re: [PATCH v2] http.proxy: also mention https_proxy and all_proxy","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-05T05:11:17Z","receivedAt":"2012-03-05T05:11:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks; will queue.\n"}]}