[pacman-dev] [PATCH 0/4] [RFC] add caller prefix to alpm_logaction
Implements the following TODO from alpm_logaction in log.c:
TODO We should add a prefix to log strings depending on who called us. If logaction was called by the frontend: USER: <the frontend log> and if called internally: ALPM: <the library log> Moreover, the frontend should be able to choose its prefix (USER by default?): pacman: "PACMAN" kpacman: "KPACMAN" This would allow us to share the log file between several frontends and know who does what
The log prefix can be added two different ways. To make it easier for frontends, handle->program can be set to the program name (defaults to "USER") which will be used for calls to alpm_logaction. The prefix can be set per call with alpm_plogaction; this is what alpm does. Resulting logfile looks like this:
[2012-12-20 16:29] [PACMAN] Running '/home/ag/devel/pacman/src/pacman/.libs/lt-pacman -Rdd pacman' [2012-12-20 16:29] [ALPM] warning: /etc/pacman.conf saved as /etc/pacman.conf.pacsave [2012-12-20 16:29] [PACMAN] removed pacman (4.0.3-5) [2012-12-20 16:29] [PACMAN] Running '/home/ag/devel/pacman/src/pacman/.libs/lt-pacman --conf /etc/pacman.conf.pacsave -S pacman' [2012-12-20 16:29] [ALPM-SCRIPTLET] >>> Run `pacman-key --init; pacman-key --populate archlinux` [2012-12-20 16:29] [ALPM-SCRIPTLET] >>> to import the data required by pacman for package verification. [2012-12-20 16:29] [ALPM-SCRIPTLET] >>> See: https://www.archlinux.org/news/having-pacman-verify-packages [2012-12-20 16:29] [PACMAN] installed pacman (4.0.3-5)
Andrew Gregory (4): alpm_handle_t: add program name for use in log prefix _alpm_logaction: add support for prefix log.c: add alpm_plogaction pacman: set program name for logging lib/libalpm/add.c | 22 +++++++++++----------- lib/libalpm/alpm.c | 2 ++ lib/libalpm/alpm.h | 4 ++++ lib/libalpm/handle.c | 18 ++++++++++++++++++ lib/libalpm/handle.h | 1 + lib/libalpm/log.c | 50 ++++++++++++++++++++++++++++++++++++++------------ lib/libalpm/remove.c | 6 +++--- lib/libalpm/util.c | 8 ++++++-- lib/libalpm/util.h | 2 +- src/pacman/conf.c | 1 + 10 files changed, 85 insertions(+), 29 deletions(-) -- 1.8.0.2
Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- lib/libalpm/alpm.c | 2 ++ lib/libalpm/alpm.h | 3 +++ lib/libalpm/handle.c | 18 ++++++++++++++++++ lib/libalpm/handle.h | 1 + lib/libalpm/log.c | 11 ----------- 5 files changed, 24 insertions(+), 11 deletions(-) diff --git a/lib/libalpm/alpm.c b/lib/libalpm/alpm.c index c58a406..255e486 100644 --- a/lib/libalpm/alpm.c +++ b/lib/libalpm/alpm.c @@ -76,6 +76,8 @@ alpm_handle_t SYMEXPORT *alpm_initialize(const char *root, const char *dbpath, bindtextdomain("libalpm", LOCALEDIR); #endif + alpm_option_set_program(myhandle, "USER"); + return myhandle; cleanup: diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 5393b3b..3388735 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -563,6 +563,9 @@ const char *alpm_option_get_arch(alpm_handle_t *handle); /** Sets the targeted architecture. */ int alpm_option_set_arch(alpm_handle_t *handle, const char *arch); +const char *alpm_option_get_program(alpm_handle_t *handle); +int alpm_option_set_program(alpm_handle_t *handle, const char *arch); + double alpm_option_get_deltaratio(alpm_handle_t *handle); int alpm_option_set_deltaratio(alpm_handle_t *handle, double ratio); diff --git a/lib/libalpm/handle.c b/lib/libalpm/handle.c index d13be1c..45c6dab 100644 --- a/lib/libalpm/handle.c +++ b/lib/libalpm/handle.c @@ -79,6 +79,7 @@ void _alpm_handle_free(alpm_handle_t *handle) FREE(handle->logfile); FREE(handle->lockfile); FREE(handle->arch); + FREE(handle->program); FREE(handle->gpgdir); FREELIST(handle->dbs_sync); FREELIST(handle->noupgrade); @@ -253,6 +254,12 @@ const char SYMEXPORT *alpm_option_get_arch(alpm_handle_t *handle) return handle->arch; } +const char SYMEXPORT *alpm_option_get_program(alpm_handle_t *handle) +{ + CHECK_HANDLE(handle, return NULL); + return handle->program; +} + double SYMEXPORT alpm_option_get_deltaratio(alpm_handle_t *handle) { CHECK_HANDLE(handle, return -1); @@ -582,6 +589,17 @@ int SYMEXPORT alpm_option_set_arch(alpm_handle_t *handle, const char *arch) } return 0; } +int SYMEXPORT alpm_option_set_program(alpm_handle_t *handle, const char *program) +{ + CHECK_HANDLE(handle, return -1); + if(handle->program) FREE(handle->program); + if(program) { + handle->program = strdup(program); + } else { + handle->program = NULL; + } + return 0; +} int SYMEXPORT alpm_option_set_deltaratio(alpm_handle_t *handle, double ratio) { diff --git a/lib/libalpm/handle.h b/lib/libalpm/handle.h index 1b9c8c8..2a68b8e 100644 --- a/lib/libalpm/handle.h +++ b/lib/libalpm/handle.h @@ -96,6 +96,7 @@ struct __alpm_handle_t { upgrade operations */ alpm_siglevel_t remotefilesiglevel; /* Signature verification level for remote file upgrade operations */ + char *program; /* error code */ alpm_errno_t pm_errno; diff --git a/lib/libalpm/log.c b/lib/libalpm/log.c index 8486716..5c4bf9a 100644 --- a/lib/libalpm/log.c +++ b/lib/libalpm/log.c @@ -65,17 +65,6 @@ int SYMEXPORT alpm_logaction(alpm_handle_t *handle, const char *fmt, ...) ret = _alpm_logaction(handle, fmt, args); va_end(args); - /* TODO We should add a prefix to log strings depending on who called us. - * If logaction was called by the frontend: - * USER: <the frontend log> - * and if called internally: - * ALPM: <the library log> - * Moreover, the frontend should be able to choose its prefix - * (USER by default?): - * pacman: "PACMAN" - * kpacman: "KPACMAN" - * This would allow us to share the log file between several frontends - * and know who does what */ return ret; } -- 1.8.0.2
Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- lib/libalpm/log.c | 2 +- lib/libalpm/util.c | 6 +++++- lib/libalpm/util.h | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/libalpm/log.c b/lib/libalpm/log.c index 5c4bf9a..0b48d26 100644 --- a/lib/libalpm/log.c +++ b/lib/libalpm/log.c @@ -62,7 +62,7 @@ int SYMEXPORT alpm_logaction(alpm_handle_t *handle, const char *fmt, ...) } va_start(args, fmt); - ret = _alpm_logaction(handle, fmt, args); + ret = _alpm_logaction(handle, handle->program, fmt, args); va_end(args); return ret; diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c index dda6a92..8a6f27c 100644 --- a/lib/libalpm/util.c +++ b/lib/libalpm/util.c @@ -453,7 +453,8 @@ ssize_t _alpm_files_in_directory(alpm_handle_t *handle, const char *path, * @return 0 or number of characters written on success, vfprintf return value * on error */ -int _alpm_logaction(alpm_handle_t *handle, const char *fmt, va_list args) +int _alpm_logaction(alpm_handle_t *handle, const char *prefix, + const char *fmt, va_list args) { int ret = 0; @@ -477,6 +478,9 @@ int _alpm_logaction(alpm_handle_t *handle, const char *fmt, va_list args) fprintf(handle->logstream, "[%04d-%02d-%02d %02d:%02d] ", tm->tm_year+1900, tm->tm_mon+1, tm->tm_mday, tm->tm_hour, tm->tm_min); + if(prefix && *prefix) { + fprintf(handle->logstream, "[%s] ", prefix); + }; ret = vfprintf(handle->logstream, fmt, args); fflush(handle->logstream); } diff --git a/lib/libalpm/util.h b/lib/libalpm/util.h index 5043476..2726443 100644 --- a/lib/libalpm/util.h +++ b/lib/libalpm/util.h @@ -119,7 +119,7 @@ int _alpm_unpack(alpm_handle_t *handle, const char *archive, const char *prefix, alpm_list_t *list, int breakfirst); ssize_t _alpm_files_in_directory(alpm_handle_t *handle, const char *path, int full_count); -int _alpm_logaction(alpm_handle_t *handle, const char *fmt, va_list args); +int _alpm_logaction(alpm_handle_t *handle, const char *prefix, const char *fmt, va_list args); int _alpm_run_chroot(alpm_handle_t *handle, const char *cmd, char *const argv[]); int _alpm_ldconfig(alpm_handle_t *handle); int _alpm_str_cmp(const void *s1, const void *s2); -- 1.8.0.2
Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- lib/libalpm/add.c | 22 +++++++++++----------- lib/libalpm/alpm.h | 1 + lib/libalpm/log.c | 37 +++++++++++++++++++++++++++++++++++++ lib/libalpm/remove.c | 6 +++--- lib/libalpm/util.c | 2 +- 5 files changed, 53 insertions(+), 15 deletions(-) diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c index 7ef6530..86f4c64 100644 --- a/lib/libalpm/add.c +++ b/lib/libalpm/add.c @@ -121,7 +121,7 @@ static int perform_extraction(alpm_handle_t *handle, struct archive *archive, } else if(ret != ARCHIVE_OK) { _alpm_log(handle, ALPM_LOG_ERROR, _("could not extract %s (%s)\n"), origname, archive_error_string(archive)); - alpm_logaction(handle, "error: could not extract %s (%s)\n", + alpm_plogaction(handle, "ALPM", "error: could not extract %s (%s)\n", origname, archive_error_string(archive)); return 1; } @@ -133,7 +133,7 @@ static int try_rename(alpm_handle_t *handle, const char *src, const char *dest) if(rename(src, dest)) { _alpm_log(handle, ALPM_LOG_ERROR, _("could not rename %s to %s (%s)\n"), src, dest, strerror(errno)); - alpm_logaction(handle, "error: could not rename %s to %s (%s)\n", + alpm_plogaction(handle, "ALPM", "error: could not rename %s to %s (%s)\n", src, dest, strerror(errno)); return 1; } @@ -184,7 +184,7 @@ static int extract_single_file(alpm_handle_t *handle, struct archive *archive, if(alpm_list_find(handle->noextract, entryname, _alpm_fnmatch)) { _alpm_log(handle, ALPM_LOG_DEBUG, "%s is in NoExtract, skipping extraction\n", entryname); - alpm_logaction(handle, "note: %s is in NoExtract, skipping extraction\n", + alpm_plogaction(handle, "ALPM", "note: %s is in NoExtract, skipping extraction\n", entryname); archive_read_data_skip(archive); return 0; @@ -224,7 +224,7 @@ static int extract_single_file(alpm_handle_t *handle, struct archive *archive, _alpm_log(handle, ALPM_LOG_WARNING, _("directory permissions differ on %s\n" "filesystem: %o package: %o\n"), entryname, lsbuf.st_mode & mask, entrymode & mask); - alpm_logaction(handle, "warning: directory permissions differ on %s\n" + alpm_plogaction(handle, "ALPM", "warning: directory permissions differ on %s\n" "filesystem: %o package: %o\n", entryname, lsbuf.st_mode & mask, entrymode & mask); } @@ -348,7 +348,7 @@ static int extract_single_file(alpm_handle_t *handle, struct archive *archive, errors++; } else { _alpm_log(handle, ALPM_LOG_WARNING, _("%s saved as %s\n"), filename, newpath); - alpm_logaction(handle, "warning: %s saved as %s\n", filename, newpath); + alpm_plogaction(handle, "ALPM", "warning: %s saved as %s\n", filename, newpath); } } free(newpath); @@ -399,7 +399,7 @@ static int extract_single_file(alpm_handle_t *handle, struct archive *archive, } else { _alpm_log(handle, ALPM_LOG_WARNING, _("%s installed as %s\n"), filename, newpath); - alpm_logaction(handle, "warning: %s installed as %s\n", + alpm_plogaction(handle, "ALPM", "warning: %s installed as %s\n", filename, newpath); } free(newpath); @@ -416,7 +416,7 @@ needbackup_cleanup: /* 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, "warning: extracting %s as %s.pacnew\n", filename, filename); + alpm_plogaction(handle, "ALPM", "warning: extracting %s as %s.pacnew\n", filename, filename); strncat(filename, ".pacnew", PATH_MAX - strlen(filename)); } else { _alpm_log(handle, ALPM_LOG_DEBUG, "extracting %s\n", filename); @@ -518,7 +518,7 @@ static int commit_single_pkg(alpm_handle_t *handle, alpm_pkg_t *newpkg, /* prepare directory for database entries so permission are correct after changelog/install script installation (FS#12263) */ if(_alpm_local_db_prepare(db, newpkg)) { - alpm_logaction(handle, "error: could not create database entry %s-%s\n", + alpm_plogaction(handle, "ALPM", "error: could not create database entry %s-%s\n", newpkg->name, newpkg->version); handle->pm_errno = ALPM_ERR_DB_WRITE; ret = -1; @@ -609,12 +609,12 @@ static int commit_single_pkg(alpm_handle_t *handle, alpm_pkg_t *newpkg, if(is_upgrade) { _alpm_log(handle, ALPM_LOG_ERROR, _("problem occurred while upgrading %s\n"), newpkg->name); - alpm_logaction(handle, "error: problem occurred while upgrading %s\n", + alpm_plogaction(handle, "ALPM", "error: problem occurred while upgrading %s\n", newpkg->name); } else { _alpm_log(handle, ALPM_LOG_ERROR, _("problem occurred while installing %s\n"), newpkg->name); - alpm_logaction(handle, "error: problem occurred while installing %s\n", + alpm_plogaction(handle, "ALPM", "error: problem occurred while installing %s\n", newpkg->name); } } @@ -629,7 +629,7 @@ static int commit_single_pkg(alpm_handle_t *handle, alpm_pkg_t *newpkg, if(_alpm_local_db_write(db, newpkg, INFRQ_ALL)) { _alpm_log(handle, ALPM_LOG_ERROR, _("could not update database entry %s-%s\n"), newpkg->name, newpkg->version); - alpm_logaction(handle, "error: could not update database entry %s-%s\n", + alpm_plogaction(handle, "ALPM", "error: could not update database entry %s-%s\n", newpkg->name, newpkg->version); handle->pm_errno = ALPM_ERR_DB_WRITE; ret = -1; diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 3388735..bece1e2 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -272,6 +272,7 @@ typedef enum _alpm_loglevel_t { typedef void (*alpm_cb_log)(alpm_loglevel_t, const char *, va_list); int alpm_logaction(alpm_handle_t *handle, const char *fmt, ...); +int alpm_plogaction(alpm_handle_t *handle, const char *prefix, const char *fmt, ...); /** * Events. diff --git a/lib/libalpm/log.c b/lib/libalpm/log.c index 0b48d26..4119b5d 100644 --- a/lib/libalpm/log.c +++ b/lib/libalpm/log.c @@ -35,6 +35,43 @@ /** A printf-like function for logging. * @param handle the context handle + * @param prefix log message prefix + * @param fmt output format + * @return 0 on success, -1 on error (pm_errno is set accordingly) + */ +int SYMEXPORT alpm_plogaction(alpm_handle_t *handle, const char *prefix, + const char *fmt, ...) +{ + int ret; + va_list args; + + ASSERT(handle != NULL, return -1); + + /* check if the logstream is open already, opening it if needed */ + if(handle->logstream == NULL) { + handle->logstream = fopen(handle->logfile, "a"); + /* if we couldn't open it, we have an issue */ + if(handle->logstream == NULL) { + if(errno == EACCES) { + handle->pm_errno = ALPM_ERR_BADPERMS; + } else if(errno == ENOENT) { + handle->pm_errno = ALPM_ERR_NOT_A_DIR; + } else { + handle->pm_errno = ALPM_ERR_SYSTEM; + } + return -1; + } + } + + va_start(args, fmt); + ret = _alpm_logaction(handle, prefix, fmt, args); + va_end(args); + + return ret; +} + +/** A printf-like function for logging. + * @param handle the context handle * @param fmt output format * @return 0 on success, -1 on error (pm_errno is set accordingly) */ diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c index 4e1c127..f47b377 100644 --- a/lib/libalpm/remove.c +++ b/lib/libalpm/remove.c @@ -479,13 +479,13 @@ static int unlink_file(alpm_handle_t *handle, alpm_pkg_t *oldpkg, if(rename(file, newpath)) { _alpm_log(handle, ALPM_LOG_ERROR, _("could not rename %s to %s (%s)\n"), file, newpath, strerror(errno)); - alpm_logaction(handle, "error: could not rename %s to %s (%s)\n", + alpm_plogaction(handle, "ALPM", "error: could not rename %s to %s (%s)\n", file, newpath, strerror(errno)); free(newpath); return -1; } _alpm_log(handle, ALPM_LOG_WARNING, _("%s saved as %s\n"), file, newpath); - alpm_logaction(handle, "warning: %s saved as %s\n", file, newpath); + alpm_plogaction(handle, "ALPM", "warning: %s saved as %s\n", file, newpath); free(newpath); return 0; } @@ -497,7 +497,7 @@ static int unlink_file(alpm_handle_t *handle, alpm_pkg_t *oldpkg, if(unlink(file) == -1) { _alpm_log(handle, ALPM_LOG_ERROR, _("cannot remove %s (%s)\n"), file, strerror(errno)); - alpm_logaction(handle, "error: cannot remove %s (%s)\n", + alpm_plogaction(handle, "ALPM", "error: cannot remove %s (%s)\n", file, strerror(errno)); return -1; } diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c index 8a6f27c..cb90a07 100644 --- a/lib/libalpm/util.c +++ b/lib/libalpm/util.c @@ -572,7 +572,7 @@ int _alpm_run_chroot(alpm_handle_t *handle, const char *cmd, char *const argv[]) char line[PATH_MAX]; if(fgets(line, PATH_MAX, pipe_file) == NULL) break; - alpm_logaction(handle, "%s", line); + alpm_plogaction(handle, "ALPM-SCRIPTLET", "%s", line); EVENT(handle, ALPM_EVENT_SCRIPTLET_INFO, line, NULL); } fclose(pipe_file); -- 1.8.0.2
Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- src/pacman/conf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/pacman/conf.c b/src/pacman/conf.c index bbec281..f012908 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -621,6 +621,7 @@ static int setup_libalpm(void) alpm_option_set_eventcb(handle, cb_event); alpm_option_set_questioncb(handle, cb_question); alpm_option_set_progresscb(handle, cb_progress); + alpm_option_set_program(handle, "PACMAN"); config->logfile = config->logfile ? config->logfile : strdup(LOGFILE); ret = alpm_option_set_logfile(handle, config->logfile); -- 1.8.0.2
On 21/12/12 08:09, Andrew Gregory wrote:
Implements the following TODO from alpm_logaction in log.c:
TODO We should add a prefix to log strings depending on who called us. If logaction was called by the frontend: USER: <the frontend log> and if called internally: ALPM: <the library log> Moreover, the frontend should be able to choose its prefix (USER by default?): pacman: "PACMAN" kpacman: "KPACMAN" This would allow us to share the log file between several frontends and know who does what
The log prefix can be added two different ways. To make it easier for frontends, handle->program can be set to the program name (defaults to "USER") which will be used for calls to alpm_logaction. The prefix can be set per call with alpm_plogaction; this is what alpm does.
Resulting logfile looks like this:
[2012-12-20 16:29] [PACMAN] Running '/home/ag/devel/pacman/src/pacman/.libs/lt-pacman -Rdd pacman' [2012-12-20 16:29] [ALPM] warning: /etc/pacman.conf saved as /etc/pacman.conf.pacsave [2012-12-20 16:29] [PACMAN] removed pacman (4.0.3-5) [2012-12-20 16:29] [PACMAN] Running '/home/ag/devel/pacman/src/pacman/.libs/lt-pacman --conf /etc/pacman.conf.pacsave -S pacman' [2012-12-20 16:29] [ALPM-SCRIPTLET] >>> Run `pacman-key --init; pacman-key --populate archlinux` [2012-12-20 16:29] [ALPM-SCRIPTLET] >>> to import the data required by pacman for package verification. [2012-12-20 16:29] [ALPM-SCRIPTLET] >>> See: https://www.archlinux.org/news/having-pacman-verify-packages [2012-12-20 16:29] [PACMAN] installed pacman (4.0.3-5)
That looks fine to me. Not it will break paclog-pkglist, but should be reasonably easy to fix... However, I think there is too much going on here with the setting a package name, creating a new function for use in libalpm, etc. We are never that attached to API around here, so I would just add a new parameter to alpm_logaction and for everything to provide a caller. Anyone else have an opinion on this? Allan
prefix defaults to "UNKOWN" if null or an empty string is provided. Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- lib/libalpm/add.c | 36 ++++++++++++++++++++++-------------- lib/libalpm/alpm.h | 3 ++- lib/libalpm/log.c | 17 ++++------------- lib/libalpm/log.h | 2 ++ lib/libalpm/remove.c | 10 ++++++---- lib/libalpm/util.c | 13 +++++++++---- lib/libalpm/util.h | 2 +- src/pacman/callback.c | 24 ++++++++++++++---------- src/pacman/pacman.c | 3 ++- src/pacman/pacman.h | 2 ++ src/pacman/sync.c | 6 ++++-- 11 files changed, 68 insertions(+), 50 deletions(-) diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c index 57c9300..9b67813 100644 --- a/lib/libalpm/add.c +++ b/lib/libalpm/add.c @@ -121,7 +121,8 @@ static int perform_extraction(alpm_handle_t *handle, struct archive *archive, } else if(ret != ARCHIVE_OK) { _alpm_log(handle, ALPM_LOG_ERROR, _("could not extract %s (%s)\n"), origname, archive_error_string(archive)); - alpm_logaction(handle, "error: could not extract %s (%s)\n", + alpm_logaction(handle, ALPM_CALLER_PREFIX, + "error: could not extract %s (%s)\n", origname, archive_error_string(archive)); return 1; } @@ -133,8 +134,8 @@ static int try_rename(alpm_handle_t *handle, const char *src, const char *dest) if(rename(src, dest)) { _alpm_log(handle, ALPM_LOG_ERROR, _("could not rename %s to %s (%s)\n"), src, dest, strerror(errno)); - alpm_logaction(handle, "error: could not rename %s to %s (%s)\n", - src, dest, strerror(errno)); + alpm_logaction(handle, ALPM_CALLER_PREFIX, + "error: could not rename %s to %s (%s)\n", src, dest, strerror(errno)); return 1; } return 0; @@ -184,8 +185,8 @@ static int extract_single_file(alpm_handle_t *handle, struct archive *archive, if(alpm_list_find(handle->noextract, entryname, _alpm_fnmatch)) { _alpm_log(handle, ALPM_LOG_DEBUG, "%s is in NoExtract, skipping extraction\n", entryname); - alpm_logaction(handle, "note: %s is in NoExtract, skipping extraction\n", - entryname); + alpm_logaction(handle, ALPM_CALLER_PREFIX, + "note: %s is in NoExtract, skipping extraction\n", entryname); archive_read_data_skip(archive); return 0; } @@ -224,7 +225,8 @@ static int extract_single_file(alpm_handle_t *handle, struct archive *archive, _alpm_log(handle, ALPM_LOG_WARNING, _("directory permissions differ on %s\n" "filesystem: %o package: %o\n"), entryname, lsbuf.st_mode & mask, entrymode & mask); - alpm_logaction(handle, "warning: directory permissions differ on %s\n" + alpm_logaction(handle, ALPM_CALLER_PREFIX, + "warning: directory permissions differ on %s\n" "filesystem: %o package: %o\n", entryname, lsbuf.st_mode & mask, entrymode & mask); } @@ -348,7 +350,8 @@ static int extract_single_file(alpm_handle_t *handle, struct archive *archive, errors++; } else { _alpm_log(handle, ALPM_LOG_WARNING, _("%s saved as %s\n"), filename, newpath); - alpm_logaction(handle, "warning: %s saved as %s\n", filename, newpath); + alpm_logaction(handle, ALPM_CALLER_PREFIX, + "warning: %s saved as %s\n", filename, newpath); } } free(newpath); @@ -399,8 +402,8 @@ static int extract_single_file(alpm_handle_t *handle, struct archive *archive, } else { _alpm_log(handle, ALPM_LOG_WARNING, _("%s installed as %s\n"), filename, newpath); - alpm_logaction(handle, "warning: %s installed as %s\n", - filename, newpath); + alpm_logaction(handle, ALPM_CALLER_PREFIX, + "warning: %s installed as %s\n", filename, newpath); } free(newpath); } @@ -416,7 +419,8 @@ needbackup_cleanup: /* 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, "warning: extracting %s as %s.pacnew\n", filename, filename); + alpm_logaction(handle, ALPM_CALLER_PREFIX, + "warning: extracting %s as %s.pacnew\n", filename, filename); strncat(filename, ".pacnew", PATH_MAX - strlen(filename)); } else { _alpm_log(handle, ALPM_LOG_DEBUG, "extracting %s\n", filename); @@ -518,7 +522,8 @@ static int commit_single_pkg(alpm_handle_t *handle, alpm_pkg_t *newpkg, /* prepare directory for database entries so permission are correct after changelog/install script installation (FS#12263) */ if(_alpm_local_db_prepare(db, newpkg)) { - alpm_logaction(handle, "error: could not create database entry %s-%s\n", + alpm_logaction(handle, ALPM_CALLER_PREFIX, + "error: could not create database entry %s-%s\n", newpkg->name, newpkg->version); handle->pm_errno = ALPM_ERR_DB_WRITE; ret = -1; @@ -609,12 +614,14 @@ static int commit_single_pkg(alpm_handle_t *handle, alpm_pkg_t *newpkg, if(is_upgrade) { _alpm_log(handle, ALPM_LOG_ERROR, _("problem occurred while upgrading %s\n"), newpkg->name); - alpm_logaction(handle, "error: problem occurred while upgrading %s\n", + alpm_logaction(handle, ALPM_CALLER_PREFIX, + "error: problem occurred while upgrading %s\n", newpkg->name); } else { _alpm_log(handle, ALPM_LOG_ERROR, _("problem occurred while installing %s\n"), newpkg->name); - alpm_logaction(handle, "error: problem occurred while installing %s\n", + alpm_logaction(handle, ALPM_CALLER_PREFIX, + "error: problem occurred while installing %s\n", newpkg->name); } } @@ -629,7 +636,8 @@ static int commit_single_pkg(alpm_handle_t *handle, alpm_pkg_t *newpkg, if(_alpm_local_db_write(db, newpkg, INFRQ_ALL)) { _alpm_log(handle, ALPM_LOG_ERROR, _("could not update database entry %s-%s\n"), newpkg->name, newpkg->version); - alpm_logaction(handle, "error: could not update database entry %s-%s\n", + alpm_logaction(handle, ALPM_CALLER_PREFIX, + "error: could not update database entry %s-%s\n", newpkg->name, newpkg->version); handle->pm_errno = ALPM_ERR_DB_WRITE; ret = -1; diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 6e8c93d..f524c19 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -268,7 +268,8 @@ typedef enum _alpm_loglevel_t { } alpm_loglevel_t; typedef void (*alpm_cb_log)(alpm_loglevel_t, const char *, va_list); -int alpm_logaction(alpm_handle_t *handle, const char *fmt, ...); +int alpm_logaction(alpm_handle_t *handle, const char *prefix, + const char *fmt, ...); /** * Events. diff --git a/lib/libalpm/log.c b/lib/libalpm/log.c index 74143fd..a8639b4 100644 --- a/lib/libalpm/log.c +++ b/lib/libalpm/log.c @@ -35,10 +35,12 @@ /** A printf-like function for logging. * @param handle the context handle + * @param prefix caller-specific prefix for the log * @param fmt output format * @return 0 on success, -1 on error (pm_errno is set accordingly) */ -int SYMEXPORT alpm_logaction(alpm_handle_t *handle, const char *fmt, ...) +int SYMEXPORT alpm_logaction(alpm_handle_t *handle, const char *prefix, const + char *fmt, ...) { int ret; va_list args; @@ -62,20 +64,9 @@ int SYMEXPORT alpm_logaction(alpm_handle_t *handle, const char *fmt, ...) } va_start(args, fmt); - ret = _alpm_logaction(handle, fmt, args); + ret = _alpm_logaction(handle, prefix, fmt, args); va_end(args); - /* TODO We should add a prefix to log strings depending on who called us. - * If logaction was called by the frontend: - * USER: <the frontend log> - * and if called internally: - * ALPM: <the library log> - * Moreover, the frontend should be able to choose its prefix - * (USER by default?): - * pacman: "PACMAN" - * kpacman: "KPACMAN" - * This would allow us to share the log file between several frontends - * and know who does what */ return ret; } diff --git a/lib/libalpm/log.h b/lib/libalpm/log.h index b44ea18..dd60c85 100644 --- a/lib/libalpm/log.h +++ b/lib/libalpm/log.h @@ -22,6 +22,8 @@ #include "alpm.h" +#define ALPM_CALLER_PREFIX "ALPM" + void _alpm_log(alpm_handle_t *handle, alpm_loglevel_t flag, const char *fmt, ...) __attribute__((format(printf,3,4))); diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c index cdc572d..50fc93c 100644 --- a/lib/libalpm/remove.c +++ b/lib/libalpm/remove.c @@ -479,13 +479,15 @@ static int unlink_file(alpm_handle_t *handle, alpm_pkg_t *oldpkg, if(rename(file, newpath)) { _alpm_log(handle, ALPM_LOG_ERROR, _("could not rename %s to %s (%s)\n"), file, newpath, strerror(errno)); - alpm_logaction(handle, "error: could not rename %s to %s (%s)\n", + alpm_logaction(handle, ALPM_CALLER_PREFIX, + "error: could not rename %s to %s (%s)\n", file, newpath, strerror(errno)); free(newpath); return -1; } _alpm_log(handle, ALPM_LOG_WARNING, _("%s saved as %s\n"), file, newpath); - alpm_logaction(handle, "warning: %s saved as %s\n", file, newpath); + alpm_logaction(handle, ALPM_CALLER_PREFIX, + "warning: %s saved as %s\n", file, newpath); free(newpath); return 0; } @@ -497,8 +499,8 @@ static int unlink_file(alpm_handle_t *handle, alpm_pkg_t *oldpkg, if(unlink(file) == -1) { _alpm_log(handle, ALPM_LOG_ERROR, _("cannot remove %s (%s)\n"), file, strerror(errno)); - alpm_logaction(handle, "error: cannot remove %s (%s)\n", - file, strerror(errno)); + alpm_logaction(handle, ALPM_CALLER_PREFIX, + "error: cannot remove %s (%s)\n", file, strerror(errno)); return -1; } } diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c index fd1d786..700a717 100644 --- a/lib/libalpm/util.c +++ b/lib/libalpm/util.c @@ -453,10 +453,15 @@ ssize_t _alpm_files_in_directory(alpm_handle_t *handle, const char *path, * @return 0 or number of characters written on success, vfprintf return value * on error */ -int _alpm_logaction(alpm_handle_t *handle, const char *fmt, va_list args) +int _alpm_logaction(alpm_handle_t *handle, const char *prefix, + const char *fmt, va_list args) { int ret = 0; + if(!(prefix && *prefix)) { + prefix = "UNKNOWN"; + } + if(handle->usesyslog) { /* we can't use a va_list more than once, so we need to copy it * so we can use the original when calling vfprintf below. */ @@ -474,9 +479,9 @@ int _alpm_logaction(alpm_handle_t *handle, const char *fmt, va_list args) tm = localtime(&t); /* Use ISO-8601 date format */ - fprintf(handle->logstream, "[%04d-%02d-%02d %02d:%02d] ", + fprintf(handle->logstream, "[%04d-%02d-%02d %02d:%02d] [%s] ", tm->tm_year + 1900, tm->tm_mon + 1, tm->tm_mday, - tm->tm_hour, tm->tm_min); + tm->tm_hour, tm->tm_min, prefix); ret = vfprintf(handle->logstream, fmt, args); fflush(handle->logstream); } @@ -568,7 +573,7 @@ int _alpm_run_chroot(alpm_handle_t *handle, const char *cmd, char *const argv[]) char line[PATH_MAX]; if(fgets(line, PATH_MAX, pipe_file) == NULL) break; - alpm_logaction(handle, "%s", line); + alpm_logaction(handle, "ALPM-SCRIPTLET", "%s", line); EVENT(handle, ALPM_EVENT_SCRIPTLET_INFO, line, NULL); } fclose(pipe_file); diff --git a/lib/libalpm/util.h b/lib/libalpm/util.h index 3a6b14a..93b6573 100644 --- a/lib/libalpm/util.h +++ b/lib/libalpm/util.h @@ -120,7 +120,7 @@ int _alpm_unpack(alpm_handle_t *handle, const char *archive, const char *prefix, alpm_list_t *list, int breakfirst); ssize_t _alpm_files_in_directory(alpm_handle_t *handle, const char *path, int full_count); -int _alpm_logaction(alpm_handle_t *handle, const char *fmt, va_list args); +int _alpm_logaction(alpm_handle_t *handle, const char *prefix, const char *fmt, va_list args); int _alpm_run_chroot(alpm_handle_t *handle, const char *cmd, char *const argv[]); int _alpm_ldconfig(alpm_handle_t *handle); int _alpm_str_cmp(const void *s1, const void *s2); diff --git a/src/pacman/callback.c b/src/pacman/callback.c index f53f59c..93418db 100644 --- a/src/pacman/callback.c +++ b/src/pacman/callback.c @@ -31,6 +31,7 @@ #include <alpm.h> /* pacman */ +#include "pacman.h" #include "callback.h" #include "util.h" #include "conf.h" @@ -176,9 +177,10 @@ void cb_event(alpm_event_t event, void *data1, void *data2) } break; case ALPM_EVENT_ADD_DONE: - alpm_logaction(config->handle, "installed %s (%s)\n", - alpm_pkg_get_name(data1), - alpm_pkg_get_version(data1)); + alpm_logaction(config->handle, PACMAN_CALLER_PREFIX, + "installed %s (%s)\n", + alpm_pkg_get_name(data1), + alpm_pkg_get_version(data1)); display_optdepends(data1); break; case ALPM_EVENT_REMOVE_START: @@ -187,9 +189,10 @@ void cb_event(alpm_event_t event, void *data1, void *data2) } break; case ALPM_EVENT_REMOVE_DONE: - alpm_logaction(config->handle, "removed %s (%s)\n", - alpm_pkg_get_name(data1), - alpm_pkg_get_version(data1)); + alpm_logaction(config->handle, PACMAN_CALLER_PREFIX, + "removed %s (%s)\n", + alpm_pkg_get_name(data1), + alpm_pkg_get_version(data1)); break; case ALPM_EVENT_UPGRADE_START: if(config->noprogressbar) { @@ -197,10 +200,11 @@ void cb_event(alpm_event_t event, void *data1, void *data2) } break; case ALPM_EVENT_UPGRADE_DONE: - alpm_logaction(config->handle, "upgraded %s (%s -> %s)\n", - alpm_pkg_get_name(data1), - alpm_pkg_get_version(data2), - alpm_pkg_get_version(data1)); + alpm_logaction(config->handle, PACMAN_CALLER_PREFIX, + "upgraded %s (%s -> %s)\n", + alpm_pkg_get_name(data1), + alpm_pkg_get_version(data2), + alpm_pkg_get_version(data1)); display_new_optdepends(data2, data1); break; case ALPM_EVENT_INTEGRITY_START: diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index f3ca8b9..8448d6b 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -744,7 +744,8 @@ static void cl_to_log(int argc, char *argv[]) *p++ = ' '; } strcpy(p, argv[i]); - alpm_logaction(config->handle, "Running '%s'\n", cl_text); + alpm_logaction(config->handle, PACMAN_CALLER_PREFIX, + "Running '%s'\n", cl_text); free(cl_text); } diff --git a/src/pacman/pacman.h b/src/pacman/pacman.h index 4eaf318..7c9c365 100644 --- a/src/pacman/pacman.h +++ b/src/pacman/pacman.h @@ -22,6 +22,8 @@ #include <alpm_list.h> +#define PACMAN_CALLER_PREFIX "PACMAN" + /* database.c */ int pacman_database(alpm_list_t *targets); /* deptest.c */ diff --git a/src/pacman/sync.c b/src/pacman/sync.c index fef2940..3decc40 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -787,7 +787,8 @@ 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"); + alpm_logaction(config->handle, PACMAN_CALLER_PREFIX, + "starting full system upgrade\n"); 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(); @@ -954,7 +955,8 @@ int pacman_sync(alpm_list_t *targets) if(config->op_s_sync) { /* grab a fresh package list */ printf(_(":: Synchronizing package databases...\n")); - alpm_logaction(config->handle, "synchronizing package lists\n"); + alpm_logaction(config->handle, PACMAN_CALLER_PREFIX, + "synchronizing package lists\n"); if(!sync_synctree(config->op_s_sync, sync_dbs)) { return 1; } -- 1.8.1.1
Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- I'm not very familiar with awk, so there is probably a better way to do this. contrib/paclog-pkglist.sh.in | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/contrib/paclog-pkglist.sh.in b/contrib/paclog-pkglist.sh.in index 222bbc4..e1bd58e 100644 --- a/contrib/paclog-pkglist.sh.in +++ b/contrib/paclog-pkglist.sh.in @@ -50,25 +50,34 @@ fi <"$logfile" awk ' { - action = $3 - pkgname = $4 - pkgver = $5 - upgver = $7 + if ($3 ~ /^\[.*\]$/) { + # new style with caller name + action = $4 + pkgname = $5 + pkgver = $6 + upgver = $8 + } else { + action = $3 + pkgname = $4 + pkgver = $5 + upgver = $7 + NF = (NF + 1) + } } -NF == 5 && action == "installed" { +NF == 6 && action == "installed" { gsub(/[()]/, "", pkgver) pkg[pkgname] = pkgver next } -NF == 7 && action == "upgraded" { +NF == 8 && action == "upgraded" { sub(/\)/, "", upgver) pkg[pkgname] = upgver next } -NF == 5 && action == "removed" { +NF == 6 && action == "removed" { pkg[pkgname] = -1 } -- 1.8.1.1
On 19/01/13 11:42, Andrew Gregory wrote:
Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> ---
I'm not very familiar with awk, so there is probably a better way to do this.
Dave: can you please comment on this?
contrib/paclog-pkglist.sh.in | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/contrib/paclog-pkglist.sh.in b/contrib/paclog-pkglist.sh.in index 222bbc4..e1bd58e 100644 --- a/contrib/paclog-pkglist.sh.in +++ b/contrib/paclog-pkglist.sh.in @@ -50,25 +50,34 @@ fi
<"$logfile" awk ' { - action = $3 - pkgname = $4 - pkgver = $5 - upgver = $7 + if ($3 ~ /^\[.*\]$/) { + # new style with caller name + action = $4 + pkgname = $5 + pkgver = $6 + upgver = $8 + } else { + action = $3 + pkgname = $4 + pkgver = $5 + upgver = $7 + NF = (NF + 1) + } }
-NF == 5 && action == "installed" { +NF == 6 && action == "installed" { gsub(/[()]/, "", pkgver) pkg[pkgname] = pkgver next }
-NF == 7 && action == "upgraded" { +NF == 8 && action == "upgraded" { sub(/\)/, "", upgver) pkg[pkgname] = upgver next }
-NF == 5 && action == "removed" { +NF == 6 && action == "removed" { pkg[pkgname] = -1 }
On Fri, Jan 18, 2013 at 08:42:22PM -0500, Andrew Gregory wrote:
Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> ---
I'm not very familiar with awk, so there is probably a better way to do this.
contrib/paclog-pkglist.sh.in | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/contrib/paclog-pkglist.sh.in b/contrib/paclog-pkglist.sh.in index 222bbc4..e1bd58e 100644 --- a/contrib/paclog-pkglist.sh.in +++ b/contrib/paclog-pkglist.sh.in @@ -50,25 +50,34 @@ fi
<"$logfile" awk ' { - action = $3 - pkgname = $4 - pkgver = $5 - upgver = $7 + if ($3 ~ /^\[.*\]$/) { + # new style with caller name + action = $4 + pkgname = $5 + pkgver = $6 + upgver = $8 + } else { + action = $3 + pkgname = $4 + pkgver = $5 + upgver = $7 + NF = (NF + 1)
Would prefer using a different varname over modifying/lying about the value of an internal var. This seems fine otherwise.
+ } }
-NF == 5 && action == "installed" { +NF == 6 && action == "installed" { gsub(/[()]/, "", pkgver) pkg[pkgname] = pkgver next }
-NF == 7 && action == "upgraded" { +NF == 8 && action == "upgraded" { sub(/\)/, "", upgver) pkg[pkgname] = upgver next }
-NF == 5 && action == "removed" { +NF == 6 && action == "removed" { pkg[pkgname] = -1 }
-- 1.8.1.1
On 21/01/13 22:56, Dave Reisner wrote:
On Fri, Jan 18, 2013 at 08:42:22PM -0500, Andrew Gregory wrote:
Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> ---
I'm not very familiar with awk, so there is probably a better way to do this.
contrib/paclog-pkglist.sh.in | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/contrib/paclog-pkglist.sh.in b/contrib/paclog-pkglist.sh.in index 222bbc4..e1bd58e 100644 --- a/contrib/paclog-pkglist.sh.in +++ b/contrib/paclog-pkglist.sh.in @@ -50,25 +50,34 @@ fi
<"$logfile" awk ' { - action = $3 - pkgname = $4 - pkgver = $5 - upgver = $7 + if ($3 ~ /^\[.*\]$/) { + # new style with caller name + action = $4 + pkgname = $5 + pkgver = $6 + upgver = $8 + } else { + action = $3 + pkgname = $4 + pkgver = $5 + upgver = $7 + NF = (NF + 1)
Would prefer using a different varname over modifying/lying about the value of an internal var.
This seems fine otherwise.
This and patch 1/2 look fine to me with this change. Allan
Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- contrib/paclog-pkglist.sh.in | 24 +++++++++++++++++------- contrib/updpkgsums.sh.in | 0 2 files changed, 17 insertions(+), 7 deletions(-) mode change 100755 => 100644 contrib/updpkgsums.sh.in diff --git a/contrib/paclog-pkglist.sh.in b/contrib/paclog-pkglist.sh.in index 222bbc4..908bd4a 100644 --- a/contrib/paclog-pkglist.sh.in +++ b/contrib/paclog-pkglist.sh.in @@ -50,25 +50,35 @@ fi <"$logfile" awk ' { - action = $3 - pkgname = $4 - pkgver = $5 - upgver = $7 + if ($3 ~ /^\[.*\]$/) { + # new style with caller name + action = $4 + pkgname = $5 + pkgver = $6 + upgver = $8 + nfields = NF + } else { + action = $3 + pkgname = $4 + pkgver = $5 + upgver = $7 + nfields = (NF + 1) # compensate for missing caller field + } } -NF == 5 && action == "installed" { +nfields == 6 && action == "installed" { gsub(/[()]/, "", pkgver) pkg[pkgname] = pkgver next } -NF == 7 && action == "upgraded" { +nfields == 8 && action == "upgraded" { sub(/\)/, "", upgver) pkg[pkgname] = upgver next } -NF == 5 && action == "removed" { +nfields == 6 && action == "removed" { pkg[pkgname] = -1 } diff --git a/contrib/updpkgsums.sh.in b/contrib/updpkgsums.sh.in old mode 100755 new mode 100644 -- 1.8.1.1
Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- Let's try that again without the sneaky permission change this time... contrib/paclog-pkglist.sh.in | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/contrib/paclog-pkglist.sh.in b/contrib/paclog-pkglist.sh.in index 222bbc4..908bd4a 100644 --- a/contrib/paclog-pkglist.sh.in +++ b/contrib/paclog-pkglist.sh.in @@ -50,25 +50,35 @@ fi <"$logfile" awk ' { - action = $3 - pkgname = $4 - pkgver = $5 - upgver = $7 + if ($3 ~ /^\[.*\]$/) { + # new style with caller name + action = $4 + pkgname = $5 + pkgver = $6 + upgver = $8 + nfields = NF + } else { + action = $3 + pkgname = $4 + pkgver = $5 + upgver = $7 + nfields = (NF + 1) # compensate for missing caller field + } } -NF == 5 && action == "installed" { +nfields == 6 && action == "installed" { gsub(/[()]/, "", pkgver) pkg[pkgname] = pkgver next } -NF == 7 && action == "upgraded" { +nfields == 8 && action == "upgraded" { sub(/\)/, "", upgver) pkg[pkgname] = upgver next } -NF == 5 && action == "removed" { +nfields == 6 && action == "removed" { pkg[pkgname] = -1 } -- 1.8.1.1
On 24/01/13 10:28, Andrew Gregory wrote:
Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> ---
Let's try that again without the sneaky permission change this time...
Let's also pretend I did not ack the last one... :P
On 24/01/13 10:20, Andrew Gregory wrote:
Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> ---
Ack.
participants (3)
-
Allan McRae
-
Andrew Gregory
-
Dave Reisner