[pacman-dev] [PATCH v2 7/7] Add events on pacnew/pacsave/pacorig file creation

[Allan McRae]

On 11/01/14 01:25, Olivier Brunel wrote:
> ALPM still adds a warning to the log, but doesn't emit an event about
> said warning, instead using a specific event to let the frontend what
> happened/how to inform the user.
> 
> Signed-off-by: Olivier Brunel <jjk at jjacky.com>
> ---
>  lib/libalpm/add.c     | 41 +++++++++++++++++++++++++++++++++++------
>  lib/libalpm/alpm.h    | 41 ++++++++++++++++++++++++++++++++++++++++-
>  lib/libalpm/remove.c  |  7 ++++++-
>  src/pacman/callback.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  src/pacman/util.c     | 13 +++++++++++++
>  src/pacman/util.h     |  1 +
>  6 files changed, 143 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c
> index 1eaea36..649d6a9 100644
> --- a/lib/libalpm/add.c
> +++ b/lib/libalpm/add.c
> @@ -363,8 +363,14 @@ static int extract_single_file(alpm_handle_t *handle, struct archive *archive,
>  				if(try_rename(handle, checkfile, newpath)) {
>  					errors++;
>  				} else {
> -					_alpm_log(handle, ALPM_LOG_WARNING, _("%s installed as %s\n"),
> -							filename, newpath);
> +					alpm_event_pacnew_created_t event = {
> +						.type = ALPM_EVENT_PACNEW_CREATED,
> +						.from_noupgrade = 0,
> +						.oldpkg = oldpkg,
> +						.newpkg = newpkg,
> +						.oldfile = filename
> +					};
> +					EVENT(handle, &event);
>  					alpm_logaction(handle, ALPM_CALLER_PREFIX,
>  							"warning: %s installed as %s\n", filename, newpath);
>  				}
> @@ -391,8 +397,12 @@ static int extract_single_file(alpm_handle_t *handle, struct archive *archive,
>  					if(try_rename(handle, checkfile, filename)) {
>  						errors++;
>  					} else {
> -						_alpm_log(handle, ALPM_LOG_WARNING,
> -								_("%s saved as %s\n"), filename, newpath);
> +						alpm_event_pacorig_created_t event = {
> +							.type = ALPM_EVENT_PACORIG_CREATED,
> +							.newpkg = newpkg,
> +							.oldfile = filename
> +						};
> +						EVENT(handle, &event);
>  						alpm_logaction(handle, ALPM_CALLER_PREFIX,
>  								"warning: %s saved as %s\n", filename, newpath);
>  					}
> @@ -407,14 +417,16 @@ needbackup_cleanup:
>  		free(hash_local);
>  		free(hash_pkg);
>  	} else {
> +		size_t len;
>  		/* we didn't need a backup */
>  		if(notouch) {
>  			/* change the path to a .pacnew extension */
>  			_alpm_log(handle, ALPM_LOG_DEBUG, "%s is in NoUpgrade -- skipping\n", filename);
> -			_alpm_log(handle, ALPM_LOG_WARNING, _("extracting %s as %s.pacnew\n"), filename, filename);
>  			alpm_logaction(handle, ALPM_CALLER_PREFIX,
>  					"warning: extracting %s as %s.pacnew\n", filename, filename);

For all other actions, the alpm_logaction() call is done right after the
event and has the the exact same warning printed by pacman.  Move this
down to the next if(notouch) block and adjust the message to past tense.


> -			strncat(filename, ".pacnew", PATH_MAX - strlen(filename));
> +			/* remember len so we can get the old filename back for the event */
> +			len = strlen(filename);
> +			strncat(filename, ".pacnew", PATH_MAX - len);
>  		} else {
>  			_alpm_log(handle, ALPM_LOG_DEBUG, "extracting %s\n", filename);
>  		}
> @@ -433,6 +445,23 @@ needbackup_cleanup:
>  			return errors;
>  		}
>  
> +		if(notouch) {
> +			alpm_event_pacnew_created_t event = {
> +				.type = ALPM_EVENT_PACNEW_CREATED,
> +				.from_noupgrade = 1,
> +				.oldpkg = oldpkg,
> +				.newpkg = newpkg,
> +				.oldfile = filename
> +			};
> +			/* "remove" the .pacnew suffix */
> +			if(len < PATH_MAX)

I don't think these checks are necessary.  If the filename exceeded
PATH_MAX in length, we would need to detect this much earlier in the
process.  So no need to check here.

> +				filename[len] = '\0';
> +			EVENT(handle, &event);
> +			/* restore */
> +			if(len < PATH_MAX)
> +				filename[len] = '.';
> +		}
> +
>  		/* calculate an hash if this is in newpkg's backup */
>  		alpm_list_t *i;
>  		for(i = alpm_pkg_get_backup(newpkg); i; i = i->next) {
> diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h
> index 40d8e7b..626d875 100644
> --- a/lib/libalpm/alpm.h
> +++ b/lib/libalpm/alpm.h
> @@ -350,7 +350,15 @@ typedef enum _alpm_event_type_t {
>  	/** Key downloading is finished. */
>  	ALPM_EVENT_KEY_DOWNLOAD_DONE,
>  	/** A log message was emitted; See alpm_event_log_t for arguments. */
> -	ALPM_EVENT_LOG
> +	ALPM_EVENT_LOG,
> +	/** A .pacnew file was created; See alpm_event_pacnew_created_t for arguments. */
> +	ALPM_EVENT_PACNEW_CREATED,
> +	/** A .pacsave file was created; See alpm_event_pacsave_created_t for
> +	 * arguments */
> +	ALPM_EVENT_PACSAVE_CREATED,
> +	/** A .pacorig file was created; See alpm_event_pacorig_created_t for
> +	 * arguments */
> +	ALPM_EVENT_PACORIG_CREATED
>  } alpm_event_type_t;
>  
>  /** Events.
> @@ -442,6 +450,37 @@ typedef struct _alpm_event_pkgdownload_t {
>  	const char *file;
>  } alpm_event_pkgdownload_t;
>  
> +typedef struct _alpm_event_pacnew_created_t {
> +	/** Type of event. */
> +	alpm_event_type_t type;
> +	/** Whether the creation was result of a NoUpgrade or not */
> +	int from_noupgrade;
> +	/** Old package. */
> +	alpm_pkg_t *oldpkg;
> +	/** New Package. */
> +	alpm_pkg_t *newpkg;
> +	/** Filename of the old file. */
> +	const char *oldfile;

Bit if a bikeshed here...   but I'd like this just referred to as "file"
being the file name of the altered file without suffix.  Having it
called oldfile makes me think it is associated with oldpkg, when it is
really being installed from newpkg.   Do the same in the other two for
consistency.  Mention the lack of suffix in the comment.

> +} alpm_event_pacnew_created_t;
> +
> +typedef struct _alpm_event_pacsave_created_t {
> +	/** Type of event. */
> +	alpm_event_type_t type;
> +	/** Old package. */
> +	alpm_pkg_t *oldpkg;
> +	/** Filename of the old file. */
> +	const char *oldfile;
> +} alpm_event_pacsave_created_t;
> +
> +typedef struct _alpm_event_pacorig_created_t {
> +	/** Type of event. */
> +	alpm_event_type_t type;
> +	/** New package. */
> +	alpm_pkg_t *newpkg;
> +	/** Filename of the old file. */
> +	const char *oldfile;
> +} alpm_event_pacorig_created_t;
> +