[pacman-dev] [PATCH 2/8] Rename ALPM_EVENT_OPTDEP_REQUIRED to _OPTDEP_REMOVED

Olivier Brunel jjk at jjacky.com
Sun Dec 15 09:43:55 EST 2013


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 at 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;
>>
> 
> 



More information about the pacman-dev mailing list