Merge pull request #218 from aplanas/master

action #3206 - auto review for delete requests

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.

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.
 
@@ -885,9 +895,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

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]))