| Message ID | 20180320044957.26489-1-morganamilo@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 20 March 2018 at 04:49, morganamilo <morganamilo@gmail.com> wrote: > From: actionless <actionless.loveless@gmail.com> > > --- > doc/rpc.txt | 4 ++-- > web/lib/aurjson.class.php | 16 +++++++++++++++- > 2 files changed, 17 insertions(+), 3 deletions(-) > > diff --git a/doc/rpc.txt b/doc/rpc.txt > index f353ff0..83cdae3 100644 > --- a/doc/rpc.txt > +++ b/doc/rpc.txt > @@ -8,8 +8,8 @@ Package searches can be performed by issuing HTTP GET requests of the form > +/rpc/?v=5&type=search&by=_field_&arg=_keywords_+ where _keywords_ is the > search argument and _field_ is one of the following values: > > -* `name` (search by package name only) > -* `name-desc` (search by package name and description) > +* `name` (search by package name or packages which provide that name) > +* `name-desc` (the same as `name` and search by description) > * `maintainer` (search by package maintainer) > > The _by_ parameter can be skipped and defaults to `name-desc`. > diff --git a/web/lib/aurjson.class.php b/web/lib/aurjson.class.php > index 9eeaafd..6e580a9 100644 > --- a/web/lib/aurjson.class.php > +++ b/web/lib/aurjson.class.php > @@ -392,12 +392,26 @@ class AurJSON { > if (strlen($keyword_string) < 2) { > return $this->json_error('Query arg too small.'); > } > + > + //packages which provide the package we are looking for: > + $providers = pkg_providers(addcslashes($keyword_string, '%_')); > + $provided_names = array(); > + foreach ($providers as $provider) { > + if ($provider[0] != 0) { // if package is not from repo > + $name = $this->dbh->quote($provider[1]); > + array_push($provided_names, $name); > + } > + } > + $provided_query = "(" . join(", ", $provided_names) . ")"; > + > $keyword_string = $this->dbh->quote("%" . addcslashes($keyword_string, '%_') . "%"); > > if ($search_by === 'name') { > - $where_condition = "(Packages.Name LIKE $keyword_string)"; > + $where_condition = "(Packages.Name LIKE $keyword_string OR "; > + $where_condition .= "Packages.Name IN $provided_query )"; > } else if ($search_by === 'name-desc') { > $where_condition = "(Packages.Name LIKE $keyword_string OR "; > + $where_condition .= "Packages.Name IN $provided_query OR "; > $where_condition .= "Description LIKE $keyword_string)"; > } > } else if ($search_by === 'maintainer') { > -- > 2.16.2 > Sent on behalf of actionless <actionless.loveless@gmail.com>. I have an intrest in this myself so if he decides he does not want to further this I wouldn't mind taking it over.
On Tue, 20 Mar 2018 at 05:49:57, morganamilo wrote: > From: actionless <actionless.loveless@gmail.com> > > --- > doc/rpc.txt | 4 ++-- > web/lib/aurjson.class.php | 16 +++++++++++++++- > 2 files changed, 17 insertions(+), 3 deletions(-) > [...] Thank you for submitting a patch! Before commenting on implementation details, I would like to discuss the overall approach, though. What is the rationale for making it part of name and name-desc? Wouldn't it make more sense to add a new search type ("providers")? Regards, Lukas
On 03/20/2018 02:36 PM, Lukas Fleischer wrote: > On Tue, 20 Mar 2018 at 05:49:57, morganamilo wrote: >> From: actionless <actionless.loveless@gmail.com> >> >> --- >> doc/rpc.txt | 4 ++-- >> web/lib/aurjson.class.php | 16 +++++++++++++++- >> 2 files changed, 17 insertions(+), 3 deletions(-) >> [...] > > Thank you for submitting a patch! Before commenting on implementation > details, I would like to discuss the overall approach, though. What is > the rationale for making it part of name and name-desc? Wouldn't it make > more sense to add a new search type ("providers")? Well, we have no precedent here as archweb does not allow searching by provides either AFAICT. But I think, generally speaking, from a usability perspective any search that is not an exact-name search is probably *interested* in these provides packages. pacman -Ss will show them, pacman -S will install them, so the web interfaces arguably should do the same. ... Aside: that commit message feels somewhat jarring as it does not match the (admittedly loose) style in use. I have no idea why this commit is a "feat" but it feels oddly self-congratulatory, as though the commit is a circus performer demonstrating a trick. Am I missing out on the latest fashion or something?
2018-03-21 2:56 GMT+08:00 Eli Schwartz <eschwartz@archlinux.org>: > > What is > > the rationale for making it part of name and name-desc? Wouldn't it make > > more sense to add a new search type ("providers")? I vote for a new search type "providers". For my use case, I'd appreciate if I can get precise information from the RPC, instead of parsing the aurweb html or crawling the entire AUR. I prefer having a bulk query here so I can keep the request count low. > Well, we have no precedent here as archweb does not allow searching by > provides either AFAICT. But I think, generally speaking, from a > usability perspective any search that is not an exact-name search is > probably *interested* in these provides packages. > pacman -Ss will show them, pacman -S will install them, so the web > interfaces arguably should do the same. Actually I just found it confusing that: pacman -Ss 'gcc>7' shows nothing, yet pacman -S 'gcc>7' will do the right stuff I'd argue that pacman should provide a search providers. But at least it has alpm_find_satisifier.
On 03/20/2018 08:47 PM, 尤立宇 wrote: > I vote for a new search type "providers". > For my use case, I'd appreciate if I can get precise information from the RPC, > instead of parsing the aurweb html or crawling the entire AUR. > I prefer having a bulk query here so I can keep the request count low. So, like, return all the provides results in one search result, then do another RPC search for multiinfo on all those packages to see exact details on the pkgbase, provides, etc? >> pacman -Ss will show them, pacman -S will install them, so the web >> interfaces arguably should do the same. > > Actually I just found it confusing that: > pacman -Ss 'gcc>7' shows nothing, yet > pacman -S 'gcc>7' will do the right stuff > I'd argue that pacman should provide a search providers. But at least > it has alpm_find_satisifier. That's because the >7 is a version specifier, not a search term. On top of which that is for the main package and has nothing to do with provides.
2018-03-21 8:56 GMT+08:00 Eli Schwartz <eschwartz@archlinux.org>: > On 03/20/2018 08:47 PM, 尤立宇 wrote: >> I vote for a new search type "providers". >> For my use case, I'd appreciate if I can get precise information from the RPC, >> instead of parsing the aurweb html or crawling the entire AUR. >> I prefer having a bulk query here so I can keep the request count low. > > So, like, return all the provides results in one search result, then do > another RPC search for multiinfo on all those packages to see exact > details on the pkgbase, provides, etc? > I think it would be like: HTTP GET /rpc/?v=5&type=provides&arg[]=java-environment&arg[]=libgala.so {"version":5,"type":"provides","resultcount":2,"results": "java-environment": [] // might be list of package names or list of "info" "libgala.so": [] } >>> pacman -Ss will show them, pacman -S will install them, so the web >>> interfaces arguably should do the same. >> >> Actually I just found it confusing that: >> pacman -Ss 'gcc>7' shows nothing, yet >> pacman -S 'gcc>7' will do the right stuff >> I'd argue that pacman should provide a search providers. But at least >> it has alpm_find_satisifier. > > That's because the >7 is a version specifier, not a search term. On top > of which that is for the main package and has nothing to do with provides. I got the reasoning about it. Still, versioned dependencies are valid in the depends array, so I'd like the provides search in the RPC mentioned above support this, much like the Dependencies section in the web interface [1]. [1] https://aur.archlinux.org/packages/nvidia-ck/ See how it lists nvidia-utils=390.42, linux-ck-headers>=4.15, etc
> I think it would be like: > HTTP GET /rpc/?v=5&type=provides&arg[]=java-environment&arg[]=libgala.so > {"version":5,"type":"provides","resultcount":2,"results": > "java-environment": [] // might be list of package names or list of "info" > "libgala.so": [] > } Pacman can "-S" and "-Ss" by provides so I would prefer to have the option to search by provides in type=info and type=search rather than a new type=provides. Also I would like each option to be indivudually settable, so "provides", "name" and "desc" can be set indipendantly.
2018-03-21 9:49 GMT+08:00 morganamilo <morganamilo@gmail.com>: >> I think it would be like: >> HTTP GET /rpc/?v=5&type=provides&arg[]=java-environment&arg[]=libgala.so >> {"version":5,"type":"provides","resultcount":2,"results": >> "java-environment": [] // might be list of package names or list of >> "info" >> "libgala.so": [] >> } > > Pacman can "-S" and "-Ss" by provides so I would prefer to have the option > to search by provides in type=info and type=search rather than a new > type=provides. Also I would like each option to be indivudually settable, so > "provides", "name" and "desc" can be set indipendantly. I think I'm looking for an alpm_find_satisifier equivalent rather than a pacman -Ss equalviant, and that's why I'm proposing a new type=provides. For type=info after thinking again, is it a) The dependencies providers' details are listed in the "Depends" "MakeDepends" "OptDepends" b) The "results" items are unchanged, but querying a name not only returns the package name that matches, but also their provides array matches. That is ?type=info&arg[]=java-environment would return resultcount > 1 or something else? I think b) would work for me, I will just have to verify the version afterwards with vercmp in that case.
On 21/03/18 02:33, 尤立宇 wrote: > 2018-03-21 9:49 GMT+08:00 morganamilo <morganamilo@gmail.com>: >>> I think it would be like: >>> HTTP GET /rpc/?v=5&type=provides&arg[]=java-environment&arg[]=libgala.so >>> {"version":5,"type":"provides","resultcount":2,"results": >>> "java-environment": [] // might be list of package names or list of >>> "info" >>> "libgala.so": [] >>> } >> Pacman can "-S" and "-Ss" by provides so I would prefer to have the option >> to search by provides in type=info and type=search rather than a new >> type=provides. Also I would like each option to be indivudually settable, so >> "provides", "name" and "desc" can be set indipendantly. > I think I'm looking for an alpm_find_satisifier equivalent rather than > a pacman -Ss equalviant, > and that's why I'm proposing a new type=provides. > > For type=info after thinking again, is it > a) The dependencies providers' details are listed in the "Depends" > "MakeDepends" "OptDepends" > b) The "results" items are unchanged, but querying a name not only > returns the package name that matches, > but also their provides array matches. That is > ?type=info&arg[]=java-environment would return > resultcount > 1 > or something else? > > I think b) would work for me, I will just have to verify the version > afterwards with vercmp in that case. An alpm_find_satisifier equivilent is what I want too, thing is once you get the package names from that alpm_find_satisifier equivelent what are you going to do with thoes names? Probably call "info" on all of them. So why not skip that step and just have info support it. Yes point b is what I was thinking. info("foo") would return "foo" because it matches by name, and it would also return "foo-git" because it provides "foo". Maybe it would be &type=info&by[]=provides&by[]=name rather than just "info". Point being you still get the same hunk of json that info gets you. Would your type=provides return just package names or the same stuff info does? If its the latter it's the same as what I want but under a different name.
> An alpm_find_satisifier equivilent is what I want too, thing is once you get > the package names from that alpm_find_satisifier equivelent what are you > going to do with thoes names? Probably call "info" on all of them. So why > not skip that step and just have info support it. > > Yes point b is what I was thinking. info("foo") would return "foo" because > it matches by name, and it would also return "foo-git" because it provides > "foo". Maybe it would be &type=info&by[]=provides&by[]=name rather than just > "info". Point being you still get the same hunk of json that info gets you. > > Would your type=provides return just package names or the same stuff info > does? If its the latter it's the same as what I want but under a different > name. I'm convinced that just putting that into info would be better. Looking at the code also shows that adding by[]=provides is easier to implement as it fits better to the current architecture.
On 21/03/18 03:12, 尤立宇 wrote: >> An alpm_find_satisifier equivilent is what I want too, thing is once you get >> the package names from that alpm_find_satisifier equivelent what are you >> going to do with thoes names? Probably call "info" on all of them. So why >> not skip that step and just have info support it. >> >> Yes point b is what I was thinking. info("foo") would return "foo" because >> it matches by name, and it would also return "foo-git" because it provides >> "foo". Maybe it would be &type=info&by[]=provides&by[]=name rather than just >> "info". Point being you still get the same hunk of json that info gets you. >> >> Would your type=provides return just package names or the same stuff info >> does? If its the latter it's the same as what I want but under a different >> name. > I'm convinced that just putting that into info would be better. > Looking at the code also shows that adding by[]=provides is easier to implement > as it fits better to the current architecture. Glad you agree. This patch was more about opening up discussion before going in and changing a bunch of stuff. Is there any offial decision by the devs and a little guidence on how the implementation should look?
On Wed, 21 Mar 2018 at 04:20:18, morganamilo wrote: > On 21/03/18 03:12, 尤立宇 wrote: > >> An alpm_find_satisifier equivilent is what I want too, thing is once you get > >> the package names from that alpm_find_satisifier equivelent what are you > >> going to do with thoes names? Probably call "info" on all of them. So why > >> not skip that step and just have info support it. > >> > >> Yes point b is what I was thinking. info("foo") would return "foo" because > >> it matches by name, and it would also return "foo-git" because it provides > >> "foo". Maybe it would be &type=info&by[]=provides&by[]=name rather than just > >> "info". Point being you still get the same hunk of json that info gets you. > >> > >> Would your type=provides return just package names or the same stuff info > >> does? If its the latter it's the same as what I want but under a different > >> name. > > I'm convinced that just putting that into info would be better. > > Looking at the code also shows that adding by[]=provides is easier to implement > > as it fits better to the current architecture. > Glad you agree. This patch was more about opening up discussion before > going in and changing a bunch of stuff. Is there any offial decision by > the devs and a little guidence on how the implementation should look? As I already suggested before, I like the idea of adding multiple "by" arguments to the search. I wonder what to do with info queries. If we do the same thing there, wouldn't it be natural to add all the "by" types from search as well? Maybe info and search could/should be able to handle the same kind of queries and only differ in the amount of details returned...
>> > I'm convinced that just putting that into info would be better. >> > Looking at the code also shows that adding by[]=provides is easier to implement >> > as it fits better to the current architecture. >> Glad you agree. This patch was more about opening up discussion before >> going in and changing a bunch of stuff. Is there any offial decision by >> the devs and a little guidence on how the implementation should look? > > As I already suggested before, I like the idea of adding multiple "by" > arguments to the search. I wonder what to do with info queries. If we do > the same thing there, wouldn't it be natural to add all the "by" types > from search as well? Maybe info and search could/should be able to > handle the same kind of queries and only differ in the amount of details > returned... From my point of view, the search should look for the queried substring in the fields, while info looks for exact match. My study on the code shows the only difference now is how the $where_condition is built [1] and the level of detail is the same. Here's some code highlights (v5 only). Except maintainer, search performs a LIKE query and info performs a IN query. Also, search only supports single argument, while info supports multiple. search/name: "(Packages.Name LIKE $keyword_string)" search/name-desc: "(Packages.Name LIKE $keyword_string OR Description LIKE $keyword_string)" search/maintainer: "Users.Username = $keyword_string " info: "Packages.Name IN ($names_value) " To support multiple "by" arguments, I came up with this, assuming by=by1&by=by2 search: any by fields contain the argument WHERE (Packages.By1 LILKE $keyword_string OR Packages.By2 LIKE $keyword_string) info: any by fields equals one of the argument WHERE (Packages.By1 IN ($names_value) OR Packages.By2 IN ($names_value) I think this way more fields can be easily supported. With this in mind, search by=name-desc would become by=name&by=desc search by=maintainer would become info by=maintainer. Does this make sense to you? [1] search: https://git.archlinux.org/aurweb.git/tree/web/lib/aurjson.class.php#n412 multiinfo: https://git.archlinux.org/aurweb.git/tree/web/lib/aurjson.class.php#n467
On Wed, 21 Mar 2018 at 09:10:10, 尤立宇 wrote: > [...] > To support multiple "by" arguments, I came up with this, assuming by=by1&by=by2 > search: any by fields contain the argument > WHERE (Packages.By1 LILKE $keyword_string OR Packages.By2 LIKE $keyword_string) > info: any by fields equals one of the argument > WHERE (Packages.By1 IN ($names_value) OR Packages.By2 IN ($names_value) > I think this way more fields can be easily supported. > > With this in mind, > search by=name-desc would become by=name&by=desc > search by=maintainer would become info by=maintainer. > Does this make sense to you? > [...] Yes, sounds reasonable. Note that we still need to support the old implementation and should probably create a new version of the RPC API. Regards, Lukas
diff --git a/doc/rpc.txt b/doc/rpc.txt index f353ff0..83cdae3 100644 --- a/doc/rpc.txt +++ b/doc/rpc.txt @@ -8,8 +8,8 @@ Package searches can be performed by issuing HTTP GET requests of the form +/rpc/?v=5&type=search&by=_field_&arg=_keywords_+ where _keywords_ is the search argument and _field_ is one of the following values: -* `name` (search by package name only) -* `name-desc` (search by package name and description) +* `name` (search by package name or packages which provide that name) +* `name-desc` (the same as `name` and search by description) * `maintainer` (search by package maintainer) The _by_ parameter can be skipped and defaults to `name-desc`. diff --git a/web/lib/aurjson.class.php b/web/lib/aurjson.class.php index 9eeaafd..6e580a9 100644 --- a/web/lib/aurjson.class.php +++ b/web/lib/aurjson.class.php @@ -392,12 +392,26 @@ class AurJSON { if (strlen($keyword_string) < 2) { return $this->json_error('Query arg too small.'); } + + //packages which provide the package we are looking for: + $providers = pkg_providers(addcslashes($keyword_string, '%_')); + $provided_names = array(); + foreach ($providers as $provider) { + if ($provider[0] != 0) { // if package is not from repo + $name = $this->dbh->quote($provider[1]); + array_push($provided_names, $name); + } + } + $provided_query = "(" . join(", ", $provided_names) . ")"; + $keyword_string = $this->dbh->quote("%" . addcslashes($keyword_string, '%_') . "%"); if ($search_by === 'name') { - $where_condition = "(Packages.Name LIKE $keyword_string)"; + $where_condition = "(Packages.Name LIKE $keyword_string OR "; + $where_condition .= "Packages.Name IN $provided_query )"; } else if ($search_by === 'name-desc') { $where_condition = "(Packages.Name LIKE $keyword_string OR "; + $where_condition .= "Packages.Name IN $provided_query OR "; $where_condition .= "Description LIKE $keyword_string)"; } } else if ($search_by === 'maintainer') {
From: actionless <actionless.loveless@gmail.com> --- doc/rpc.txt | 4 ++-- web/lib/aurjson.class.php | 16 +++++++++++++++- 2 files changed, 17 insertions(+), 3 deletions(-)