[pacman-dev] [PATCH] Feedback from installed package while searching
From: Laszlo Papp <djszapi2@gmail.com> To get feedback while searching instead of using another utility for this purpose, whether the desired packages are installed. You can see example for it in case of yaourt. Signed-off-by: Laszlo Papp <djszapi2@gmail.com> --- src/pacman/sync.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/src/pacman/sync.c b/src/pacman/sync.c index dc93621..9935d6d 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -330,6 +330,10 @@ static int sync_search(alpm_list_t *syncs, alpm_list_t *targets) printf(")"); } + if (alpm_db_get_pkg(db_local, alpm_pkg_get_name(pkg))) { + printf(" [%s]", _("installed")); + } + /* we need a newline and initial indent first */ printf("\n "); indentprint(alpm_pkg_get_desc(pkg), 4); -- 1.6.4.1
On Thu, Sep 3, 2009 at 9:18 PM, <djszapi2@gmail.com> wrote:
From: Laszlo Papp <djszapi2@gmail.com>
To get feedback while searching instead of using another utility for this purpose, whether the desired packages are installed. You can see example for it in case of yaourt.
Signed-off-by: Laszlo Papp <djszapi2@gmail.com> --- src/pacman/sync.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/src/pacman/sync.c b/src/pacman/sync.c index dc93621..9935d6d 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -330,6 +330,10 @@ static int sync_search(alpm_list_t *syncs, alpm_list_t *targets) printf(")"); }
+ if (alpm_db_get_pkg(db_local, alpm_pkg_get_name(pkg))) { + printf(" [%s]", _("installed")); + } + /* we need a newline and initial indent first */ printf("\n "); indentprint(alpm_pkg_get_desc(pkg), 4); -- 1.6.4.1
stopped using root account, heh ? I submitted a similar patch 1 year and a half ago, but it was never accepted : http://www.mail-archive.com/pacman-dev@archlinux.org/msg00109.html So I am afraid you are stuck with yaourt / pacsearch / any other pacman wrapper to get that feature.
Xavier wrote:
On Thu, Sep 3, 2009 at 9:18 PM, <djszapi2@gmail.com> wrote:
From: Laszlo Papp <djszapi2@gmail.com>
To get feedback while searching instead of using another utility for this purpose, whether the desired packages are installed. You can see example for it in case of yaourt.
Signed-off-by: Laszlo Papp <djszapi2@gmail.com> --- src/pacman/sync.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/src/pacman/sync.c b/src/pacman/sync.c index dc93621..9935d6d 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -330,6 +330,10 @@ static int sync_search(alpm_list_t *syncs, alpm_list_t *targets) printf(")"); }
+ if (alpm_db_get_pkg(db_local, alpm_pkg_get_name(pkg))) { + printf(" [%s]", _("installed")); + } + /* we need a newline and initial indent first */ printf("\n "); indentprint(alpm_pkg_get_desc(pkg), 4); -- 1.6.4.1
stopped using root account, heh ?
I submitted a similar patch 1 year and a half ago, but it was never accepted : http://www.mail-archive.com/pacman-dev@archlinux.org/msg00109.html
So I am afraid you are stuck with yaourt / pacsearch / any other pacman wrapper to get that feature.
Well... this is nothing if not persistent given Laslo had a bug report closed as "Won't Implement" for this feature request... So lets take a step back and see why this feature was dismissed. As far as I can tell, it was rejected because it is slower as we need to parse the local db as well as the sync db - of course only applies before the local db is cached. I actually had Xavier's patch bookmarked because I was going to revisit this idea with this awesome suggestion.... How about adding a configuration option called "SearchLocal" or something that directs whether this is enabled. Allan
On Fri, Sep 4, 2009 at 1:09 AM, Allan McRae <allan@archlinux.org> wrote:
Xavier wrote:
On Thu, Sep 3, 2009 at 9:18 PM, <djszapi2@gmail.com> wrote:
From: Laszlo Papp <djszapi2@gmail.com>
To get feedback while searching instead of using another utility for this purpose, whether the desired packages are installed. You can see example for it in case of yaourt.
Signed-off-by: Laszlo Papp <djszapi2@gmail.com> --- src/pacman/sync.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/src/pacman/sync.c b/src/pacman/sync.c index dc93621..9935d6d 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -330,6 +330,10 @@ static int sync_search(alpm_list_t *syncs, alpm_list_t *targets) printf(")"); }
+ if (alpm_db_get_pkg(db_local, alpm_pkg_get_name(pkg))) { + printf(" [%s]", _("installed")); + } + /* we need a newline and initial indent first */ printf("\n "); indentprint(alpm_pkg_get_desc(pkg), 4); -- 1.6.4.1
stopped using root account, heh ?
I submitted a similar patch 1 year and a half ago, but it was never accepted : http://www.mail-archive.com/pacman-dev@archlinux.org/msg00109.html
So I am afraid you are stuck with yaourt / pacsearch / any other pacman wrapper to get that feature.
Well... this is nothing if not persistent given Laslo had a bug report closed as "Won't Implement" for this feature request...
My name is Laszlo ;)
So lets take a step back and see why this feature was dismissed. As far as I can tell, it was rejected because it is slower as we need to parse the local db as well as the sync db - of course only applies before the local db is cached.
The uses of strlen, strcmp and memory allocations far outweight time taken on-disk for me I just did a callgrind test on pacman with [installed], the number of calls to alpm_db_get_pkg was 27, the cost itself was 0%, while memory allocations, strlen, fgets are 7% 6% and 6% of the overall number of calls. The number of calls are 194,000 469,000 223,000 and those numbers are cut for simplicit, so just benchmark pacman, valgrind --tool=callgrind pacman -Ss pacman.
I actually had Xavier's patch bookmarked because I was going to revisit this idea with this awesome suggestion.... How about adding a configuration option called "SearchLocal" or something that directs whether this is enabled.
Allan
In last case a separate option can be good too. (who would like to use it all the time, make an alias, bash function or something for it). Best Regards, Laszlo Papp
On Fri, Sep 4, 2009 at 1:09 AM, Allan McRae<allan@archlinux.org> wrote:
Xavier wrote:
I submitted a similar patch 1 year and a half ago, but it was never accepted : http://www.mail-archive.com/pacman-dev@archlinux.org/msg00109.html
So I am afraid you are stuck with yaourt / pacsearch / any other pacman wrapper to get that feature.
Well... this is nothing if not persistent given Laslo had a bug report closed as "Won't Implement" for this feature request...
So lets take a step back and see why this feature was dismissed. As far as I can tell, it was rejected because it is slower as we need to parse the local db as well as the sync db - of course only applies before the local db is cached.
I actually had Xavier's patch bookmarked because I was going to revisit this idea with this awesome suggestion.... How about adding a configuration option called "SearchLocal" or something that directs whether this is enabled.
I did some more testing and the performance differences are pretty insignificant. For the -Sl operating, without fs cache, it is 2.2s vs 2.6s with fs cache, I get the same result than before : 0.1s vs 0.2s but who is going to notice that ? :) besides, this should only be done for the non-quiet output, so the quiet output should remain the same. In the case of -Sl, we are already looking at 4300 dir for the sync databases. Considering local db just add a few hundreds more. In the case of -Ss (Laszlo patch), the difference is even smaller, because we already have to look at 4300 dir + 4300 desc files! Considering local database only add a few hundred directories to that. If we have a performance problem, I think it's already here, and it's not going to change much with that patch ("pacman -Ss" after dropping fs caches takes ~10 seconds in both cases here :) )
2009. 09. 4, péntek keltezéssel 09.09-kor Allan McRae ezt írta:
Xavier wrote:
On Thu, Sep 3, 2009 at 9:18 PM, <djszapi2@gmail.com> wrote:
From: Laszlo Papp <djszapi2@gmail.com>
To get feedback while searching instead of using another utility for this purpose, whether the desired packages are installed. You can see example for it in case of yaourt.
Signed-off-by: Laszlo Papp <djszapi2@gmail.com> --- src/pacman/sync.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/src/pacman/sync.c b/src/pacman/sync.c index dc93621..9935d6d 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -330,6 +330,10 @@ static int sync_search(alpm_list_t *syncs, alpm_list_t *targets) printf(")"); }
+ if (alpm_db_get_pkg(db_local, alpm_pkg_get_name(pkg))) { + printf(" [%s]", _("installed")); + } + /* we need a newline and initial indent first */ printf("\n "); indentprint(alpm_pkg_get_desc(pkg), 4); -- 1.6.4.1
stopped using root account, heh ?
I submitted a similar patch 1 year and a half ago, but it was never accepted : http://www.mail-archive.com/pacman-dev@archlinux.org/msg00109.html
So I am afraid you are stuck with yaourt / pacsearch / any other pacman wrapper to get that feature.
Well... this is nothing if not persistent given Laslo had a bug report closed as "Won't Implement" for this feature request...
So lets take a step back and see why this feature was dismissed. As far as I can tell, it was rejected because it is slower as we need to parse the local db as well as the sync db - of course only applies before the local db is cached.
However, this is very cheap localdb parsing here, because we need package name and version only, so there is no need to read any local depend/desc/... files.
2009. 09. 3, csütörtök keltezéssel 21.18-kor djszapi2@gmail.com ezt írta:
From: Laszlo Papp <djszapi2@gmail.com>
To get feedback while searching instead of using another utility for this purpose, whether the desired packages are installed. You can see example for it in case of yaourt.
Signed-off-by: Laszlo Papp <djszapi2@gmail.com> --- src/pacman/sync.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/src/pacman/sync.c b/src/pacman/sync.c index dc93621..9935d6d 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -330,6 +330,10 @@ static int sync_search(alpm_list_t *syncs, alpm_list_t *targets) printf(")"); }
+ if (alpm_db_get_pkg(db_local, alpm_pkg_get_name(pkg))) { + printf(" [%s]", _("installed")); + } + /* we need a newline and initial indent first */ printf("\n "); indentprint(alpm_pkg_get_desc(pkg), 4); -- 1.6.4.1
What is wrong with printf(_(" [installed]"));? For my taste, printing [installed] when I have _older version_ installed, a bit strange. In this case [installed: 2.0-1] would be better (but harder to parser).
On Thu, Sep 3, 2009 at 9:54 PM, Nagy Gabor<ngaba@bibl.u-szeged.hu> wrote:
What is wrong with printf(_(" [installed]"));?
For my taste, printing [installed] when I have _older version_ installed, a bit strange. In this case [installed: 2.0-1] would be better (but harder to parser).
Afaik, it is always safer to avoid putting non-fixed strings as the first argument of printf. if a bogus translation put some format strings as the translation of installed, it could do weird things.
On Thu, Sep 3, 2009 at 9:54 PM, Nagy Gabor<ngaba@bibl.u-szeged.hu> wrote:
What is wrong with printf(_(" [installed]"));?
For my taste, printing [installed] when I have _older version_ installed, a bit strange. In this case [installed: 2.0-1] would be better (but harder to parser).
Afaik, it is always safer to avoid putting non-fixed strings as the first argument of printf.
if a bogus translation put some format strings as the translation of installed, it could do weird things.
That is true, but then these kind of lines are also "dangerous": printf(_(" This program may be freely redistributed under\n" " the terms of the GNU General Public License.\n")); Then e should use printf("%s",_()) or puts. Btw, with c-format, the appropriate gettext tool should catch bogus translation. Bye
participants (5)
-
Allan McRae
-
djszapi2＠gmail.com
-
Laszlo Papp
-
Nagy Gabor
-
Xavier