- Aur-dev - lists.archlinux.org

[PATCH] feat(rpc): return "providers" packages when querying by `name` or `name-desc`
by morganamilo 18 Feb '19

18 Feb '19

From: actionless <actionless.loveless(a)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

6 25

Django-hosted aurweb Proposal
by Kevin Morris 18 Feb '19

18 Feb '19

To whom it may concern, The following is a proposal for a Django-hosted aurweb application. It is meant to be a drop-in replacement for aur.archlinux.org; effectively cloning its user interface and capabilities into a Python Django extension. Following https://patchwork.archlinux.org/patch/1000/, I would like to put together a new aurweb. The current revamped version of archweb runs inside of a django server as a django extension (or app). I would like to do this same thing for `aurweb`. The new django `aurweb` shall support all of the current v1-6 capabilities that aurweb provides, as well as the front-end user website located at https://aur.archlinux.org. It shall be an exact clone from the user's perspective. The major differences between maintaining a PHP vs Django server would be that the Django server would be: 1. Integrated with any Django server and easily removed. 2. Developed via Python, which has vastly more support and familiarity than PHP within the community. 3. Django models, which greatly reduce the complexity of SQL queries inside of `aurweb` backend. 4. Baked-in extendable templates with templatetags. 5. View decorators which enforce requirements of a specific route. No more manually handling $http_data user errors. 6. Everybody loves Python; there will most likely be many more code-base contributions in Python than in PHP. I would like to hear your thoughts on this. If approved, I would love to begin this project within the next few weeks. Thanks, Kevin

6 6

[PATCH] Introduce API v6
by Kevin Morris 12 Feb '19

12 Feb '19

