Merge pull request #211 from aplanas/master

Fix a problem found by max. Refactor check_repo to be independent of Factory.
This commit is contained in:
Ludwig Nussel 2014-08-15 13:40:03 +02:00
commit e0774e41ab
6 changed files with 100 additions and 130 deletions

View File

@ -35,10 +35,9 @@ sys.path.append(_plugin_dir)
from osclib.checkrepo import CheckRepo
from osclib.cycle import CycleDetector
from osclib.memoize import CACHEDIR
from osclib.stagingapi import StagingAPI
def _check_repo_download(self, request, opts):
def _check_repo_download(self, request):
request.downloads = defaultdict(list)
# Found cached version for the request, but the cached can be
@ -67,7 +66,8 @@ def _check_repo_download(self, request, opts):
if request.error:
return set()
if 'openSUSE:Factory:Staging:' in str(request.group):
staging_prefix = 'openSUSE:{}:Staging:'.format(self.checkrepo.opensuse)
if staging_prefix in str(request.group):
todownload = [ToDownload(request.group, 'standard', 'x86_64',
fn[0], fn[3]) for fn in
self.checkrepo.get_package_list_from_repository(
@ -94,7 +94,7 @@ def _check_repo_download(self, request, opts):
_errors_printed = set()
def _check_repo_group(self, id_, requests, opts):
def _check_repo_group(self, id_, requests):
print '> Check group [%s]' % ', '.join(r.str_compact() for r in requests)
if not all(self.checkrepo.is_buildsuccess(r) for r in requests):
@ -102,11 +102,11 @@ def _check_repo_group(self, id_, requests, opts):
toignore = set()
destdir = os.path.expanduser('~/co/%s' % str(requests[0].group))
fetched = dict((r, False) for r in opts.groups.get(id_, []))
fetched = dict((r, False) for r in self.checkrepo.groups.get(id_, []))
packs = []
for request in requests:
i = self._check_repo_download(request, opts)
i = self._check_repo_download(request)
if request.error and request.error not in _errors_printed:
_errors_printed.add(request.error)
if not request.updated:
@ -140,17 +140,16 @@ def _check_repo_group(self, id_, requests, opts):
# we need to call it to fetch the good repos to download
# but the return value is of no interest right now.
self.checkrepo.is_buildsuccess(rq)
i = self._check_repo_download(rq, opts)
i = self._check_repo_download(rq)
if rq.error:
print 'ERROR (ALREADY ACEPTED?):', rq.error
rq.updated = True
toignore.update(i)
# Detect cycles into the current Factory graph after we update the
# links with the current list of request.
api = StagingAPI(opts.apiurl)
cycle_detector = CycleDetector(api)
# Detect cycles into the current Factory / openSUSE graph after we
# update the links with the current list of request.
cycle_detector = CycleDetector(self.checkrepo.staging)
for (cycle, new_edges) in cycle_detector.cycles(requests=packs):
print
print ' - New cycle detected:', sorted(cycle)
@ -170,7 +169,7 @@ def _check_repo_group(self, id_, requests, opts):
continue
request = self.checkrepo.find_request_id(rq.tgt_project, package)
if request:
greqs = opts.groups.get(rq.group, [])
greqs = self.checkrepo.groups.get(rq.group, [])
if request in greqs:
continue
package = '#[%s](%s)' % (request, package)
@ -308,9 +307,10 @@ def _check_repo_group(self, id_, requests, opts):
# Detect if this error message comes from a staging project.
# Store it in the repo_checker_error, that is the text that
# will be published in the error message.
if 'openSUSE:Factory:Staging:' in project_repo[0]:
staging_prefix = 'openSUSE:{}:Staging:'.format(self.checkrepo.opensuse)
if staging_prefix in project_repo[0]:
repo_checker_error = stdoutdata
if not any('openSUSE:Factory:Staging:' in p_r[0] for p_r in execution_plan):
if not any(staging_prefix in p_r[0] for p_r in execution_plan):
repo_checker_error += '\nExecution plan: %s\n%s' % ('/'.join(project_repo), stdoutdata)
# print ret, stdoutdata, stderrdata
@ -347,9 +347,9 @@ def _check_repo_group(self, id_, requests, opts):
updated[rq.request_id] = 1
def mirror_full(plugin_dir, repo_dir):
def _mirror_full(self, plugin_dir, repo_dir):
"""Call bs_mirrorfull script to mirror packages."""
url = 'https://build.opensuse.org/build/%s/%s/x86_64' % ('openSUSE:Factory', 'standard')
url = 'https://build.opensuse.org/build/openSUSE:%s/%s/x86_64' % (self.checkrepo.opensuse, 'standard')
if not os.path.exists(repo_dir):
os.mkdir(repo_dir)
@ -365,6 +365,8 @@ def _print_request_and_specs(self, request_and_specs):
@cmdln.alias('check', 'cr')
@cmdln.option('-p', '--project', dest='project', metavar='PROJECT', default='Factory',
help='select a different project instead of openSUSE:Factory')
@cmdln.option('-s', '--skip', action='store_true', help='skip review')
def do_check_repo(self, subcmd, opts, *args):
"""${cmd_name}: Checker review of submit requests.
@ -377,16 +379,7 @@ def do_check_repo(self, subcmd, opts, *args):
${cmd_option_list}
"""
opts.mode = ''
opts.verbose = False
opts.apiurl = self.get_api_url()
self.checkrepo = CheckRepo(opts.apiurl)
# XXX TODO - Remove this the all access to opt.group[s|ed] comes
# from checkrepo.
opts.grouped = self.checkrepo.grouped
opts.groups = self.checkrepo.groups
self.checkrepo = CheckRepo(self.get_api_url(), opts.project)
if opts.skip:
if not len(args):
@ -453,8 +446,8 @@ def do_check_repo(self, subcmd, opts, *args):
# Mirror the packages locally in the CACHEDIR
plugin = '~/.osc-plugins/osc-check_repo.py'
self.plugin_dir = os.path.dirname(os.path.realpath(os.path.expanduser(plugin)))
self.repo_dir = '%s/repo-%s-%s-x86_64' % (CACHEDIR, 'openSUSE:Factory', 'standard')
mirror_full(self.plugin_dir, self.repo_dir)
self.repo_dir = '%s/repo-%s-%s-x86_64' % (CACHEDIR, 'openSUSE:{}'.format(opts.project), 'standard')
self._mirror_full(self.plugin_dir, self.repo_dir)
print
print 'Analysis results'
@ -464,6 +457,6 @@ def do_check_repo(self, subcmd, opts, *args):
# Sort the groups, from high to low. This put first the stating
# projects also
for id_, reqs in sorted(groups.items(), reverse=True):
self._check_repo_group(id_, reqs, opts)
self._check_repo_group(id_, reqs)
print
print

View File

@ -37,14 +37,14 @@ def _silent_running(fn):
checkout_pkg = _silent_running(checkout_package)
def _checker_parse(self, apiurl, project, package,
revision=None, brief=False, verbose=False):
def _checker_parse(self, apiurl, project, package, revision=None,
brief=False, verbose=False):
query = {'view': 'info', 'parse': 1}
if revision:
query['rev'] = revision
url = makeurl(apiurl, ['source', project, package], query)
ret = {'name': None, 'version': None }
ret = {'name': None, 'version': None}
try:
xml = ET.parse(http_GET(url)).getroot()
@ -53,19 +53,22 @@ def _checker_parse(self, apiurl, project, package,
return ret
# ET's boolean check is screwed
if xml.find('name') != None:
if xml.find('name') is not None:
ret['name'] = xml.find('name').text
if xml.find('version') != None:
if xml.find('version') is not None:
ret['version'] = xml.find('version').text
return ret
def _checker_change_review_state(self, opts, id, newstate, by_group='', by_user='', message='', supersed=None):
def _checker_change_review_state(self, opts, id_, newstate,
by_group='', by_user='', message='',
supersed=None):
""" taken from osc/osc/core.py, improved:
- verbose option added,
- empty by_user=& removed.
- numeric id can be int().
- numeric id_ can be int().
"""
query = {'cmd': 'changereviewstate', 'newstate': newstate}
if by_group:
@ -74,9 +77,9 @@ def _checker_change_review_state(self, opts, id, newstate, by_group='', by_user=
query['by_user'] = by_user
if supersed:
query['superseded_by'] = supersed
url = makeurl(opts.apiurl, ['request', str(id)], query=query)
url = makeurl(opts.apiurl, ['request', str(id_)], query=query)
if opts.dry_run:
print "dry-run: change review state: %s"%url
print 'dry-run: change review state: %s' % url
return
root = ET.parse(http_POST(url, data=message)).getroot()
return root.attrib['code']
@ -89,7 +92,8 @@ def _checker_prepare_dir(self, dir):
os.chdir(olddir)
def _checker_add_review(self, opts, id, by_group=None, by_user=None, msg=None):
def _checker_add_review(self, opts, id_, by_group=None, by_user=None,
msg=None):
query = {'cmd': 'addreview'}
if by_group:
query['by_group'] = by_group
@ -98,9 +102,9 @@ def _checker_add_review(self, opts, id, by_group=None, by_user=None, msg=None):
else:
raise Exception('we need either by_group or by_user')
url = makeurl(opts.apiurl, ['request', str(id)], query)
url = makeurl(opts.apiurl, ['request', str(id_)], query)
if opts.dry_run:
print "dry-run: adding review %s"%url
print 'dry-run: adding review %s' % url
return 0
try:
@ -114,25 +118,25 @@ def _checker_add_review(self, opts, id, by_group=None, by_user=None, msg=None):
return 0
def _checker_forward_to_staging(self, opts, id):
return self._checker_add_review(opts, id, by_group='factory-staging', msg="Pick Staging Project")
def _checker_forward_to_staging(self, opts, id_):
return self._checker_add_review(opts, id_, by_group='factory-staging', msg="Pick Staging Project")
def _checker_add_review_team(self, opts, id):
return self._checker_add_review(opts, id, by_group='opensuse-review-team', msg="Please review sources")
def _checker_add_review_team(self, opts, id_):
return self._checker_add_review(opts, id_, by_group='opensuse-review-team', msg="Please review sources")
def _checker_accept_request(self, opts, id, msg, diff=10000):
def _checker_accept_request(self, opts, id_, msg, diff=10000):
if diff > 8:
self._checker_add_review_team(opts, id)
#else:
# self._checker_add_review(opts, id, by_user='coolo', msg='Does it look harmless?')
self._checker_add_review_team(opts, id_)
# else:
# self._checker_add_review(opts, id_, by_user='coolo', msg='Does it look harmless?')
self._checker_add_review(opts, id, by_user='factory-repo-checker', msg='Please review build success')
self._checker_add_review(opts, id_, by_user='factory-repo-checker', msg='Please review build success')
self._checker_forward_to_staging(opts, id)
self._checker_forward_to_staging(opts, id_)
self._checker_change_review_state(opts, id, 'accepted', by_group='factory-auto', message=msg)
self._checker_change_review_state(opts, id_, 'accepted', by_group='factory-auto', message=msg)
print("accepted " + msg)
@ -140,12 +144,12 @@ def _checker_one_request(self, rq, opts):
if (opts.verbose):
ET.dump(rq)
print(opts)
id = int(rq.get('id'))
id_ = int(rq.get('id'))
act_id = 0
actions = rq.findall('action')
if len(actions) > 1:
msg = "2 actions in one SR is not supported - https://github.com/openSUSE/osc-plugin-factory/fork_select"
self._checker_change_review_state(opts, id, 'declined', by_group='factory-auto', message=msg)
msg = '2 actions in one SR is not supported - https://github.com/openSUSE/osc-plugin-factory/fork_select'
self._checker_change_review_state(opts, id_, 'declined', by_group='factory-auto', message=msg)
print("declined " + msg)
return
@ -162,20 +166,20 @@ def _checker_one_request(self, rq, opts):
src = {'package': pkg, 'project': prj, 'rev': rev, 'error': None}
e = []
if not pkg:
e.append('no source/package in request %d, action %d' % (id, act_id))
e.append('no source/package in request %d, action %d' % (id_, act_id))
if not prj:
e.append('no source/project in request %d, action %d' % (id, act_id))
e.append('no source/project in request %d, action %d' % (id_, act_id))
if e:
src.error = '; '.join(e)
e = []
if not tpkg:
e.append('no target/package in request %d, action %d; ' % (id, act_id))
e.append('no target/package in request %d, action %d; ' % (id_, act_id))
if not prj:
e.append('no target/project in request %d, action %d; ' % (id, act_id))
e.append('no target/project in request %d, action %d; ' % (id_, act_id))
# it is no error, if the target package dies not exist
subm_id = "SUBMIT(%d):" % id
subm_id = "SUBMIT(%d):" % id_
print ("\n%s %s/%s -> %s/%s" % (subm_id,
prj, pkg,
tprj, tpkg))
@ -186,18 +190,18 @@ def _checker_one_request(self, rq, opts):
[dprj, dpkg] = dpkg.split('/')
else:
dprj = None
if dprj and (dprj != prj or dpkg != pkg) and ("IGNORE_DEVEL_PROJECTS" not in os.environ):
if dprj and (dprj != prj or dpkg != pkg) and ('IGNORE_DEVEL_PROJECTS' not in os.environ):
msg = "'%s/%s' is the devel package, submission is from '%s'" % (dprj, dpkg, prj)
self._checker_change_review_state(opts, id, 'declined', by_group='factory-auto', message=msg)
print("declined " + msg)
self._checker_change_review_state(opts, id_, 'declined', by_group='factory-auto', message=msg)
continue
if not dprj and (prj + "/") not in self._devel_projects:
msg = "'%s' is not a valid devel project of %s - please pick one of the existent" % (prj, tprj)
self._checker_change_review_state(opts, id, 'declined', by_group='factory-auto', message=msg)
print("declined " + msg)
self._checker_change_review_state(opts, id_, 'declined', by_group='factory-auto', message=msg)
continue
dir = os.path.expanduser("~/co/%s" % str(id))
dir = os.path.expanduser("~/co/%s" % str(id_))
if os.path.exists(dir):
print("%s already exists" % dir)
continue
@ -222,7 +226,7 @@ def _checker_one_request(self, rq, opts):
new_infos = self._checker_parse(opts.apiurl, prj, pkg, revision=rev)
if new_infos['name'] != tpkg:
msg = "A pkg submitted as %s has to build as 'Name: %s' - found Name '%s'" % (tpkg, tpkg, new_infos['name'])
self._checker_change_review_state(opts, id, 'declined', by_group='factory-auto', message=msg)
self._checker_change_review_state(opts, id_, 'declined', by_group='factory-auto', message=msg)
continue
sourcechecker = os.path.dirname(os.path.realpath(os.path.expanduser('~/.osc-plugins/osc-check_source.py')))
@ -242,7 +246,7 @@ def _checker_one_request(self, rq, opts):
if ret != 0:
msg = "Output of check script:\n" + output
self._checker_change_review_state(opts, id, 'declined', by_group='factory-auto', message=msg)
self._checker_change_review_state(opts, id_, 'declined', by_group='factory-auto', message=msg)
print("declined " + msg)
shutil.rmtree(dir)
continue
@ -261,12 +265,12 @@ def _checker_one_request(self, rq, opts):
if len(checked):
msg = msg + "\n\nOutput of check script (non-fatal):\n" + output
if self._checker_accept_request(opts, id, msg, diff=diff):
if self._checker_accept_request(opts, id_, msg, diff=diff):
continue
else:
self._checker_forward_to_staging(opts, id)
self._checker_change_review_state(opts, id, 'accepted',
self._checker_forward_to_staging(opts, id_)
self._checker_change_review_state(opts, id_, 'accepted',
by_group='factory-auto',
message="Unchecked request type %s" % _type)
@ -292,12 +296,13 @@ def _checker_check_devel_package(self, opts, project, package):
return self._devel_projects[key]
if project == 'openSUSE:13.2':
if opts.verbose:
print "no devel project for %s found in %s, retry using Factory projects"%(package, project)
print 'no devel project for %s found in %s, retry using Factory projects' % (package, project)
return self._checker_check_devel_package(opts, 'openSUSE:Factory', package)
return None
@cmdln.option('-v', '--verbose', action='store_true', help="verbose output")
@cmdln.option('-p', '--project', dest='project', metavar='PROJECT', default='Factory',
help='select a different project instead of openSUSE:Factory')
@cmdln.option('-n', '--dry-run', action='store_true', help="dry run")
def do_check_source(self, subcmd, opts, *args):
"""${cmd_name}: checker review of submit requests.
@ -310,29 +315,31 @@ def do_check_source(self, subcmd, opts, *args):
"""
self._devel_projects = {}
self.opensuse = opts.project
opts.apiurl = self.get_api_url()
if len(args) and args[0] == 'skip':
for id in args[1:]:
self._checker_accept_request(opts, id, 'skip review')
for id_ in args[1:]:
self._checker_accept_request(opts, id_, 'skip review')
return
ids = {}
for a in args:
if re.match('\d+', a):
ids[a] = 1
if (not len(ids)):
where = "@by_group='factory-auto'+and+@state='new'"
url = makeurl(opts.apiurl, ['search', 'request'], "match=state/@name='review'+and+review[" + where + "]")
if not ids:
review = "@by_group='factory-auto'+and+@state='new'"
target = "@project='openSUSE:{}'".format(self.opensuse)
url = makeurl(opts.apiurl, ('search', 'request'),
"match=state/@name='review'+and+review[%s]+and+target[%s]" % (review, target))
root = ET.parse(http_GET(url)).getroot()
for rq in root.findall('request'):
self._checker_one_request(rq, opts)
else:
# we have a list, use them.
for id in ids.keys():
url = makeurl(opts.apiurl, ['request', id])
for id_ in ids.keys():
url = makeurl(opts.apiurl, ('request', id_))
f = http_GET(url)
xml = ET.parse(f)
root = xml.getroot()

View File

@ -101,10 +101,11 @@ class Request(object):
class CheckRepo(object):
def __init__(self, apiurl):
def __init__(self, apiurl, opensuse='Factory'):
"""CheckRepo constructor."""
self.apiurl = apiurl
self.staging = StagingAPI(apiurl)
self.opensuse = opensuse
self.staging = StagingAPI(apiurl, opensuse)
# grouped = { id: staging, }
self.grouped = {}
@ -189,10 +190,11 @@ class CheckRepo(object):
def pending_requests(self):
"""Search pending requests to review."""
requests = []
where = "@by_user='factory-repo-checker'+and+@state='new'"
review = "@by_user='factory-repo-checker'+and+@state='new'"
target = "@project='openSUSE:{}'".format(self.opensuse)
try:
url = makeurl(self.apiurl, ('search', 'request'),
"match=state/@name='review'+and+review[%s]" % where)
"match=state/@name='review'+and+review[%s]+and+target[%s]" % (review, target))
root = ET.parse(http_GET(url)).getroot()
requests = root.findall('request')
except urllib2.HTTPError, e:

View File

@ -30,31 +30,6 @@ class RequestFinder(object):
self.api = api
self.srs = {}
def _filter_review_by_project(self, element, state):
"""
Takes a XML that contains a list of reviews and take the ones
that in state 'state'.
:param element: xml list with reviews
:param state: state we filter for
"""
reviews = [r.get('by_project')
for r in element.findall('review')
if r.get('by_project') and r.get('state') == state]
return reviews
def _new_review_by_project(self, request_id, element):
"""
Takes a XML that contains a list of reviews and return the
staging project currantly assigned for review that is in 'new'
state. Makes sure that there is a most one.
:param request_id: request id
:param element: XML with list of reviews
"""
reviews = self._filter_review_by_project(element, 'new')
msg = 'Request "{}" have multiple review by project in new state "{}"'.format(request_id, reviews)
assert len(reviews) <= 1, msg
return reviews[0] if reviews else None
def find_request_id(self, request_id):
"""
Look up the request by ID to verify if it is correct
@ -82,10 +57,6 @@ class RequestFinder(object):
raise oscerr.WrongArgs(msg)
self.srs[int(request_id)] = {'project': project}
review = self._new_review_by_project(request_id, root)
if review and review.startswith('openSUSE:{}:Staging:'.format(self.api.opensuse)):
self.srs[int(request_id)]['staging'] = review
return True
def find_request_package(self, package):
@ -113,10 +84,6 @@ class RequestFinder(object):
self.srs[request] = {'project': 'openSUSE:{}'.format(self.api.opensuse), 'state': state}
review = self._new_review_by_project(request, sr)
if review and review.startswith('openSUSE:{}:Staging:'.format(self.api.opensuse)):
self.srs[int(request)]['staging'] = review
if last_rq:
if self.srs[last_rq]['state'] == 'declined':
# ignore previous requests if they are declined
@ -153,9 +120,6 @@ class RequestFinder(object):
request = int(sr.attrib['id'])
state = sr.find('state').get('name')
self.srs[request] = {'project': 'openSUSE:{}'.format(self.api.opensuse), 'state': state}
review = self._new_review_by_project(request, sr)
if review and review.startswith('openSUSE:{}:Staging:'.format(self.api.opensuse)):
self.srs[int(request)]['staging'] = review
ret = True
return ret

View File

@ -56,22 +56,26 @@ class SelectCommand(object):
return candidates[0] if candidates else None
def select_request(self, request, request_project, move, from_):
def select_request(self, request, move, from_):
supersede = self._supersede(request)
if 'staging' not in request_project and not supersede:
staged_requests = {
self.api.packages_staged[package]['rq_id']: package for package in self.api.packages_staged
}
if request not in staged_requests and not supersede:
# Normal 'select' command
print('Adding request "{}" to project "{}"'.format(request, self.target_project))
return self.api.rq_to_prj(request, self.target_project)
elif 'staging' in request_project and (move or supersede):
elif request in staged_requests and (move or supersede):
# 'select' command becomes a 'move'
fprj = None
if from_:
fprj = self.api.prj_from_letter(from_)
else:
# supersede = (new_rq, package, project)
fprj = request_project['staging'] if not supersede else supersede[2]
fprj = staged_requests[request] if not supersede else supersede[2]
if supersede:
print('"{} ({}) is superseded by {}'.format(request, supersede[1], supersede[0]))
@ -86,13 +90,13 @@ class SelectCommand(object):
self.affected_projects.add(fprj)
return self.api.move_between_project(fprj, request, self.target_project)
elif 'staging' in request_project and not move:
elif request in staged_requests and not move:
# Previously selected, but not explicit move
msg = 'Request {} is already tracked in "{}".'
msg = msg.format(request, request_project['staging'])
if request_project['staging'] != self.target_project:
msg = msg.format(request, staged_requests[request])
if staged_requests[request] != self.target_project:
msg += '\nUse --move modifier to move the request from "{}" to "{}"'
msg = msg.format(request_project['staging'], self.target_project)
msg = msg.format(staged_requests[request], self.target_project)
print(msg)
return True
else:
@ -114,8 +118,8 @@ class SelectCommand(object):
# FreezeCommand(self.api).perform(target_project)
self.target_project = target_project
for request, request_project in RequestFinder.find_sr(requests, self.api).items():
if not self.select_request(request, request_project, move, from_):
for request in RequestFinder.find_sr(requests, self.api):
if not self.select_request(request, move, from_):
return False
# Notify everybody about the changes

View File

@ -665,7 +665,7 @@ class OBS(object):
query = urlparse.urlparse(uri).query
assert query in (
"match=state/@name='review'+and+review[@by_group='factory-staging'+and+@state='new']",
"match=state/@name='review'+and+review[@by_user='factory-repo-checker'+and+@state='new']"
"match=state/@name='review'+and+review[@by_user='factory-repo-checker'+and+@state='new']+and+target[@project='openSUSE:Factory']"
)
response = (404, headers, '<result>Not found</result>')