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

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 --- 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; +