Add new method of multiple by[] search in API v6. The 'provides' by field is also supported in the new multiple by[] method included with this patch. 'provides' may not be used in legacy (v5) singular by search. A helpful json error message is returned to the user to indicate so. This patch is a follow-up implementation to https://patchwork.archlinux.org/patch/479/ Signed-off-by: Kevin Morris <kevr.gtalk(a)gmail.com> --- doc/rpc.txt | 24 ++++++--- web/lib/aurjson.class.php | 105 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 116 insertions(+), 13 deletions(-) diff --git a/doc/rpc.txt b/doc/rpc.txt index 3148ebe..486a49d 100644 --- a/doc/rpc.txt +++ b/doc/rpc.txt @@ -5,7 +5,7 @@ Package Search -------------- 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 ++/rpc/?v=6&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) @@ -15,31 +15,39 @@ search argument and _field_ is one of the following values: * `makedepends` (search for packages that makedepend on _keywords_) * `optdepends` (search for packages that optdepend on _keywords_) * `checkdepends` (search for packages that checkdepend on _keywords_) +* `provides` (search by package provides; v6 multiple by[] only) The _by_ parameter can be skipped and defaults to `name-desc`. If a maintainer search is performed and the search argument is left empty, a list of orphan packages is returned. +Note: Legacy v5 support is still enabled for singular _by_ searches. + Package Details --------------- Package information can be obtained by issuing HTTP GET requests of the form -+/rpc/?v=5&type=info&arg[]=_pkg1_&arg[]=_pkg2_&...+ where _pkg1_, _pkg2_, ... ++/rpc/?v=6&type=info&arg[]=_pkg1_&arg[]=_pkg2_&...+ where _pkg1_, _pkg2_, ... are the names of packages to retrieve package details for. Examples -------- `search`:: - `/rpc/?v=5&type=search&arg=foobar` + `/rpc/?v=6&type=search&arg=foobar` `search` by maintainer:: - `/rpc/?v=5&type=search&by=maintainer&arg=john` + `/rpc/?v=6&type=search&by[]=maintainer&arg=john` `search` packages that have _boost_ as `makedepends`:: - `/rpc/?v=5&type=search&by=makedepends&arg=boost` + `/rpc/?v=6&type=search&by[]=makedepends&arg=boost` `search` with callback:: - `/rpc/?v=5&type=search&arg=foobar&callback=jsonp1192244621103` + `/rpc/?v=6&type=search&arg=foobar&callback=jsonp1192244621103` +`search` by provides:: + `/rpc/?v=6&type=search&by[]=provides&arg[]=gcc` +`search` by provides and name:: + `/rpc/?v=6&type=search&by[]=name&by[]=provides&arg[]=gcc` `info`:: - `/rpc/?v=5&type=info&arg[]=foobar` + `/rpc/?v=6&type=info&arg[]=foobar` `info` with multiple packages:: - `/rpc/?v=5&type=info&arg[]=foo&arg[]=bar` + `/rpc/?v=6&type=info&arg[]=foo&arg[]=bar` + diff --git a/web/lib/aurjson.class.php b/web/lib/aurjson.class.php index c275d21..ab4eb19 100644 --- a/web/lib/aurjson.class.php +++ b/web/lib/aurjson.class.php @@ -80,7 +80,8 @@ class AurJSON { if (isset($http_data['v'])) { $this->version = intval($http_data['v']); } - if ($this->version < 1 || $this->version > 5) { + + if ($this->version < 1 || $this->version > 6) { return $this->json_error('Invalid version specified.'); } @@ -94,8 +95,14 @@ class AurJSON { if (isset($http_data['search_by']) && !isset($http_data['by'])) { $http_data['by'] = $http_data['search_by']; } - if (isset($http_data['by']) && !in_array($http_data['by'], self::$exposed_fields)) { - return $this->json_error('Incorrect by field specified.'); + + if (isset($http_data['by']) && !is_array($http_data['by'])) { + if ($http_data['by'] === 'provides') { + return $this->json_error("The 'provides' by field " . + "may only be used via multiple by[] search."); + } elseif (!in_array($http_data['by'], self::$exposed_fields)) { + return $this->json_error('Incorrect by field specified.'); + } } $this->dbh = DB::connect(); @@ -360,7 +367,7 @@ class AurJSON { } elseif ($this->version >= 2) { if ($this->version == 2 || $this->version == 3) { $fields = implode(',', self::$fields_v2); - } else if ($this->version == 4 || $this->version == 5) { + } else if ($this->version == 4 || $this->version == 5 || $this->version == 6) { $fields = implode(',', self::$fields_v4); } $query = "SELECT {$fields} " . @@ -470,6 +477,10 @@ class AurJSON { * @return mixed Returns an array of package matches. */ private function search($http_data) { + if ($this->version == 6 && is_array($http_data['by'])) { + return $this->search_v6($http_data); + } + $keyword_string = $http_data['arg']; if (isset($http_data['by'])) { @@ -497,6 +508,8 @@ class AurJSON { $keyword_string = $this->dbh->quote($keyword_string); $where_condition = "Users.Username = $keyword_string "; } + } else if ($search_by === 'provides') { + return $this->json_error("The 'provides' by field is only available via multiple by[] search."); } else if (in_array($search_by, self::$exposed_depfields)) { if (empty($keyword_string)) { return $this->json_error('Query arg is empty.'); @@ -515,6 +528,89 @@ class AurJSON { return $this->process_query('search', $where_condition); } + /* + * Returns multiple-by, multiple-arg and/or search information + * a feature not included in the v5 and before RPC API, + * which introduces the beginnings of the v6 API. + * + * @param array $http_data Query parameters. + * + * @return mixed Returns an array of results containing the package data + */ + private function search_v6($http_data) { + if (!is_array($http_data['arg'])) { + $http_data['arg'] = array($http_data['arg']); + } + + $max_results = config_get_int('options', 'max_rpc_results'); + $fields = implode(',', self::$fields_v4); + + $query = "SELECT {$fields} FROM " . + "Packages LEFT JOIN PackageBases " . + "ON PackageBases.ID = Packages.PackageBaseID " . + "LEFT JOIN Users " . + "ON PackageBases.MaintainerUID = Users.ID " . + "LEFT JOIN PackageRelations " . + "ON PackageRelations.PackageID = Packages.ID " . + "AND PackageRelations.RelTypeID = 2 "; + + $where = array(); + + foreach($http_data['by'] as $by) { + if($by == 'provides') { + foreach ($http_data['arg'] as $provide) { + array_push($where, + "PackageRelations.RelName = " . + $this->dbh->quote($provide)); + } + } elseif($by == 'name') { + foreach ($http_data['arg'] as $name) { + array_push($where, "Packages.NAME LIKE " . + $this->dbh->quote('%' . $name . '%_')); + array_push($where, "Packages.Name = " . + $this->dbh->quote($name)); + } + } elseif($by == 'name-desc') { + foreach ($http_data['arg'] as $name) { + array_push($where, "Packages.NAME LIKE " . + $this->dbh->quote('%' . $name . '%_')); + array_push($where, "Packages.Name = " . + $this->dbh->quote($name)); + array_push($where, "Packages.Description LIKE " . + $this->dbh->quote('%' . $name . '%_')); + } + } elseif (in_array($by, self::$exposed_depfields)) { + foreach ($http_data['arg'] as $dep) { + $subquery = "SELECT PackageDepends.DepName FROM PackageDepends "; + $subquery .= "LEFT JOIN DependencyTypes "; + $subquery .= "ON PackageDepends.DepTypeID = DependencyTypes.ID "; + $subquery .= "WHERE PackageDepends.PackageID = Packages.ID "; + $subquery .= "AND DependencyTypes.Name = " . $this->dbh->quote($by); + $sub_condition = $this->dbh->quote($dep) . " IN (${subquery})"; + array_push($where, $sub_condition); + } + } else { + return $this->json_error( + "${by} is not supported in v6 multi-argument by search." + ); + } + } + + $query .= "WHERE " . implode(" OR ", $where) . " "; + "AND PackageBases.PackagerUID IS NOT NULL " . + "LIMIT ${max_results}"; + + $packages = array(); // Final list of packages + $result = $this->dbh->query($query); + + if ($result) { + while ($row = $result->fetch(PDO::FETCH_ASSOC)) + array_push($packages, $row); + } + + return $this->json_results('search', count($packages), $packages, NULL); + } + /* * Returns the info on a specific package. * @@ -569,7 +665,6 @@ class AurJSON { return $this->process_query('multiinfo', $where_condition); } - /* * Returns all the packages for a specific maintainer. * -- 2.20.1

