From: Christian Hesse mail@eworm.de
Add LowSpeedLimit and LowSpeedTime configuration options to correspond to libcurl's CURLOPT_LOW_SPEED_LIMIT and CURLOPT_LOW_SPEED_TIME options. This allows, e.g., transfers behind corporate virus-scanning firewalls to survive the delays. Increasing the timeout may not be desirable in all situations; similarly, disabling the check prevents detection of disappearing networks.
Adds a utility function, parse_positive_long(), which yields a positive (but not unsigned) long integer from a null-terminated string using strtol(). A string containing anything but a base-10 number will cause the function to return -1.
Original-Patch-by: Andrew Hills ahills@ednos.net Signed-off-by: Christian Hesse mail@eworm.de --- doc/pacman.conf.5.txt | 12 ++++++++++++ etc/pacman.conf.in | 4 ++++ lib/libalpm/alpm.h | 10 ++++++++++ lib/libalpm/dload.c | 4 ++-- lib/libalpm/handle.c | 32 ++++++++++++++++++++++++++++++++ lib/libalpm/handle.h | 9 +++++++++ src/pacman/conf.c | 24 ++++++++++++++++++++++++ src/pacman/conf.h | 4 ++++ src/pacman/util.c | 19 +++++++++++++++++++ src/pacman/util.h | 1 + 10 files changed, 117 insertions(+), 2 deletions(-)
diff --git a/doc/pacman.conf.5.txt b/doc/pacman.conf.5.txt index c665870..2346331 100644 --- a/doc/pacman.conf.5.txt +++ b/doc/pacman.conf.5.txt @@ -209,6 +209,18 @@ Options Displays name, version and size of target packages formatted as a table for upgrade, sync and remove operations.
+*LowSpeedLimit* = speed:: + Sets the speed in bytes per second that a download should be below during + `LowSpeedTime` seconds to abort the transfer for being too slow. Setting + 'speed' to 0 will disable the speed check. Defaults to 1 byte per second. + Note that this option will not affect external programs specified by + `XferCommand`. + +*LowSpeedTime* = time:: + Sets the time in seconds that a download should be below the `LowSpeedLimit` + transfer speed to abort the transfer for being too slow. Setting 'time' to + 0 will disable the speed check. Defaults to 10 seconds. Note that this + option will not affect external programs specified by `XferCommand`.
Repository Sections ------------------- diff --git a/etc/pacman.conf.in b/etc/pacman.conf.in index 53071e5..ddc7397 100644 --- a/etc/pacman.conf.in +++ b/etc/pacman.conf.in @@ -29,6 +29,10 @@ Architecture = auto #NoUpgrade = #NoExtract =
+# Downloader options +#LowSpeedLimit = 1 +#LowSpeedTime = 10 + # Misc options #UseSyslog #Color diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 168d71b..c338119 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -811,6 +811,16 @@ const char *alpm_option_get_dbpath(alpm_handle_t *handle); /** Get the name of the database lock file. Read-only. */ const char *alpm_option_get_lockfile(alpm_handle_t *handle);
+/** Returns the libcurl low speed limit in bytes per second. */ +long alpm_option_get_download_lowspeedlimit(alpm_handle_t *handle); +/** Sets the libcurl low speed limit in bytes per second. */ +int alpm_option_set_download_lowspeedlimit(alpm_handle_t *handle, long lowspeedlimit); + +/** Returns the libcurl low speed time in seconds. */ +long alpm_option_get_download_lowspeedtime(alpm_handle_t *handle); +/** Sets the libcurl low speed time in seconds. */ +int alpm_option_set_download_lowspeedtime(alpm_handle_t *handle, long lowspeedtime); + /** @name Accessors to the list of package cache directories. * @{ */ diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index f4e6a27..7490dd6 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -305,8 +305,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->download.lowspeedlimit); + curl_easy_setopt(curl, CURLOPT_LOW_SPEED_TIME, handle->download.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.c b/lib/libalpm/handle.c index e9439a0..8cfe0a1 100644 --- a/lib/libalpm/handle.c +++ b/lib/libalpm/handle.c @@ -247,6 +247,18 @@ const char SYMEXPORT *alpm_option_get_lockfile(alpm_handle_t *handle) return handle->lockfile; }
+long SYMEXPORT alpm_option_get_download_lowspeedlimit(alpm_handle_t *handle) +{ + CHECK_HANDLE(handle, return -1); + return handle->download.lowspeedlimit; +} + +long SYMEXPORT alpm_option_get_download_lowspeedtime(alpm_handle_t *handle) +{ + CHECK_HANDLE(handle, return -1); + return handle->download.lowspeedtime; +} + const char SYMEXPORT *alpm_option_get_gpgdir(alpm_handle_t *handle) { CHECK_HANDLE(handle, return NULL); @@ -362,6 +374,26 @@ int SYMEXPORT alpm_option_set_progresscb(alpm_handle_t *handle, alpm_cb_progress return 0; }
+int SYMEXPORT alpm_option_set_download_lowspeedlimit(alpm_handle_t *handle, long lowspeedlimit) +{ + CHECK_HANDLE(handle, return -1); + if(lowspeedlimit < 0) { + RET_ERR(handle, ALPM_ERR_WRONG_ARGS, -1); + } + handle->download.lowspeedlimit = lowspeedlimit; + return 0; +} + +int SYMEXPORT alpm_option_set_download_lowspeedtime(alpm_handle_t *handle, long lowspeedtime) +{ + CHECK_HANDLE(handle, return -1); + if(lowspeedtime < 0) { + RET_ERR(handle, ALPM_ERR_WRONG_ARGS, -1); + } + handle->download.lowspeedtime = lowspeedtime; + return 0; +} + static char *canonicalize_path(const char *path) { char *new_path; diff --git a/lib/libalpm/handle.h b/lib/libalpm/handle.h index a1d0f9a..73fa584 100644 --- a/lib/libalpm/handle.h +++ b/lib/libalpm/handle.h @@ -50,6 +50,14 @@ do { \ } \ } while(0)
+#ifdef HAVE_LIBCURL +struct download { + /* Curl timeouts */ + long lowspeedlimit; + long lowspeedtime; +}; +#endif + struct __alpm_handle_t { /* internal usage */ alpm_db_t *db_local; /* local db pointer */ @@ -60,6 +68,7 @@ struct __alpm_handle_t { #ifdef HAVE_LIBCURL /* libcurl handle */ CURL *curl; /* reusable curl_easy handle */ + struct download download; #endif
#ifdef HAVE_LIBGPGME diff --git a/src/pacman/conf.c b/src/pacman/conf.c index 25de7af..21db633 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -122,6 +122,9 @@ config_t *config_new(void) newconfig->colstr.err = ""; newconfig->colstr.nocolor = "";
+ newconfig->lowspeedlimit = 1L; + newconfig->lowspeedtime = 10L; + return newconfig; }
@@ -604,6 +607,24 @@ static int _parse_options(const char *key, char *value, return 1; } FREELIST(values); + } else if(strcmp(key, "LowSpeedLimit") == 0) { + config->lowspeedlimit = parse_positive_long(value); + if(config->lowspeedlimit < 0) { + pm_printf(ALPM_LOG_WARNING, + _("config file %s, line %d: invalid %s, using default.\n"), + file, linenum, "LowSpeedLimit"); + config->lowspeedlimit = 1L; + } + pm_printf(ALPM_LOG_DEBUG, "config: lowspeedlimit: %ld\n", config->lowspeedlimit); + } else if(strcmp(key, "LowSpeedTime") == 0) { + config->lowspeedtime = parse_positive_long(value); + if(config->lowspeedtime < 0) { + pm_printf(ALPM_LOG_WARNING, + _("config file %s, line %d: invalid %s, using default.\n"), + file, linenum, "LowSpeedTime"); + config->lowspeedtime = 10L; + } + pm_printf(ALPM_LOG_DEBUG, "config: lowspeedtime: %ld\n", config->lowspeedtime); } else { pm_printf(ALPM_LOG_WARNING, _("config file %s, line %d: directive '%s' in section '%s' not recognized.\n"), @@ -734,6 +755,9 @@ static int setup_libalpm(void) alpm_option_set_questioncb(handle, cb_question); alpm_option_set_progresscb(handle, cb_progress);
+ alpm_option_set_download_lowspeedlimit(handle, config->lowspeedlimit); + alpm_option_set_download_lowspeedtime(handle, config->lowspeedtime); + if(config->op == PM_OP_FILES) { alpm_option_set_dbext(handle, ".files"); } diff --git a/src/pacman/conf.h b/src/pacman/conf.h index 2aba8bf..c61ed2a 100644 --- a/src/pacman/conf.h +++ b/src/pacman/conf.h @@ -131,6 +131,10 @@ typedef struct __config_t { /* Color strings for output */ colstr_t colstr;
+ /* Curl timeouts */ + long lowspeedlimit; + long lowspeedtime; + alpm_list_t *repos; } config_t;
diff --git a/src/pacman/util.c b/src/pacman/util.c index b979083..b1875bd 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -1620,6 +1620,25 @@ int noyes(const char *format, ...) return ret; }
+/* Attempts to extract a positive long from a string; failure yields -1. */ +long parse_positive_long(const char *str) +{ + char *end; + long num; + + errno = 0; + num = strtol(str, &end, 10); + + if(end == str || /* Not a decimal number */ + *end != 0 || /* Extra characters */ + (num == LONG_MAX && errno == ERANGE) || /* Out of range */ + num < 0) { /* Out of range (negative) */ + return -1; + } + + return num; +} + int colon_printf(const char *fmt, ...) { int ret; diff --git a/src/pacman/util.h b/src/pacman/util.h index f5e37c8..d3eb77f 100644 --- a/src/pacman/util.h +++ b/src/pacman/util.h @@ -75,6 +75,7 @@ int multiselect_question(char *array, int count); int colon_printf(const char *format, ...) __attribute__((format(printf, 1, 2))); int yesno(const char *format, ...) __attribute__((format(printf, 1, 2))); int noyes(const char *format, ...) __attribute__((format(printf, 1, 2))); +long parse_positive_long(const char *str);
int pm_printf(alpm_loglevel_t level, const char *format, ...) __attribute__((format(printf,2,3))); int pm_asprintf(char **string, const char *format, ...) __attribute__((format(printf,2,3)));
On Tue, Aug 30, 2016 at 02:12:23PM +0200, Christian Hesse wrote:
From: Christian Hesse mail@eworm.de
Add LowSpeedLimit and LowSpeedTime configuration options to correspond to libcurl's CURLOPT_LOW_SPEED_LIMIT and CURLOPT_LOW_SPEED_TIME options. This allows, e.g., transfers behind corporate virus-scanning firewalls to survive the delays. Increasing the timeout may not be desirable in all situations; similarly, disabling the check prevents detection of disappearing networks.
FWIW, I'm strongly opposed to having a 1:1 mapping between pacman options and curl config options. Please look at the bigger picture -- it's dead connection detection. We might reimplement that in the future in some other way (e.g. via the progress callback or by some other transfer library entirely).
To that end, I think it would be reasonable to add a boolean toggle for the dead connection detection (default on). The patch in its current state makes me rather itchy from an API perspective.
Adds a utility function, parse_positive_long(), which yields a positive (but not unsigned) long integer from a null-terminated string using strtol(). A string containing anything but a base-10 number will cause the function to return -1.
Original-Patch-by: Andrew Hills ahills@ednos.net Signed-off-by: Christian Hesse mail@eworm.de
doc/pacman.conf.5.txt | 12 ++++++++++++ etc/pacman.conf.in | 4 ++++ lib/libalpm/alpm.h | 10 ++++++++++ lib/libalpm/dload.c | 4 ++-- lib/libalpm/handle.c | 32 ++++++++++++++++++++++++++++++++ lib/libalpm/handle.h | 9 +++++++++ src/pacman/conf.c | 24 ++++++++++++++++++++++++ src/pacman/conf.h | 4 ++++ src/pacman/util.c | 19 +++++++++++++++++++ src/pacman/util.h | 1 + 10 files changed, 117 insertions(+), 2 deletions(-)
diff --git a/doc/pacman.conf.5.txt b/doc/pacman.conf.5.txt index c665870..2346331 100644 --- a/doc/pacman.conf.5.txt +++ b/doc/pacman.conf.5.txt @@ -209,6 +209,18 @@ Options Displays name, version and size of target packages formatted as a table for upgrade, sync and remove operations.
+*LowSpeedLimit* = speed::
- Sets the speed in bytes per second that a download should be below during
- `LowSpeedTime` seconds to abort the transfer for being too slow. Setting
- 'speed' to 0 will disable the speed check. Defaults to 1 byte per second.
- Note that this option will not affect external programs specified by
- `XferCommand`.
+*LowSpeedTime* = time::
- Sets the time in seconds that a download should be below the `LowSpeedLimit`
- transfer speed to abort the transfer for being too slow. Setting 'time' to
- 0 will disable the speed check. Defaults to 10 seconds. Note that this
- option will not affect external programs specified by `XferCommand`.
Repository Sections
diff --git a/etc/pacman.conf.in b/etc/pacman.conf.in index 53071e5..ddc7397 100644 --- a/etc/pacman.conf.in +++ b/etc/pacman.conf.in @@ -29,6 +29,10 @@ Architecture = auto #NoUpgrade = #NoExtract =
+# Downloader options +#LowSpeedLimit = 1 +#LowSpeedTime = 10
# Misc options #UseSyslog #Color diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 168d71b..c338119 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -811,6 +811,16 @@ const char *alpm_option_get_dbpath(alpm_handle_t *handle); /** Get the name of the database lock file. Read-only. */ const char *alpm_option_get_lockfile(alpm_handle_t *handle);
+/** Returns the libcurl low speed limit in bytes per second. */ +long alpm_option_get_download_lowspeedlimit(alpm_handle_t *handle); +/** Sets the libcurl low speed limit in bytes per second. */ +int alpm_option_set_download_lowspeedlimit(alpm_handle_t *handle, long lowspeedlimit);
+/** Returns the libcurl low speed time in seconds. */ +long alpm_option_get_download_lowspeedtime(alpm_handle_t *handle); +/** Sets the libcurl low speed time in seconds. */ +int alpm_option_set_download_lowspeedtime(alpm_handle_t *handle, long lowspeedtime);
/** @name Accessors to the list of package cache directories.
- @{
*/ diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index f4e6a27..7490dd6 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -305,8 +305,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->download.lowspeedlimit);
- curl_easy_setopt(curl, CURLOPT_LOW_SPEED_TIME, handle->download.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.c b/lib/libalpm/handle.c index e9439a0..8cfe0a1 100644 --- a/lib/libalpm/handle.c +++ b/lib/libalpm/handle.c @@ -247,6 +247,18 @@ const char SYMEXPORT *alpm_option_get_lockfile(alpm_handle_t *handle) return handle->lockfile; }
+long SYMEXPORT alpm_option_get_download_lowspeedlimit(alpm_handle_t *handle) +{
- CHECK_HANDLE(handle, return -1);
- return handle->download.lowspeedlimit;
+}
+long SYMEXPORT alpm_option_get_download_lowspeedtime(alpm_handle_t *handle) +{
- CHECK_HANDLE(handle, return -1);
- return handle->download.lowspeedtime;
+}
const char SYMEXPORT *alpm_option_get_gpgdir(alpm_handle_t *handle) { CHECK_HANDLE(handle, return NULL); @@ -362,6 +374,26 @@ int SYMEXPORT alpm_option_set_progresscb(alpm_handle_t *handle, alpm_cb_progress return 0; }
+int SYMEXPORT alpm_option_set_download_lowspeedlimit(alpm_handle_t *handle, long lowspeedlimit) +{
- CHECK_HANDLE(handle, return -1);
- if(lowspeedlimit < 0) {
RET_ERR(handle, ALPM_ERR_WRONG_ARGS, -1);- }
- handle->download.lowspeedlimit = lowspeedlimit;
- return 0;
+}
+int SYMEXPORT alpm_option_set_download_lowspeedtime(alpm_handle_t *handle, long lowspeedtime) +{
- CHECK_HANDLE(handle, return -1);
- if(lowspeedtime < 0) {
RET_ERR(handle, ALPM_ERR_WRONG_ARGS, -1);- }
- handle->download.lowspeedtime = lowspeedtime;
- return 0;
+}
static char *canonicalize_path(const char *path) { char *new_path; diff --git a/lib/libalpm/handle.h b/lib/libalpm/handle.h index a1d0f9a..73fa584 100644 --- a/lib/libalpm/handle.h +++ b/lib/libalpm/handle.h @@ -50,6 +50,14 @@ do { \ } \ } while(0)
+#ifdef HAVE_LIBCURL +struct download {
- /* Curl timeouts */
- long lowspeedlimit;
- long lowspeedtime;
+}; +#endif
struct __alpm_handle_t { /* internal usage */ alpm_db_t *db_local; /* local db pointer */ @@ -60,6 +68,7 @@ struct __alpm_handle_t { #ifdef HAVE_LIBCURL /* libcurl handle */ CURL *curl; /* reusable curl_easy handle */
- struct download download;
#endif
#ifdef HAVE_LIBGPGME diff --git a/src/pacman/conf.c b/src/pacman/conf.c index 25de7af..21db633 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -122,6 +122,9 @@ config_t *config_new(void) newconfig->colstr.err = ""; newconfig->colstr.nocolor = "";
- newconfig->lowspeedlimit = 1L;
- newconfig->lowspeedtime = 10L;
- return newconfig;
}
@@ -604,6 +607,24 @@ static int _parse_options(const char *key, char *value, return 1; } FREELIST(values);
} else if(strcmp(key, "LowSpeedLimit") == 0) {config->lowspeedlimit = parse_positive_long(value);if(config->lowspeedlimit < 0) {pm_printf(ALPM_LOG_WARNING,_("config file %s, line %d: invalid %s, using default.\n"),file, linenum, "LowSpeedLimit");config->lowspeedlimit = 1L;}pm_printf(ALPM_LOG_DEBUG, "config: lowspeedlimit: %ld\n", config->lowspeedlimit);} else if(strcmp(key, "LowSpeedTime") == 0) {config->lowspeedtime = parse_positive_long(value);if(config->lowspeedtime < 0) {pm_printf(ALPM_LOG_WARNING,_("config file %s, line %d: invalid %s, using default.\n"),file, linenum, "LowSpeedTime");config->lowspeedtime = 10L;}pm_printf(ALPM_LOG_DEBUG, "config: lowspeedtime: %ld\n", config->lowspeedtime);@@ -734,6 +755,9 @@ static int setup_libalpm(void) alpm_option_set_questioncb(handle, cb_question); alpm_option_set_progresscb(handle, cb_progress);
- alpm_option_set_download_lowspeedlimit(handle, config->lowspeedlimit);
- alpm_option_set_download_lowspeedtime(handle, config->lowspeedtime);
- if(config->op == PM_OP_FILES) { alpm_option_set_dbext(handle, ".files"); }
diff --git a/src/pacman/conf.h b/src/pacman/conf.h index 2aba8bf..c61ed2a 100644 --- a/src/pacman/conf.h +++ b/src/pacman/conf.h @@ -131,6 +131,10 @@ typedef struct __config_t { /* Color strings for output */ colstr_t colstr;
- /* Curl timeouts */
- long lowspeedlimit;
- long lowspeedtime;
- alpm_list_t *repos;
} config_t;
diff --git a/src/pacman/util.c b/src/pacman/util.c index b979083..b1875bd 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -1620,6 +1620,25 @@ int noyes(const char *format, ...) return ret; }
+/* Attempts to extract a positive long from a string; failure yields -1. */ +long parse_positive_long(const char *str) +{
- char *end;
- long num;
- errno = 0;
- num = strtol(str, &end, 10);
- if(end == str || /* Not a decimal number */
*end != 0 || /* Extra characters */(num == LONG_MAX && errno == ERANGE) || /* Out of range */num < 0) { /* Out of range (negative) */return -1;- }
- return num;
+}
int colon_printf(const char *fmt, ...) { int ret; diff --git a/src/pacman/util.h b/src/pacman/util.h index f5e37c8..d3eb77f 100644 --- a/src/pacman/util.h +++ b/src/pacman/util.h @@ -75,6 +75,7 @@ int multiselect_question(char *array, int count); int colon_printf(const char *format, ...) __attribute__((format(printf, 1, 2))); int yesno(const char *format, ...) __attribute__((format(printf, 1, 2))); int noyes(const char *format, ...) __attribute__((format(printf, 1, 2))); +long parse_positive_long(const char *str);
int pm_printf(alpm_loglevel_t level, const char *format, ...) __attribute__((format(printf,2,3))); int pm_asprintf(char **string, const char *format, ...) __attribute__((format(printf,2,3))); -- 2.9.3
Dave Reisner d@falconindy.com on Tue, 2016/08/30 08:46:
On Tue, Aug 30, 2016 at 02:12:23PM +0200, Christian Hesse wrote:
From: Christian Hesse mail@eworm.de
Add LowSpeedLimit and LowSpeedTime configuration options to correspond to libcurl's CURLOPT_LOW_SPEED_LIMIT and CURLOPT_LOW_SPEED_TIME options. This allows, e.g., transfers behind corporate virus-scanning firewalls to survive the delays. Increasing the timeout may not be desirable in all situations; similarly, disabling the check prevents detection of disappearing networks.
FWIW, I'm strongly opposed to having a 1:1 mapping between pacman options and curl config options. Please look at the bigger picture -- it's dead connection detection. We might reimplement that in the future in some other way (e.g. via the progress callback or by some other transfer library entirely).
To that end, I think it would be reasonable to add a boolean toggle for the dead connection detection (default on). The patch in its current state makes me rather itchy from an API perspective.
That is what my stupid-proxy patch does... Now it is up to Allan to decide. ;)
On 30/08/16 22:48, Christian Hesse wrote:
Dave Reisner d@falconindy.com on Tue, 2016/08/30 08:46:
On Tue, Aug 30, 2016 at 02:12:23PM +0200, Christian Hesse wrote:
From: Christian Hesse mail@eworm.de
Add LowSpeedLimit and LowSpeedTime configuration options to correspond to libcurl's CURLOPT_LOW_SPEED_LIMIT and CURLOPT_LOW_SPEED_TIME options. This allows, e.g., transfers behind corporate virus-scanning firewalls to survive the delays. Increasing the timeout may not be desirable in all situations; similarly, disabling the check prevents detection of disappearing networks.
FWIW, I'm strongly opposed to having a 1:1 mapping between pacman options and curl config options. Please look at the bigger picture -- it's dead connection detection. We might reimplement that in the future in some other way (e.g. via the progress callback or by some other transfer library entirely).
To that end, I think it would be reasonable to add a boolean toggle for the dead connection detection (default on). The patch in its current state makes me rather itchy from an API perspective.
That is what my stupid-proxy patch does... Now it is up to Allan to decide. ;)
Crap... Why do I need to make decisions?
OK - lets think on the go here... The two options we "want" to support based on submitted patches are: 1) Disabling the low speed timeout 2) Setting maximum download speed
In the future, we might also add a parallel download option.
I'm guessing these want both global config values and command line options. Anyone want to suggest what these could be. I'll choose the colour of the bikeshed.
Allan
On Sat, Sep 03, 2016 at 10:45:42PM +1000, Allan McRae wrote:
On 30/08/16 22:48, Christian Hesse wrote:
Dave Reisner d@falconindy.com on Tue, 2016/08/30 08:46:
On Tue, Aug 30, 2016 at 02:12:23PM +0200, Christian Hesse wrote:
From: Christian Hesse mail@eworm.de
Add LowSpeedLimit and LowSpeedTime configuration options to correspond to libcurl's CURLOPT_LOW_SPEED_LIMIT and CURLOPT_LOW_SPEED_TIME options. This allows, e.g., transfers behind corporate virus-scanning firewalls to survive the delays. Increasing the timeout may not be desirable in all situations; similarly, disabling the check prevents detection of disappearing networks.
FWIW, I'm strongly opposed to having a 1:1 mapping between pacman options and curl config options. Please look at the bigger picture -- it's dead connection detection. We might reimplement that in the future in some other way (e.g. via the progress callback or by some other transfer library entirely).
To that end, I think it would be reasonable to add a boolean toggle for the dead connection detection (default on). The patch in its current state makes me rather itchy from an API perspective.
That is what my stupid-proxy patch does... Now it is up to Allan to decide. ;)
Crap... Why do I need to make decisions?
OK - lets think on the go here... The two options we "want" to support based on submitted patches are:
- Disabling the low speed timeout
- Setting maximum download speed
In the future, we might also add a parallel download option.
I'm guessing these want both global config values and command line options. Anyone want to suggest what these could be. I'll choose the colour of the bikeshed.
Allan
1) DetectDeadConnections 2) MaxDownloadKBps future) MaxConcurrentDownloads
For #2, I'll suggest that you can already do this with a program like netbrake. Curl's internal rate limiting is not good and only uses a flat average, rather than a rolling window of recent samples. It's idea of "limiting" bandwidth is inserting an artificial delay before the next recv call, which might just be draining a buffer of already received data from the kernel. This is particularly true if you're behind a router and not the actual gateway device (and I suspect this is the common case).
d
On Sat, 3 Sep 2016 09:40:52 -0400 Dave Reisner d@falconindy.com wrote:
On Sat, Sep 03, 2016 at 10:45:42PM +1000, Allan McRae wrote:
On 30/08/16 22:48, Christian Hesse wrote:
Dave Reisner d@falconindy.com on Tue, 2016/08/30 08:46:
On Tue, Aug 30, 2016 at 02:12:23PM +0200, Christian Hesse wrote:
From: Christian Hesse mail@eworm.de
Add LowSpeedLimit and LowSpeedTime configuration options to correspond to libcurl's CURLOPT_LOW_SPEED_LIMIT and CURLOPT_LOW_SPEED_TIME options. This allows, e.g., transfers behind corporate virus-scanning firewalls to survive the delays. Increasing the timeout may not be desirable in all situations; similarly, disabling the check prevents detection of disappearing networks.
FWIW, I'm strongly opposed to having a 1:1 mapping between pacman options and curl config options. Please look at the bigger picture -- it's dead connection detection. We might reimplement that in the future in some other way (e.g. via the progress callback or by some other transfer library entirely).
To that end, I think it would be reasonable to add a boolean toggle for the dead connection detection (default on). The patch in its current state makes me rather itchy from an API perspective.
That is what my stupid-proxy patch does... Now it is up to Allan to decide. ;)
Crap... Why do I need to make decisions?
OK - lets think on the go here... The two options we "want" to support based on submitted patches are:
- Disabling the low speed timeout
- Setting maximum download speed
In the future, we might also add a parallel download option.
I'm guessing these want both global config values and command line options. Anyone want to suggest what these could be. I'll choose the colour of the bikeshed.
Allan
- DetectDeadConnections
- MaxDownloadKBps
future) MaxConcurrentDownloads
For #2, I'll suggest that you can already do this with a program like netbrake. Curl's internal rate limiting is not good and only uses a flat average, rather than a rolling window of recent samples. It's
FWIW I've actually submitted a patch[1] recently to improve curl's limiting algo that should improve this. (Of course that'll only apply to users with a quite recent libcurl.)
In my patch[2] (for pacman this time) I used MaxDlSpeed as name so one can specify a unit, whereas MaxDownloadKBps implies one (though likely the most common one I suppose).
Using "Dl" instead of "Download" in the option name seemed like still pretty common/understandable, all the while making things a bit shorter, especially nice for the command line option.
With similar intent, maybe a shorter one for #1 might be better... e.g. NoLowSpeedTimeout, or simply NoSlowTimeout ? (Also I guess yours should be NoDetectDeadConnections or similar, as the option is actually meant to *disable* the low speed timeout.)
idea of "limiting" bandwidth is inserting an artificial delay before the next recv call, which might just be draining a buffer of already received data from the kernel. This is particularly true if you're behind a router and not the actual gateway device (and I suspect this is the common case).
d
[1] https://github.com/curl/curl/pull/971 [2] https://lists.archlinux.org/pipermail/pacman-dev/2016-August/021265.html
From: Christian Hesse mail@eworm.de
Signed-off-by: Christian Hesse mail@eworm.de --- doc/pacman.8.txt | 13 +++++++++++++ src/pacman/conf.h | 4 +++- src/pacman/pacman.c | 14 ++++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/doc/pacman.8.txt b/doc/pacman.8.txt index 231e0bc..83f63e6 100644 --- a/doc/pacman.8.txt +++ b/doc/pacman.8.txt @@ -266,6 +266,19 @@ Upgrade Options (apply to '-S' and '-U')[[UO]] *--needed*:: Do not reinstall the targets that are already up-to-date.
+*--lowspeedlimit* <speed>:: + Sets the speed in bytes per second that a download should be below during + `LowSpeedTime` seconds to abort the transfer for being too slow. Setting + 'speed' to 0 will disable the speed check. Defaults to 1 byte per second. + Note that this option will not affect external programs specified by + `XferCommand`. + +*--lowspeedlimit* <time>:: + Sets the time in seconds that a download should be below the `LowSpeedLimit` + transfer speed to abort the transfer for being too slow. Setting 'time' to + 0 will disable the speed check. Defaults to 10 seconds. Note that this + option will not affect external programs specified by `XferCommand`. +
Query Options (apply to '-Q')[[QO]] ----------------------------------- diff --git a/src/pacman/conf.h b/src/pacman/conf.h index c61ed2a..fe296cc 100644 --- a/src/pacman/conf.h +++ b/src/pacman/conf.h @@ -205,7 +205,9 @@ enum { OP_VERBOSE, OP_DOWNLOADONLY, OP_REFRESH, - OP_ASSUMEINSTALLED + OP_ASSUMEINSTALLED, + OP_LOWSPEEDLIMIT, + OP_LOWSPEEDTIME };
/* clean method */ diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index be52d1b..f6a5f33 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -194,6 +194,12 @@ static void usage(int op, const char * const myname) addlist(_(" --ignore <pkg> ignore a package upgrade (can be used more than once)\n")); addlist(_(" --ignoregroup <grp>\n" " ignore a group upgrade (can be used more than once)\n")); +#ifdef HAVE_LIBCURL + addlist(_(" --lowspeedlimit <speed>\n" + " bytes per second that a download should be below\n")); + addlist(_(" --lowspeedtime <time>\n" + " time in seconds that a download should be below lowspeedlimit\n")); +#endif /* pass through */ case PM_OP_REMOVE: addlist(_(" -d, --nodeps skip dependency version checks (-dd to skip all checks)\n")); @@ -713,6 +719,12 @@ static int parsearg_upgrade(int opt) case OP_IGNOREGROUP: parsearg_util_addlist(&(config->ignoregrp)); break; + case OP_LOWSPEEDLIMIT: + config->lowspeedlimit = parse_positive_long(optarg); + break; + case OP_LOWSPEEDTIME: + config->lowspeedtime = parse_positive_long(optarg); + break; default: return 1; } return 0; @@ -928,6 +940,8 @@ static int parseargs(int argc, char *argv[]) {"logfile", required_argument, 0, OP_LOGFILE}, {"ignoregroup", required_argument, 0, OP_IGNOREGROUP}, {"needed", no_argument, 0, OP_NEEDED}, + {"lowspeedlimit", required_argument, 0, OP_LOWSPEEDLIMIT}, + {"lowspeedtime", required_argument, 0, OP_LOWSPEEDTIME}, {"asexplicit", no_argument, 0, OP_ASEXPLICIT}, {"arch", required_argument, 0, OP_ARCH}, {"print-format", required_argument, 0, OP_PRINTFORMAT},
Hi,
On 30.08.2016 15:16, Christian Hesse wrote:
From: Christian Hesse mail@eworm.de
Signed-off-by: Christian Hesse mail@eworm.de
doc/pacman.8.txt | 13 +++++++++++++ src/pacman/conf.h | 4 +++- src/pacman/pacman.c | 14 ++++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/doc/pacman.8.txt b/doc/pacman.8.txt index 231e0bc..83f63e6 100644 --- a/doc/pacman.8.txt +++ b/doc/pacman.8.txt @@ -266,6 +266,19 @@ Upgrade Options (apply to '-S' and '-U')[[UO]] *--needed*:: Do not reinstall the targets that are already up-to-date.
+*--lowspeedlimit* <speed>::
- Sets the speed in bytes per second that a download should be below during
- `LowSpeedTime` seconds to abort the transfer for being too slow. Setting
- 'speed' to 0 will disable the speed check. Defaults to 1 byte per second.
- Note that this option will not affect external programs specified by
- `XferCommand`.
+*--lowspeedlimit* <time>::
'--lowspeedlimit' is used a second time here. Probably a copy-paste error and meant to be '--lowspeedtime'?
- Sets the time in seconds that a download should be below the `LowSpeedLimit`
- transfer speed to abort the transfer for being too slow. Setting 'time' to
- 0 will disable the speed check. Defaults to 10 seconds. Note that this
- option will not affect external programs specified by `XferCommand`.
Query Options (apply to '-Q')[[QO]]
diff --git a/src/pacman/conf.h b/src/pacman/conf.h index c61ed2a..fe296cc 100644 --- a/src/pacman/conf.h +++ b/src/pacman/conf.h @@ -205,7 +205,9 @@ enum { OP_VERBOSE, OP_DOWNLOADONLY, OP_REFRESH,
- OP_ASSUMEINSTALLED
- OP_ASSUMEINSTALLED,
- OP_LOWSPEEDLIMIT,
- OP_LOWSPEEDTIME
};
/* clean method */ diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index be52d1b..f6a5f33 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -194,6 +194,12 @@ static void usage(int op, const char * const myname) addlist(_(" --ignore <pkg> ignore a package upgrade (can be used more than once)\n")); addlist(_(" --ignoregroup <grp>\n" " ignore a group upgrade (can be used more than once)\n")); +#ifdef HAVE_LIBCURL
addlist(_(" --lowspeedlimit <speed>\n"" bytes per second that a download should be below\n"));addlist(_(" --lowspeedtime <time>\n"" time in seconds that a download should be below lowspeedlimit\n"));+#endif /* pass through */ case PM_OP_REMOVE: addlist(_(" -d, --nodeps skip dependency version checks (-dd to skip all checks)\n")); @@ -713,6 +719,12 @@ static int parsearg_upgrade(int opt) case OP_IGNOREGROUP: parsearg_util_addlist(&(config->ignoregrp)); break;
case OP_LOWSPEEDLIMIT:config->lowspeedlimit = parse_positive_long(optarg);break;case OP_LOWSPEEDTIME:config->lowspeedtime = parse_positive_long(optarg);break;@@ -928,6 +940,8 @@ static int parseargs(int argc, char *argv[]) {"logfile", required_argument, 0, OP_LOGFILE}, {"ignoregroup", required_argument, 0, OP_IGNOREGROUP}, {"needed", no_argument, 0, OP_NEEDED},
{"lowspeedlimit", required_argument, 0, OP_LOWSPEEDLIMIT},{"lowspeedtime", required_argument, 0, OP_LOWSPEEDTIME},
On 14.11.2016 00:33, brainpower wrote:
Hi,
On 30.08.2016 15:16, Christian Hesse wrote:
Ah, sorry didn'T notice this was quite old, sorry for the noise. My mail client sorted it between all the new messages.
brainpower brainpower@mailbox.org on Mon, 2016/11/14 00:40:
On 14.11.2016 00:33, brainpower wrote:
Hi,
On 30.08.2016 15:16, Christian Hesse wrote:
Ah, sorry didn'T notice this was quite old, sorry for the noise. My mail client sorted it between all the new messages.
The original patches are quite old, but I sent the rebase just three days ago. And the issue is still there, so your point is still valid.
I will send an updated patch soon.
From: Christian Hesse mail@eworm.de
Add LowSpeedLimit and LowSpeedTime configuration options to correspond to libcurl's CURLOPT_LOW_SPEED_LIMIT and CURLOPT_LOW_SPEED_TIME options. This allows, e.g., transfers behind corporate virus-scanning firewalls to survive the delays. Increasing the timeout may not be desirable in all situations; similarly, disabling the check prevents detection of disappearing networks.
Adds a utility function, parse_positive_long(), which yields a positive (but not unsigned) long integer from a null-terminated string using strtol(). A string containing anything but a base-10 number will cause the function to return -1.
Original-Patch-by: Andrew Hills ahills@ednos.net Signed-off-by: Christian Hesse mail@eworm.de --- doc/pacman.conf.5.txt | 12 ++++++++++++ etc/pacman.conf.in | 4 ++++ lib/libalpm/alpm.h | 10 ++++++++++ lib/libalpm/dload.c | 4 ++-- lib/libalpm/handle.c | 32 ++++++++++++++++++++++++++++++++ lib/libalpm/handle.h | 9 +++++++++ src/pacman/conf.c | 24 ++++++++++++++++++++++++ src/pacman/conf.h | 4 ++++ src/pacman/util.c | 19 +++++++++++++++++++ src/pacman/util.h | 1 + 10 files changed, 117 insertions(+), 2 deletions(-)
diff --git a/doc/pacman.conf.5.txt b/doc/pacman.conf.5.txt index c665870..2346331 100644 --- a/doc/pacman.conf.5.txt +++ b/doc/pacman.conf.5.txt @@ -209,6 +209,18 @@ Options Displays name, version and size of target packages formatted as a table for upgrade, sync and remove operations.
+*LowSpeedLimit* = speed:: + Sets the speed in bytes per second that a download should be below during + `LowSpeedTime` seconds to abort the transfer for being too slow. Setting + 'speed' to 0 will disable the speed check. Defaults to 1 byte per second. + Note that this option will not affect external programs specified by + `XferCommand`. + +*LowSpeedTime* = time:: + Sets the time in seconds that a download should be below the `LowSpeedLimit` + transfer speed to abort the transfer for being too slow. Setting 'time' to + 0 will disable the speed check. Defaults to 10 seconds. Note that this + option will not affect external programs specified by `XferCommand`.
Repository Sections ------------------- diff --git a/etc/pacman.conf.in b/etc/pacman.conf.in index 53071e5..ddc7397 100644 --- a/etc/pacman.conf.in +++ b/etc/pacman.conf.in @@ -29,6 +29,10 @@ Architecture = auto #NoUpgrade = #NoExtract =
+# Downloader options +#LowSpeedLimit = 1 +#LowSpeedTime = 10 + # Misc options #UseSyslog #Color diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 2d2491d..bea79da 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -812,6 +812,16 @@ const char *alpm_option_get_dbpath(alpm_handle_t *handle); /** Get the name of the database lock file. Read-only. */ const char *alpm_option_get_lockfile(alpm_handle_t *handle);
+/** Returns the libcurl low speed limit in bytes per second. */ +long alpm_option_get_download_lowspeedlimit(alpm_handle_t *handle); +/** Sets the libcurl low speed limit in bytes per second. */ +int alpm_option_set_download_lowspeedlimit(alpm_handle_t *handle, long lowspeedlimit); + +/** Returns the libcurl low speed time in seconds. */ +long alpm_option_get_download_lowspeedtime(alpm_handle_t *handle); +/** Sets the libcurl low speed time in seconds. */ +int alpm_option_set_download_lowspeedtime(alpm_handle_t *handle, long lowspeedtime); + /** @name Accessors to the list of package cache directories. * @{ */ diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index 9d80358..c3bb200 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -305,8 +305,8 @@ static void curl_set_handle_opts(struct dload_payload *payload, curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1L); curl_easy_setopt(curl, CURLOPT_XFERINFOFUNCTION, dload_progress_cb); curl_easy_setopt(curl, CURLOPT_XFERINFODATA, (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->download.lowspeedlimit); + curl_easy_setopt(curl, CURLOPT_LOW_SPEED_TIME, handle->download.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.c b/lib/libalpm/handle.c index 28e8148..6479798 100644 --- a/lib/libalpm/handle.c +++ b/lib/libalpm/handle.c @@ -247,6 +247,18 @@ const char SYMEXPORT *alpm_option_get_lockfile(alpm_handle_t *handle) return handle->lockfile; }
+long SYMEXPORT alpm_option_get_download_lowspeedlimit(alpm_handle_t *handle) +{ + CHECK_HANDLE(handle, return -1); + return handle->download.lowspeedlimit; +} + +long SYMEXPORT alpm_option_get_download_lowspeedtime(alpm_handle_t *handle) +{ + CHECK_HANDLE(handle, return -1); + return handle->download.lowspeedtime; +} + const char SYMEXPORT *alpm_option_get_gpgdir(alpm_handle_t *handle) { CHECK_HANDLE(handle, return NULL); @@ -362,6 +374,26 @@ int SYMEXPORT alpm_option_set_progresscb(alpm_handle_t *handle, alpm_cb_progress return 0; }
+int SYMEXPORT alpm_option_set_download_lowspeedlimit(alpm_handle_t *handle, long lowspeedlimit) +{ + CHECK_HANDLE(handle, return -1); + if(lowspeedlimit < 0) { + RET_ERR(handle, ALPM_ERR_WRONG_ARGS, -1); + } + handle->download.lowspeedlimit = lowspeedlimit; + return 0; +} + +int SYMEXPORT alpm_option_set_download_lowspeedtime(alpm_handle_t *handle, long lowspeedtime) +{ + CHECK_HANDLE(handle, return -1); + if(lowspeedtime < 0) { + RET_ERR(handle, ALPM_ERR_WRONG_ARGS, -1); + } + handle->download.lowspeedtime = lowspeedtime; + return 0; +} + static char *canonicalize_path(const char *path) { char *new_path; diff --git a/lib/libalpm/handle.h b/lib/libalpm/handle.h index 652f17d..0c66469 100644 --- a/lib/libalpm/handle.h +++ b/lib/libalpm/handle.h @@ -50,6 +50,14 @@ do { \ } \ } while(0)
+#ifdef HAVE_LIBCURL +struct download { + /* Curl timeouts */ + long lowspeedlimit; + long lowspeedtime; +}; +#endif + struct __alpm_handle_t { /* internal usage */ alpm_db_t *db_local; /* local db pointer */ @@ -60,6 +68,7 @@ struct __alpm_handle_t { #ifdef HAVE_LIBCURL /* libcurl handle */ CURL *curl; /* reusable curl_easy handle */ + struct download download; #endif
#ifdef HAVE_LIBGPGME diff --git a/src/pacman/conf.c b/src/pacman/conf.c index d8d64fb..520f4d0 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -122,6 +122,9 @@ config_t *config_new(void) newconfig->colstr.err = ""; newconfig->colstr.nocolor = "";
+ newconfig->lowspeedlimit = 1L; + newconfig->lowspeedtime = 10L; + return newconfig; }
@@ -603,6 +606,24 @@ static int _parse_options(const char *key, char *value, return 1; } FREELIST(values); + } else if(strcmp(key, "LowSpeedLimit") == 0) { + config->lowspeedlimit = parse_positive_long(value); + if(config->lowspeedlimit < 0) { + pm_printf(ALPM_LOG_WARNING, + _("config file %s, line %d: invalid %s, using default.\n"), + file, linenum, "LowSpeedLimit"); + config->lowspeedlimit = 1L; + } + pm_printf(ALPM_LOG_DEBUG, "config: lowspeedlimit: %ld\n", config->lowspeedlimit); + } else if(strcmp(key, "LowSpeedTime") == 0) { + config->lowspeedtime = parse_positive_long(value); + if(config->lowspeedtime < 0) { + pm_printf(ALPM_LOG_WARNING, + _("config file %s, line %d: invalid %s, using default.\n"), + file, linenum, "LowSpeedTime"); + config->lowspeedtime = 10L; + } + pm_printf(ALPM_LOG_DEBUG, "config: lowspeedtime: %ld\n", config->lowspeedtime); } else { pm_printf(ALPM_LOG_WARNING, _("config file %s, line %d: directive '%s' in section '%s' not recognized.\n"), @@ -733,6 +754,9 @@ static int setup_libalpm(void) alpm_option_set_questioncb(handle, cb_question); alpm_option_set_progresscb(handle, cb_progress);
+ alpm_option_set_download_lowspeedlimit(handle, config->lowspeedlimit); + alpm_option_set_download_lowspeedtime(handle, config->lowspeedtime); + if(config->op == PM_OP_FILES) { alpm_option_set_dbext(handle, ".files"); } diff --git a/src/pacman/conf.h b/src/pacman/conf.h index 945de7c..17e5f27 100644 --- a/src/pacman/conf.h +++ b/src/pacman/conf.h @@ -133,6 +133,10 @@ typedef struct __config_t { /* Color strings for output */ colstr_t colstr;
+ /* Curl timeouts */ + long lowspeedlimit; + long lowspeedtime; + alpm_list_t *repos; } config_t;
diff --git a/src/pacman/util.c b/src/pacman/util.c index fd7438b..bd85d8a 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -1620,6 +1620,25 @@ int noyes(const char *format, ...) return ret; }
+/* Attempts to extract a positive long from a string; failure yields -1. */ +long parse_positive_long(const char *str) +{ + char *end; + long num; + + errno = 0; + num = strtol(str, &end, 10); + + if(end == str || /* Not a decimal number */ + *end != 0 || /* Extra characters */ + (num == LONG_MAX && errno == ERANGE) || /* Out of range */ + num < 0) { /* Out of range (negative) */ + return -1; + } + + return num; +} + int colon_printf(const char *fmt, ...) { int ret; diff --git a/src/pacman/util.h b/src/pacman/util.h index 0a6ce97..fa84ecc 100644 --- a/src/pacman/util.h +++ b/src/pacman/util.h @@ -76,6 +76,7 @@ int multiselect_question(char *array, int count); int colon_printf(const char *format, ...) __attribute__((format(printf, 1, 2))); int yesno(const char *format, ...) __attribute__((format(printf, 1, 2))); int noyes(const char *format, ...) __attribute__((format(printf, 1, 2))); +long parse_positive_long(const char *str);
int pm_printf(alpm_loglevel_t level, const char *format, ...) __attribute__((format(printf,2,3))); int pm_asprintf(char **string, const char *format, ...) __attribute__((format(printf,2,3)));
From: Christian Hesse mail@eworm.de
Signed-off-by: Christian Hesse mail@eworm.de --- doc/pacman.8.txt | 13 +++++++++++++ src/pacman/conf.h | 4 +++- src/pacman/pacman.c | 14 ++++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/doc/pacman.8.txt b/doc/pacman.8.txt index 0fa727e..0eda4fb 100644 --- a/doc/pacman.8.txt +++ b/doc/pacman.8.txt @@ -266,6 +266,19 @@ Upgrade Options (apply to '-S' and '-U')[[UO]] *--needed*:: Do not reinstall the targets that are already up-to-date.
+*--lowspeedlimit* <speed>:: + Sets the speed in bytes per second that a download should be below during + `LowSpeedTime` seconds to abort the transfer for being too slow. Setting + 'speed' to 0 will disable the speed check. Defaults to 1 byte per second. + Note that this option will not affect external programs specified by + `XferCommand`. + +*--lowspeedlimit* <time>:: + Sets the time in seconds that a download should be below the `LowSpeedLimit` + transfer speed to abort the transfer for being too slow. Setting 'time' to + 0 will disable the speed check. Defaults to 10 seconds. Note that this + option will not affect external programs specified by `XferCommand`. +
Query Options (apply to '-Q')[[QO]] ----------------------------------- diff --git a/src/pacman/conf.h b/src/pacman/conf.h index 17e5f27..9b77238 100644 --- a/src/pacman/conf.h +++ b/src/pacman/conf.h @@ -207,7 +207,9 @@ enum { OP_VERBOSE, OP_DOWNLOADONLY, OP_REFRESH, - OP_ASSUMEINSTALLED + OP_ASSUMEINSTALLED, + OP_LOWSPEEDLIMIT, + OP_LOWSPEEDTIME };
/* clean method */ diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index be52d1b..f6a5f33 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -194,6 +194,12 @@ static void usage(int op, const char * const myname) addlist(_(" --ignore <pkg> ignore a package upgrade (can be used more than once)\n")); addlist(_(" --ignoregroup <grp>\n" " ignore a group upgrade (can be used more than once)\n")); +#ifdef HAVE_LIBCURL + addlist(_(" --lowspeedlimit <speed>\n" + " bytes per second that a download should be below\n")); + addlist(_(" --lowspeedtime <time>\n" + " time in seconds that a download should be below lowspeedlimit\n")); +#endif /* pass through */ case PM_OP_REMOVE: addlist(_(" -d, --nodeps skip dependency version checks (-dd to skip all checks)\n")); @@ -713,6 +719,12 @@ static int parsearg_upgrade(int opt) case OP_IGNOREGROUP: parsearg_util_addlist(&(config->ignoregrp)); break; + case OP_LOWSPEEDLIMIT: + config->lowspeedlimit = parse_positive_long(optarg); + break; + case OP_LOWSPEEDTIME: + config->lowspeedtime = parse_positive_long(optarg); + break; default: return 1; } return 0; @@ -928,6 +940,8 @@ static int parseargs(int argc, char *argv[]) {"logfile", required_argument, 0, OP_LOGFILE}, {"ignoregroup", required_argument, 0, OP_IGNOREGROUP}, {"needed", no_argument, 0, OP_NEEDED}, + {"lowspeedlimit", required_argument, 0, OP_LOWSPEEDLIMIT}, + {"lowspeedtime", required_argument, 0, OP_LOWSPEEDTIME}, {"asexplicit", no_argument, 0, OP_ASEXPLICIT}, {"arch", required_argument, 0, OP_ARCH}, {"print-format", required_argument, 0, OP_PRINTFORMAT},
On Tue, Aug 30, 2016 at 02:12:23PM +0200, Christian Hesse wrote:
From: Christian Hesse mail@eworm.de
Add LowSpeedLimit and LowSpeedTime configuration options to correspond to libcurl's CURLOPT_LOW_SPEED_LIMIT and CURLOPT_LOW_SPEED_TIME options. This allows, e.g., transfers behind corporate virus-scanning firewalls to survive the delays. Increasing the timeout may not be desirable in all situations; similarly, disabling the check prevents detection of disappearing networks.
Adds a utility function, parse_positive_long(), which yields a positive (but not unsigned) long integer from a null-terminated string using strtol(). A string containing anything but a base-10 number will cause the function to return -1.
Original-Patch-by: Andrew Hills ahills@ednos.net Signed-off-by: Christian Hesse mail@eworm.de
I'm going to restate my prior objection to this patch series, as presented:
https://lists.archlinux.org/pipermail/pacman-dev/2016-August/021283.html https://lists.archlinux.org/pipermail/pacman-dev/2016-September/021367.html
What you want is to provide an *interface*, not a 1:1 mapping of curl internals to public pacman options.
doc/pacman.conf.5.txt | 12 ++++++++++++ etc/pacman.conf.in | 4 ++++ lib/libalpm/alpm.h | 10 ++++++++++ lib/libalpm/dload.c | 4 ++-- lib/libalpm/handle.c | 32 ++++++++++++++++++++++++++++++++ lib/libalpm/handle.h | 9 +++++++++ src/pacman/conf.c | 24 ++++++++++++++++++++++++ src/pacman/conf.h | 4 ++++ src/pacman/util.c | 19 +++++++++++++++++++ src/pacman/util.h | 1 + 10 files changed, 117 insertions(+), 2 deletions(-)
diff --git a/doc/pacman.conf.5.txt b/doc/pacman.conf.5.txt index c665870..2346331 100644 --- a/doc/pacman.conf.5.txt +++ b/doc/pacman.conf.5.txt @@ -209,6 +209,18 @@ Options Displays name, version and size of target packages formatted as a table for upgrade, sync and remove operations.
+*LowSpeedLimit* = speed::
- Sets the speed in bytes per second that a download should be below during
- `LowSpeedTime` seconds to abort the transfer for being too slow. Setting
- 'speed' to 0 will disable the speed check. Defaults to 1 byte per second.
- Note that this option will not affect external programs specified by
- `XferCommand`.
+*LowSpeedTime* = time::
- Sets the time in seconds that a download should be below the `LowSpeedLimit`
- transfer speed to abort the transfer for being too slow. Setting 'time' to
- 0 will disable the speed check. Defaults to 10 seconds. Note that this
- option will not affect external programs specified by `XferCommand`.
Repository Sections
diff --git a/etc/pacman.conf.in b/etc/pacman.conf.in index 53071e5..ddc7397 100644 --- a/etc/pacman.conf.in +++ b/etc/pacman.conf.in @@ -29,6 +29,10 @@ Architecture = auto #NoUpgrade = #NoExtract =
+# Downloader options +#LowSpeedLimit = 1 +#LowSpeedTime = 10
# Misc options #UseSyslog #Color diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 168d71b..c338119 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -811,6 +811,16 @@ const char *alpm_option_get_dbpath(alpm_handle_t *handle); /** Get the name of the database lock file. Read-only. */ const char *alpm_option_get_lockfile(alpm_handle_t *handle);
+/** Returns the libcurl low speed limit in bytes per second. */ +long alpm_option_get_download_lowspeedlimit(alpm_handle_t *handle); +/** Sets the libcurl low speed limit in bytes per second. */ +int alpm_option_set_download_lowspeedlimit(alpm_handle_t *handle, long lowspeedlimit);
+/** Returns the libcurl low speed time in seconds. */ +long alpm_option_get_download_lowspeedtime(alpm_handle_t *handle); +/** Sets the libcurl low speed time in seconds. */ +int alpm_option_set_download_lowspeedtime(alpm_handle_t *handle, long lowspeedtime);
/** @name Accessors to the list of package cache directories.
- @{
*/ diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index f4e6a27..7490dd6 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -305,8 +305,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->download.lowspeedlimit);
- curl_easy_setopt(curl, CURLOPT_LOW_SPEED_TIME, handle->download.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.c b/lib/libalpm/handle.c index e9439a0..8cfe0a1 100644 --- a/lib/libalpm/handle.c +++ b/lib/libalpm/handle.c @@ -247,6 +247,18 @@ const char SYMEXPORT *alpm_option_get_lockfile(alpm_handle_t *handle) return handle->lockfile; }
+long SYMEXPORT alpm_option_get_download_lowspeedlimit(alpm_handle_t *handle) +{
- CHECK_HANDLE(handle, return -1);
- return handle->download.lowspeedlimit;
+}
+long SYMEXPORT alpm_option_get_download_lowspeedtime(alpm_handle_t *handle) +{
- CHECK_HANDLE(handle, return -1);
- return handle->download.lowspeedtime;
+}
const char SYMEXPORT *alpm_option_get_gpgdir(alpm_handle_t *handle) { CHECK_HANDLE(handle, return NULL); @@ -362,6 +374,26 @@ int SYMEXPORT alpm_option_set_progresscb(alpm_handle_t *handle, alpm_cb_progress return 0; }
+int SYMEXPORT alpm_option_set_download_lowspeedlimit(alpm_handle_t *handle, long lowspeedlimit) +{
- CHECK_HANDLE(handle, return -1);
- if(lowspeedlimit < 0) {
RET_ERR(handle, ALPM_ERR_WRONG_ARGS, -1);- }
- handle->download.lowspeedlimit = lowspeedlimit;
- return 0;
+}
+int SYMEXPORT alpm_option_set_download_lowspeedtime(alpm_handle_t *handle, long lowspeedtime) +{
- CHECK_HANDLE(handle, return -1);
- if(lowspeedtime < 0) {
RET_ERR(handle, ALPM_ERR_WRONG_ARGS, -1);- }
- handle->download.lowspeedtime = lowspeedtime;
- return 0;
+}
static char *canonicalize_path(const char *path) { char *new_path; diff --git a/lib/libalpm/handle.h b/lib/libalpm/handle.h index a1d0f9a..73fa584 100644 --- a/lib/libalpm/handle.h +++ b/lib/libalpm/handle.h @@ -50,6 +50,14 @@ do { \ } \ } while(0)
+#ifdef HAVE_LIBCURL +struct download {
- /* Curl timeouts */
- long lowspeedlimit;
- long lowspeedtime;
+}; +#endif
struct __alpm_handle_t { /* internal usage */ alpm_db_t *db_local; /* local db pointer */ @@ -60,6 +68,7 @@ struct __alpm_handle_t { #ifdef HAVE_LIBCURL /* libcurl handle */ CURL *curl; /* reusable curl_easy handle */
- struct download download;
#endif
#ifdef HAVE_LIBGPGME diff --git a/src/pacman/conf.c b/src/pacman/conf.c index 25de7af..21db633 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -122,6 +122,9 @@ config_t *config_new(void) newconfig->colstr.err = ""; newconfig->colstr.nocolor = "";
- newconfig->lowspeedlimit = 1L;
- newconfig->lowspeedtime = 10L;
- return newconfig;
}
@@ -604,6 +607,24 @@ static int _parse_options(const char *key, char *value, return 1; } FREELIST(values);
} else if(strcmp(key, "LowSpeedLimit") == 0) {config->lowspeedlimit = parse_positive_long(value);if(config->lowspeedlimit < 0) {pm_printf(ALPM_LOG_WARNING,_("config file %s, line %d: invalid %s, using default.\n"),file, linenum, "LowSpeedLimit");config->lowspeedlimit = 1L;}pm_printf(ALPM_LOG_DEBUG, "config: lowspeedlimit: %ld\n", config->lowspeedlimit);} else if(strcmp(key, "LowSpeedTime") == 0) {config->lowspeedtime = parse_positive_long(value);if(config->lowspeedtime < 0) {pm_printf(ALPM_LOG_WARNING,_("config file %s, line %d: invalid %s, using default.\n"),file, linenum, "LowSpeedTime");config->lowspeedtime = 10L;}pm_printf(ALPM_LOG_DEBUG, "config: lowspeedtime: %ld\n", config->lowspeedtime);@@ -734,6 +755,9 @@ static int setup_libalpm(void) alpm_option_set_questioncb(handle, cb_question); alpm_option_set_progresscb(handle, cb_progress);
- alpm_option_set_download_lowspeedlimit(handle, config->lowspeedlimit);
- alpm_option_set_download_lowspeedtime(handle, config->lowspeedtime);
- if(config->op == PM_OP_FILES) { alpm_option_set_dbext(handle, ".files"); }
diff --git a/src/pacman/conf.h b/src/pacman/conf.h index 2aba8bf..c61ed2a 100644 --- a/src/pacman/conf.h +++ b/src/pacman/conf.h @@ -131,6 +131,10 @@ typedef struct __config_t { /* Color strings for output */ colstr_t colstr;
- /* Curl timeouts */
- long lowspeedlimit;
- long lowspeedtime;
- alpm_list_t *repos;
} config_t;
diff --git a/src/pacman/util.c b/src/pacman/util.c index b979083..b1875bd 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -1620,6 +1620,25 @@ int noyes(const char *format, ...) return ret; }
+/* Attempts to extract a positive long from a string; failure yields -1. */ +long parse_positive_long(const char *str) +{
- char *end;
- long num;
- errno = 0;
- num = strtol(str, &end, 10);
- if(end == str || /* Not a decimal number */
*end != 0 || /* Extra characters */(num == LONG_MAX && errno == ERANGE) || /* Out of range */num < 0) { /* Out of range (negative) */return -1;- }
- return num;
+}
int colon_printf(const char *fmt, ...) { int ret; diff --git a/src/pacman/util.h b/src/pacman/util.h index f5e37c8..d3eb77f 100644 --- a/src/pacman/util.h +++ b/src/pacman/util.h @@ -75,6 +75,7 @@ int multiselect_question(char *array, int count); int colon_printf(const char *format, ...) __attribute__((format(printf, 1, 2))); int yesno(const char *format, ...) __attribute__((format(printf, 1, 2))); int noyes(const char *format, ...) __attribute__((format(printf, 1, 2))); +long parse_positive_long(const char *str);
int pm_printf(alpm_loglevel_t level, const char *format, ...) __attribute__((format(printf,2,3))); int pm_asprintf(char **string, const char *format, ...) __attribute__((format(printf,2,3))); -- 2.9.3
pacman-dev@lists.archlinux.org