On 12/16/12 15:05, Allan McRae wrote:
> On 13/12/12 23:19, Olivier Brunel wrote:
>> The "starting sysupgrade" message was logged quite soon, making it be added
>> even when nothing was actually done, because there was nothing to do, the
>> user
>> didn't confirm, or asked to only download packages.
>>
>> The log message is now added only when (before) committing the transaction.
>> And
>> we also log a message at the end (in case of success or failure).
>>
>> Signed-off-by: Olivier Brunel <[email protected]>
>> ---
>> src/pacman/sync.c | 9 ++++++++-
>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/pacman/sync.c b/src/pacman/sync.c
>> index f8fce7f..77fd573 100644
>> --- a/src/pacman/sync.c
>> +++ b/src/pacman/sync.c
>> @@ -793,7 +793,6 @@ static int sync_trans(alpm_list_t *targets)
>>
>> if(config->op_s_upgrade) {
>> printf(_(":: Starting full system upgrade...\n"));
>> - alpm_logaction(config->handle, "starting full system
>> upgrade\n");
>
> I really do not like that moving this prints effectively the same
> message at two different times. Also, the line below really is the
> start of the system upgrade.
By two different times, you mean to the user and in the log? In such a
case, this first message could be something else, e.g. "preparing full
system upgrade" or something to that effect, and moving the "starting"
along with the log one.
What I don't like with the current situation, is that the message (in
the log) says "starting sysupgrade" while it might not really be the
case, because of a conflict, user didn't confirmed, or there was nothing
to do.
That's why I wanted the message to do logged before the operation was
confirmed, and the sysupgrade will effectively be attempted/started.
>
>> if(alpm_sync_sysupgrade(config->handle, config->op_s_upgrade >=
>> 2) == -1) {
>> pm_printf(ALPM_LOG_ERROR, "%s\n",
>> alpm_strerror(alpm_errno(config->handle)));
>> trans_release();
>> @@ -879,6 +878,9 @@ int sync_prepare_execute(void)
>> goto cleanup;
>> }
>>
>> + if(config->op_s_upgrade) {
>> + alpm_logaction(config->handle, "starting full system
>> upgrade\n");
>> + }
>> if(alpm_trans_commit(config->handle, &data) == -1) {
>> alpm_errno_t err = alpm_errno(config->handle);
>> pm_printf(ALPM_LOG_ERROR, _("failed to commit transaction
>> (%s)\n"),
>> @@ -911,11 +913,16 @@ int sync_prepare_execute(void)
>> default:
>> break;
>> }
>> + alpm_logaction(config->handle, "failed to commit transaction
>> (%s)\n",
>> + alpm_strerror(err));
>
> Why only this error? There are other just as valid error conditions to log.
I wanted to log something after the "start" message, to either say it
was successful, or failed. Other errors occur before the start message
is logged, that's why they're not logged.
>
>> /* TODO: stderr? */
>> printf(_("Errors occurred, no packages were upgraded.\n"));
>> retval = 1;
>> goto cleanup;
>> }
>> + alpm_logaction(config->handle, (config->op_s_upgrade)
>> + ? "full system upgrade completed\n"
>> + : "installation completed\n");
Just realized, that this probably should account for another case:
"download completed"
>>
>> /* Step 4: release transaction resources */
>> cleanup:
>>
>
>