1 0

PR Regarding v6 multiple by and provides field
by Kevin Morris 12 Feb '19

12 Feb '19

Hello AUR Maintainers, This pull request is in reference to https://patchwork.archlinux.org/patch/578/ and satisfies back-compat requirements and multiple by[] search implementation that the original requester intended, including the 'provides' field search. > The following changes since commit > 042f3f2622e1adbc7546cfa25a5d138486b99341: > Quote MySql 8.0 reserved keywords (2019-01-14 14:45:37 -0500) > are available in the Git repository at: > https://github.com/kevr/aurweb master > for you to fetch changes up to ac80bee46ae3fe0362c134fe598304d58d1db3fb: > Introduce API v6 (2019-02-12 13:39:08 -0800) > ---------------------------------------------------------------- > Kevin Morris (1): > Introduce API v6 > doc/rpc.txt | 15 ++++++++++++++- > web/lib/aurjson.class.php | 99 > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---- > 2 files changed, 109 insertions(+), 5 deletions(-) Please let me know if there are any blockers or questions you may have, I hope it works out well! Regards, Kevin -- Kevin Morris Personal Inquiries: kevr.gtalk(a)gmail.com Technologies: C++, Python, Django, Ruby, Rails, ReactJS, jQuery, Javascript, SQL, Redux

2 1

Search or info by "provides"
by Sergey Petrenko 02 Feb '19

02 Feb '19

Hi, I’m interested in implementing this one way or another. I have found thread from march 2018 concerning this feature, but without any final resolution. What is the status? Will you accept extension of /search over "provides" or I’ll have to wait till RPC v6 is ready?

1 0

[PATCH] aurblup: make provider updates more robust
by Lukas Fleischer 21 Jan '19

21 Jan '19

