[PATCH] Improve "d" to support more workflows
Hi,
The attached 5 patches add three new features to "d" that I would like
to merge into master. All items here can correctly be testing from
respighi.d.o with my "local" copy of d via:
ssh respighi.debian.org ~nthykier/release-tools/scripts/d ...
If there are no objections, I intend to merge this by the end the week.
The proposed new features:
==========================
* Choose the target suite
- Example: "d openlp/experimental"
* Choose the base and target pkg+suite
- Example: "d mscgen/stable mscgen/testing"
* Diff *any* two versions still available in the pool (even if they
are not in any suite any longer)
- Example: "d clamav/0.99.2+dfsg-5 clamav/testing"
NB: clamav/0.99.2+dfsg-5 is currently *not* in any suite and
haven't been for the past 24+ hours. The example will eventually
expire as dak cleans up.
The existing "d <src>" will of course stay as it is.
The rationale behind the features:
==================================
At times, I have found d's current implementation a bit limiting and I
believe these changes can resolve some of that. A known case:
* Contributor uploads version pkg/1.0-2 to unstable to fix a bug
* RT member reviews diff between pkg/1.0-1..1.0-2 and unblocks
* Contributor notices a blocking issue and uploads pkg/1.0-3 to fix it
* RT member now /either/ has to review pkg/1.0-1..1.0-3 with "d" OR
pull pkg/1.0-2 from snapshots plus 1.0-3 from unstable and compare
them manually
Admittedly, in many cases, the changes are rather small and
self-contained. But unfortunately, the cases, where the changes are
"huge" are the ones where the extra work can be a(n extra) demotivating
factor.
The new features gives us a (time-limited) method to trivially work
around this issue by using the new call:
d src/<last-reviwed-version> src/unstable
Thanks,
~Niels
From 23d2024ac0411133a37b00eeda00890ef371cca6 Mon Sep 17 00:00:00 2001
From: Niels Thykier <niels@thykier.net>
Date: Tue, 7 Feb 2017 18:15:21 +0000
Subject: [PATCH 1/5] d: Support <src>/<suite>
Signed-off-by: Niels Thykier <niels@thykier.net>
---
scripts/d | 67 +++++++++++++++++++++++++++++++++++++++++----------------------
1 file changed, 44 insertions(+), 23 deletions(-)
diff --git a/scripts/d b/scripts/d
index 2e0e912..1a53b86 100755
--- a/scripts/d
+++ b/scripts/d
@@ -111,6 +111,7 @@ def invoke_grepexcuses(srcpkg, suite):
def usage():
print("Usage: %s <srcpkg>" % sys.argv[0])
+ print(" %s <srcpkg>/<suite>" % sys.argv[0])
return 1
def locate_hint_tool(argv0):
@@ -145,34 +146,51 @@ def main():
debdiffer = DebDiffFactory(projectb, config)
# Start working
srcpkg = sys.argv[1]
+ req_target_suite = None
versions = {}
+ suites = ['testing']
+
+ if '/' in srcpkg:
+ srcpkg, req_target_suite = srcpkg.split('/')
+ suites.append(req_target_suite)
+ else:
+ suites.extend(['testing-proposed-updates', 'unstable'])
locate_hint_tool(sys.argv[0])
# Fetch versions
- for suite in ['testing', 'testing-proposed-updates', 'unstable']:
+ for suite in suites:
versions[suite] = projectb.get_version(suite, srcpkg, False, True)
+
# Sanity check: is package present in testing, if not: bail out.
if versions['testing'] is None:
- if versions['unstable'] is None:
- print('E: package %s neither in testing nor unstable; maybe a typo or binary package?' % srcpkg,
+ if versions[req_target_suite] is None:
+ print('E: package %s neither in testing nor %s; maybe a typo or binary package?' % (srcpkg, req_target_suite),
file=sys.stderr)
else:
print('E: package %s not in testing' % srcpkg, file=sys.stderr)
return 1
- # Fetch version from tpu if there is a version available there,
- # otherwise take the version from unstable.
+
base_version = versions['testing']
- if (versions['testing-proposed-updates'] is None
- or apt_pkg.version_compare(versions['testing-proposed-updates'],
- versions['testing']) <= 0):
- target_suite = 'unstable'
- target_version = versions['unstable']
+ if req_target_suite is not None:
+ target_suite = req_target_suite
+ target_version = versions[target_suite]
+ print('I: using version %s from %s' % (target_version, target_suite),
+ file=sys.stderr)
else:
- print('I: using version %s from testing-proposed-updates'
- % versions['testing-proposed-updates'],
- file=sys.stderr)
- target_suite = 'testing-proposed-updates'
- target_version = versions['testing-proposed-updates']
+ # Fetch version from tpu if there is a version available there,
+ # otherwise take the version from unstable.
+ if (versions['testing-proposed-updates'] is None
+ or apt_pkg.version_compare(versions['testing-proposed-updates'],
+ versions['testing']) <= 0):
+ target_suite = 'unstable'
+ target_version = versions['unstable']
+ else:
+ print('I: using version %s from testing-proposed-updates'
+ % versions['testing-proposed-updates'],
+ file=sys.stderr)
+ target_suite = 'testing-proposed-updates'
+ target_version = versions['testing-proposed-updates']
+
# Identical versions do not need an action.
if base_version == target_version:
print('I: versions identical, nothing to diff', file=sys.stderr)
@@ -225,17 +243,20 @@ def main():
debdiff.diffstat, b"\n", the_diff)
close_pager(pager)
- print("Hints needed:")
- if target_suite != 'testing-proposed-updates':
- print(invoke_hint('unblock', srcpkg))
+ if req_target_suite is not None and req_target_suite not in ('unstable', 'testing-proposed-updates'):
+ print("Can only generate hints for versions from unstable and testing-proposed-updates")
else:
- print(invoke_hint('approve', srcpkg))
- if need_udeb_unblock:
- print("# XXXX: Confirm with d-i RM")
+ print("Hints needed:")
if target_suite != 'testing-proposed-updates':
- print(invoke_hint('unblock-udeb', srcpkg))
+ print(invoke_hint('unblock', srcpkg))
else:
- print("unblock-udeb %s/%s" % (srcpkg, target_version))
+ print(invoke_hint('approve', srcpkg))
+ if need_udeb_unblock:
+ print("# XXXX: Confirm with d-i RM")
+ if target_suite != 'testing-proposed-updates':
+ print(invoke_hint('unblock-udeb', srcpkg))
+ else:
+ print("unblock-udeb %s/%s" % (srcpkg, target_version))
return 0
if __name__ == '__main__':
--
2.1.4
From f0e68f74d2cef0196d92db81f55068e775cc94f6 Mon Sep 17 00:00:00 2001
From: Niels Thykier <niels@thykier.net>
Date: Tue, 7 Feb 2017 18:32:41 +0000
Subject: [PATCH 2/5] d: Check if a suite is valid before using it
Signed-off-by: Niels Thykier <niels@thykier.net>
---
lib/debrelease/projectb.py | 10 ++++++++++
scripts/d | 3 +++
2 files changed, 13 insertions(+)
diff --git a/lib/debrelease/projectb.py b/lib/debrelease/projectb.py
index 9d93bdc..4cab2b3 100644
--- a/lib/debrelease/projectb.py
+++ b/lib/debrelease/projectb.py
@@ -274,3 +274,13 @@ class ProjectB(object):
for m in self.BUGS_CLOSED_RE.findall(changelog):
bugs.update(re.findall("\d+", m))
return bugs
+
+
+ is_valid_suite_name_query = """SELECT 1 FROM suite WHERE suite_name = '%s' LIMIT 1"""
+
+ def is_valid_suite_name(self, suite):
+ query = self.is_valid_suite_name_query % (suite)
+ cursor = self.conn.cursor()
+ cursor.execute(query)
+ result = cursor.fetchall()
+ return len(result) > 0
diff --git a/scripts/d b/scripts/d
index 1a53b86..1fcbf40 100755
--- a/scripts/d
+++ b/scripts/d
@@ -152,6 +152,9 @@ def main():
if '/' in srcpkg:
srcpkg, req_target_suite = srcpkg.split('/')
+ if not projectb.is_valid_suite_name(req_target_suite):
+ print("E: %s is not a valid suite name" % req_target_suite)
+ return 1
suites.append(req_target_suite)
else:
suites.extend(['testing-proposed-updates', 'unstable'])
--
2.1.4
From 2b996d12ad4b2664774a7ac5716ce50aa749f36e Mon Sep 17 00:00:00 2001
From: Niels Thykier <niels@thykier.net>
Date: Tue, 7 Feb 2017 19:15:20 +0000
Subject: [PATCH 3/5] d: Support explicit passing the base dsc (src/suite)
Signed-off-by: Niels Thykier <niels@thykier.net>
---
scripts/d | 60 +++++++++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 45 insertions(+), 15 deletions(-)
diff --git a/scripts/d b/scripts/d
index 1fcbf40..3e3d1fb 100755
--- a/scripts/d
+++ b/scripts/d
@@ -112,6 +112,7 @@ def invoke_grepexcuses(srcpkg, suite):
def usage():
print("Usage: %s <srcpkg>" % sys.argv[0])
print(" %s <srcpkg>/<suite>" % sys.argv[0])
+ print(" %s <srcpkg>/<base-suite> <srcpkg>/<target-suite>" % sys.argv[0])
return 1
def locate_hint_tool(argv0):
@@ -136,8 +137,25 @@ def use_auto_filter():
return True
return False
+
+def parse_package_reference(projectb, given_pkg, *, require_explicit_suite=False):
+ pkg = given_pkg
+ suite = None
+ if '/' in given_pkg:
+ pkg, suite = given_pkg.split('/')
+ if not projectb.is_valid_suite_name(suite):
+ print("E: %s is not a valid suite name (in argument %s)" % (suite, given_pkg),
+ file=sys.stderr)
+ sys.exit(1)
+ elif require_explicit_suite:
+ print("E: %s is must be annotated with a valid suite name" % (given_pkg),
+ file=sys.stderr)
+ sys.exit(1)
+ return (pkg, suite)
+
+
def main():
- if len(sys.argv) != 2:
+ if len(sys.argv) < 2 or len(sys.argv) > 3:
return usage()
# Initialisation
apt_pkg.init_system()
@@ -145,16 +163,27 @@ def main():
projectb = ProjectB()
debdiffer = DebDiffFactory(projectb, config)
# Start working
- srcpkg = sys.argv[1]
req_target_suite = None
+ req_base_suite = 'testing'
versions = {}
- suites = ['testing']
- if '/' in srcpkg:
- srcpkg, req_target_suite = srcpkg.split('/')
- if not projectb.is_valid_suite_name(req_target_suite):
- print("E: %s is not a valid suite name" % req_target_suite)
- return 1
+ srcpkg = sys.argv[1]
+ base_src_pkg = None
+ if len(sys.argv) == 3:
+ base_src_pkg, req_base_suite = parse_package_reference(projectb,
+ sys.argv[1],
+ require_explicit_suite=True)
+ srcpkg, req_target_suite = parse_package_reference(projectb,
+ sys.argv[2],
+ require_explicit_suite=True)
+ else:
+ srcpkg, req_target_suite = parse_package_reference(projectb,
+ sys.argv[1],
+ require_explicit_suite=False)
+
+ suites = [req_base_suite]
+
+ if req_target_suite is not None:
suites.append(req_target_suite)
else:
suites.extend(['testing-proposed-updates', 'unstable'])
@@ -165,15 +194,15 @@ def main():
versions[suite] = projectb.get_version(suite, srcpkg, False, True)
# Sanity check: is package present in testing, if not: bail out.
- if versions['testing'] is None:
+ if versions[req_base_suite] is None:
if versions[req_target_suite] is None:
- print('E: package %s neither in testing nor %s; maybe a typo or binary package?' % (srcpkg, req_target_suite),
+ print('E: package %s neither in %s nor %s; maybe a typo or binary package?' % (srcpkg, req_base_suite, req_target_suite),
file=sys.stderr)
else:
- print('E: package %s not in testing' % srcpkg, file=sys.stderr)
+ print('E: package %s not in %s' % (srcpkg, req_base_suite), file=sys.stderr)
return 1
- base_version = versions['testing']
+ base_version = versions[req_base_suite]
if req_target_suite is not None:
target_suite = req_target_suite
target_version = versions[target_suite]
@@ -200,7 +229,7 @@ def main():
return 0
# Generate a header.
header = "Base version: %s_%s from %s\nTarget version: %s_%s from %s\n\n" % \
- (srcpkg, base_version, 'testing', srcpkg, target_version, target_suite)
+ (srcpkg, base_version, req_base_suite, srcpkg, target_version, target_suite)
header = header.encode('utf-8')
# Generate the diff.
debdiff = debdiffer.create_pool_pool_debdiff(
@@ -246,8 +275,9 @@ def main():
debdiff.diffstat, b"\n", the_diff)
close_pager(pager)
- if req_target_suite is not None and req_target_suite not in ('unstable', 'testing-proposed-updates'):
- print("Can only generate hints for versions from unstable and testing-proposed-updates")
+ if req_base_suite != 'testing' or (req_target_suite is not None
+ and req_target_suite not in ('unstable', 'testing-proposed-updates')):
+ print("Can only generate hints for versions from unstable and testing-proposed-updates to testing")
else:
print("Hints needed:")
if target_suite != 'testing-proposed-updates':
--
2.1.4
From ba510d21dc492dc46468e9cda725072d81beadec Mon Sep 17 00:00:00 2001
From: Niels Thykier <niels@thykier.net>
Date: Tue, 7 Feb 2017 19:19:05 +0000
Subject: [PATCH 4/5] d: Rename a variable
Signed-off-by: Niels Thykier <niels@thykier.net>
---
scripts/d | 36 ++++++++++++++++++------------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/scripts/d b/scripts/d
index 3e3d1fb..bbd05ea 100755
--- a/scripts/d
+++ b/scripts/d
@@ -167,19 +167,19 @@ def main():
req_base_suite = 'testing'
versions = {}
- srcpkg = sys.argv[1]
+ target_src_pkg = sys.argv[1]
base_src_pkg = None
if len(sys.argv) == 3:
base_src_pkg, req_base_suite = parse_package_reference(projectb,
sys.argv[1],
require_explicit_suite=True)
- srcpkg, req_target_suite = parse_package_reference(projectb,
- sys.argv[2],
- require_explicit_suite=True)
+ target_src_pkg, req_target_suite = parse_package_reference(projectb,
+ sys.argv[2],
+ require_explicit_suite=True)
else:
- srcpkg, req_target_suite = parse_package_reference(projectb,
- sys.argv[1],
- require_explicit_suite=False)
+ target_src_pkg, req_target_suite = parse_package_reference(projectb,
+ sys.argv[1],
+ require_explicit_suite=False)
suites = [req_base_suite]
@@ -191,15 +191,15 @@ def main():
locate_hint_tool(sys.argv[0])
# Fetch versions
for suite in suites:
- versions[suite] = projectb.get_version(suite, srcpkg, False, True)
+ versions[suite] = projectb.get_version(suite, target_src_pkg, False, True)
# Sanity check: is package present in testing, if not: bail out.
if versions[req_base_suite] is None:
if versions[req_target_suite] is None:
- print('E: package %s neither in %s nor %s; maybe a typo or binary package?' % (srcpkg, req_base_suite, req_target_suite),
+ print('E: package %s neither in %s nor %s; maybe a typo or binary package?' % (target_src_pkg, req_base_suite, req_target_suite),
file=sys.stderr)
else:
- print('E: package %s not in %s' % (srcpkg, req_base_suite), file=sys.stderr)
+ print('E: package %s not in %s' % (target_src_pkg, req_base_suite), file=sys.stderr)
return 1
base_version = versions[req_base_suite]
@@ -229,15 +229,15 @@ def main():
return 0
# Generate a header.
header = "Base version: %s_%s from %s\nTarget version: %s_%s from %s\n\n" % \
- (srcpkg, base_version, req_base_suite, srcpkg, target_version, target_suite)
+ (target_src_pkg, base_version, req_base_suite, target_src_pkg, target_version, target_suite)
header = header.encode('utf-8')
# Generate the diff.
debdiff = debdiffer.create_pool_pool_debdiff(
- srcpkg, base_version, target_version)
+ target_src_pkg, base_version, target_version)
# Provide these on the pager, so that review can be aborted if the package
# is already unblocked.
- existing_hints = invoke_hint('grep', '/^%s$/' % re.escape(srcpkg))
+ existing_hints = invoke_hint('grep', '/^%s$/' % re.escape(target_src_pkg))
if existing_hints:
existing_hints = 'Hints in place:\n' + existing_hints + '\n\n'
@@ -248,7 +248,7 @@ def main():
need_udeb_unblock = False
existing_hints = existing_hints.encode('utf-8')
- excuses = 'Excuses:\n\n%s\n\n' % (invoke_grepexcuses(srcpkg, target_suite))
+ excuses = 'Excuses:\n\n%s\n\n' % (invoke_grepexcuses(target_src_pkg, target_suite))
excuses = excuses.encode('utf-8')
use_colordiff = can_use_colordiff()
@@ -281,15 +281,15 @@ def main():
else:
print("Hints needed:")
if target_suite != 'testing-proposed-updates':
- print(invoke_hint('unblock', srcpkg))
+ print(invoke_hint('unblock', target_src_pkg))
else:
- print(invoke_hint('approve', srcpkg))
+ print(invoke_hint('approve', target_src_pkg))
if need_udeb_unblock:
print("# XXXX: Confirm with d-i RM")
if target_suite != 'testing-proposed-updates':
- print(invoke_hint('unblock-udeb', srcpkg))
+ print(invoke_hint('unblock-udeb', target_src_pkg))
else:
- print("unblock-udeb %s/%s" % (srcpkg, target_version))
+ print("unblock-udeb %s/%s" % (target_src_pkg, target_version))
return 0
if __name__ == '__main__':
--
2.1.4
From c463f7367bc9f62d0a2211aeb43c2b8080b2e287 Mon Sep 17 00:00:00 2001
From: Niels Thykier <niels@thykier.net>
Date: Tue, 7 Feb 2017 20:01:15 +0000
Subject: [PATCH 5/5] d: Support "src/version" for versions not in any suites
Signed-off-by: Niels Thykier <niels@thykier.net>
---
lib/debrelease/projectb.py | 23 ++++++++++++
scripts/d | 90 ++++++++++++++++++++++++++++------------------
2 files changed, 79 insertions(+), 34 deletions(-)
diff --git a/lib/debrelease/projectb.py b/lib/debrelease/projectb.py
index 4cab2b3..d6ff017 100644
--- a/lib/debrelease/projectb.py
+++ b/lib/debrelease/projectb.py
@@ -284,3 +284,26 @@ class ProjectB(object):
cursor.execute(query)
result = cursor.fetchall()
return len(result) > 0
+
+ # Abuse LEFT JOINs to get 1 result if the version exists but is not in a suite
+ # (makes the suite name field empty)
+ check_version_query = \
+ """
+ SELECT 1, suite.suite_name FROM source
+ LEFT JOIN src_associations sa ON source.id = sa.source
+ LEFT JOIN suite ON sa.suite = suite.id
+ WHERE source.source = '%s' AND source.version = '%s'
+ LIMIT 1
+ """
+
+ def check_version(self, srcpkg, version):
+ query = self.check_version_query % (srcpkg, version)
+ cursor = self.conn.cursor()
+ cursor.execute(query)
+ result = cursor.fetchall()
+ if len(result) < 1:
+ return (False, None)
+ suite = result[0][1]
+ if suite is not None and suite == '':
+ suite = None
+ return (True, suite)
diff --git a/scripts/d b/scripts/d
index bbd05ea..eabd823 100755
--- a/scripts/d
+++ b/scripts/d
@@ -37,6 +37,7 @@ from debrelease.debdiff import DebDiffFactory
from debrelease.proposed import Deb822
ALWAYS_EXCLUDE = ['**/*.po', '**/*.pot']
+NOT_IN_A_SUITE = '<none>'
def read_config():
config_file = os.path.dirname(os.path.realpath(__file__)) + '/../config.ini'
@@ -141,17 +142,25 @@ def use_auto_filter():
def parse_package_reference(projectb, given_pkg, *, require_explicit_suite=False):
pkg = given_pkg
suite = None
+ version = None
if '/' in given_pkg:
- pkg, suite = given_pkg.split('/')
- if not projectb.is_valid_suite_name(suite):
- print("E: %s is not a valid suite name (in argument %s)" % (suite, given_pkg),
- file=sys.stderr)
- sys.exit(1)
+ pkg, suite_or_version = given_pkg.split('/')
+ if projectb.is_valid_suite_name(suite_or_version):
+ suite = suite_or_version
+ else:
+ ok, suite = projectb.check_version(pkg, suite_or_version)
+ if not ok:
+ print("E: %s is not a valid suite name (in argument %s)" % (suite, given_pkg),
+ file=sys.stderr)
+ sys.exit(1)
+ version = suite_or_version
+ if suite is None:
+ suite = NOT_IN_A_SUITE
elif require_explicit_suite:
print("E: %s is must be annotated with a valid suite name" % (given_pkg),
file=sys.stderr)
sys.exit(1)
- return (pkg, suite)
+ return (pkg, suite, version)
def main():
@@ -163,6 +172,8 @@ def main():
projectb = ProjectB()
debdiffer = DebDiffFactory(projectb, config)
# Start working
+ base_version = None
+ target_version = None
req_target_suite = None
req_base_suite = 'testing'
versions = {}
@@ -170,42 +181,53 @@ def main():
target_src_pkg = sys.argv[1]
base_src_pkg = None
if len(sys.argv) == 3:
- base_src_pkg, req_base_suite = parse_package_reference(projectb,
- sys.argv[1],
- require_explicit_suite=True)
- target_src_pkg, req_target_suite = parse_package_reference(projectb,
- sys.argv[2],
- require_explicit_suite=True)
- else:
- target_src_pkg, req_target_suite = parse_package_reference(projectb,
- sys.argv[1],
- require_explicit_suite=False)
-
- suites = [req_base_suite]
-
- if req_target_suite is not None:
- suites.append(req_target_suite)
+ orig_target = sys.argv[2]
+ base_src_pkg, req_base_suite, base_version = parse_package_reference(projectb,
+ sys.argv[1],
+ require_explicit_suite=True)
+ target_src_pkg, req_target_suite, target_version = parse_package_reference(projectb,
+ orig_target,
+ require_explicit_suite=True)
else:
- suites.extend(['testing-proposed-updates', 'unstable'])
+ orig_target = sys.argv[1]
+ target_src_pkg, req_target_suite, target_version = parse_package_reference(projectb,
+ orig_target,
+ require_explicit_suite=False)
+ suites = []
+ if base_version is None:
+ suites = [req_base_suite]
+
+ if target_version is None:
+ if req_target_suite is not None:
+ suites.append(req_target_suite)
+ else:
+ suites.extend(['testing-proposed-updates', 'unstable'])
locate_hint_tool(sys.argv[0])
- # Fetch versions
+
+ # Fetch versions (if needed)
for suite in suites:
versions[suite] = projectb.get_version(suite, target_src_pkg, False, True)
- # Sanity check: is package present in testing, if not: bail out.
- if versions[req_base_suite] is None:
- if versions[req_target_suite] is None:
- print('E: package %s neither in %s nor %s; maybe a typo or binary package?' % (target_src_pkg, req_base_suite, req_target_suite),
- file=sys.stderr)
- else:
- print('E: package %s not in %s' % (target_src_pkg, req_base_suite), file=sys.stderr)
- return 1
+ # Sanity check: assert that the packages exist in the given suites
+ if base_version is None and (req_base_suite == NOT_IN_A_SUITE or versions[req_base_suite] is None):
+ # Base version is missing; something is wrong
+ if target_version is None:
+ if req_target_suite == NOT_IN_A_SUITE:
+ print("E: Target version %s was not found in the database; maybe a typo or a binary package" % orig_target, file=sys.stderr)
+ elif versions[req_target_suite] is None:
+ print('E: package %s neither in %s nor %s; maybe a typo or binary package?' % (target_src_pkg, req_base_suite, req_target_suite),
+ file=sys.stderr)
+ else:
+ print('E: package %s not in %s' % (target_src_pkg, req_base_suite), file=sys.stderr)
+ return 1
- base_version = versions[req_base_suite]
- if req_target_suite is not None:
+ if base_version is None:
+ base_version = versions[req_base_suite]
+ if req_target_suite is not None or target_version is not None:
target_suite = req_target_suite
- target_version = versions[target_suite]
+ if target_version is None:
+ target_version = versions[target_suite]
print('I: using version %s from %s' % (target_version, target_suite),
file=sys.stderr)
else:
--
2.1.4
Reply to: