From c0259013a42756b916a9bb4d765017503dd305de Mon Sep 17 00:00:00 2001 From: Alberto Planas Date: Thu, 21 Aug 2014 17:00:37 +0200 Subject: [PATCH 1/3] action #3206 - auto review for delete requests --- osc-check_repo.py | 20 ++++++++++++----- osclib/checkrepo.py | 53 +++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 66 insertions(+), 7 deletions(-) diff --git a/osc-check_repo.py b/osc-check_repo.py index c8bcbcac..b1f78599 100644 --- a/osc-check_repo.py +++ b/osc-check_repo.py @@ -97,7 +97,7 @@ _errors_printed = set() 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): + if not all(self.checkrepo.is_buildsuccess(r) for r in requests if r.action_type != 'delete'): return toignore = set() @@ -106,6 +106,9 @@ def _check_repo_group(self, id_, requests): packs = [] for request in requests: + if request.action_type == 'delete': + continue + i = self._check_repo_download(request) if request.error and request.error not in _errors_printed: _errors_printed.add(request.error) @@ -118,7 +121,8 @@ def _check_repo_group(self, id_, requests): return toignore.update(i) fetched[request.request_id] = True - packs.append(request) + + packs.extend(requests) # Extend packs array with the packages and .spec files of the # not-fetched requests. The not fetched ones are the requests of @@ -130,12 +134,18 @@ def _check_repo_group(self, id_, requests): # Download the repos from the request of the same group not # explicited in the command line. for rq in packs: - if fetched[rq.request_id]: + if rq.request_id in fetched and fetched[rq.request_id]: continue i = set() if rq.action_type == 'delete': - # for delete requests we only care for toignore + # for delete requests we care for toignore i = self.checkrepo._toignore(rq) + # We also check that nothing depends on the package and + # that the request originates by the package maintainer + if not self.checkrepo.is_secure_to_delete(rq): + rq.error = 'This request is not secure to remove. Check dependencies or author.' + print ' - %s' % rq.error + rq.updated = True else: # we need to call it to fetch the good repos to download # but the return value is of no interest right now. @@ -195,7 +205,7 @@ def _check_repo_group(self, id_, requests): execution_plan = defaultdict(list) - DEBUG_PLAN = 0 + DEBUG_PLAN = 1 # Get all the (project, repo, disturl) where the disturl is # compatible with the request. For the same package we can have diff --git a/osclib/checkrepo.py b/osclib/checkrepo.py index 4afd7dbd..a6d82c10 100644 --- a/osclib/checkrepo.py +++ b/osclib/checkrepo.py @@ -885,9 +885,58 @@ class CheckRepo(object): """ if request.is_shadow_devel: - url = self.staging.makeurl(['source', request.shadow_src_project, request.src_package]) + url = makeurl(self.apiurl, ('source', request.shadow_src_project, request.src_package)) http_DELETE(url) for sub_prj, sub_pkg in self.staging.get_sub_packages(request.src_package, request.shadow_src_project): - url = self.staging.makeurl(['source', sub_prj, sub_pkg]) + url = makeurl(self.apiurl, ('source', sub_prj, sub_pkg)) http_DELETE(url) + + def _whatdependson(self, request): + """Return the list of packages that depends on the one in the + request. + + """ + deps = set() + query = { + 'package': request.tgt_package, + 'view': 'revpkgnames', + } + for arch in ('i586', 'x86_64'): + url = makeurl(self.apiurl, ('build', request.tgt_project, 'standard', arch, '_builddepinfo'), + query=query) + root = ET.parse(http_GET(url)).getroot() + deps.update(pkgdep.text for pkgdep in root.findall('.//pkgdep')) + return deps + + def _maintainers(self, request): + """Get the maintainer of the package involved in the request.""" + query = { + 'binary': request.tgt_package, + } + url = makeurl(self.apiurl, ('search', 'owner'), query=query) + root = ET.parse(http_GET(url)).getroot() + return [p.get('name') for p in root.findall('.//person') if p.get('role') == 'maintainer'] + + def _author(self, request): + """Get the author of the request.""" + url = makeurl(self.apiurl, ('request', str(request.request_id))) + root = ET.parse(http_GET(url)).getroot() + + state = root.find('state') + if state.get('name') == 'new': + return state.get('who') + return root.find('history').get('who') + + def is_secure_to_delete(self, request): + """Return True is the request is secure to remove: + + - Nothing depends on the package anymore. + - The request originates by the package maintainer. + + """ + whatdependson = self._whatdependson(request) + maintainers = self._maintainers(request) + author = self._author(request) + + return (not whatdependson) and author in maintainers From a65f4d5844f30bcffbc228dfbcb42ba16206b7e7 Mon Sep 17 00:00:00 2001 From: Alberto Planas Date: Thu, 21 Aug 2014 17:31:07 +0200 Subject: [PATCH 2/3] Fix the from_project in select_command. --- osclib/select_command.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osclib/select_command.py b/osclib/select_command.py index 3d7588c3..100dd6a4 100644 --- a/osclib/select_command.py +++ b/osclib/select_command.py @@ -75,7 +75,7 @@ class SelectCommand(object): fprj = self.api.prj_from_letter(from_) else: # supersede = (new_rq, package, project) - fprj = staged_requests[request] if not supersede else supersede[2] + fprj = self.api.packages_staged[staged_requests[request]]['prj'] if not supersede else supersede[2] if supersede: print('"{} ({}) is superseded by {}'.format(request, supersede[1], supersede[0])) From 8acd18874fd5d8708e1a5477ea19d24a96bd5fae Mon Sep 17 00:00:00 2001 From: Alberto Planas Date: Fri, 22 Aug 2014 09:56:05 +0200 Subject: [PATCH 3/3] Add a new check. When the SPEC file is different from the package name. --- osc-check_repo.py | 2 +- osclib/checkrepo.py | 14 ++++++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/osc-check_repo.py b/osc-check_repo.py index b1f78599..9af12931 100644 --- a/osc-check_repo.py +++ b/osc-check_repo.py @@ -205,7 +205,7 @@ def _check_repo_group(self, id_, requests): execution_plan = defaultdict(list) - DEBUG_PLAN = 1 + DEBUG_PLAN = 0 # Get all the (project, repo, disturl) where the disturl is # compatible with the request. For the same package we can have diff --git a/osclib/checkrepo.py b/osclib/checkrepo.py index a6d82c10..fe7d05dd 100644 --- a/osclib/checkrepo.py +++ b/osclib/checkrepo.py @@ -407,8 +407,18 @@ class CheckRepo(object): specs = [en.attrib['name'][:-5] for en in root.findall('entry') if en.attrib['name'].endswith('.spec')] - # source checker validated it exists - specs.remove(rq.src_package) + # source checker already validated it + if rq.src_package in specs: + specs.remove(rq.src_package) + elif rq.tgt_package in specs: + specs.remove(rq.tgt_package) + else: + msg = 'The name of the SPEC files %s do not match with the name of the package (%s)' + msg = msg % (specs, rq.src_package) + print('DECLINED', msg) + self.change_review_state(request_id, 'declined', message=msg) + rq.updated = True + return requests # Makes sure that the .spec file builds properly.