[pacman-dev] [PATCH 1/3] pacman-conf: add support for new ParallelDownloads config option
This was forgotten in the initial implementation, so it was impossible to figure out the value from a script, or correctly roundtrip the config file. Signed-off-by: Eli Schwartz <eschwartz@archlinux.org> --- src/pacman/pacman-conf.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/pacman/pacman-conf.c b/src/pacman/pacman-conf.c index 60739bf6..d76c0985 100644 --- a/src/pacman/pacman-conf.c +++ b/src/pacman/pacman-conf.c @@ -142,6 +142,14 @@ static void show_str(const char *directive, const char *val) printf("%s%c", val, sep); } +static void show_int(const char *directive, unsigned int val) +{ + if (verbose) { + printf("%s = ", directive); + } + printf("%u%c", val, sep); +} + static void show_list_str(const char *directive, alpm_list_t *list) { alpm_list_t *i; @@ -261,6 +269,8 @@ static void dump_config(void) show_bool("ILoveCandy", config->chomp); show_bool("NoProgressBar", config->noprogressbar); + show_int("ParallelDownloads", config->parallel_downloads); + show_cleanmethod("CleanMethod", config->cleanmethod); show_siglevel("SigLevel", config->siglevel, 0); @@ -372,6 +382,9 @@ static int list_directives(void) } else if(strcasecmp(i->data, "NoProgressBar") == 0) { show_bool("NoProgressBar", config->noprogressbar); + } else if(strcasecmp(i->data, "ParallelDownloads") == 0) { + show_int("ParallelDownloads", config->parallel_downloads); + } else if(strcasecmp(i->data, "CleanMethod") == 0) { show_cleanmethod("CleanMethod", config->cleanmethod); -- 2.26.2
This is not a warning, _parse_options() returns failure without even parsing further lines and the attempted pacman/pacman-conf program execution immediately aborts. Warnings are for when e.g. later on if we don't recognize a setting at all, we skip over it and have enough confidence in this to continue executing the program. The current implementation results in pacman-conf aborting with: warning: config file /etc/pacman.conf, line 60: invalid value for 'ParallelDownloads' : '2.5' error parsing '/etc/pacman.conf' or pacman -Syu aborting with the entirely more cryptic: warning: config file /etc/pacman.conf, line 59: invalid value for 'ParallelDownloads' : '2.5' and this isn't just a problem for the newly added ParallelDownloads setting, either, you could get the same problem if you specified a broken XferCommand, but that's harder as it's more accepting of input and you probably don't hit this except with unbalanced quotes. Signed-off-by: Eli Schwartz <eschwartz@archlinux.org> --- src/pacman/conf.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/pacman/conf.c b/src/pacman/conf.c index ac5a5329..3a3ef605 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -665,7 +665,7 @@ static int _parse_options(const char *key, char *value, } else if(strcmp(key, "XferCommand") == 0) { char **c; if((config->xfercommand_argv = wordsplit(value)) == NULL) { - pm_printf(ALPM_LOG_WARNING, + pm_printf(ALPM_LOG_ERROR, _("config file %s, line %d: invalid value for '%s' : '%s'\n"), file, linenum, "XferCommand", value); return 1; @@ -717,21 +717,21 @@ static int _parse_options(const char *key, char *value, err = parse_number(value, &number); if(err) { - pm_printf(ALPM_LOG_WARNING, + pm_printf(ALPM_LOG_ERROR, _("config file %s, line %d: invalid value for '%s' : '%s'\n"), file, linenum, "ParallelDownloads", value); return 1; } if(number < 1) { - pm_printf(ALPM_LOG_WARNING, + pm_printf(ALPM_LOG_ERROR, _("config file %s, line %d: value for '%s' has to be positive : '%s'\n"), file, linenum, "ParallelDownloads", value); return 1; } if(number > INT_MAX) { - pm_printf(ALPM_LOG_WARNING, + pm_printf(ALPM_LOG_ERROR, _("config file %s, line %d: value for '%s' is too large : '%s'\n"), file, linenum, "ParallelDownloads", value); return 1; -- 2.26.2
On 10/5/20 2:32 pm, Eli Schwartz wrote:
Is this the fix we want? I think it would be better to get to the end of parsing the conf file spitting errors as we go so that all issues are printed before exiting. Allan
On 5/10/20 4:09 AM, Allan McRae wrote:
Maybe? All I have done thus far is change the log level. To collect multiple errors I guess we'd need to stop aborting in ini.c:parse_ini the first time one callback returns an error. It's currently documented with: @note Parsing will immediately stop if the callback returns non-zero. -- Eli Schwartz Bug Wrangler and Trusted User
On 10/5/20 6:22 pm, Eli Schwartz wrote:
OK - I'll take your patch as is, and we may want to consider that later. We also may have issues continuing to parse after a failure. A
This was only partially implemented in the original implementation. `pacman-conf | grep ILoveCandy` would tell you if it was set, but querying directly by name would not. Signed-off-by: Eli Schwartz <eschwartz@archlinux.org> --- src/pacman/pacman-conf.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/pacman/pacman-conf.c b/src/pacman/pacman-conf.c index d76c0985..463badf1 100644 --- a/src/pacman/pacman-conf.c +++ b/src/pacman/pacman-conf.c @@ -379,6 +379,8 @@ static int list_directives(void) show_bool("VerbosePkgLists", config->verbosepkglists); } else if(strcasecmp(i->data, "DisableDownloadTimeout") == 0) { show_bool("DisableDownloadTimeout", config->disable_dl_timeout); + } else if(strcasecmp(i->data, "ILoveCandy") == 0) { + show_bool("ILoveCandy", config->chomp); } else if(strcasecmp(i->data, "NoProgressBar") == 0) { show_bool("NoProgressBar", config->noprogressbar); -- 2.26.2
participants (2)
-
Allan McRae
-
Eli Schwartz