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

Reply via email to