On 11/5/20 6:50 pm, Anatol Pomozov wrote: > Installing remote packages using its URL is an interesting case for ALPM > API. Unlike package sync ('pacman -S pkg1 pkg2') '-U' does not deal with > server mirror list. Thus _alpm_multi_download() should be able to > handle file download for payloads that either have 'fileurl' field > or pair of fields ('servers' and 'filepath') set. > > Signature for alpm_fetch_pkgurl() has changed and it accepts an > output list that is populated with filepaths to fetched packages. > > Signed-off-by: Anatol Pomozov <anatol.pomo...@gmail.com>
Thanks. Very minor comments belown. > --- > lib/libalpm/alpm.h | 11 ++- > lib/libalpm/dload.c | 204 +++++++++++++++++++++++++------------------ > lib/libalpm/dload.h | 7 +- > src/pacman/upgrade.c | 102 ++++++++++------------ > 4 files changed, 181 insertions(+), 143 deletions(-) > > diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h > index 3ea66ccc..c6a13273 100644 > --- a/lib/libalpm/alpm.h > +++ b/lib/libalpm/alpm.h > @@ -755,12 +755,15 @@ typedef void (*alpm_cb_totaldl)(off_t total); > typedef int (*alpm_cb_fetch)(const char *url, const char *localpath, > int force); > > -/** Fetch a remote pkg. > +/** Fetch a list of remote packages. > * @param handle the context handle > - * @param url URL of the package to download > - * @return the downloaded filepath on success, NULL on error > + * @param urls list of package URLs to download > + * @param fetched list of filepaths to the fetched packages, each item > + * corresponds to one in `urls` list Can you make it clear that this is filled by the function, and not something that should be passed. > + * @return 0 on success or -1 on failure > */ > -char *alpm_fetch_pkgurl(alpm_handle_t *handle, const char *url); > +int alpm_fetch_pkgurl(alpm_handle_t *handle, const alpm_list_t *urls, > + alpm_list_t **fetched); > > /** @addtogroup alpm_api_options Options > * Libalpm option getters and setters > diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c > index 13aa4086..43fe9847 100644 > --- a/lib/libalpm/dload.c > +++ b/lib/libalpm/dload.c > @@ -613,7 +613,9 @@ static int curl_multi_retry_next_server(CURLM *curlm, > CURL *curl, struct dload_p > size_t len; > alpm_handle_t *handle = payload->handle; > > - payload->servers = payload->servers->next; > + if(payload->servers) { > + payload->servers = payload->servers->next; > + } OK. > if(!payload->servers) { > _alpm_log(payload->handle, ALPM_LOG_DEBUG, > "%s: no more servers to retry\n", > payload->remote_name); > @@ -622,7 +624,7 @@ static int curl_multi_retry_next_server(CURLM *curlm, > CURL *curl, struct dload_p > server = payload->servers->data; > > /* regenerate a new fileurl */ > - free(payload->fileurl); > + FREE(payload->fileurl); Change unrelated to this patch, but OK. > len = strlen(server) + strlen(payload->filepath) + 2; > MALLOC(payload->fileurl, len, RET_ERR(handle, ALPM_ERR_MEMORY, -1)); > snprintf(payload->fileurl, len, "%s/%s", server, payload->filepath); > @@ -849,20 +851,28 @@ static int curl_multi_add_payload(alpm_handle_t > *handle, CURLM *curlm, > struct dload_payload *payload, const char *localpath) > { > size_t len; > - const char *server; > CURL *curl = NULL; > char hostname[HOSTNAME_SIZE]; > int ret = -1; > > - ASSERT(payload->servers, RET_ERR(handle, ALPM_ERR_SERVER_NONE, -1)); > - server = payload->servers->data; > - > curl = curl_easy_init(); > payload->curl = curl; > > - len = strlen(server) + strlen(payload->filepath) + 2; > - MALLOC(payload->fileurl, len, GOTO_ERR(handle, ALPM_ERR_MEMORY, > cleanup)); > - snprintf(payload->fileurl, len, "%s/%s", server, payload->filepath); > + if(payload->fileurl) { > + ASSERT(!payload->servers, GOTO_ERR(handle, ALPM_ERR_WRONG_ARGS, > cleanup)); > + ASSERT(!payload->filepath, GOTO_ERR(handle, > ALPM_ERR_WRONG_ARGS, cleanup)); OK. > + } else { > + const char *server; > + > + ASSERT(payload->servers, GOTO_ERR(handle, ALPM_ERR_SERVER_NONE, > cleanup)); > + ASSERT(payload->filepath, GOTO_ERR(handle, ALPM_ERR_WRONG_ARGS, > cleanup)); > + > + server = payload->servers->data; > + > + len = strlen(server) + strlen(payload->filepath) + 2; > + MALLOC(payload->fileurl, len, GOTO_ERR(handle, ALPM_ERR_MEMORY, > cleanup)); > + snprintf(payload->fileurl, len, "%s/%s", server, > payload->filepath); > + } > OK. > payload->tempfile_openmode = "wb"; > if(!payload->remote_name) { > @@ -1046,22 +1056,29 @@ int _alpm_multi_download(alpm_handle_t *handle, > alpm_list_t *s; > int success = 0; > > - for(s = payload->servers; s; s = s->next) { > - const char *server = s->data; > - char *fileurl; > - int ret; > - > - size_t len = strlen(server) + > strlen(payload->filepath) + 2; > - MALLOC(fileurl, len, RET_ERR(handle, > ALPM_ERR_MEMORY, -1)); > - snprintf(fileurl, len, "%s/%s", server, > payload->filepath); > - > - ret = handle->fetchcb(fileurl, localpath, > payload->force); > - free(fileurl); > - > - if (ret != -1) { > + if(payload->fileurl) { > + if (handle->fetchcb(payload->fileurl, > localpath, payload->force) != -1) { > success = 1; > break; > } > + } else { > + for(s = payload->servers; s; s = s->next) { > + const char *server = s->data; > + char *fileurl; > + int ret; > + > + size_t len = strlen(server) + > strlen(payload->filepath) + 2; > + MALLOC(fileurl, len, RET_ERR(handle, > ALPM_ERR_MEMORY, -1)); > + snprintf(fileurl, len, "%s/%s", server, > payload->filepath); > + > + ret = handle->fetchcb(fileurl, > localpath, payload->force); > + free(fileurl); > + > + if (ret != -1) { > + success = 1; > + break; > + } > + } > } OK. > if(!success && !payload->errors_ok) { > RET_ERR(handle, ALPM_ERR_EXTERNAL_DOWNLOAD, -1); > @@ -1087,82 +1104,101 @@ static char *filecache_find_url(alpm_handle_t > *handle, const char *url) > return _alpm_filecache_find(handle, filebase); > } > > -char SYMEXPORT *alpm_fetch_pkgurl(alpm_handle_t *handle, const char *url) > +int SYMEXPORT alpm_fetch_pkgurl(alpm_handle_t *handle, const alpm_list_t > *urls, > + alpm_list_t **fetched) > { > - char *filepath; > - const char *cachedir, *final_pkg_url = NULL; > - char *final_file = NULL; > - struct dload_payload payload = {0}; > - int ret = 0; > + const char *cachedir; > + alpm_list_t *payloads = NULL; > + int download_sigs; > + const alpm_list_t *i; > + alpm_event_t event; > > - CHECK_HANDLE(handle, return NULL); > - ASSERT(url, RET_ERR(handle, ALPM_ERR_WRONG_ARGS, NULL)); > + CHECK_HANDLE(handle, return -1); > + download_sigs = handle->siglevel & ALPM_SIG_PACKAGE; > Should we check *fetched is NULL? > /* find a valid cache dir to download to */ > cachedir = _alpm_filecache_setup(handle); > > - /* attempt to find the file in our pkgcache */ > - filepath = filecache_find_url(handle, url); > - if(filepath == NULL) { > - STRDUP(payload.fileurl, url, RET_ERR(handle, ALPM_ERR_MEMORY, > NULL)); > - payload.allow_resume = 1; > - payload.handle = handle; > - payload.trust_remote_name = 1; > - > - /* download the file */ > - ret = _alpm_download(&payload, cachedir, &final_file, > &final_pkg_url); > - _alpm_dload_payload_reset(&payload); > - if(ret == -1) { > - _alpm_log(handle, ALPM_LOG_WARNING, _("failed to > download %s\n"), url); > - free(final_file); > - return NULL; > + for(i = urls; i; i = i->next) { > + char *url = i->data; > + > + /* attempt to find the file in our pkgcache */ > + char *filepath = filecache_find_url(handle, url); > + if(filepath) { > + /* the file is locally cached so add it to the output > right away */ > + alpm_list_append(fetched, filepath); > + } else { > + struct dload_payload *payload = NULL; > + > + ASSERT(url, GOTO_ERR(handle, ALPM_ERR_WRONG_ARGS, err)); > + CALLOC(payload, 1, sizeof(*payload), GOTO_ERR(handle, > ALPM_ERR_MEMORY, err)); > + STRDUP(payload->fileurl, url, FREE(payload); > GOTO_ERR(handle, ALPM_ERR_MEMORY, err)); > + payload->allow_resume = 1; > + payload->handle = handle; > + payload->trust_remote_name = 1; > + payloads = alpm_list_add(payloads, payload); > + > + if(download_sigs) { > + int len = strlen(url) + 5; > + CALLOC(payload, 1, sizeof(*payload), > GOTO_ERR(handle, ALPM_ERR_MEMORY, err)); > + payload->signature = 1; > + MALLOC(payload->fileurl, len, FREE(payload); > GOTO_ERR(handle, ALPM_ERR_MEMORY, err)); > + snprintf(payload->fileurl, len, "%s.sig", url); > + payload->handle = handle; > + payload->trust_remote_name = 1; > + payload->errors_ok = (handle->siglevel & > ALPM_SIG_PACKAGE_OPTIONAL); > + /* set hard upper limit of 16KiB */ > + payload->max_size = 16 * 1024; > + payloads = alpm_list_add(payloads, payload); > + } OK (realising this will be updated in a couple of patches time to handle redircting stuff) > } > - _alpm_log(handle, ALPM_LOG_DEBUG, "successfully downloaded > %s\n", url); > } > > - /* attempt to download the signature */ > - if(ret == 0 && final_pkg_url && (handle->siglevel & ALPM_SIG_PACKAGE)) { > - char *sig_filepath, *sig_final_file = NULL; > - size_t len; > - > - len = strlen(final_pkg_url) + 5; > - MALLOC(payload.fileurl, len, free(final_file); RET_ERR(handle, > ALPM_ERR_MEMORY, NULL)); > - snprintf(payload.fileurl, len, "%s.sig", final_pkg_url); > - > - sig_filepath = filecache_find_url(handle, payload.fileurl); > - if(sig_filepath == NULL) { > - payload.signature = 1; > - payload.handle = handle; > - payload.trust_remote_name = 1; > - payload.force = 1; > - payload.errors_ok = (handle->siglevel & > ALPM_SIG_PACKAGE_OPTIONAL); > - > - /* set hard upper limit of 16KiB */ > - payload.max_size = 16 * 1024; > - > - ret = _alpm_download(&payload, cachedir, > &sig_final_file, NULL); > - if(ret == -1 && !payload.errors_ok) { > - _alpm_log(handle, ALPM_LOG_WARNING, > - _("failed to download %s\n"), > payload.fileurl); > - /* Warn now, but don't return NULL. We will > fail later during package > - * load time. */ > - } else if(ret == 0) { > - _alpm_log(handle, ALPM_LOG_DEBUG, > - "successfully downloaded %s\n", > payload.fileurl); OK. > + if(payloads) { > + event.type = ALPM_EVENT_PKG_RETRIEVE_START; > + EVENT(handle, &event); > + if(_alpm_multi_download(handle, payloads, cachedir) == -1) { > + _alpm_log(handle, ALPM_LOG_WARNING, _("failed to > retrieve some files\n")); > + event.type = ALPM_EVENT_PKG_RETRIEVE_FAILED; > + EVENT(handle, &event); > + > + GOTO_ERR(handle, ALPM_ERR_RETRIEVE, err); > + } else { > + event.type = ALPM_EVENT_PKG_RETRIEVE_DONE; > + EVENT(handle, &event); > + } > + > + for(i = payloads; i; i = i->next) { > + struct dload_payload *payload = i->data; > + const char *filename; > + char *filepath; > + > + if(payload->signature) { > + continue; > + } > + > + filename = mbasename(payload->destfile_name); > + filepath = _alpm_filecache_find(handle, filename); > + if(filepath) { > + alpm_list_append(fetched, filepath); > + } else { > + _alpm_log(handle, ALPM_LOG_WARNING, _("download > completed successfully but no file in the cache\n")); > + GOTO_ERR(handle, ALPM_ERR_RETRIEVE, err); > } > - FREE(sig_final_file); > } OK. > - free(sig_filepath); > - _alpm_dload_payload_reset(&payload); > - } > > - /* we should be able to find the file the second time around */ > - if(filepath == NULL) { > - filepath = _alpm_filecache_find(handle, final_file); > + alpm_list_free_inner(payloads, > (alpm_list_fn_free)_alpm_dload_payload_reset); > + FREELIST(payloads); > } > - free(final_file); > > - return filepath; > + return 0; > + > +err: > + alpm_list_free_inner(payloads, > (alpm_list_fn_free)_alpm_dload_payload_reset); > + FREELIST(payloads); > + FREELIST(*fetched); > + > + return -1; > } > OK. > void _alpm_dload_payload_reset(struct dload_payload *payload) > diff --git a/lib/libalpm/dload.h b/lib/libalpm/dload.h > index 1e4af755..f119fc2e 100644 > --- a/lib/libalpm/dload.h > +++ b/lib/libalpm/dload.h > @@ -30,9 +30,14 @@ struct dload_payload { > char *tempfile_name; > char *destfile_name; > char *content_disp_name; > + /* client has to provide either > + * 1) fileurl - full URL to the file > + * 2) pair of (servers, filepath), in this case ALPM iterates over the > + * server list and tries to download "$server/$filepath" > + */ OK. > char *fileurl; > - char *filepath; /* download URL path */ > alpm_list_t *servers; > + char *filepath; /* download URL path */ Random churn. > long respcode; > off_t initial_size; > off_t max_size; > diff --git a/src/pacman/upgrade.c b/src/pacman/upgrade.c > index 5f984e44..840e3a31 100644 > --- a/src/pacman/upgrade.c > +++ b/src/pacman/upgrade.c > @@ -30,6 +30,34 @@ > #include "conf.h" > #include "util.h" > > +/* add targets to the created transaction */ > +static int load_packages(alpm_list_t *targets, int siglevel) > +{ > + alpm_list_t *i; > + int retval = 0; > + > + for(i = targets; i; i = alpm_list_next(i)) { > + const char *targ = i->data; > + alpm_pkg_t *pkg; > + > + if(alpm_pkg_load(config->handle, targ, 1, siglevel, &pkg) != 0) > { > + pm_printf(ALPM_LOG_ERROR, "'%s': %s\n", > + targ, > alpm_strerror(alpm_errno(config->handle))); > + retval = 1; > + continue; > + } > + if(alpm_add_pkg(config->handle, pkg) == -1) { > + pm_printf(ALPM_LOG_ERROR, "'%s': %s\n", > + targ, > alpm_strerror(alpm_errno(config->handle))); > + alpm_pkg_free(pkg); > + retval = 1; > + continue; > + } > + config->explicit_adds = alpm_list_add(config->explicit_adds, > pkg); > + } > + return retval; > +} > + > /** > * @brief Upgrade a specified list of packages. > * > @@ -39,43 +67,30 @@ > */ > int pacman_upgrade(alpm_list_t *targets) > { > - int retval = 0, *file_is_remote; > + int retval = 0; > + alpm_list_t *remote_targets = NULL, *fetched_files = NULL; > + alpm_list_t *local_targets = NULL; > alpm_list_t *i; > - unsigned int n, num_targets; > > if(targets == NULL) { > pm_printf(ALPM_LOG_ERROR, _("no targets specified (use -h for > help)\n")); > return 1; > } > > - num_targets = alpm_list_count(targets); > - > - /* Check for URL targets and process them */ > - file_is_remote = malloc(num_targets * sizeof(int)); > - if(file_is_remote == NULL) { > - pm_printf(ALPM_LOG_ERROR, _("memory exhausted\n")); > - return 1; > - } > - > - for(i = targets, n = 0; i; i = alpm_list_next(i), n++) { > + /* carve out remote targets and move it into a separate list */ > + for(i = targets; i; i = alpm_list_next(i)) { > if(strstr(i->data, "://")) { > - char *str = alpm_fetch_pkgurl(config->handle, i->data); > - if(str == NULL) { > - pm_printf(ALPM_LOG_ERROR, "'%s': %s\n", > - (char *)i->data, > alpm_strerror(alpm_errno(config->handle))); > - retval = 1; > - } else { > - free(i->data); > - i->data = str; > - file_is_remote[n] = 1; > - } > + remote_targets = alpm_list_add(remote_targets, i->data); > } else { > - file_is_remote[n] = 0; > + local_targets = alpm_list_add(local_targets, i->data); > } > } > > - if(retval) { > - goto fail_free; > + if(remote_targets) { > + retval = alpm_fetch_pkgurl(config->handle, remote_targets, > &fetched_files); > + if(retval) { > + goto fail_free; > + } > } > OK. > /* Step 1: create a new transaction */ > @@ -85,39 +100,16 @@ int pacman_upgrade(alpm_list_t *targets) > } > > printf(_("loading packages...\n")); > - /* add targets to the created transaction */ > - for(i = targets, n = 0; i; i = alpm_list_next(i), n++) { > - const char *targ = i->data; > - alpm_pkg_t *pkg; > - int siglevel; > - > - if(file_is_remote[n]) { > - siglevel = > alpm_option_get_remote_file_siglevel(config->handle); > - } else { > - siglevel = > alpm_option_get_local_file_siglevel(config->handle); > - } > - > - if(alpm_pkg_load(config->handle, targ, 1, siglevel, &pkg) != 0) > { > - pm_printf(ALPM_LOG_ERROR, "'%s': %s\n", > - targ, > alpm_strerror(alpm_errno(config->handle))); > - retval = 1; > - continue; > - } > - if(alpm_add_pkg(config->handle, pkg) == -1) { > - pm_printf(ALPM_LOG_ERROR, "'%s': %s\n", > - targ, > alpm_strerror(alpm_errno(config->handle))); > - alpm_pkg_free(pkg); > - retval = 1; > - continue; > - } > - config->explicit_adds = alpm_list_add(config->explicit_adds, > pkg); > - } > + retval |= load_packages(local_targets, > alpm_option_get_local_file_siglevel(config->handle)); > + retval |= load_packages(fetched_files, > alpm_option_get_remote_file_siglevel(config->handle)); > OK. > if(retval) { > goto fail_release; > } > > - free(file_is_remote); > + alpm_list_free(remote_targets); > + alpm_list_free(local_targets); > + FREELIST(fetched_files); > > /* now that targets are resolved, we can hand it all off to the sync > code */ > return sync_prepare_execute(); > @@ -125,7 +117,9 @@ int pacman_upgrade(alpm_list_t *targets) > fail_release: > trans_release(); > fail_free: > - free(file_is_remote); > + alpm_list_free(remote_targets); > + alpm_list_free(local_targets); > + FREELIST(fetched_files); > > return retval; > } > OK.