[pacman-dev] [PATCH] Add configuration options for libcurl's "low speed" timeout

Allan McRae allan at archlinux.org
Tue Mar 25 21:19:31 EDT 2014


On 26/03/14 09:01, Andrew Hills wrote:
> Hi all,
> 
> (Resending; looks like it didn't make it through the first time.)
> 
> Behind an obnoxious "transparent" corporate virus-scanning firewall, I
> find pacman frequently failing to download packages, yielding an
> "Operation too slow" error. Setting the transfer command to a more
> forgiving program like wget is suboptimal, as it's often unclear what
> failed and why: most mirrors are missing the signatures for the package
> databases, for example, littering the terminal with irrelevant errors.
> 
> This patch adds the "LowSpeedLimit" and "LowSpeedTime" configuration
> parameters to pacman.conf rather than hard-coding the defaults in
> libalpm. It adds two fields to the configuration structure and the ALPM
> handle structure to get the data all the way into curl_set_handle_opts().
> 
> Unfortunately, I had to include ALPM's handle.h in pacman's conf.h, else
> setting the handle's new fields fails (the type is incomplete at that
> point). I couldn't see a better way to do it. Comments requested.

Did this even compile?  The include should fail given libalpm is not in
the pacman source inlcude path.

Anyway, as with all the other options you will need to add a
alpm_option_set/get_...() function.  This gives all frontends access.

> Thanks,
> Andrew
> 
> From 85b5392be37fa6b8cc4392f6b308a8141255ab8f Mon Sep 17 00:00:00 2001
> From: Andrew Hills <ahills at ednos.net>
> Date: Wed, 19 Mar 2014 14:03:30 -0400
> Subject: [PATCH] Add LowSpeedLimit and LowSpeedTime configuration options to
>  correspond to libcurl's CURLOPT_LOW_SPEED_LIMIT and CURLOPT_LOW_SPEED_TIME
>  options.
> 
> Signed-off-by: Andrew Hills <ahills at ednos.net>
> ---
>  lib/libalpm/dload.c  |  4 ++--
>  lib/libalpm/handle.h |  4 ++++
>  src/pacman/conf.c    | 24 ++++++++++++++++++++++++
>  src/pacman/conf.h    |  5 +++++

No documentation updates to be seen.  pacman.conf man page and example
pacman.conf will need changed.

Small comments below.

@Dave: downloading is your thing.  Any other comments?