Prevent from corner cases, such as failing unique key constraints when a provided package changes from lower case to upper case. Signed-off-by: Lukas Fleischer <lfleischer(a)archlinux.org> --- aurweb/scripts/aurblup.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/aurweb/scripts/aurblup.py b/aurweb/scripts/aurblup.py index 1433173..a7d43f1 100755 --- a/aurweb/scripts/aurblup.py +++ b/aurweb/scripts/aurblup.py @@ -39,13 +39,13 @@ def main(): cur = conn.execute("SELECT Name, Provides FROM OfficialProviders") oldproviders = set(cur.fetchall()) + for pkg, provides in oldproviders.difference(providers): + conn.execute("DELETE FROM OfficialProviders " + "WHERE Name = ? AND Provides = ?", [pkg, provides]) for pkg, provides in providers.difference(oldproviders): repo = repomap[(pkg, provides)] conn.execute("INSERT INTO OfficialProviders (Name, Repo, Provides) " "VALUES (?, ?, ?)", [pkg, repo, provides]) - for pkg, provides in oldproviders.difference(providers): - conn.execute("DELETE FROM OfficialProviders " - "WHERE Name = ? AND Provides = ?", [pkg, provides]) conn.commit() conn.close() -- 2.20.1

2 2

[PATCH] Quote MySql 8.0 reserved keywords
by Florian Pritz 14 Jan '19

14 Jan '19

