On 12/15/13 13:00, Allan McRae wrote:
On 03/12/13 06:45, Olivier Brunel wrote:
Because this event is triggered when an optdepend for another package is being removed.
Signed-off-by: Olivier Brunel <jjk@jjacky.com>
I am not convinced by this. The event happens when the dependencies are being checked before removing a package. pacman current just prints a warning, but another frontend might want to query the user before removing the package. So at that stage, the optdep is required and not removed.
Well, I think it's wrong to say the optdep is required. Not just because there's no real sense of "requirement" (as in, it's optional), but more importantly because the only thing is that a package that will be removed happens to be listed as optdep of another (installed) package. It doesn't mean that this was the reason the package was installed, or that the user ever made any use of the package as such optdep. Indicating that there's a notion of requirement here feels just wrong to me. And the name ALPM_EVENT_OPTDEP_REQUIRED implies (to me) that this is what the event is about, the requirement part. When going over this I got confused about what it meant, and this is why I suggested a renaming. To go back to your example, I don't see an problem with the following: - ALPM emits an event OPTDEP_REMOVED to indicate an optdep is (going to be) removed. - the frontend then queries the user, to confirm this removal (as this is a removal we're talking about here). To be pedantic, we might want to say ALPM_EVENT_OPTDEP_REMOVAL_PENDING or something, which would be more precise/correct, but also quite long. For the sake or being short, I just went with REMOVED, which I think remains valid. REQUIRED OTOH does not. I'm not opposed to using something other than REMOVED, but IMHO it should be renamed, as REQUIRED is wrong/confusing (it doesn't even convey anything regarding the removal, which again is really what this (event) is about, the (pending) *removal* of an optdep). -j
--- lib/libalpm/alpm.h | 2 +- lib/libalpm/remove.c | 2 +- src/pacman/callback.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index e9b0feb..c6d9064 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -377,7 +377,7 @@ typedef enum _alpm_event_t { ALPM_EVENT_DISKSPACE_DONE, /** An optdepend for another package is being removed * The requiring package and its dependency are passed to the callback */ - ALPM_EVENT_OPTDEP_REQUIRED, + ALPM_EVENT_OPTDEP_REMOVED, /** A configured repository database is missing */ ALPM_EVENT_DATABASE_MISSING, /** Checking keys used to create signatures are in keyring. */ diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c index 9417a61..8884495 100644 --- a/lib/libalpm/remove.c +++ b/lib/libalpm/remove.c @@ -179,7 +179,7 @@ static void remove_notify_needed_optdepends(alpm_handle_t *handle, alpm_list_t * for(j = optdeps; j; j = alpm_list_next(j)) { alpm_depend_t *optdep = j->data; if(alpm_pkg_find(lp, optdep->name)) { - EVENT(handle, ALPM_EVENT_OPTDEP_REQUIRED, pkg, optdep); + EVENT(handle, ALPM_EVENT_OPTDEP_REMOVED, pkg, optdep); } } } diff --git a/src/pacman/callback.c b/src/pacman/callback.c index c412f6f..e80a071 100644 --- a/src/pacman/callback.c +++ b/src/pacman/callback.c @@ -247,7 +247,7 @@ void cb_event(alpm_event_t event, void *data1, void *data2) printf(_("checking available disk space...\n")); } break; - case ALPM_EVENT_OPTDEP_REQUIRED: + case ALPM_EVENT_OPTDEP_REMOVED: colon_printf(_("%s optionally requires %s\n"), alpm_pkg_get_name(data1), alpm_dep_compute_string(data2)); break;