findmissing: Improve SHA matching using SQL queries
SHAs provided by the command line need to be matched against kernel_sha
and fixedby_upstream_sha in the two fixes tables. So far we used
get_fix_status_and_changeid_from_list() to first evaluate if a SHA is
an upstream SHA (fixedby_upstream_sha) or a ChromeOS/stable release SHA
(kernel_sha). In general it is either/or. This means we can use SQL
queries "kernel_sha IN (sha_list)" and "fixedby_upstream_sha in
(sha_list)" to find the SHAs in the database. Only question is if to use
ther AND or the OR operator when searching for entries with kernel_sha
and/or fixedby_upstream_sha. As it turns out, this is straightforward:
- For "abandon" and "restore" commands, we have one or two SHAs.
- If the user provided one SHA, it can be a kernel_sha or a
fixedby_upstream_sha, and we need to use the OR operator.
- If the user provided two SHAs, expectation is that one is a kernel_sha
and one is a fixedby_upstream_sha. Use the AND operator in this case.
- For the "status" command, we want to list SHAs found anywhere.
Use the "OR" operator.
To implement this,
- Replace "kernel_sha" and "fixedby_upstream_sha" arguments to
get_fix_status_and_changeid() with a list of SHAs.
- Replace query
[kernel_sha = <kernel_sha>] [AND]
[fixedby_upstream_sha = <fixedby_upstream_sha>]
with
kernel_sha IN (sha_list) {AND,OR} fixedby_upstream_sha IN (sha_list)
- Pass "strict" argument to get_fix_status_and_changeid() to determine
if the search needs to match both kernel_sha and fixedby_upstream_sha
(AND) or if any match is sufficient (OR).
With this change, get_fix_status_and_changeid_from_list() and the query
of the linux_upstream table for each SHA is no longer needed. Also, we no
longer need to examine SHAs one by one in status_fix_cl() but can pass
the entire SHA list provided by the user to get_fix_status_and_changeid().
BUG=None
TEST=Run various findmissing commands
Change-Id: I959b80ec2e7eb7f428162aeab7c405ceecc6ce5f
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/dev-util/+/2319574
Commit-Queue: Guenter Roeck <groeck@chromium.org>
Tested-by: Guenter Roeck <groeck@chromium.org>
Reviewed-by: Curtis Malainey <cujomalainey@chromium.org>
diff --git a/contrib/findmissing/cloudsql_interface.py b/contrib/findmissing/cloudsql_interface.py
index f377956..a5874ad 100755
--- a/contrib/findmissing/cloudsql_interface.py
+++ b/contrib/findmissing/cloudsql_interface.py
@@ -47,21 +47,25 @@
return (row['kernel_sha'], row['fixedby_upstream_sha'])
-def get_fix_status_and_changeid(db, fixes_tables, kernel_sha, fixedby_upstream_sha):
+def get_fix_status_and_changeid(db, fixes_tables, sha_list, strict):
"""Get branch, fix_change_id, initial_status and status for one or more rows in fixes table."""
c = db.cursor(MySQLdb.cursors.DictCursor)
+ # If sha_list has only one entry, there will be only one database match.
+ # Use OR in the query if this is the case.
+ if len(sha_list) < 2:
+ strict = False
+
pre_q = """SELECT '{fixes_table}' AS 'table', branch, kernel_sha, fixedby_upstream_sha,
fix_change_id, initial_status, status
FROM {fixes_table}
WHERE """
- if kernel_sha:
- pre_q += ' kernel_sha = "%s"' % kernel_sha
- if fixedby_upstream_sha:
- pre_q += ' AND'
- if fixedby_upstream_sha:
- pre_q += ' fixedby_upstream_sha = "%s"' % fixedby_upstream_sha
+ joined_list = str(tuple(sha_list))
+
+ pre_q += ' kernel_sha IN %s' % joined_list
+ pre_q += ' AND' if strict else ' OR'
+ pre_q += ' fixedby_upstream_sha IN %s' % joined_list
q = pre_q.format(fixes_table=fixes_tables.pop(0))
while fixes_tables:
@@ -72,35 +76,6 @@
return c.fetchall()
-def get_fix_status_and_changeid_from_list(db, fixes_tables, sha_list):
- """Get branch, fix_change_id, initial_status and status for one or more rows in fixes table.
-
- The SHA or SHAs to identify commits are provided as anonymous SHA list. SHAs may either
- be from the upstream kernel or from the ChromeOS kernel. One or two SHAs must be provided.
- If there is one SHA, it must ieither be from the upstream kernel or from a ChromeOS branch.
- If there are two SHAs, one must be from the upstream kernel, the other must be from a ChromeOS
- branch.
- """
- c = db.cursor(MySQLdb.cursors.DictCursor)
- # find out which SHA is which
- kernel_sha = None
- fixedby_upstream_sha = None
- q = """SELECT sha FROM linux_upstream
- WHERE sha = %s"""
- c.execute(q, [sha_list[0]])
- if c.fetchone():
- # First SHA is upstream SHA
- fixedby_upstream_sha = sha_list[0]
- if len(sha_list) > 1:
- kernel_sha = sha_list[1]
- else:
- kernel_sha = sha_list[0]
- if len(sha_list) > 1:
- fixedby_upstream_sha = sha_list[1]
-
- return get_fix_status_and_changeid(db, fixes_tables, kernel_sha, fixedby_upstream_sha)
-
-
def update_change_abandoned(db, fixes_table, kernel_sha, fixedby_upstream_sha, reason=None):
"""Updates fixes_table unique fix row to indicate fix cl has been abandoned.
@@ -119,7 +94,7 @@
def update_change_restored(db, fixes_table, kernel_sha, fixedby_upstream_sha, reason=None):
"""Updates fixes_table unique fix row to indicate fix cl has been reopened."""
- rows = get_fix_status_and_changeid(db, [fixes_table], kernel_sha, fixedby_upstream_sha)
+ rows = get_fix_status_and_changeid(db, [fixes_table], [kernel_sha, fixedby_upstream_sha], True)
row = rows[0]
status = 'OPEN' if row['fix_change_id'] else row['initial_status']
diff --git a/contrib/findmissing/main.py b/contrib/findmissing/main.py
index c0abd42..9cf6713 100755
--- a/contrib/findmissing/main.py
+++ b/contrib/findmissing/main.py
@@ -57,7 +57,7 @@
row['branch'], row['kernel_sha'], row['fixedby_upstream_sha'], row['status']))
-def get_fixes_rows(cloudsql_db, fixes_table, sha_list):
+def get_fixes_rows(cloudsql_db, fixes_table, sha_list, strict):
"""Get all table rows for provided fixes table, or for both tables if none is proviced."""
if not fixes_table:
@@ -65,8 +65,8 @@
else:
fixes_tables = [fixes_table]
- return cloudsql_interface.get_fix_status_and_changeid_from_list(cloudsql_db, fixes_tables,
- sha_list)
+ return cloudsql_interface.get_fix_status_and_changeid(cloudsql_db, fixes_tables, sha_list,
+ strict)
@util.cloud_sql_proxy_decorator
@@ -76,7 +76,7 @@
cloudsql_db = MySQLdb.Connect(user='linux_patches_robot', host='127.0.0.1', db='linuxdb')
try:
- rows = get_fixes_rows(cloudsql_db, fixes_table, sha_list)
+ rows = get_fixes_rows(cloudsql_db, fixes_table, sha_list, True)
if not rows:
print('Patch identified by "%s" not found in fixes table(s)' % sha_list)
sys.exit(1)
@@ -120,8 +120,7 @@
rows = []
# Remove duplicate SHAs
sha_list = list(set(sha_list))
- for sha in sha_list:
- rows += get_fixes_rows(db, fixes_table, [sha])
+ rows = get_fixes_rows(db, fixes_table, sha_list, False)
if not rows:
print('No patches identified by "%s" found in fixes table(s)' % sha_list)
else:
@@ -136,7 +135,7 @@
"""Restores an abandoned change + updates database fix table."""
cloudsql_db = MySQLdb.Connect(user='linux_patches_robot', host='127.0.0.1', db='linuxdb')
try:
- rows = get_fixes_rows(cloudsql_db, fixes_table, sha_list)
+ rows = get_fixes_rows(cloudsql_db, fixes_table, sha_list, True)
if not rows:
print('Patch identified by "%s" not found in fixes table(s)' % sha_list)
sys.exit(1)