Signed-off-by: Florian Pritz <bluewind(a)xinu.at> --- aurweb/git/update.py | 4 ++-- schema/aur-schema.sql | 4 ++-- upgrading/3.0.0.txt | 4 ++-- upgrading/3.1.0.txt | 2 +- web/lib/aurjson.class.php | 6 +++--- web/lib/pkgfuncs.inc.php | 2 +- 6 files changed, 11 insertions(+), 11 deletions(-) diff --git a/aurweb/git/update.py b/aurweb/git/update.py index 0614e9e..39128f8 100755 --- a/aurweb/git/update.py +++ b/aurweb/git/update.py @@ -180,13 +180,13 @@ def save_metadata(metadata, conn, user): # Add package groups. if 'groups' in pkginfo: for group in pkginfo['groups']: - cur = conn.execute("SELECT ID FROM Groups WHERE Name = ?", + cur = conn.execute("SELECT ID FROM `Groups` WHERE Name = ?", [group]) row = cur.fetchone() if row: groupid = row[0] else: - cur = conn.execute("INSERT INTO Groups (Name) VALUES (?)", + cur = conn.execute("INSERT INTO `Groups` (Name) VALUES (?)", [group]) conn.commit() groupid = cur.lastrowid diff --git a/schema/aur-schema.sql b/schema/aur-schema.sql index 79de3f2..f05f40a 100644 --- a/schema/aur-schema.sql +++ b/schema/aur-schema.sql @@ -149,7 +149,7 @@ CREATE TABLE PackageLicenses ( -- Information about groups -- -CREATE TABLE Groups ( +CREATE TABLE `Groups` ( ID INTEGER UNSIGNED NOT NULL AUTO_INCREMENT, Name VARCHAR(255) NOT NULL, PRIMARY KEY (ID), @@ -164,7 +164,7 @@ CREATE TABLE PackageGroups ( GroupID INTEGER UNSIGNED NOT NULL, PRIMARY KEY (PackageID, GroupID), FOREIGN KEY (PackageID) REFERENCES Packages(ID) ON DELETE CASCADE, - FOREIGN KEY (GroupID) REFERENCES Groups(ID) ON DELETE CASCADE + FOREIGN KEY (GroupID) REFERENCES `Groups`(ID) ON DELETE CASCADE ) ENGINE = InnoDB; diff --git a/upgrading/3.0.0.txt b/upgrading/3.0.0.txt index b134b79..4f9c70f 100644 --- a/upgrading/3.0.0.txt +++ b/upgrading/3.0.0.txt @@ -200,7 +200,7 @@ CREATE TABLE PackageRelations ( 17. Create tables to store package groups: ---- -CREATE TABLE Groups ( +CREATE TABLE `Groups` ( ID INTEGER UNSIGNED NOT NULL AUTO_INCREMENT, Name VARCHAR(64) NOT NULL, PRIMARY KEY (ID), @@ -211,7 +211,7 @@ CREATE TABLE PackageGroups ( GroupID INTEGER UNSIGNED NOT NULL, PRIMARY KEY (PackageID, GroupID), FOREIGN KEY (PackageID) REFERENCES Packages(ID) ON DELETE CASCADE, - FOREIGN KEY (GroupID) REFERENCES Groups(ID) ON DELETE CASCADE + FOREIGN KEY (GroupID) REFERENCES `Groups`(ID) ON DELETE CASCADE ) ENGINE = InnoDB; ---- diff --git a/upgrading/3.1.0.txt b/upgrading/3.1.0.txt index 2fc1614..e883a6c 100644 --- a/upgrading/3.1.0.txt +++ b/upgrading/3.1.0.txt @@ -7,7 +7,7 @@ ALTER TABLE Packages MODIFY Name VARCHAR(255) NOT NULL, MODIFY Version VARCHAR(255) NOT NULL DEFAULT ''; ALTER TABLE Licenses MODIFY Name VARCHAR(255) NOT NULL; -ALTER TABLE Groups MODIFY Name VARCHAR(255) NOT NULL; +ALTER TABLE `Groups` MODIFY Name VARCHAR(255) NOT NULL; ALTER TABLE PackageDepends MODIFY DepCondition VARCHAR(255); ALTER TABLE PackageRelations MODIFY RelCondition VARCHAR(255); ---- diff --git a/web/lib/aurjson.class.php b/web/lib/aurjson.class.php index c51e9c2..c275d21 100644 --- a/web/lib/aurjson.class.php +++ b/web/lib/aurjson.class.php @@ -284,10 +284,10 @@ private function get_extended_fields($pkgid, $base_id) { "LEFT JOIN RelationTypes " . "ON RelationTypes.ID = PackageRelations.RelTypeID " . "WHERE PackageRelations.PackageID = " . $pkgid . " " . - "UNION SELECT 'groups' AS Type, Groups.Name, '' AS Cond " . - "FROM Groups INNER JOIN PackageGroups " . + "UNION SELECT 'groups' AS Type, `Groups`.`Name`, '' AS Cond " . + "FROM `Groups` INNER JOIN PackageGroups " . "ON PackageGroups.PackageID = " . $pkgid . " " . - "AND PackageGroups.GroupID = Groups.ID " . + "AND PackageGroups.GroupID = `Groups`.ID " . "UNION SELECT 'license' AS Type, Licenses.Name, '' AS Cond " . "FROM Licenses INNER JOIN PackageLicenses " . "ON PackageLicenses.PackageID = " . $pkgid . " " . diff --git a/web/lib/pkgfuncs.inc.php b/web/lib/pkgfuncs.inc.php index ced1f8e..00f0cc4 100644 --- a/web/lib/pkgfuncs.inc.php +++ b/web/lib/pkgfuncs.inc.php @@ -188,7 +188,7 @@ function pkg_groups($pkgid) { $pkgid = intval($pkgid); if ($pkgid > 0) { $dbh = DB::connect(); - $q = "SELECT g.Name FROM Groups g "; + $q = "SELECT g.Name FROM `Groups` g "; $q.= "INNER JOIN PackageGroups pg ON pg.GroupID = g.ID "; $q.= "WHERE pg.PackageID = ". $pkgid; $result = $dbh->query($q); -- 2.20.1

2 1

Fwd: [PATCH] Make delete and merge links create an auto-accepted request
by Eli Schwartz 23 Dec '18

23 Dec '18

Plain copy of reply -------- Forwarded Message -------- Subject: Re: [PATCH] Make delete and merge links create an auto-accepted request Date: Sun, 23 Dec 2018 23:19:47 +0100 From: Johannes Löthberg <johannes(a)kyriasis.com> To: Eli Schwartz <eschwartz(a)archlinux.org> Excerpts from Eli Schwartz's message of December 23, 2018 22:58: > On 12/23/18 4:14 PM, Johannes Löthberg wrote: >> This lets us have a better paper-trail over what happens in the AUR. > I'm opposed to this change. > > The purpose of those links is arguably to do things in exceptional > circumstances. There are cases where an auto-accepted request is simply > uninteresting information. e.g., the following cases: > > - user submits deletion request, Eli Schwartz uses "close" instead of > "accept", with the message "merged instead". Do we really need > additional notifications here? > I think that is a better portrayal of reality, yes. I do not see there being any really justified reasons for anyone being able to delete a package without a request. > - deletion of spam packages > I believe that that is still just as interesting to have in the history. We've already had quite a few cases where there was confusion as to what happened to a package just because it was deleted directly. > Moreover, the current proposal is simply inferior in every possible way > compared to simply submitting a request, then accepting it. That way you > get to leave a message saying why it happened... I would simply never > ever use the delete/merge links if I actually wanted to send out > notifications. > It's hardly inferior, it's just different. There are obviously TUs that currently use them but don't want to leave a message, this still lets them do exactly that. And even if we would not actually create an auto-accepted request for it, I believe that it is wrong to not always send out at least a notification when a package is deleted. Having an actual request created just makes it easier to see them along with all other changes. > On top of this, where is the notification for orphaning packages against > the will of the maintainer? > That is fundamentally a different change and feature that has nothing to do with this patch. > > It's basically accepted practice regardless that when TUs adopt a > package into community, they submit a deletion request and then accept > it. This will traditionally include the high-content comment "moved to > community". > Sure. Except for when they don't. > The current patchset was proposed in response to one TU on IRC, Incorrect. I have been planning on doing this for months, and talked to Lukas about it months ago as well. > who feels strongly about the goal of said paper trail and desired to have > the entire feature removed from aurweb altogether. I propose instead > that we follow my recommendation to document on the wiki at > https://wiki.archlinux.org/index.php/AUR_Trusted_User_Guidelines that > TUs should submit a deletion request *with the relevant comment* rather > than deleting the package. > > I see no reason to modify the emergency override tools. Before you get to call them "emergency override tools" we're going to both need to *clearly* label them as such, and preferably move them to a separate section. As it is they are, and will continue to be, "just tools," especially since they are the easiest option for performing these actions. > Note: regarding the person who suggested on IRC that the links should be > removed, the orphan link in particular is utterly crucial to remain, > since aurweb includes a feature to accept an orphan request early by > leaving a comment and *not* actually orphaning the package. This > requires the TU to manually use the orphan link. If orphan requests were > given fair treatment with the other two request types, this would result > in a second notification every time it was used. > More generally, if you wish to leave a comment in the acceptance > notification, you must use the same comment form followed by manual > followup when closing a deletion or merge request as well (although > those are not locked for the duration of a 14-day grace period). > But then again, there is no reason to actually implement it that simplistically either. -- Sincerely, Johannes Löthberg

1 0

Re: [PATCH] Make delete and merge links create an auto-accepted request
by Eli Schwartz 23 Dec '18

23 Dec '18

This accidentally did not get CC'ed to the list... I've inlined my response. On 12/23/18 5:19 PM, Johannes Löthberg wrote: > Excerpts from Eli Schwartz's message of December 23, 2018 22:58: >> On 12/23/18 4:14 PM, Johannes Löthberg wrote: >>> This lets us have a better paper-trail over what happens in the AUR. >> I'm opposed to this change. >> >> The purpose of those links is arguably to do things in exceptional >> circumstances. There are cases where an auto-accepted request is simply >> uninteresting information. e.g., the following cases: >> >> - user submits deletion request, Eli Schwartz uses "close" instead of >> "accept", with the message "merged instead". Do we really need >> additional notifications here? >> > > I think that is a better portrayal of reality, yes. I do not see there > being any really justified reasons for anyone being able to delete a > package without a request. There was a request. :p >> - deletion of spam packages >> > > I believe that that is still just as interesting to have in the history. > > We've already had quite a few cases where there was confusion as to what > happened to a package just because it was deleted directly. Eh, not sure I really agree that spam is interesting. >> Moreover, the current proposal is simply inferior in every possible way >> compared to simply submitting a request, then accepting it. That way you >> get to leave a message saying why it happened... I would simply never >> ever use the delete/merge links if I actually wanted to send out >> notifications. >> > > It's hardly inferior, it's just different. There are obviously TUs that > currently use them but don't want to leave a message, this still lets > them do exactly that. And even if we would not actually create an > auto-accepted request for it, I believe that it is wrong to not always > send out at least a notification when a package is deleted. Having an > actual request created just makes it easier to see them along with all > other changes. I'd much prefer to see the ability to include a comment on the direct action interface then. :D I don't believe that would make it mandatory to leave a comment -- it would just give one the option to. >> On top of this, where is the notification for orphaning packages against >> the will of the maintainer? >> > > That is fundamentally a different change and feature that has nothing to > do with this patch. I see it as the same change/feature, adding a paper trail to to request-like actions. Could you clarify why you see it as different? >> It's basically accepted practice regardless that when TUs adopt a >> package into community, they submit a deletion request and then accept >> it. This will traditionally include the high-content comment "moved to >> community". >> > > Sure. Except for when they don't. > >> The current patchset was proposed in response to one TU on IRC, > > Incorrect. I have been planning on doing this for months, and talked to > Lukas about it months ago as well. Sorry for jumping to conclusions. :( All I saw was today's mention on IRC. >> who feels strongly about the goal of said paper trail and desired to have >> the entire feature removed from aurweb altogether. I propose instead >> that we follow my recommendation to document on the wiki at >> https://wiki.archlinux.org/index.php/AUR_Trusted_User_Guidelines that >> TUs should submit a deletion request *with the relevant comment* rather >> than deleting the package. >> >> I see no reason to modify the emergency override tools. > > Before you get to call them "emergency override tools" we're going to > both need to *clearly* label them as such, and preferably move them to a > separate section. As it is they are, and will continue to be, "just > tools," especially since they are the easiest option for performing > these actions. Fair point, although I do rather consider them as such I guess this is not really clear. Especially since I still don't want to use them without a comment box... (I would like to see more tools, "just" or otherwise, since in some ways the TU interface is rather rough. Deleting comments one by one is still the only way to deal with the repeated essay writing/gaming sites/MS Word spam too.) >> Note: regarding the person who suggested on IRC that the links should be >> removed, the orphan link in particular is utterly crucial to remain, >> since aurweb includes a feature to accept an orphan request early by >> leaving a comment and *not* actually orphaning the package. This >> requires the TU to manually use the orphan link. If orphan requests were >> given fair treatment with the other two request types, this would result >> in a second notification every time it was used. >> More generally, if you wish to leave a comment in the acceptance >> notification, you must use the same comment form followed by manual >> followup when closing a deletion or merge request as well (although >> those are not locked for the duration of a 14-day grace period). >> > > But then again, there is no reason to actually implement it that > simplistically either. Okay. I think, as I said above, that the current available tools are a bit primitive in how they expose functionality regardless. Maybe there is some way we could revamp all this in the process? Maybe if we exposed the comment box in the "delete/merge/orphan this package" landing page, we could unify comments and notifications across all three action paths? (request-accept, request-close, and direct action links) Rather than submitting an auto-accepted request, we'd simply prompt for an optional comment, then perform the final notification (and if "via" is passed, do so using pkgreq_close, else just use notify). This would also mean we only send out one email, instead of two. -- Eli Schwartz Bug Wrangler and Trusted User

1 0

[PATCH] Make delete and merge links create an auto-accepted request
by Johannes Löthberg 23 Dec '18

23 Dec '18

This lets us have a better paper-trail over what happens in the AUR. Signed-off-by: Johannes Löthberg <johannes(a)kyriasis.com> --- web/html/pkgbase.php | 23 +++++++++++++---------- web/lib/pkgreqfuncs.inc.php | 14 +++++++------- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/web/html/pkgbase.php b/web/html/pkgbase.php index 46ad77e..8efa51e 100644 --- a/web/html/pkgbase.php +++ b/web/html/pkgbase.php @@ -70,12 +70,10 @@ if (check_token()) { list($ret, $output) = pkgbase_vote($ids, false); } elseif (current_action("do_Delete")) { if (isset($_POST['confirm'])) { - if (!isset($_POST['merge_Into']) || empty($_POST['merge_Into'])) { - list($ret, $output) = pkgbase_delete($ids, NULL, $via); - unset($_GET['ID']); - unset($base_id); - } - else { + $type = "deletion"; + $merge_base_id = NULL; + if (isset($_POST['merge_Into']) && !empty($_POST['merge_Into'])) { + $type = "merge"; $merge_base_id = pkgbase_from_name($_POST['merge_Into']); if (!$merge_base_id) { $output = __("Cannot find package to merge votes and comments into."); @@ -83,12 +81,17 @@ if (check_token()) { } elseif (in_array($merge_base_id, $ids)) { $output = __("Cannot merge a package base with itself."); $ret = false; - } else { + } + } + if ($type == "deletion" || ($type == "merge" && $merge_base_id)) { + list($ret, $output, $request_id) = pkgreq_file($ids, $type, $_POST['merge_Into'], + "Trusted User-requested auto-accepted ".$type."."); + if ($ret) { list($ret, $output) = pkgbase_delete($ids, $merge_base_id, $via); - unset($_GET['ID']); - unset($base_id); } } + unset($_GET['ID']); + unset($base_id); } else { $output = __("The selected packages have not been deleted, check the confirmation checkbox."); @@ -112,7 +115,7 @@ if (check_token()) { } elseif (current_action("do_SetKeywords")) { list($ret, $output) = pkgbase_set_keywords($base_id, preg_split("/[\s,;]+/", $_POST['keywords'], -1, PREG_SPLIT_NO_EMPTY)); } elseif (current_action("do_FileRequest")) { - list($ret, $output) = pkgreq_file($ids, $_POST['type'], $_POST['merge_into'], $_POST['comments']); + list($ret, $output, $_reqid) = pkgreq_file($ids, $_POST['type'], $_POST['merge_into'], $_POST['comments']); } elseif (current_action("do_CloseRequest")) { list($ret, $output) = pkgreq_close($_POST['reqid'], $_POST['reason'], $_POST['comments']); } elseif (current_action("do_EditComaintainers")) { diff --git a/web/lib/pkgreqfuncs.inc.php b/web/lib/pkgreqfuncs.inc.php index 774ebe7..a15f101 100644 --- a/web/lib/pkgreqfuncs.inc.php +++ b/web/lib/pkgreqfuncs.inc.php @@ -124,19 +124,19 @@ function pkgreq_get_creator_email($id) { */ function pkgreq_file($ids, $type, $merge_into, $comments) { if (!has_credential(CRED_PKGREQ_FILE)) { - return array(false, __("You must be logged in to file package requests.")); + return array(false, __("You must be logged in to file package requests."), NULL); } if (!empty($merge_into) && !preg_match("/^[a-z0-9][a-z0-9\.+_-]*$/D", $merge_into)) { - return array(false, __("Invalid name: only lowercase letters are allowed.")); + return array(false, __("Invalid name: only lowercase letters are allowed."), NULL); } if (!empty($merge_into) && !pkgbase_from_name($merge_into)) { - return array(false, __("Cannot find package to merge votes and comments into.")); + return array(false, __("Cannot find package to merge votes and comments into."), NULL); } if (empty($comments)) { - return array(false, __("The comment field must not be empty.")); + return array(false, __("The comment field must not be empty."), NULL); } $dbh = DB::connect(); @@ -147,7 +147,7 @@ function pkgreq_file($ids, $type, $merge_into, $comments) { $pkgbase_name = pkgbase_name_from_id($base_id); if ($merge_into == $pkgbase_name) { - return array(false, __("Cannot merge a package base with itself.")); + return array(false, __("Cannot merge a package base with itself."), NULL); } $q = "SELECT ID FROM RequestTypes WHERE Name = " . $dbh->quote($type); @@ -155,7 +155,7 @@ function pkgreq_file($ids, $type, $merge_into, $comments) { if ($row = $result->fetch(PDO::FETCH_ASSOC)) { $type_id = $row['ID']; } else { - return array(false, __("Invalid request type.")); + return array(false, __("Invalid request type."), NULL); } $q = "INSERT INTO PackageRequests "; @@ -208,7 +208,7 @@ function pkgreq_file($ids, $type, $merge_into, $comments) { pkgbase_delete(array($base_id), NULL, NULL, true); } - return array(true, __("Added request successfully.")); + return array(true, __("Added request successfully."), $request_id); } /** -- 2.20.1

2 1