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

[Andrew Gregory]

On 12/15/13 at 03:43pm, Olivier Brunel wrote:
> 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

Just to be clear, transactions should not be altered once
trans_prepare has started.  The front-end can only display a warning
or abort the entire transaction (and it can only even do that after
trans_prepare is finished).  If we want front-ends to be able to
choose not to remove the individual package, this needs to be
a question so that the optdep is never added to the transaction in the
first place, not an event.  Of course, this would only apply to
packages added to the transaction by alpm.  Personally, I think we
should just emit a generic package-added-to-transaction event and let
the front-end do checks and display whatever warnings it wants and/or
abort the transaction.

apg