[pacman-dev] [PATCH 2/2] Move logging of sysupgrade start, add log when done

[jjacky]

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 <i.am.jack.mail at gmail.com>
>> ---
>>  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:
>>
> 
>