>  4 files changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
> index 53867f5..31edbf4 100644
> --- a/lib/libalpm/dload.c
> +++ b/lib/libalpm/dload.c
> @@ -294,8 +294,8 @@ static void curl_set_handle_opts(struct
> dload_payload *payload,
>  	curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1L);
>  	curl_easy_setopt(curl, CURLOPT_PROGRESSFUNCTION, dload_progress_cb);
>  	curl_easy_setopt(curl, CURLOPT_PROGRESSDATA, (void *)payload);
> -	curl_easy_setopt(curl, CURLOPT_LOW_SPEED_LIMIT, 1L);
> -	curl_easy_setopt(curl, CURLOPT_LOW_SPEED_TIME, 10L);
> +	curl_easy_setopt(curl, CURLOPT_LOW_SPEED_LIMIT, handle->lowspeedlimit);
> +	curl_easy_setopt(curl, CURLOPT_LOW_SPEED_TIME, handle->lowspeedtime);
>  	curl_easy_setopt(curl, CURLOPT_HEADERFUNCTION, dload_parseheader_cb);
>  	curl_easy_setopt(curl, CURLOPT_WRITEHEADER, (void *)payload);
>  	curl_easy_setopt(curl, CURLOPT_NETRC, CURL_NETRC_OPTIONAL);
> diff --git a/lib/libalpm/handle.h b/lib/libalpm/handle.h
> index 27241ea..25a51ff 100644
> --- a/lib/libalpm/handle.h
> +++ b/lib/libalpm/handle.h
> @@ -104,6 +104,10 @@ struct __alpm_handle_t {
>  	/* for delta parsing efficiency */
>  	int delta_regex_compiled;
>  	regex_t delta_regex;
> +
> +	/* Curl timeouts */
> +	long lowspeedlimit;
> +	long lowspeedtime;
>  };
> 
>  alpm_handle_t *_alpm_handle_new(void);
> diff --git a/src/pacman/conf.c b/src/pacman/conf.c
> index f75f3a7..3e5041f 100644
> --- a/src/pacman/conf.c
> +++ b/src/pacman/conf.c
> @@ -120,6 +120,9 @@ config_t *config_new(void)
>  	newconfig->colstr.err     = "";
>  	newconfig->colstr.nocolor = "";
> 
> +	newconfig->lowspeedlimit = 1L;
> +	newconfig->lowspeedtime = 10L;
> +
>  	return newconfig;
>  }
> 
> @@ -596,6 +599,24 @@ static int _parse_options(const char *key, char *value,
>  				return 1;
>  			}
>  			FREELIST(values);
> +    } else if(strcmp(key, "LowSpeedLimit") == 0) {

Indenting is always tabs in pacman.  This varies throughout your patch...

> +       config->lowspeedlimit = strtol(value, NULL, 0);
> +       if((config->lowspeedlimit == LONG_MAX || config->lowspeedlimit
> == LONG_MIN) && errno == ERANGE) {
> +                                pm_printf(ALPM_LOG_WARNING,
> +                                                _("config file %s, line
> %d: LowSpeedLimit overflow, using default.\n"),
> +                                                file, linenum);
> +                                config->lowspeedlimit = 1L;
> +                        }
> +                        pm_printf(ALPM_LOG_DEBUG, "config:
> lowspeedlimit: %lu\n", config->lowspeedlimit);

How does curl handle negative values for the limits?  Perhaps we should
ensure this is positive too.

(aside: do we check the range for the delta config value?)

> +     } else if(strcmp(key, "LowSpeedTime") == 0) {
> +       config->lowspeedtime = strtol(value, NULL, 0);
> +       if((config->lowspeedtime == LONG_MAX || config->lowspeedtime ==
> LONG_MIN) && errno == ERANGE) {
> +                                pm_printf(ALPM_LOG_WARNING,
> +                                                _("config file %s, line
> %d: LowSpeedTime overflow, using default.\n"),
> +                                                file, linenum);
> +                                config->lowspeedtime = 10L;
> +                        }
> +                        pm_printf(ALPM_LOG_DEBUG, "config:
> lowspeedtime: %lu\n", config->lowspeedtime);
>  		} else {
>  			pm_printf(ALPM_LOG_WARNING,
>  					_("config file %s, line %d: directive '%s' in section '%s' not
> recognized.\n"),
> @@ -690,6 +711,9 @@ static int setup_libalpm(void)
>  	alpm_option_set_questioncb(handle, cb_question);
>  	alpm_option_set_progresscb(handle, cb_progress);
> 
> +  handle->lowspeedlimit = config->lowspeedlimit;
> +  handle->lowspeedtime = config->lowspeedtime;
> +
>  	config->logfile = config->logfile ? config->logfile : strdup(LOGFILE);
>  	ret = alpm_option_set_logfile(handle, config->logfile);
>  	if(ret != 0) {
> diff --git a/src/pacman/conf.h b/src/pacman/conf.h
> index e8cac50..6ab8b52 100644
> --- a/src/pacman/conf.h
> +++ b/src/pacman/conf.h
> @@ -21,6 +21,7 @@
>  #define _PM_CONF_H
> 
>  #include <alpm.h>
> +#include <handle.h>
> 
>  typedef struct __colstr_t {
>  	const char *colon;
> @@ -113,6 +114,10 @@ typedef struct __config_t {
> 
>  	/* Color strings for output */
>  	colstr_t colstr;
> +
> +	/* Curl timeouts */
> +	long lowspeedlimit;
> +	long lowspeedtime;
>  } config_t;
> 
>  /* Operations */
> --
> 1.8.5.5




More information about the pacman-dev mailing list