findmissing: Improve SHA handling
Experience shows that in almost all cases, we want to abandon all
patches associated with a given upstream patch (fix) or with a given
stable release or Chrome OS patch.
At the same time, we need to be careful to not make too many uncontrolled
database changes.
To simplify the abandon/restore command line interface, only require
a single SHA. It does not matter which SHA is provided; it can be the
upstream SHA (the fix) or the stable release/ChromeOS SHA.
If that SHA reflects exactly one entry in the database, go ahead and
make the requested change. If there is more than one entry in the database,
only make the change if the '-f' or '--force' flag is provided. If the flag
is not provided, list the database entries which would be associated with
the change and exit.
Other changes:
- Convert "reason" argument into mandatory optional argument. This was
necessary since the list of SHAs has to come last (as positional
arguments)
- Do not require specific SHA order if two SHAs are specified.
- Ignore pointless pylint error when importing MySQLdb to make 'repo
upload' happy.
- When abandoning patches, skip already abandoned patches.
Also, do not let the user mark merged patches as "ABANDONED".
BUG=None
TEST=Run findmissing command and abandon/restore various patches
Change-Id: I8ad1a606508e6715c4e3e6fbb276493d0f5aa620
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/dev-util/+/2252420
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 7398a97..4507c9b 100755
--- a/contrib/findmissing/cloudsql_interface.py
+++ b/contrib/findmissing/cloudsql_interface.py
@@ -9,7 +9,7 @@
from __future__ import print_function
-import MySQLdb
+import MySQLdb # pylint: disable=import-error
import common
@@ -48,17 +48,54 @@
def get_fix_status_and_changeid(db, fixes_table, kernel_sha, fixedby_upstream_sha):
- """Get branch, fix_change_id, initial_status and status for a unique row in fixes table."""
+ """Get branch, fix_change_id, initial_status and status for one or more rows in fixes table."""
c = db.cursor(MySQLdb.cursors.DictCursor)
- q = """SELECT branch, fix_change_id, initial_status, status
- FROM {fixes_table}
- WHERE kernel_sha = %s
- AND fixedby_upstream_sha = %s""".format(fixes_table=fixes_table)
+ q = """SELECT branch, kernel_sha, fixedby_upstream_sha, fix_change_id,
+ initial_status, status
+ FROM {fixes_table}""".format(fixes_table=fixes_table)
- c.execute(q, [kernel_sha, fixedby_upstream_sha])
- row = c.fetchone()
- return row
+ if kernel_sha and fixedby_upstream_sha:
+ q += ' WHERE kernel_sha = %s AND fixedby_upstream_sha = %s'
+ sha_list = [kernel_sha, fixedby_upstream_sha]
+ elif fixedby_upstream_sha:
+ q += ' WHERE fixedby_upstream_sha = %s'
+ sha_list = [fixedby_upstream_sha]
+ else:
+ q += ' WHERE kernel_sha = %s'
+ sha_list = [kernel_sha]
+
+ c.execute(q, sha_list)
+ return c.fetchall()
+
+
+def get_fix_status_and_changeid_from_list(db, fixes_table, 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""".format(fixes_table=fixes_table)
+ 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_table, kernel_sha, fixedby_upstream_sha)
def update_change_abandoned(db, fixes_table, kernel_sha, fixedby_upstream_sha, reason=None):
@@ -79,7 +116,8 @@
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."""
- row = 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)
+ row = rows[0]
status = 'OPEN' if row['fix_change_id'] else row['initial_status']
c = db.cursor()
@@ -88,8 +126,7 @@
WHERE kernel_sha = %s
AND fixedby_upstream_sha = %s
AND status = 'ABANDONED'""".format(fixes_table=fixes_table)
- close_time = None
- c.execute(q, [status, close_time, reason, kernel_sha, fixedby_upstream_sha])
+ c.execute(q, [status, None, reason, kernel_sha, fixedby_upstream_sha])
db.commit()
diff --git a/contrib/findmissing/findmissing.py b/contrib/findmissing/findmissing.py
index a2d6b7b..6f751a1 100755
--- a/contrib/findmissing/findmissing.py
+++ b/contrib/findmissing/findmissing.py
@@ -19,6 +19,7 @@
from __future__ import print_function
import argparse
+import sys
import main
@@ -35,17 +36,20 @@
help='Function to either abandon/restore changes.')
parser.add_argument('fix', type=str, choices=('stable', 'chrome'),
help='Table that contains primary key you want to update.')
- parser.add_argument('patch', type=str,
- help='kernel_sha of row you want to update.')
- parser.add_argument('fixed_by', type=str,
- help='fixedby_upstream_sha of row you want to update.')
- parser.add_argument('reason', type=str, help='Reason for performing action.')
+ parser.add_argument('-f', '--force', action='store_true',
+ help='Force action if only one SHA provided and more than one database entry is affected.')
+ parser.add_argument('-r', '--reason', type=str, required=True,
+ help='Reason for performing action.')
+ parser.add_argument('sha', type=str, nargs='+',
+ help='To-be-fixed and/or fixing SHA for row you want to update.')
args = parser.parse_args()
- fixes_table = args.fix + '_fixes'
- abandon_restore_function_map[args.command](fixes_table, args.patch,
- args.fixed_by, args.reason)
+ if len(args.sha) > 2:
+ print('Must specify one or two SHAs.')
+ sys.exit(1)
+ fixes_table = args.fix + '_fixes'
+ abandon_restore_function_map[args.command](fixes_table, args.sha, args.reason, args.force)
if __name__ == '__main__':
diff --git a/contrib/findmissing/main.py b/contrib/findmissing/main.py
index bd3ebaf..a84ca6c 100755
--- a/contrib/findmissing/main.py
+++ b/contrib/findmissing/main.py
@@ -13,7 +13,7 @@
from __future__ import print_function
import sys
-import MySQLdb
+import MySQLdb # pylint: disable=import-error
import cloudsql_interface
import common
@@ -48,27 +48,48 @@
create_new_patches()
+def print_rows(rows):
+ """Print list of SHAs in database"""
+ print('Branch SHA Fixed by SHA Status')
+ for row in rows:
+ print('%-8s%-16s%-16s%s' %
+ (row['branch'], row['kernel_sha'], row['fixedby_upstream_sha'], row['status']))
+
+
@util.cloud_sql_proxy_decorator
@util.preliminary_check_decorator(False)
-def abandon_fix_cl(fixes_table, kernel_sha, fixedby_upstream_sha, reason):
+def abandon_fix_cl(fixes_table, sha_list, reason, force):
"""Abandons an fix CL + updates database fix table."""
cloudsql_db = MySQLdb.Connect(user='linux_patches_robot', host='127.0.0.1', db='linuxdb')
try:
- row = cloudsql_interface.get_fix_status_and_changeid(cloudsql_db, fixes_table,
- kernel_sha, fixedby_upstream_sha)
- if not row:
- print('Patch %s Fixed By %s doesnt exist in list of fixes in %s'
- % (kernel_sha, fixedby_upstream_sha, fixes_table))
+ rows = cloudsql_interface.get_fix_status_and_changeid_from_list(cloudsql_db,
+ fixes_table, sha_list)
+ if not rows:
+ print('Patch identified by "%s" not found in %s' % (sha_list, fixes_table))
sys.exit(1)
- if row['status'] == common.Status.OPEN.name:
- fix_change_id = row['fix_change_id']
+ if len(rows) > 1 and not force:
+ print('More than one database entry. Force flag needed to continue.')
+ print_rows(rows)
+ sys.exit(1)
+ for row in rows:
branch = row['branch']
- gerrit_interface.abandon_change(fix_change_id, branch, reason)
- print('Abandoned Change %s on Gerrit with reason %s' % (fix_change_id, reason))
- cloudsql_interface.update_change_abandoned(cloudsql_db, fixes_table,
- kernel_sha, fixedby_upstream_sha, reason)
- print('Updated status to abandoned for Patch %s Fixed by %s'
- % (kernel_sha, fixedby_upstream_sha))
+ fixedby_upstream_sha = row['fixedby_upstream_sha']
+ kernel_sha = row['kernel_sha']
+ status = row['status']
+ if status == common.Status.ABANDONED.name:
+ continue
+ if status not in (common.Status.OPEN.name, common.Status.CONFLICT.name):
+ print('Status for SHA %s fixed by %s is %s, can not abandon' %
+ (kernel_sha, fixedby_upstream_sha, status))
+ continue
+ if status == common.Status.OPEN.name:
+ fix_change_id = row['fix_change_id']
+ gerrit_interface.abandon_change(fix_change_id, branch, reason)
+ print('Abandoned Change %s on Gerrit with reason %s' % (fix_change_id, reason))
+ cloudsql_interface.update_change_abandoned(cloudsql_db, fixes_table,
+ kernel_sha, fixedby_upstream_sha, reason)
+ print('Updated status to abandoned for patch %s in %s, fixed by %s' %
+ (kernel_sha, branch, fixedby_upstream_sha))
sys.exit(0)
except KeyError:
print("""Could not retrieve fix row with primary key kernel_sha %s
@@ -80,27 +101,34 @@
@util.cloud_sql_proxy_decorator
@util.preliminary_check_decorator(False)
-def restore_fix_cl(fixes_table, kernel_sha, fixedby_upstream_sha, reason):
+def restore_fix_cl(fixes_table, sha_list, reason, force):
"""Restores an abandoned change + updates database fix table."""
cloudsql_db = MySQLdb.Connect(user='linux_patches_robot', host='127.0.0.1', db='linuxdb')
try:
- row = cloudsql_interface.get_fix_status_and_changeid(cloudsql_db, fixes_table,
- kernel_sha, fixedby_upstream_sha)
- if not row:
- print('Patch %s Fixed By %s doesnt exist in list of fixes in %s'
- % (kernel_sha, fixedby_upstream_sha, fixes_table))
+ rows = cloudsql_interface.get_fix_status_and_changeid_from_list(cloudsql_db,
+ fixes_table, sha_list)
+ if not rows:
+ print('Patch identified by "%s" not found in %s' % (sha_list, fixes_table))
sys.exit(1)
- if row['status'] == common.Status.ABANDONED.name:
- fix_change_id = row.get('fix_change_id')
+ if len(rows) > 1 and not force:
+ print('More than one database entry. Force flag needed to continue.')
+ print_rows(rows)
+ sys.exit(1)
+ for row in rows:
+ if row['status'] != common.Status.ABANDONED.name:
+ continue
+ fix_change_id = row['fix_change_id']
+ branch = row['branch']
+ fixedby_upstream_sha = row['fixedby_upstream_sha']
+ kernel_sha = row['kernel_sha']
if fix_change_id:
- branch = row['branch']
gerrit_interface.restore_change(fix_change_id, branch, reason)
print('Restored Change %s on Gerrit with reason %s' % (fix_change_id, reason))
cloudsql_interface.update_change_restored(cloudsql_db, fixes_table,
- kernel_sha, fixedby_upstream_sha, reason)
- print('Updated status to restored for Patch %s Fixed by %s'
- % (kernel_sha, fixedby_upstream_sha))
- sys.exit(0)
+ kernel_sha, fixedby_upstream_sha, reason)
+ print('Updated status to restored for patch %s in %s, fixed by %s'
+ % (kernel_sha, branch, fixedby_upstream_sha))
+ sys.exit(0)
except KeyError:
print("""Could not retrieve fix row with primary key kernel_sha %s
and fixedby_upstream_sha %s""" % (kernel_sha, fixedby_upstream_sha))
