Re: [pacman-dev] [PATCH v2] libalpm: process needed before group selection
On 10/17/18 at 04:40pm, morganamilo wrote:
> When --needed is used, up to date packages are now filtered out
> before showing the group select.
>
> Fixes FS#22870.
>
> Signed-off-by: morganamilo
> ---
> v2: Changed per Andrew's feedback.
I wasn't concerned about the misleading message for this patch, but
the failing test is another thing. If you're not testing your patches
with `make check`, please do. We could update the test, but let's
just go ahead and fix the "error" after all. I recommend, in
a separate patch, adding a group_exists function and using that to
distinguish between a non-existent group and a group with no packages
selected after find_group_packages returns NULL.
> diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
> index b6ae7b72..05f58fad 100644
> --- a/lib/libalpm/sync.c
> +++ b/lib/libalpm/sync.c
> @@ -277,10 +277,21 @@ alpm_list_t SYMEXPORT *alpm_find_group_pkgs(alpm_list_t
> *dbs,
>
> for(j = grp->packages; j; j = j->next) {
> alpm_pkg_t *pkg = j->data;
> + alpm_trans_t *trans = db->handle->trans;
>
> if(alpm_pkg_find(ignorelist, pkg->name)) {
> continue;
> }
> + if(trans != NULL && trans->flags &
> ALPM_TRANS_FLAG_NEEDED) {
> + alpm_pkg_t *local =
> _alpm_db_get_pkgfromcache(db->handle->db_local, pkg->name);
> + if(local && _alpm_pkg_compare_versions(pkg,
> local) == 0) {
> + /* with the NEEDED flag, packages up to
> date are not reinstalled */
> + _alpm_log(db->handle, ALPM_LOG_WARNING,
> _("%s-%s is up to date -- skipping\n"),
> + local->name,
> local->version);
> + ignorelist = alpm_list_add(ignorelist,
> pkg);
> + continue;
> + }
> + }
> if(alpm_pkg_should_ignore(db->handle, pkg)) {
> alpm_question_install_ignorepkg_t question = {
> .type = ALPM_QUESTION_INSTALL_IGNOREPKG,
> --
> 2.19.1
[pacman-dev] [PATCH v2] libalpm: process needed before group selection
When --needed is used, up to date packages are now filtered out
before showing the group select.
Fixes FS#22870.
Signed-off-by: morganamilo
---
v2: Changed per Andrew's feedback.
diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
index b6ae7b72..05f58fad 100644
--- a/lib/libalpm/sync.c
+++ b/lib/libalpm/sync.c
@@ -277,10 +277,21 @@ alpm_list_t SYMEXPORT *alpm_find_group_pkgs(alpm_list_t
*dbs,
for(j = grp->packages; j; j = j->next) {
alpm_pkg_t *pkg = j->data;
+ alpm_trans_t *trans = db->handle->trans;
if(alpm_pkg_find(ignorelist, pkg->name)) {
continue;
}
+ if(trans != NULL && trans->flags &
ALPM_TRANS_FLAG_NEEDED) {
+ alpm_pkg_t *local =
_alpm_db_get_pkgfromcache(db->handle->db_local, pkg->name);
+ if(local && _alpm_pkg_compare_versions(pkg,
local) == 0) {
+ /* with the NEEDED flag, packages up to
date are not reinstalled */
+ _alpm_log(db->handle, ALPM_LOG_WARNING,
_("%s-%s is up to date -- skipping\n"),
+ local->name,
local->version);
+ ignorelist = alpm_list_add(ignorelist,
pkg);
+ continue;
+ }
+ }
if(alpm_pkg_should_ignore(db->handle, pkg)) {
alpm_question_install_ignorepkg_t question = {
.type = ALPM_QUESTION_INSTALL_IGNOREPKG,
--
2.19.1
[pacman-dev] [PATCH v2] alpm: Fix SIGINT handling re: aborting download
Upon receiving SIGINT a flag is set to abort the (curl) download. However, since it was never reset/initialized, if a front-end doesn't actually exit on SIGINT, and later tries any operation that needs to perform a new download, said download would always get aborted right away due to the flag not having been reset. --- v2: Correctly reset variable before setting signal handler Signed-off-by: Olivier Brunel --- lib/libalpm/dload.c | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c index cca39470..4d0adb19 100644 --- a/lib/libalpm/dload.c +++ b/lib/libalpm/dload.c @@ -431,6 +431,7 @@ static int curl_download_internal(struct dload_payload *payload, /* Ignore any SIGPIPE signals. With libcurl, these shouldn't be happening, * but better safe than sorry. Store the old signal handler first. */ mask_signal(SIGPIPE, SIG_IGN, _sig_pipe); + dload_interrupted = 0; mask_signal(SIGINT, , _sig_int); /* perform transfer */ -- 2.19.0
Re: [pacman-dev] [PATCH] alpm: Do not raise SIGINT when filesize goes over limit
On 10/09/18 at 06:29pm, Olivier Brunel wrote: > Variable dload_interrupted is used both to abort a download because > SIGINT was caught, and when a file limit is reached. But raising SIGINT > is only meant to happen in the first case. > > Signed-off-by: Olivier Brunel > --- ACK.
Re: [pacman-dev] [PATCH] alpm: Fix SIGINT handling re: aborting download
On 10/09/18 at 06:29pm, Olivier Brunel wrote:
> Upon receiving SIGINT a flag is set to abort the (curl) download.
> However, since it was never reset/initialized, if a front-end doesn't
> actually exit on SIGINT, and later tries any operation that needs to
> perform a new download, said download would always get aborted right
> away due to the flag not having been reset.
>
> Signed-off-by: Olivier Brunel
> ---
> lib/libalpm/dload.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
> index cca39470..0a3293cf 100644
> --- a/lib/libalpm/dload.c
> +++ b/lib/libalpm/dload.c
> @@ -253,6 +253,7 @@ static void curl_set_handle_opts(struct dload_payload
> *payload,
> const char *useragent = getenv("HTTP_USER_AGENT");
> struct stat st;
>
> + dload_interrupted = 0;
I think set_handle_opts is the wrong place to reset it since it is in
fact not a handle option. Let's put it right before the signal
handlers are registered to keep signal-related things together.
> /* the curl_easy handle is initialized with the alpm handle, so we only
> need
>* to reset the handle's parameters for each time it's used. */
> curl_easy_reset(curl);
> --
> 2.19.0
Re: [pacman-dev] [PATCH] libalpm: process needed before group selection
On 09/22/18 at 05:24pm, morganamilo wrote:
> When --needed is used, up to date packages are now filtered out
> before showing the group select.
>
> Signed-off-by: morganamilo
> ---
>
> This patch set is currently incomplete. There is a problem where if every
> package in the group is already installed you end up with the eror.
> "error: target not found: groupname". Instead "there is nothing to do" should
> be produced instead.
>
> I'm unsure of how to solve this so I am submitting this for discussion.
> Currently my idea is to have alpm_find_group_pkgs gain a new param `int
> *exists`
> which the front end can then check instead of the length of the return. Or
> instead the needed check could just be moved to the front end. Let me know
> if theres a better way.
I really hate this function. The logic is way too complex and
specific for it to fit comfortably in the backend. I'd really prefer
the whole thing move to the frontend. Regardless, I'm not concerned
about the somewhat erroneous message. That already happens if the
entire group is ignored, so that's really an existing problem that can
be solved separately. I'd like to see the patch cleaned up a little,
but I think this solution is fine for now. If you resubmit, please
include a note in the commit message that this fixes FS#22870.
> diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
> index b6ae7b72..f1c02417 100644
> --- a/lib/libalpm/sync.c
> +++ b/lib/libalpm/sync.c
> @@ -270,6 +270,8 @@ alpm_list_t SYMEXPORT *alpm_find_group_pkgs(alpm_list_t
> *dbs,
> for(i = dbs; i; i = i->next) {
> alpm_db_t *db = i->data;
> alpm_group_t *grp = alpm_db_get_group(db, name);
> + alpm_handle_t *handle = db->handle;
> + alpm_trans_t *trans = handle->trans;
If you're going to save db->handle to a variable, go ahead and replace
the other db->handle references in the function.
> if(!grp) {
> continue;
> @@ -277,10 +279,26 @@ alpm_list_t SYMEXPORT *alpm_find_group_pkgs(alpm_list_t
> *dbs,
>
> for(j = grp->packages; j; j = j->next) {
> alpm_pkg_t *pkg = j->data;
> + alpm_pkg_t *local =
> _alpm_db_get_pkgfromcache(handle->db_local, pkg->name);
>
> if(alpm_pkg_find(ignorelist, pkg->name)) {
> continue;
> }
> + if(local) {
> + const char *localpkgname = local->name;
> + const char *localpkgver = local->version;
> + int cmp = _alpm_pkg_compare_versions(pkg,
> local);
These variables are unnecessary. They are each used once, don't add
any descriptive detail, and the two strings have names longer than the
struct member they reference.
> +
> + if(cmp == 0) {
> + if(trans != NULL && trans->flags &
> ALPM_TRANS_FLAG_NEEDED) {
Of the three conditions, this will be the fastest to check, so it
should be the outermost. The other two can then be combined to
avoid unnecessary indentation:
if(trans != NULL && trans->flags & ALPM_TRANS_FLAG_NEEDED) {
alpm_pkg_t *local =
_alpm_db_get_pkgfromcache(handle->db_local, pkg->name);
if(local && _alpm_pkg_compare_versions(pkg, local) ==
0) {
> + /* with the NEEDED flag,
> packages up to date are not reinstalled */
> + _alpm_log(handle,
> ALPM_LOG_WARNING, _("%s-%s is up to date -- skipping\n"),
> + localpkgname,
> localpkgver);
> + ignorelist =
> alpm_list_add(ignorelist, pkg);
> + continue;
> + }
> + }
> + }
> if(alpm_pkg_should_ignore(db->handle, pkg)) {
> alpm_question_install_ignorepkg_t question = {
> .type = ALPM_QUESTION_INSTALL_IGNOREPKG,
> --
> 2.19.0
Re: [pacman-dev] [PATCH 1/2] Port pactest to python3
On 09/20/18 at 05:43am, Dave Reisner wrote: > Use BytesIO instead of StringIO, and ensure that we unicode-encode data > where needed. > --- Any particular reason for the bump or just dropping the (allegedly) dead python2? I've held off from doing this so far because asciidoc is python2, hoping they'd update to python3 eventually. It appears to be dead now, though, so I guess it's time to move on. ... > diff --git a/test/pacman/pmdb.py b/test/pacman/pmdb.py > index f7671987..26a8d9a4 100644 > --- a/test/pacman/pmdb.py > +++ b/test/pacman/pmdb.py > @@ -17,9 +17,10 @@ > > import os > import shutil > -from StringIO import StringIO > import tarfile > > +from io import BytesIO > + Why the extra line breaks around the new imports? They were grouped by built-in vs local.
