Re: [pacman-dev] [PATCH v2] Swap alpm_db_update() implementation to multiplexed version
On 12/5/20 12:27 am, guilla...@manjaro.org wrote: > Hi Allan, Hi Anatol, > > I was looking to this new multiplexed implementation and I found some > problems: > 1) This implementation can lead to download databases from different > servers, I think it can't be a problem if they are not synced As I said elsewhere, this could happen in the single download code too. So not a new issue. This could be fixed (but note not all repos have the same server - only those provided by a distro), but it is beyond this patch. > 2) Differents download payloads are created for databases and its > signature files, this can this time lead to download the database and > its signature from differents server, I'm sure it's not wanted. Signature files coming from different servers is fine. You probably have that all the time with source code downloaded from places with various load balancing. Signature files for servers in different sync states is an issue - only for databases, packages are fine. > 3) Because signature files have their own download payloads, if the > signature is missing on a the first server (e.g. database signature is > optional), all next servers will be tried and all will give the same error. This is an issue. And I guess the fix would fix #2 too. It would be good to open a bug for this. > I really wanted to suggest a patch but I didn't find a easy way > (understand a small patch) to fix it and I would like your opinion on > point 1) first. > > Regards, > Guillaume. > .
Re: [pacman-dev] [PATCH v2] Swap alpm_db_update() implementation to multiplexed version
On 12/5/20 3:14 am, guilla...@manjaro.org wrote: > Le 2020-05-11 17:10, Eli Schwartz a écrit : >> On 5/11/20 10:27 AM, guilla...@manjaro.org wrote: >>> Hi Allan, Hi Anatol, >>> >>> I was looking to this new multiplexed implementation and I found some >>> problems: >>> 1) This implementation can lead to download databases from different >>> servers, I think it can't be a problem if they are not synced >> >> This should already be the case in the single-download version too, no? >> > > I agree and nothing can be done except ensuring all databases are > downloaded > from the same server. > Except... those database for repos that are hosted on different servers. Because not all repos are provided by the distro. As this was the case in single download code, I don't consider this a bug in the new code. Allan
Re: [pacman-dev] [PATCH v2] Swap alpm_db_update() implementation to multiplexed version
Hello Guillaume On Mon, May 11, 2020 at 7:51 AM wrote: > > Hi Allan, Hi Anatol, > > I was looking to this new multiplexed implementation and I found some > problems: > 1) This implementation can lead to download databases from different > servers, I think it can't be a problem if they are not synced Database files are downloaded independently. There is no any grouping or tracking where did each file come from. It is true both for current (multiplexed) and for the previous (sequential) downloader implementation. Thus there was and is always a possibility that databases can come from different mirrors e.g. in case of a flaky server. > 2) Differents download payloads are created for databases and its > signature files, this can this time lead to download the database and > its signature from differents server, I'm sure it's not wanted. > 3) Because signature files have their own download payloads, if the > signature is missing on a the first server (e.g. database signature is > optional), all next servers will be tried and all will give the same > error. > > I really wanted to suggest a patch but I didn't find a easy way > (understand a small patch) to fix it and I would like your opinion on > point 1) first. > > Regards, > Guillaume.
[pacman-dev] [PATCH] Cleanup the old sequential download code
All users of _alpm_download() have been refactored to the new API.
It is time to remove the old _alpm_download() functionality now.
This change also removes obsolete SIGPIPE signal handler functionality
(this is a leftover from libfetch days).
Signed-off-by: Anatol Pomozov
---
lib/libalpm/dload.c | 323 +---
lib/libalpm/dload.h | 4 -
2 files changed, 3 insertions(+), 324 deletions(-)
diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
index 43fe9847..4dbb011f 100644
--- a/lib/libalpm/dload.c
+++ b/lib/libalpm/dload.c
@@ -78,10 +78,6 @@ enum {
};
static int dload_interrupted;
-static void inthandler(int UNUSED signum)
-{
- dload_interrupted = ABORT_SIGINT;
-}
static int dload_progress_cb(void *file, curl_off_t dltotal, curl_off_t dlnow,
curl_off_t UNUSED ultotal, curl_off_t UNUSED ulnow)
@@ -236,8 +232,7 @@ static size_t dload_parseheader_cb(void *ptr, size_t size,
size_t nmemb, void *u
return realsize;
}
-static void curl_set_handle_opts(struct dload_payload *payload,
- CURL *curl, char *error_buffer)
+static void curl_set_handle_opts(CURL *curl, struct dload_payload *payload)
{
alpm_handle_t *handle = payload->handle;
const char *useragent = getenv("HTTP_USER_AGENT");
@@ -247,7 +242,7 @@ static void curl_set_handle_opts(struct dload_payload
*payload,
* to reset the handle's parameters for each time it's used. */
curl_easy_reset(curl);
curl_easy_setopt(curl, CURLOPT_URL, payload->fileurl);
- curl_easy_setopt(curl, CURLOPT_ERRORBUFFER, error_buffer);
+ curl_easy_setopt(curl, CURLOPT_ERRORBUFFER, payload->error_buffer);
curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT, 10L);
curl_easy_setopt(curl, CURLOPT_MAXREDIRS, 10L);
curl_easy_setopt(curl, CURLOPT_FILETIME, 1L);
@@ -301,24 +296,6 @@ static void curl_set_handle_opts(struct dload_payload
*payload,
}
}
-static void mask_signal(int signum, void (*handler)(int),
- struct sigaction *origaction)
-{
- struct sigaction newaction;
-
- newaction.sa_handler = handler;
- sigemptyset(_mask);
- newaction.sa_flags = 0;
-
- sigaction(signum, NULL, origaction);
- sigaction(signum, , NULL);
-}
-
-static void unmask_signal(int signum, struct sigaction *sa)
-{
- sigaction(signum, sa, NULL);
-}
-
static FILE *create_tempfile(struct dload_payload *payload, const char
*localpath)
{
int fd;
@@ -353,259 +330,6 @@ static FILE *create_tempfile(struct dload_payload
*payload, const char *localpat
/* RFC1123 states applications should support this length */
#define HOSTNAME_SIZE 256
-static int curl_download_internal(struct dload_payload *payload,
- const char *localpath, char **final_file, const char
**final_url)
-{
- int ret = -1;
- FILE *localf = NULL;
- char *effective_url;
- char hostname[HOSTNAME_SIZE];
- char error_buffer[CURL_ERROR_SIZE] = {0};
- struct stat st;
- long timecond, remote_time = -1;
- double remote_size, bytes_dl;
- struct sigaction orig_sig_pipe, orig_sig_int;
- CURLcode curlerr;
- alpm_download_event_init_t init_cb_data = {0};
- alpm_download_event_completed_t completed_cb_data = {0};
- /* shortcut to our handle within the payload */
- alpm_handle_t *handle = payload->handle;
- CURL *curl = curl_easy_init();
- handle->pm_errno = ALPM_ERR_OK;
- payload->curl = curl;
-
- /* make sure these are NULL */
- FREE(payload->tempfile_name);
- FREE(payload->destfile_name);
- FREE(payload->content_disp_name);
-
- payload->tempfile_openmode = "wb";
- if(!payload->remote_name) {
- STRDUP(payload->remote_name, get_filename(payload->fileurl),
- RET_ERR(handle, ALPM_ERR_MEMORY, -1));
- }
- if(curl_gethost(payload->fileurl, hostname, sizeof(hostname)) != 0) {
- _alpm_log(handle, ALPM_LOG_ERROR, _("url '%s' is invalid\n"),
payload->fileurl);
- RET_ERR(handle, ALPM_ERR_SERVER_BAD_URL, -1);
- }
-
- if(payload->remote_name && strlen(payload->remote_name) > 0 &&
- strcmp(payload->remote_name, ".sig") != 0) {
- payload->destfile_name = get_fullpath(localpath,
payload->remote_name, "");
- payload->tempfile_name = get_fullpath(localpath,
payload->remote_name, ".part");
- if(!payload->destfile_name || !payload->tempfile_name) {
- goto cleanup;
- }
- } else {
- /* URL doesn't contain a filename, so make a tempfile. We can't
support
-* resuming this kind of download; partial transfers will be
destroyed */
- payload->unlink_on_fail = 1;
-
- localf = create_tempfile(payload, localpath);
- if(localf ==
Re: [pacman-dev] [PATCH v2] Swap alpm_db_update() implementation to multiplexed version
On 5/11/20 10:27 AM, guilla...@manjaro.org wrote: > Hi Allan, Hi Anatol, > > I was looking to this new multiplexed implementation and I found some > problems: > 1) This implementation can lead to download databases from different > servers, I think it can't be a problem if they are not synced This should already be the case in the single-download version too, no? > 2) Differents download payloads are created for databases and its > signature files, this can this time lead to download the database and > its signature from differents server, I'm sure it's not wanted. > 3) Because signature files have their own download payloads, if the > signature is missing on a the first server (e.g. database signature is > optional), all next servers will be tried and all will give the same error. re 2) I cannot remember offhand if this was already the case, but anyway it's not impossible for one to download the database successfully from one mirror, then get a sudden network issue on the signature. Falling back to a second server for optional signatures will just lead to download errors (which are ignored for sigs), and for required signatures it will fail the signature check the same way failure to download the signature at all would fail, because one way or another we *need* an updated signature. re both 2) and 3) I guess this could be optimized in the database download handling to abort on the first 404 but fall through to additional servers for connection failures. I'm not sure whether this would be a good thing... how would we detect mirrors that can be connected to, but no longer host any pacman repos? Checking 5 mirrors for 404 errors isn't much of an issue. I guess it could also solve this error I got: error: failed retrieving file 'core.db.sig' from mirror.epiphyte.network : Could not resolve host: mirror.epiphyte.network error: failed retrieving file 'testing.db.sig' from mirror.epiphyte.network : Could not resolve host: mirror.epiphyte.network error: failed retrieving file 'extra.db.sig' from mirror.epiphyte.network : Could not resolve host: mirror.epiphyte.network error: failed retrieving file 'community-testing.db.sig' from mirror.epiphyte.network : Could not resolve host: mirror.epiphyte.network error: failed retrieving file 'community.db.sig' from mirror.epiphyte.network : Could not resolve host: mirror.epiphyte.network error: failed retrieving file 'multilib.db.sig' from mirror.epiphyte.network : Could not resolve host: mirror.epiphyte.network error: failed retrieving file 'multilib-testing.db.sig' from mirror.epiphyte.network : Could not resolve host: mirror.epiphyte.network But I'd already solved this one by removing the dead mirror from the bottom of my mirrorlist (it used to be a pretty good fallback one). When all mirrors fail in the more normal manner at retrieving a signature, there's no UI results. > I really wanted to suggest a patch but I didn't find a easy way > (understand a small patch) to fix it and I would like your opinion on > point 1) first. > > Regards, > Guillaume. -- Eli Schwartz Bug Wrangler and Trusted User signature.asc Description: OpenPGP digital signature
Re: [pacman-dev] [PATCH v2] Swap alpm_db_update() implementation to multiplexed version
Hi Allan, Hi Anatol, I was looking to this new multiplexed implementation and I found some problems: 1) This implementation can lead to download databases from different servers, I think it can't be a problem if they are not synced 2) Differents download payloads are created for databases and its signature files, this can this time lead to download the database and its signature from differents server, I'm sure it's not wanted. 3) Because signature files have their own download payloads, if the signature is missing on a the first server (e.g. database signature is optional), all next servers will be tried and all will give the same error. I really wanted to suggest a patch but I didn't find a easy way (understand a small patch) to fix it and I would like your opinion on point 1) first. Regards, Guillaume.
Re: [pacman-dev] [PATCH] Use absolute path for pacman and pacman-conf in makepkg
On 5/11/20 9:01 AM, wwijs...@live.nl wrote: > Right now an unpatched pacman almost works for my use case, but there > still is one issue with building it. Currently ninja will install the > bash-completion scripts system wide, regardless of which user runs > ninja install or what the prefix is. Only if bash-completion is not > available, will the scripts be installed inside the prefix. The code > which makes this happen is here: > https://git.archlinux.org/pacman.git/tree/meson.build#n46 > > This is why I submitted the patch which makes bash-completion optional. > It may need a different solution, though. Would you take a patch which > adds an option to install the bash-completion scripts inside the > prefix? I'd prefer not to maintain any private patches if that's > possible. https://git.archlinux.org/users/eschwartz/pacman.git/commit/?h=queue2=b4e3e1c7d93463dffdbb63ff78008df0f57780db It's simple enough to do, I've just been trying to decide whether I think it is the right solution before proposing it. -- Eli Schwartz Bug Wrangler and Trusted User signature.asc Description: OpenPGP digital signature
Re: [pacman-dev] [PATCH] Use absolute path for pacman and pacman-conf in makepkg
On Mon, 2020-05-11 at 12:42 +1000, Allan McRae wrote: > On 8/5/20 4:13 am, Wouter Wijsman wrote: > > Currently makepkg requires pacman and pacman-conf to be in the path > > of > > the user. Since these executables should never move, it should be > > safe > > to add the full paths to the scripts at compile time, assuming the > > user > > uses the install command as well. > > > > This change works for both autotools and meson. > > > > Signed-off-by: Wouter Wijsman > > --- > > Hi, > > Can you let us know more detail about the use case for this > patch? Why > would the user not add the directory pacman and scripts are installed > in > to their path? > > I have concerns that hardcoding paths on build will lead to > difficulty > when in the future we have a test suite for makepkg. > > Allan Hi, I've since realized that this patch is not really needed. It was supposed to go together with another patch which didn't end up working. Sorry for the spam. I'm working on making pacman manage libraries for the Playstation Portable in a homebrew PSP software development kit. For this to work on both systems which already have pacman and systems which do not, I initially tried to make pacman build with different binary names. This approach had many issues and would require forking, which I'm not keen on doing. The Playstation Vita homebrew SDK and the devkit pro (multiple systems, mostly Nintendo) also use pacman, but they have these forks which are old and have some bugs not found in upstream pacman. Now I'm working on a different solution, which is to change the bindir in the meson options while building (to keep the binaries out of the path of the user) and using wrapper scripts for pacman and makepkg. Now I realize that I can just set the path in the wrapper script and be done with the issue this patch was supposed to solve. Right now an unpatched pacman almost works for my use case, but there still is one issue with building it. Currently ninja will install the bash-completion scripts system wide, regardless of which user runs ninja install or what the prefix is. Only if bash-completion is not available, will the scripts be installed inside the prefix. The code which makes this happen is here: https://git.archlinux.org/pacman.git/tree/meson.build#n46 This is why I submitted the patch which makes bash-completion optional. It may need a different solution, though. Would you take a patch which adds an option to install the bash-completion scripts inside the prefix? I'd prefer not to maintain any private patches if that's possible. Kind regards, Wouter P.S. For the people interested in the wrapper, here is a link: https://github.com/sharkwouter/psp-pacman-1/tree/upstream-pr The pacman.sh script builds and installs pacman. The wrapper scripts are in scripts. If the user already has pacman, it doesn't build it at all and the wrapper will use the system's pacman with a different configuration, root and dbpath.
[pacman-dev] [PATCH] Convert '-U pkg1 pkg2' codepath to parallel download
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
---
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
+ * @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;
+ }
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);
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));
+ } 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);
+ }
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,
