[pacman-dev] [PATCH] alpm_dep_get_string
Hi! If you look into libalpm code, you will realize that usually only depend->name is used in log/error messages; which can be confusing in some context (sometimes, alpm assumes that the missing dependency is a canonical package name). See my note here, for example: http://www.archlinux.org/pipermail/pacman-dev/2007-October/009671.html So I started to work on this. Most cases dependency is "created" with alpm_splitdep from a string; in these cases we can use that string in error messages. However, in few cases dependency "comes from" pmdepmissing_t, so we have to convert pmdepend_t to string. As a first step, I coded this converter-function; which is going to be used in resolvedeps only (in libalpm). ---------------------------- From 074e80216543c631f51079b4c27efbf8dcefff16 Mon Sep 17 00:00:00 2001 From: Nagy Gabor <ngaba@@bibl..u-szeged..hu> Date: Thu, 25 Oct 2007 02:16:48 +0200 Subject: [PATCH] alpm_dep_get_string Public alpm_dep_get_string function is introduced, which converts a pmdepend_t structure to printable string in %DEPENDS% format. This function is now used in pacman to print dependency error messages. --- lib/libalpm/alpm.h | 1 + lib/libalpm/deps.c | 34 ++++++++++++++++++++++++++++++++++ src/pacman/add.c | 18 +++--------------- src/pacman/remove.c | 4 +++- src/pacman/sync.c | 18 +++--------------- 5 files changed, 44 insertions(+), 31 deletions(-) diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 880bbeb..2000db4 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -379,6 +379,7 @@ pmdepend_t *alpm_miss_get_dep(pmdepmissing_t *miss); pmdepmod_t alpm_dep_get_mod(pmdepend_t *dep); const char *alpm_dep_get_name(pmdepend_t *dep); const char *alpm_dep_get_version(pmdepend_t *dep); +char *alpm_dep_get_string(pmdepend_t *dep); /* * File conflicts diff --git a/lib/libalpm/deps.c b/lib/libalpm/deps.c index b7e49be..55d3ee7 100644 --- a/lib/libalpm/deps.c +++ b/lib/libalpm/deps.c @@ -835,4 +835,38 @@ const char SYMEXPORT *alpm_dep_get_version(pmdepend_t *dep) return dep->version; } +/* the return-string must be freed! */ +char SYMEXPORT *alpm_dep_get_string(pmdepend_t *dep) +{ + ALPM_LOG_FUNC; + + /* Sanity checks */ + ASSERT(handle != NULL, return(NULL)); + ASSERT(dep != NULL, return(NULL)); + + char *ptr; + char *depstring = malloc(sizeof(pmdepend_t)); + if(depstring == NULL) { + _alpm_log(PM_LOG_ERROR, _("malloc failure: could not allocate %d bytes\n"), sizeof(pmdepend_t)); + return NULL; + } + + strcpy(depstring, dep->name); + ptr = depstring + strlen(depstring); + switch(dep->mod) { + case PM_DEP_MOD_ANY: + break; + case PM_DEP_MOD_EQ: + sprintf(ptr, "=%s", dep->version); + break; + case PM_DEP_MOD_GE: + sprintf(ptr, ">=%s", dep->version); + break; + case PM_DEP_MOD_LE: + sprintf(ptr, "<=%s", dep->version); + break; + } + + return(depstring); +} /* vim: set ts=2 sw=2 noet: */ diff --git a/src/pacman/add.c b/src/pacman/add.c index 0b59a23..f883a6b 100644 --- a/src/pacman/add.c +++ b/src/pacman/add.c @@ -137,26 +137,14 @@ int pacman_add(alpm_list_t *targets) for(i = data; i; i = alpm_list_next(i)) { pmdepmissing_t *miss = alpm_list_getdata(i); pmdepend_t *dep = alpm_miss_get_dep(miss); + char *depstring = alpm_dep_get_string(dep); /* TODO indicate if the error was a virtual package or not: * :: %s: requires %s, provided by %s */ printf(_(":: %s: requires %s\n"), alpm_miss_get_target(miss), - alpm_dep_get_name(dep)); - switch(alpm_dep_get_mod(dep)) { - case PM_DEP_MOD_ANY: - break; - case PM_DEP_MOD_EQ: - printf("=%s", alpm_dep_get_version(dep)); - break; - case PM_DEP_MOD_GE: - printf(">=%s", alpm_dep_get_version(dep)); - break; - case PM_DEP_MOD_LE: - printf("<=%s", alpm_dep_get_version(dep)); - break; - } - printf("\n"); + depstring); + free(depstring); } break; case PM_ERR_CONFLICTING_DEPS: diff --git a/src/pacman/remove.c b/src/pacman/remove.c index 860bf49..dce479e 100644 --- a/src/pacman/remove.c +++ b/src/pacman/remove.c @@ -129,8 +129,10 @@ int pacman_remove(alpm_list_t *targets) for(i = data; i; i = alpm_list_next(i)) { pmdepmissing_t *miss = alpm_list_getdata(i); pmdepend_t *dep = alpm_miss_get_dep(miss); + char *depstring = alpm_dep_get_string(dep); printf(_(":: %s: requires %s\n"), alpm_miss_get_target(miss), - alpm_dep_get_name(dep)); + depstring); + free(depstring); } alpm_list_free(data); break; diff --git a/src/pacman/sync.c b/src/pacman/sync.c index bf6eed1..2da7f55 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -558,22 +558,10 @@ int sync_trans(alpm_list_t *targets, int sync_only) for(i = data; i; i = alpm_list_next(i)) { pmdepmissing_t *miss = alpm_list_getdata(i); pmdepend_t *dep = alpm_miss_get_dep(miss); + char *depstring = alpm_dep_get_string(dep); printf(_(":: %s: requires %s\n"), alpm_miss_get_target(miss), - alpm_dep_get_name(dep)); - switch(alpm_dep_get_mod(dep)) { - case PM_DEP_MOD_ANY: - break; - case PM_DEP_MOD_EQ: - printf("=%s", alpm_dep_get_version(dep)); - break; - case PM_DEP_MOD_GE: - printf(">=%s", alpm_dep_get_version(dep)); - break; - case PM_DEP_MOD_LE: - printf("<=%s", alpm_dep_get_version(dep)); - break; - } - printf("\n"); + depstring); + free(depstring); } break; case PM_ERR_CONFLICTING_DEPS: -- 1.5.3.4
On 10/24/07, Nagy Gabor <ngaba@bibl.u-szeged.hu> wrote:
Hi! If you look into libalpm code, you will realize that usually only depend->name is used in log/error messages; which can be confusing in some context (sometimes, alpm assumes that the missing dependency is a canonical package name). See my note here, for example: http://www.archlinux.org/pipermail/pacman-dev/2007-October/009671.html So I started to work on this. Most cases dependency is "created" with alpm_splitdep from a string; in these cases we can use that string in error messages. However, in few cases dependency "comes from" pmdepmissing_t, so we have to convert pmdepend_t to string. As a first step, I coded this converter-function; which is going to be used in resolvedeps only (in libalpm). ----------------------------
From 074e80216543c631f51079b4c27efbf8dcefff16 Mon Sep 17 00:00:00 2001 From: Nagy Gabor <ngaba@@bibl..u-szeged..hu> Date: Thu, 25 Oct 2007 02:16:48 +0200 Subject: [PATCH] alpm_dep_get_string Public alpm_dep_get_string function is introduced, which converts a pmdepend_t structure to printable string in %DEPENDS% format. This function is now used in pacman to print dependency error messages.
--- lib/libalpm/alpm.h | 1 + lib/libalpm/deps.c | 34 ++++++++++++++++++++++++++++++++++ src/pacman/add.c | 18 +++--------------- src/pacman/remove.c | 4 +++- src/pacman/sync.c | 18 +++--------------- 5 files changed, 44 insertions(+), 31 deletions(-)
diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 880bbeb..2000db4 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -379,6 +379,7 @@ pmdepend_t *alpm_miss_get_dep(pmdepmissing_t *miss); pmdepmod_t alpm_dep_get_mod(pmdepend_t *dep); const char *alpm_dep_get_name(pmdepend_t *dep); const char *alpm_dep_get_version(pmdepend_t *dep); +char *alpm_dep_get_string(pmdepend_t *dep);
/* * File conflicts diff --git a/lib/libalpm/deps.c b/lib/libalpm/deps.c index b7e49be..55d3ee7 100644 --- a/lib/libalpm/deps.c +++ b/lib/libalpm/deps.c @@ -835,4 +835,38 @@ const char SYMEXPORT *alpm_dep_get_version(pmdepend_t *dep) return dep->version; }
+/* the return-string must be freed! */ +char SYMEXPORT *alpm_dep_get_string(pmdepend_t *dep) +{ + ALPM_LOG_FUNC; + + /* Sanity checks */ + ASSERT(handle != NULL, return(NULL)); + ASSERT(dep != NULL, return(NULL)); + + char *ptr; + char *depstring = malloc(sizeof(pmdepend_t)); + if(depstring == NULL) { + _alpm_log(PM_LOG_ERROR, _("malloc failure: could not allocate %d bytes\n"), sizeof(pmdepend_t)); + return NULL; + } + + strcpy(depstring, dep->name); + ptr = depstring + strlen(depstring); + switch(dep->mod) { + case PM_DEP_MOD_ANY: + break; + case PM_DEP_MOD_EQ: + sprintf(ptr, "=%s", dep->version); + break; + case PM_DEP_MOD_GE: + sprintf(ptr, ">=%s", dep->version); + break; + case PM_DEP_MOD_LE: + sprintf(ptr, "<=%s", dep->version); + break; + } + + return(depstring); +} /* vim: set ts=2 sw=2 noet: */ diff --git a/src/pacman/add.c b/src/pacman/add.c index 0b59a23..f883a6b 100644 --- a/src/pacman/add.c +++ b/src/pacman/add.c @@ -137,26 +137,14 @@ int pacman_add(alpm_list_t *targets) for(i = data; i; i = alpm_list_next(i)) { pmdepmissing_t *miss = alpm_list_getdata(i); pmdepend_t *dep = alpm_miss_get_dep(miss); + char *depstring = alpm_dep_get_string(dep);
/* TODO indicate if the error was a virtual package or not: * :: %s: requires %s, provided by %s */ printf(_(":: %s: requires %s\n"), alpm_miss_get_target(miss), - alpm_dep_get_name(dep)); - switch(alpm_dep_get_mod(dep)) { - case PM_DEP_MOD_ANY: - break; - case PM_DEP_MOD_EQ: - printf("=%s", alpm_dep_get_version(dep)); - break; - case PM_DEP_MOD_GE: - printf(">=%s", alpm_dep_get_version(dep)); - break; - case PM_DEP_MOD_LE: - printf("<=%s", alpm_dep_get_version(dep)); - break; - } - printf("\n"); + depstring); + free(depstring); } break; case PM_ERR_CONFLICTING_DEPS: diff --git a/src/pacman/remove.c b/src/pacman/remove.c index 860bf49..dce479e 100644 --- a/src/pacman/remove.c +++ b/src/pacman/remove.c @@ -129,8 +129,10 @@ int pacman_remove(alpm_list_t *targets) for(i = data; i; i = alpm_list_next(i)) { pmdepmissing_t *miss = alpm_list_getdata(i); pmdepend_t *dep = alpm_miss_get_dep(miss); + char *depstring = alpm_dep_get_string(dep); printf(_(":: %s: requires %s\n"), alpm_miss_get_target(miss), - alpm_dep_get_name(dep)); + depstring); + free(depstring); } alpm_list_free(data); break; diff --git a/src/pacman/sync.c b/src/pacman/sync.c index bf6eed1..2da7f55 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -558,22 +558,10 @@ int sync_trans(alpm_list_t *targets, int sync_only) for(i = data; i; i = alpm_list_next(i)) { pmdepmissing_t *miss = alpm_list_getdata(i); pmdepend_t *dep = alpm_miss_get_dep(miss); + char *depstring = alpm_dep_get_string(dep); printf(_(":: %s: requires %s\n"), alpm_miss_get_target(miss), - alpm_dep_get_name(dep)); - switch(alpm_dep_get_mod(dep)) { - case PM_DEP_MOD_ANY: - break; - case PM_DEP_MOD_EQ: - printf("=%s", alpm_dep_get_version(dep)); - break; - case PM_DEP_MOD_GE: - printf(">=%s", alpm_dep_get_version(dep)); - break; - case PM_DEP_MOD_LE: - printf("<=%s", alpm_dep_get_version(dep)); - break; - } - printf("\n"); + depstring); + free(depstring); } break; case PM_ERR_CONFLICTING_DEPS: -- 1.5.3.4
Seems sane to me. Aaron, any objections? -Dan
On 10/24/07, Dan McGee <dpmcgee@gmail.com> wrote:
On 10/24/07, Nagy Gabor <ngaba@bibl.u-szeged.hu> wrote:
Hi! If you look into libalpm code, you will realize that usually only depend->name is used in log/error messages; which can be confusing in some context (sometimes, alpm assumes that the missing dependency is a canonical package name). See my note here, for example: http://www.archlinux.org/pipermail/pacman-dev/2007-October/009671.html So I started to work on this. Most cases dependency is "created" with alpm_splitdep from a string; in these cases we can use that string in error messages. However, in few cases dependency "comes from" pmdepmissing_t, so we have to convert pmdepend_t to string. As a first step, I coded this converter-function; which is going to be used in resolvedeps only (in libalpm). ----------------------------
From 074e80216543c631f51079b4c27efbf8dcefff16 Mon Sep 17 00:00:00 2001 From: Nagy Gabor <ngaba@@bibl..u-szeged..hu> Date: Thu, 25 Oct 2007 02:16:48 +0200 Subject: [PATCH] alpm_dep_get_string Public alpm_dep_get_string function is introduced, which converts a pmdepend_t structure to printable string in %DEPENDS% format. This function is now used in pacman to print dependency error messages.
--- lib/libalpm/alpm.h | 1 + lib/libalpm/deps.c | 34 ++++++++++++++++++++++++++++++++++ src/pacman/add.c | 18 +++--------------- src/pacman/remove.c | 4 +++- src/pacman/sync.c | 18 +++--------------- 5 files changed, 44 insertions(+), 31 deletions(-)
diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 880bbeb..2000db4 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -379,6 +379,7 @@ pmdepend_t *alpm_miss_get_dep(pmdepmissing_t *miss); pmdepmod_t alpm_dep_get_mod(pmdepend_t *dep); const char *alpm_dep_get_name(pmdepend_t *dep); const char *alpm_dep_get_version(pmdepend_t *dep); +char *alpm_dep_get_string(pmdepend_t *dep);
/* * File conflicts diff --git a/lib/libalpm/deps.c b/lib/libalpm/deps.c index b7e49be..55d3ee7 100644 --- a/lib/libalpm/deps.c +++ b/lib/libalpm/deps.c @@ -835,4 +835,38 @@ const char SYMEXPORT *alpm_dep_get_version(pmdepend_t *dep) return dep->version; }
+/* the return-string must be freed! */ +char SYMEXPORT *alpm_dep_get_string(pmdepend_t *dep) +{ + ALPM_LOG_FUNC; + + /* Sanity checks */ + ASSERT(handle != NULL, return(NULL)); + ASSERT(dep != NULL, return(NULL)); + + char *ptr; + char *depstring = malloc(sizeof(pmdepend_t)); + if(depstring == NULL) { + _alpm_log(PM_LOG_ERROR, _("malloc failure: could not allocate %d bytes\n"), sizeof(pmdepend_t)); + return NULL; + } + + strcpy(depstring, dep->name); + ptr = depstring + strlen(depstring); + switch(dep->mod) { + case PM_DEP_MOD_ANY: + break; + case PM_DEP_MOD_EQ: + sprintf(ptr, "=%s", dep->version); + break; + case PM_DEP_MOD_GE: + sprintf(ptr, ">=%s", dep->version); + break; + case PM_DEP_MOD_LE: + sprintf(ptr, "<=%s", dep->version); + break; + } + + return(depstring); +} /* vim: set ts=2 sw=2 noet: */ diff --git a/src/pacman/add.c b/src/pacman/add.c index 0b59a23..f883a6b 100644 --- a/src/pacman/add.c +++ b/src/pacman/add.c @@ -137,26 +137,14 @@ int pacman_add(alpm_list_t *targets) for(i = data; i; i = alpm_list_next(i)) { pmdepmissing_t *miss = alpm_list_getdata(i); pmdepend_t *dep = alpm_miss_get_dep(miss); + char *depstring = alpm_dep_get_string(dep);
/* TODO indicate if the error was a virtual package or not: * :: %s: requires %s, provided by %s */ printf(_(":: %s: requires %s\n"), alpm_miss_get_target(miss), - alpm_dep_get_name(dep)); - switch(alpm_dep_get_mod(dep)) { - case PM_DEP_MOD_ANY: - break; - case PM_DEP_MOD_EQ: - printf("=%s", alpm_dep_get_version(dep)); - break; - case PM_DEP_MOD_GE: - printf(">=%s", alpm_dep_get_version(dep)); - break; - case PM_DEP_MOD_LE: - printf("<=%s", alpm_dep_get_version(dep)); - break; - } - printf("\n"); + depstring); + free(depstring); } break; case PM_ERR_CONFLICTING_DEPS: diff --git a/src/pacman/remove.c b/src/pacman/remove.c index 860bf49..dce479e 100644 --- a/src/pacman/remove.c +++ b/src/pacman/remove.c @@ -129,8 +129,10 @@ int pacman_remove(alpm_list_t *targets) for(i = data; i; i = alpm_list_next(i)) { pmdepmissing_t *miss = alpm_list_getdata(i); pmdepend_t *dep = alpm_miss_get_dep(miss); + char *depstring = alpm_dep_get_string(dep); printf(_(":: %s: requires %s\n"), alpm_miss_get_target(miss), - alpm_dep_get_name(dep)); + depstring); + free(depstring); } alpm_list_free(data); break; diff --git a/src/pacman/sync.c b/src/pacman/sync.c index bf6eed1..2da7f55 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -558,22 +558,10 @@ int sync_trans(alpm_list_t *targets, int sync_only) for(i = data; i; i = alpm_list_next(i)) { pmdepmissing_t *miss = alpm_list_getdata(i); pmdepend_t *dep = alpm_miss_get_dep(miss); + char *depstring = alpm_dep_get_string(dep); printf(_(":: %s: requires %s\n"), alpm_miss_get_target(miss), - alpm_dep_get_name(dep)); - switch(alpm_dep_get_mod(dep)) { - case PM_DEP_MOD_ANY: - break; - case PM_DEP_MOD_EQ: - printf("=%s", alpm_dep_get_version(dep)); - break; - case PM_DEP_MOD_GE: - printf(">=%s", alpm_dep_get_version(dep)); - break; - case PM_DEP_MOD_LE: - printf("<=%s", alpm_dep_get_version(dep)); - break; - } - printf("\n"); + depstring); + free(depstring); } break; case PM_ERR_CONFLICTING_DEPS: -- 1.5.3.4
Seems sane to me. Aaron, any objections?
Actually, seems more than sane. I like this patch alot.
participants (3)
-
Aaron Griffin
-
Dan McGee
-
Nagy Gabor