Re: proxy support for wget retriever
Tollef Fog Heen wrote:
> And I did. And I added ftp support to choose-mirror as well. It
> should be pretty trivial to add support for ftp to the wget retriever
> - I'll look into that as well.
I hesitate to commit this because of one little problem: it unconditionally
builds in the list of ftp mirrors as well as the list of http mirrors.
This makes choose_mirror increase in size by some 5k. It would be much
better if it could be compiled with only http support, or only ftp
support, or with both. I think you're almost there, really.
> + debconf->command(debconf, "GET", DEBCONF_BASE "protocol", NULL);
> +
> + if (strcasecmp(debconf->value,"http") == 0) {
> + mirrors = mirrors_http;
> + } else if (strcasecmp(debconf->value,"ftp") == 0) {
> + mirrors = mirrors_ftp;
> + } else {
> + /* FIXME, don't use fprintf */
> + fprintf(stderr,"Unknown protocol: %s\n",debconf->value);
> + }
I suspect you could save some space by breaking out this repeated block
of code into a function.
> - debconf->command(debconf, "INPUT", "critical", DEBCONF_BASE "http/hostname", NULL);
> - debconf->command(debconf, "INPUT", "critical", DEBCONF_BASE "http/directory", NULL);
> + if (strcasecmp(protocol,"http") == 0) {
> + debconf->command(debconf, "INPUT", "critical", DEBCONF_BASE "http/hostname", NULL);
> +
> + debconf->command(debconf, "INPUT", "critical", DEBCONF_BASE "http/directory", NULL);
> + } else if (strcasecmp(protocol,"ftp") == 0) {
> + debconf->command(debconf, "INPUT", "critical", DEBCONF_BASE "ftp/hostname", NULL);
> + debconf->command(debconf, "INPUT", "critical", DEBCONF_BASE "ftp/directory", NULL);
> + }
These rather heavy-handed doubling of everything via conditionals could
surely be improved on.
> /* Always ask about a proxy. */
> + if (strcasecmp(protocol,"http") == 0) {
> +
> debconf->command(debconf, "INPUT", "high", DEBCONF_BASE "http/proxy", NULL);
> + } else if (strcasecmp(protocol,"ftp") == 0) {
> + debconf->command(debconf, "INPUT", "high", DEBCONF_BASE "http/proxy", NULL);
Surely the http/proxy in the line above is a typo?
--
see shy jo
Reply to: