[pacman-dev] [PATCH 4/8] Update the event callback

[Allan McRae]

On 10/01/14 23:14, Olivier Brunel wrote:
> 
> On 12/15/13 13:10, Allan McRae wrote:
>> On 03/12/13 06:45, Olivier Brunel wrote:
>>> Instead of using two void* arguments for all events, we now send one
>>> pointer to an alpm_event_t struct. This contains the type of event that
>>> was triggered.
>>>
>>> With this information, the pointer can then be typecasted to the
>>> event-specific struct in order to get additional arguments.
>>>
>>> Signed-off-by: Olivier Brunel <jjk at jjacky.com>
>>
>> I have given couple of suggestions below for naming that I think makes
>> things clearer.  Also a query regarding the delta event struct.
>>
>> Otherwise, this looks good.
> 
> Alright, I've rebased & updated the patches regarding all the comments
> made. A couple of things before I resend them though:
> 
> - What's your opinion regarding what I mentioned before:
> I don't think addressing FS#36504 was right. This was about moving the
> log messages about a package being installed/upgraded/etc before the
> scriptlet messages, and not after as they were. I think this might have
> been a mistake:
> I guess it all depends on when you consider the operation to be
> completed, once pacman's extraction reached 100%, or after the scriptlet
> have run. I feel the later is right, since they are part of the process
> of installing/upgrading/etc a package; Of course, you may disagree.
> 
> Either way, the current situation is wrong, because right now ALPM does:
>   1. add log message about the operation being done
>   2. run scriptlet
>   3. signal frontend (via event) that operation is done
> 
> IOW if a frontend was to actually use the event to present its output to
> the user, it wouldn't match the order in the log, which is wrong. This
> was done because pacman ignores that event, only showing the progress of
> the extraction, and thus people feel that it goes 1. install done; then
> 2. scriptlet.
> 
> In fact, pacman doesn't really ignore the event, and uses it to show
> (new) optdep. Should this be done before or after the scriptlet?
> One could also ask, when using --noprogressbar pacman doesn't say
> anything at the end of the operation (save for optdep), imagine it
> would: should it be before or after the scriptlet?
> 
> As I said, I think the logging should be moved after the scriptlet have
> run, as this is when the operation is really done. If you were to
> disagree though, then the event should be moved before the scriptlet
> run, to keep consistency between the frontend and the log (which is
> broken ATM), but this just feels wrong to me, since then a frontend
> would consider a package upgrade/etc done, when in fact the scriptlet
> have yet to run (and produce new output relating to the operation
> supposedly done).

I'd like Andrew to comment here as I think that was his patch.  I think
arguments can be make both ways here.  Maybe signal the frontend it is
done installingg at the same time as the log and add signals that
install scriptlets are running?  I think this is independent of your
patchset, or am I missing something?

> - Any comments on the last 3 patches (adding new events)?

On patchwork [1], I have flagged "Add events ALPM_EVENT_RETRIEVE_{DONE,
FAILED}" as accepted.  I think "Add events _PKGDOWNLOAD_{START, DONE,
FAILED}" looks OK too at first glance.  Can you put "Add events on
pacnew/pacsave/pacorig file creation" at the end of your patch set?   It
is probably OK, but I have not looked too hard yet and it will be easier
to merge that last if it is at the end.

Cheers,
Allan

[1] https://patchwork.archlinux.org/project/pacman/list/?state=*