[pacman-dev] [PATCH] libalpm: fix alpm_fetch_pkgurl with a fetch callback
Guillaume Benoit
guillaume at manjaro.org
Wed May 5 13:54:21 UTC 2021
Le 04/05/2021 à 01:30, Allan McRae a écrit :
> On 3/5/21 9:28 pm, Guillaume Benoit wrote:
>>
>> If I have time to work on another version of a patch that pass the full
>> payload to
>> the front-end, is there a chance that it could be included in pacman 6.0 ?
>
> There is a chance...
>
> The things going against me accepting it for 6.0 are I have called a
> freeze apart from already submitted patches and regressions. This is
> not a regression. I guess there will also be an API change.
>
> However, I am slow at reviewing the last patches I need to apply, so a
> patch that appears soon and I judge as having minimal risk for the
> internal download may be accepted.
>
> Allan
>
v2: I choosed to create another alpm_download_payload struct to only expose
required fields to the API, alpm_cb_fetch callback now has this struct as argument.
Those are the only API changes.
I also rewrote the download_with_xfercommand function in pacman code.
What is fixed:
- download from an url with a fetch callback for any front-end
- download from an standard url with pacman with a xfercommand
What is not fixed:
- download from an url, with pacman with an xfercommand, when this url doesn't
contain the filename like https://archlinux.org/packages/core/x86_64/pacman/download/
---
lib/libalpm/alpm.h | 30 ++++++-
lib/libalpm/dload.c | 36 ++++----
src/pacman/conf.c | 202 ++++++++++++++++++++++++++++++++------------
3 files changed, 193 insertions(+), 75 deletions(-)
diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
index 101d686b..48c94052 100644
--- a/lib/libalpm/alpm.h
+++ b/lib/libalpm/alpm.h
@@ -1203,6 +1203,28 @@ typedef struct _alpm_download_event_completed_t {
int result;
} alpm_download_event_completed_t;
+/** Infos on the file to download when using a fetch callback */
+typedef struct _alpm_dload_payload_t {
+ /** the path of the downloaded file, MUST BE SET by the front-end after a successful download */
+ char *destfile_name;
+ /** alpm provides either
+ * 1) fileurl - full URL to the file
+ * 2) pair of (servers, filepath), in this case front-end need to iterate over the
+ * server list and tries to download "$server/$filepath"
+ */
+ char *fileurl;
+ char *filepath;
+ alpm_list_t *servers;
+ /** if the download must be forced even if an up to date file already exists */
+ int force;
+ /** if the download can be resumed from a already partial download */
+ int allow_resume;
+ /** specifies if an accompanion *.sig file need to be downloaded */
+ int download_signature;
+ /** *.sig file is optional */
+ int signature_optional;
+} alpm_dload_payload_t;
+
/** Type of download progress callbacks.
* @param ctx user-provided context
* @param filename the name of the file being downloaded
@@ -1215,14 +1237,14 @@ typedef void (*alpm_cb_download)(void *ctx, const char *filename,
/** A callback for downloading files
* @param ctx user-provided context
- * @param url the URL of the file to be downloaded
+ * @param payload infos on the file to download
* @param localpath the directory to which the file should be downloaded
- * @param force whether to force an update, even if the file is the same
* @return 0 on success, 1 if the file exists and is identical, -1 on
* error.
+ * On success, the destfile_name field of the payload MUST BE SET by the front-end
*/
-typedef int (*alpm_cb_fetch)(void *ctx, const char *url, const char *localpath,
- int force);
+typedef int (*alpm_cb_fetch)(void *ctx, alpm_dload_payload_t *payload,
+ const char *localpath);
/* End of libalpm_cb */
/** @} */
diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
index 8fa46bb2..66954182 100644
--- a/lib/libalpm/dload.c
+++ b/lib/libalpm/dload.c
@@ -882,30 +882,30 @@ int _alpm_download(alpm_handle_t *handle,
int updated = 0;
for(p = payloads; p; p = p->next) {
struct dload_payload *payload = p->data;
- alpm_list_t *s;
- int ret = -1;
+ int ret;
+ struct _alpm_dload_payload_t *alpm_payload = NULL;
- if(payload->fileurl) {
- ret = handle->fetchcb(handle->fetchcb_ctx, payload->fileurl, localpath, payload->force);
- } else {
- for(s = payload->servers; s && ret == -1; s = s->next) {
- const char *server = s->data;
- char *fileurl;
-
- 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);
+ CALLOC(alpm_payload, 1, sizeof(*alpm_payload), RET_ERR(handle, ALPM_ERR_MEMORY, -1));
+ alpm_payload->fileurl = payload->fileurl;
+ alpm_payload->filepath = payload->filepath;
+ alpm_payload->servers = payload->servers;
+ alpm_payload->allow_resume = payload->allow_resume;
+ alpm_payload->download_signature = payload->download_signature;
+ alpm_payload->signature_optional = payload->signature_optional;
- ret = handle->fetchcb(handle->fetchcb_ctx, fileurl, localpath, payload->force);
- free(fileurl);
- }
- }
+ ret = handle->fetchcb(handle->fetchcb_ctx, alpm_payload, localpath);
if(ret == -1 && !payload->errors_ok) {
RET_ERR(handle, ALPM_ERR_EXTERNAL_DOWNLOAD, -1);
- } else if(ret == 0) {
- updated = 1;
+ } else {
+ /* needed by alpm_fetch_pkgurl */
+ payload->destfile_name = alpm_payload->destfile_name;
+ alpm_payload->destfile_name = NULL;
+ if(ret == 0) {
+ updated = 1;
+ }
}
+ FREE(alpm_payload);
}
return updated ? 0 : 1;
}
diff --git a/src/pacman/conf.c b/src/pacman/conf.c
index 12fee64c..445ca6fa 100644
--- a/src/pacman/conf.c
+++ b/src/pacman/conf.c
@@ -277,37 +277,12 @@ static int systemvp(const char *file, char *const argv[])
return ret;
}
-/** External fetch callback */
-static int download_with_xfercommand(void *ctx, const char *url,
- const char *localpath, int force)
+static int download_url_with_xfercommand(const char *url, const char *destfile)
{
- int ret = 0, retval;
- int usepart = 0;
- int cwdfd = -1;
- struct stat st;
- char *destfile, *tempfile, *filename;
const char **argv;
size_t i;
-
- (void)ctx;
-
- if(!config->xfercommand_argv) {
- return -1;
- }
-
- filename = get_filename(url);
- if(!filename) {
- return -1;
- }
- destfile = get_destfile(localpath, filename);
- tempfile = get_tempfile(localpath, filename);
-
- if(force && stat(tempfile, &st) == 0) {
- unlink(tempfile);
- }
- if(force && stat(destfile, &st) == 0) {
- unlink(destfile);
- }
+ int retval;
+ int ret = -1;
if((argv = calloc(config->xfercommand_argc + 1, sizeof(char*))) == NULL) {
size_t bytes = (config->xfercommand_argc + 1) * sizeof(char*);
@@ -316,35 +291,19 @@ static int download_with_xfercommand(void *ctx, const char *url,
"malloc failure: could not allocate %zu bytes\n",
bytes),
bytes);
- goto cleanup;
+ goto end;
}
for(i = 0; i <= config->xfercommand_argc; i++) {
const char *val = config->xfercommand_argv[i];
if(val && strcmp(val, "%o") == 0) {
- usepart = 1;
- val = tempfile;
+ val = destfile;
} else if(val && strcmp(val, "%u") == 0) {
val = url;
}
argv[i] = val;
}
- /* save the cwd so we can restore it later */
- do {
- cwdfd = open(".", O_RDONLY);
- } while(cwdfd == -1 && errno == EINTR);
- if(cwdfd < 0) {
- pm_printf(ALPM_LOG_ERROR, _("could not get current working directory\n"));
- }
-
- /* cwd to the download directory */
- if(chdir(localpath)) {
- pm_printf(ALPM_LOG_WARNING, _("could not chdir to download directory %s\n"), localpath);
- ret = -1;
- goto cleanup;
- }
-
if(config->logmask & ALPM_LOG_DEBUG) {
char *cmd = arg_to_string(config->xfercommand_argc, (char**)argv);
if(cmd) {
@@ -356,21 +315,159 @@ static int download_with_xfercommand(void *ctx, const char *url,
if(retval == -1) {
pm_printf(ALPM_LOG_WARNING, _("running XferCommand: fork failed!\n"));
- ret = -1;
} else if(retval != 0) {
/* download failed */
pm_printf(ALPM_LOG_DEBUG, "XferCommand command returned non-zero status "
"code (%d)\n", retval);
- ret = -1;
} else {
/* download was successful */
ret = 0;
- if(usepart) {
- if(rename(tempfile, destfile)) {
- pm_printf(ALPM_LOG_ERROR, _("could not rename %s to %s (%s)\n"),
- tempfile, destfile, strerror(errno));
+ }
+
+end:
+ free(argv);
+
+ return ret;
+}
+
+/** External fetch callback */
+static int download_with_xfercommand(void *ctx, alpm_dload_payload_t *payload,
+ const char *localpath)
+{
+ int ret = -1;
+ int cwdfd = -1;
+ struct stat st;
+ char *destfile, *tempfile, *filename;
+
+ (void)ctx;
+
+ if(!config->xfercommand_argv) {
+ return -1;
+ }
+
+ if(payload->fileurl) {
+ filename = get_filename(payload->fileurl);
+ } else {
+ filename = strdup(payload->filepath);
+ }
+ if(!filename) {
+ return -1;
+ }
+ destfile = get_destfile(localpath, filename);
+ tempfile = get_tempfile(localpath, filename);
+
+ if(payload->force && stat(tempfile, &st) == 0) {
+ unlink(tempfile);
+ }
+ if(payload->force && stat(destfile, &st) == 0) {
+ unlink(destfile);
+ }
+
+ /* save the cwd so we can restore it later */
+ do {
+ cwdfd = open(".", O_RDONLY);
+ } while(cwdfd == -1 && errno == EINTR);
+ if(cwdfd < 0) {
+ pm_printf(ALPM_LOG_ERROR, _("could not get current working directory\n"));
+ }
+
+ /* cwd to the download directory */
+ if(chdir(localpath)) {
+ pm_printf(ALPM_LOG_WARNING, _("could not chdir to download directory %s\n"), localpath);
+ goto cleanup;
+ }
+
+ if (payload->fileurl) {
+ ret = download_url_with_xfercommand(payload->fileurl, tempfile);
+ /* Let's check if client requested downloading accompanion *.sig file */
+ if(payload->download_signature) {
+ int sig_ret;
+ size_t sig_url_len;
+ size_t sig_dest_len;
+ char *sig_fileurl;
+ char *sig_destfile;
+
+ sig_url_len = strlen(payload->fileurl) + 5;
+ sig_fileurl = calloc(sig_url_len, sizeof(char));
+ snprintf(sig_fileurl, sig_url_len, "%s.sig", payload->fileurl);
+
+ /* Don't use a .part file for the signature */
+ sig_dest_len = strlen(destfile) + 5;
+ sig_destfile = calloc(sig_dest_len, sizeof(char));
+ snprintf(sig_destfile, sig_dest_len, "%s.sig", destfile);
+
+ if(payload->force && stat(sig_destfile, &st) == 0) {
+ unlink(sig_destfile);
+ }
+
+ sig_ret = download_url_with_xfercommand(sig_fileurl, sig_destfile);
+ if (sig_ret == -1 && !payload->signature_optional) {
ret = -1;
}
+ free(sig_fileurl);
+ free(sig_destfile);
+ }
+ } else if(payload->filepath) {
+ while(payload->servers) {
+ const char *server = payload->servers->data;
+ char *url;
+ size_t len;
+
+ len = strlen(server) + strlen(payload->filepath) + 2;
+ url = calloc(len, sizeof(char));
+ snprintf(url, len, "%s/%s", server, payload->filepath);
+
+ ret = download_url_with_xfercommand(url, tempfile);
+ free(url);
+
+ if (ret == -1) {
+ payload->servers = payload->servers->next;
+ } else {
+ /* Let's check if client requested downloading accompanion *.sig file */
+ if(payload->download_signature) {
+ int sig_ret;
+ size_t sig_url_len;
+ size_t sig_dest_len;
+ char *sig_fileurl;
+ char *sig_destfile;
+
+ /* Use url with current server */
+ sig_url_len = strlen(url) + 5;
+ sig_fileurl = calloc(sig_url_len, sizeof(char));
+ snprintf(sig_fileurl, sig_url_len, "%s.sig", url);
+
+ /* Don't use a .part file for the signature */
+ sig_dest_len = strlen(destfile) + 5;
+ sig_destfile = calloc(sig_dest_len, sizeof(char));
+ snprintf(sig_destfile, sig_dest_len, "%s.sig", destfile);
+
+ if(payload->force && stat(sig_destfile, &st) == 0) {
+ unlink(sig_destfile);
+ }
+
+ sig_ret = download_url_with_xfercommand(sig_fileurl, sig_destfile);
+ if (sig_ret == -1 && !payload->signature_optional) {
+ ret = -1;
+ }
+ free(sig_fileurl);
+ free(sig_destfile);
+ }
+ /* Break whatever the state of download signature is,
+ * the package download succeed and we don't to download the signature from another mirror.
+ * If the signature is required, it will failed later */
+ break;
+ }
+ }
+ }
+
+ if (ret == 0) {
+ if(rename(tempfile, destfile)) {
+ pm_printf(ALPM_LOG_ERROR, _("could not rename %s to %s (%s)\n"),
+ tempfile, destfile, strerror(errno));
+ ret = -1;
+ } else {
+ payload->destfile_name = destfile;
+ destfile = NULL;
}
}
@@ -390,7 +487,6 @@ cleanup:
}
free(destfile);
free(tempfile);
- free(argv);
return ret;
}
--
2.31.1
More information about the pacman-dev
mailing list