[pacman-dev] small bug in libalpm/remove.c
Hi! Look at line 122-123: _alpm_log(PM_LOG_ERROR, _("could not find %s in database -- skipping\n"), miss->depend.name); Probably you want to say miss->target here. You can treat this mail as a patch, since it is very easy to fix it (now I'm working on removing pmdepend_t and I don't want to create a new branch;-) Bye, ngaba
On 10/23/07, Nagy Gabor <ngaba@bibl.u-szeged.hu> wrote:
You can treat this mail as a patch
Just for clarity... no, we can't. A patch is actually applicable with the 'patch' binary.
On Wed, Oct 24, 2007 at 01:15:39AM +0200, Nagy Gabor wrote:
Hi! Look at line 122-123: _alpm_log(PM_LOG_ERROR, _("could not find %s in database -- skipping\n"), miss->depend.name); Probably you want to say miss->target here. You can treat this mail as a patch, since it is very easy to fix it (now I'm working on removing pmdepend_t and I don't want to create a new branch;-)
Creating a branch is very easy with git, generating an one line patch too, this can be done in seconds :) It's true that a patch becomes more interesting with bigger changes, but it really doesn't cost anything to make one, and besides it makes Aaron and Dan less angry :d
On 10/23/07, Xavier <shiningxc@gmail.com> wrote:
On Wed, Oct 24, 2007 at 01:15:39AM +0200, Nagy Gabor wrote:
Hi! Look at line 122-123: _alpm_log(PM_LOG_ERROR, _("could not find %s in database -- skipping\n"), miss->depend.name); Probably you want to say miss->target here. You can treat this mail as a patch, since it is very easy to fix it (now I'm working on removing pmdepend_t and I don't want to create a new branch;-)
Creating a branch is very easy with git, generating an one line patch too, this can be done in seconds :) It's true that a patch becomes more interesting with bigger changes, but it really doesn't cost anything to make one, and besides it makes Aaron and Dan less angry :d
Hah, it's not that. It's just the _resistance_ to sending a properly formatted patch is silly.
On 10/23/07, Xavier <shiningxc@gmail.com> wrote:
On Wed, Oct 24, 2007 at 01:15:39AM +0200, Nagy Gabor wrote:
Hi! Look at line 122-123: _alpm_log(PM_LOG_ERROR, _("could not find %s in database -- skipping\n"), miss->depend.name); Probably you want to say miss->target here. You can treat this mail as a patch, since it is very easy to fix it (now I'm working on removing pmdepend_t and I don't want to create a new branch;-)
Creating a branch is very easy with git, generating an one line patch too, this can be done in seconds :) It's true that a patch becomes more interesting with bigger changes, but it really doesn't cost anything to make one, and besides it makes Aaron and Dan less angry :d
Hah, it's not that. It's just the _resistance_ to sending a properly formatted patch is silly. As I wrote in the patch description; that message must be NEVER listed. There are some parts of libalpm/pacman code which mustn't be reached by a bug-free libalpm/pacman (like this). Sometimes these code-parts induce a debug message only. However, I think, they are really good bug-indicators, so I would prefer something like this: _alpm_log(PM_LOG_ERROR, _("FATAL internal error occurred. Please report this to bugs.archlinux.org. Bug-code: %d")). You may say, that pactests cover all common cases, so I'm a bit paranoid. This may be true, but we have some unfixed bugs (imho conflict005.py is the most serious one); and in these cases these type of error messages can be treated as feedbacks too (in the example above, pacman should shout if it overwrites a file). What about this idea? This can be introduced step-by-step. So, here is the patch:
From 9036af619e04339c86c0019101bc2a0622591eed Mon Sep 17 00:00:00 2001 From: Nagy Gabor <ngaba@bibl.u-szeged.hu> Date: Fri, 26 Oct 2007 16:11:13 +0200 Subject: [PATCH] dependency error message fix in package.c Normally you must never see that error message. --- lib/libalpm/remove.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c index 734c365..0ed4dd0 100644 --- a/lib/libalpm/remove.c +++ b/lib/libalpm/remove.c @@ -120,7 +120,7 @@ int _alpm_remove_prepare(pmtrans_t *trans, pmdb_t *db, alpm_list_t **data) } } else { _alpm_log(PM_LOG_ERROR, _("could not find %s in database -- skipping\n"), - miss->depend.name); + miss->target); } } FREELIST(lp); -- 1.5.3.4
Hm. I dunno why I said package.c; probably there was a glitch in my brain. Due to your wish I resubmit this trivial change too: From 9036af619e04339c86c0019101bc2a0622591eed Mon Sep 17 00:00:00 2001 From: Nagy Gabor <ngaba@bibl.u-szeged.hu> Date: Fri, 26 Oct 2007 16:11:13 +0200 Subject: [PATCH] dependency error message fix in libalpm/remove.c Normally you must never see that error message. --- lib/libalpm/remove.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c index 734c365..0ed4dd0 100644 --- a/lib/libalpm/remove.c +++ b/lib/libalpm/remove.c @@ -120,7 +120,7 @@ int _alpm_remove_prepare(pmtrans_t *trans, pmdb_t *db, alpm_list_t **data) } } else { _alpm_log(PM_LOG_ERROR, _("could not find %s in database -- skipping\n"), - miss->depend.name); + miss->target); } } FREELIST(lp); -- 1.5.3.4
On 10/26/07, Nagy Gabor <ngaba@bibl.u-szeged.hu> wrote:
Hm. I dunno why I said package.c; probably there was a glitch in my brain. Due to your wish I resubmit this trivial change too:
From 9036af619e04339c86c0019101bc2a0622591eed Mon Sep 17 00:00:00 2001 From: Nagy Gabor <ngaba@bibl.u-szeged.hu> Date: Fri, 26 Oct 2007 16:11:13 +0200 Subject: [PATCH] dependency error message fix in libalpm/remove.c Normally you must never see that error message.
--- lib/libalpm/remove.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c index 734c365..0ed4dd0 100644 --- a/lib/libalpm/remove.c +++ b/lib/libalpm/remove.c @@ -120,7 +120,7 @@ int _alpm_remove_prepare(pmtrans_t *trans, pmdb_t *db, alpm_list_t **data) } } else { _alpm_log(PM_LOG_ERROR, _("could not find %s in database -- skipping\n"), - miss->depend.name); + miss->target); } } FREELIST(lp); -- 1.5.3.4
_______________________________________________ pacman-dev mailing list pacman-dev@archlinux.org http://archlinux.org/mailman/listinfo/pacman-dev
Applied. -Dan
On 10/26/07, Nagy Gabor <ngaba@bibl.u-szeged.hu> wrote:
Hm. I dunno why I said package.c; probably there was a glitch in my brain. Due to your wish I resubmit this trivial change too:
Note that to compile with --enable-debug, I had to make the following additional patch. Please make sure you test future patches with this option enabled (it makes gcc fail on all errors and warnings). -Dan lib/libalpm/deps.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/libalpm/deps.c b/lib/libalpm/deps.c index 14261ae..b04c0f2 100644 --- a/lib/libalpm/deps.c +++ b/lib/libalpm/deps.c @@ -306,7 +306,7 @@ alpm_list_t *_alpm_checkdeps(pmdb_t *db, pmtranstype_t op, if(alpm_depcmp(pkg, depend)) { _alpm_log(PM_LOG_DEBUG, "checkdeps: dependency '%s' has moved from '%s' to '%s'\n", - k->data, alpm_pkg_get_name(oldpkg), alpm_pkg_get_name(pkg)); + (char*)k->data, alpm_pkg_get_name(oldpkg), alpm_pkg_get_name(pkg)); satisfied = 1; break; } @@ -322,7 +322,7 @@ alpm_list_t *_alpm_checkdeps(pmdb_t *db, pmtranstype_t op, /* we ignore packages that will be updated because we know * that the updated ones don't satisfy depend */ _alpm_log(PM_LOG_DEBUG, "checkdeps: dependency '%s' satisfied by installed package '%s'\n", - k->data, alpm_pkg_get_name(pkg)); + (char*)k->data, alpm_pkg_get_name(pkg)); satisfied = 1; break; } @@ -380,7 +380,7 @@ alpm_list_t *_alpm_checkdeps(pmdb_t *db, pmtranstype_t op, /* else if still not found... */ if(!found) { _alpm_log(PM_LOG_DEBUG, "missing dependency '%s' for package '%s'\n", - j->data, alpm_pkg_get_name(tp)); + (char*)j->data, alpm_pkg_get_name(tp)); miss = _alpm_depmiss_new(alpm_pkg_get_name(tp), PM_DEP_TYPE_DEPEND, depend->mod, depend->name, depend->version); if(!_alpm_depmiss_isin(miss, baddeps)) { @@ -426,7 +426,7 @@ alpm_list_t *_alpm_checkdeps(pmdb_t *db, pmtranstype_t op, pmpkg_t *pkg = l->data; if(alpm_depcmp(pkg, depend) && !_alpm_pkg_find(alpm_pkg_get_name(pkg), packages)) { _alpm_log(PM_LOG_DEBUG, "checkdeps: dependency '%s' satisfied by installed package '%s'\n", - k->data, alpm_pkg_get_name(pkg)); + (char*)k->data, alpm_pkg_get_name(pkg)); satisfied = 1; break; }
On 10/26/07, Nagy Gabor <ngaba@bibl.u-szeged.hu> wrote:
Hm. I dunno why I said package.c; probably there was a glitch in my brain. Due to your wish I resubmit this trivial change too:
Note that to compile with --enable-debug, I had to make the following additional patch. Please make sure you test future patches with this option enabled (it makes gcc fail on all errors and warnings).
-Dan Hm. Thanks for the info. Just a question (a bit off): To tell the truth, I thought that I should use (char *)k->data, but I saw that alpm_splitdep(k->data) is used everywhere, and gcc is not so strict there. Why? Because of const char*? -- because when I changed the alpm_splitdep definition to "pmdepend_t *alpm_splitdep(const char *depstring);", pacman failed to compile with --enable-debug (incompatible pointer type warning). Note: The patch is malformed (linebreaks), but I could easily fix it to apply. Bye, ngaba
... because when I changed the alpm_splitdep definition to "pmdepend_t *alpm_splitdep(const char *depstring);", pacman failed to Oops. I wanted to say "pmdepend_t *alpm_splitdep(char *depstring);". (copy-paste from a wrong place ;-) Bye, ngaba
participants (4)
-
Aaron Griffin
-
Dan McGee
-
Nagy Gabor
-
Xavier