-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Dominique Leuenberger wrote: > Hi everybody, > > Sorry, it took a bit longer than I expected, but some other things just came > in between. > > But finally, here it is: my patch which will add libproxy support to wget. > Of course it's all configurable using configure and the wget parameters > --no-proxy and proxyrc have precedence.
Sorry it's taken me so long to respond, as well. I've finally had an opportunity to review the patch; here are my thoughts. . It looks like it may be missing the appropriate configure.ac or Makefile.am directive to actually add libproxy for linking in wget? . It looks like it will leak memory for "proxies" on every invocation. Maybe you can copy proxies[0] to rewritten_storage, and then free up proxies? . The most important thing to me is maintaining backward-compatibility. I gather that libproxy supports the typical environment variables; but it's not clear to me whether they take precedence over proxies specified elsewhere (in GNOME configuration, for instance), which I need to be the case, or how compatible its env var support is with wget's, or if those are both satisfactory right now, whether they can be guaranteed to be in the future. I'd feel more comfortable letting getproxy's current code have a go first, and then if it turns up nothing, fall back on libproxy. . I'm trying to move away from #if and #ifdef within the body of functions. Not much has actually been done yet to effect this move, but it would be nice to maybe ensure that if HAVE_LIBPROXY isn't defined, it gets defined to 0, and then change the #ifdef to a normal if? . We'll want to advertise libproxy support in our output for --version; please add an appropriate entry to build_info.c.in: ENABLE_LIBPROXY libproxy . It'd be great if you could improve wget.texi by updating its description of how proxies work, when configured for use with libproxy. . It would be _really_ great if libproxy could report where it got its proxy from: not knowing makes debugging a PITA. Obviously, this has nothing directly to do with your patch for Wget, but please consider adding this to libproxy's API at some point. . I think we should probably have a --no-libproxy option, and (especially) a corresponding wgetrc option (see http://wget.addictivecode.org/OptionsHowto for how to do those), so that users have the option of using only Wget's historical behavior, if they want to. . One additional minor note: "available" is misspelled "availabel" in configure.ac :) Also, if I haven't mentioned it to you before: in order to accept your changes into Wget proper, you'll need to have papers signed with the Free Software Foundation (and possibly your employer, too). Would you be willing to assign copyright over your changes to the FSF? If so, do you want to assign copyright over just these specific changes, or do you want to assign over "past and future changes" (which would allow you to continue to contribute in the future, without having to go through the paper process again)? - -- Micah J. Cowan Programmer, musician, typesetting enthusiast, gamer. Maintainer of GNU Wget and GNU Teseq http://micah.cowan.name/ -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEARECAAYFAkrGNesACgkQ7M8hyUobTrF8IACfZpKeynnI/UiYtGUTsWzf4XCt afsAn3i0Deb0N5o0t++YYuXdWx266TpW =+2Ht -----END PGP SIGNATURE-----
