When --needed is used, up to date packages are now filtered out before showing the group select.
Signed-off-by: morganamilo morganamilo@gmail.com ---
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.
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(!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); + + if(cmp == 0) { + if(trans != NULL && trans->flags & ALPM_TRANS_FLAG_NEEDED) { + /* 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,
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 morganamilo@gmail.com
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
When --needed is used, up to date packages are now filtered out before showing the group select.
Fixes FS#22870.
Signed-off-by: morganamilo morganamilo@gmail.com --- 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,
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 morganamilo@gmail.com
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
On Thu, 18 Oct 2018 at 01:34, Andrew Gregory andrew.gregory.8@gmail.com wrote:
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.
I did say the patch set was incomplete ;) group_exists does seem like the best idea though so will do. Would you prefer that function be added to the backend or frontend?
pacman-dev@lists.archlinux.org