#7 Move possibly reusable code into mass-pagure-prs
Opened 4 years ago by ishcherb. Modified 4 years ago

file modified
+52 -124
@@ -27,14 +27,14 @@ 

  # python%{?fedora:2}-foo in requires if not available.

  

  import logging

- import pathlib

- import tempfile

  

  import click

  

- from fixrequires import LocalRepo, PagureFork

  from fixrequires.modifier import RequiresModifier

- from qa import test_in_mock, test_in_koji

+ from fixrequires.dnf_info import DNFInfo

+ from qa import check_mock_build, check_koji_build

+ 

+ from mass_pagure_prs import BasePRsGenerator

  

  

  def get_logger(name=__name__, level=logging.DEBUG):
@@ -70,20 +70,36 @@ 

  )

  

  

- def fix_specfile(specfile, modifier):

-     """Fix deps in a spec file.

+ class PagurePRsGenerator(BasePRsGenerator):

  

-     Return: True if fixed, False if no changes were made

+     """A class to generate Pagure PRs for

+     ambiguous requires change.

      """

-     with open(specfile, 'rt') as f:

-         spec = f.read()

  

-     new_spec = modifier.run(spec)

+     def modify_spec(self, specfile):

+         """Modify a spec file and return a modified version.

+ 

+         Args:

+             specfile (str): A spec file as a string.

  

-     with open(specfile, 'wt') as out:

-         out.write(new_spec)

+         Return:

+             (str) A modified spec file.

+         """

+         modifier = RequiresModifier(self.logger)

+         new_spec = modifier.run(specfile)

+         self.set_pr_description(

+             f'{PR_DESCRIPTION}\n\n'

+             f'{modifier.check_for_other_fedora_branches()}'

+             '*This PR was opened automatically, for source code see '

+             '[here](https://pagure.io/python-fixrequires)*')

+         return new_spec

  

-     return spec != new_spec

+     def check_mock_build_results(self, output):

+         dnf_info = DNFInfo(self.logger)

+         check_mock_build(output, dnf_info)

+ 

+     def check_koji_scratch_build_results(self, output):

+         check_koji_build(output)

  

  

  @click.command(help=__doc__)
@@ -127,117 +143,29 @@ 

          logger = get_logger(level=logging.DEBUG)

      else:

          logger = get_logger(level=logging.ERROR)

-     with open(packages, 'rt') as f:

-         require_misnamed = f.read().splitlines()

- 

-     if not require_misnamed:

-         logger.info('No packages with misnamed requires found')

- 

-     if not dirname:

-         dirname = tempfile.mkdtemp()

- 

-     modifier = RequiresModifier(logger)

- 

-     non_fedora_packages = []

-     fixed_packages = []

-     problem_packages = []

-     opened_pr_links = []

- 

-     logger.debug(f'Cloning packages into {dirname}')

-     for package_name in require_misnamed:

- 

-         try:

-             local_repo = LocalRepo(

-                 package_name,

-                 pathlib.Path(f'{dirname}/{package_name}'),

-                 logger)

-             pagure_fork = PagureFork(

-                 package_name, pagure_token, pagure_user, logger)

- 

-             local_repo.clone()

-             local_repo.create_branch(GIT_BRANCH)

- 

-             spec_fixed = fix_specfile(local_repo.specfile, modifier)

-             if not spec_fixed:

-                 raise Exception(

-                     'No changes made to the spec file. '

-                     'The package might already be fixed')

- 

-             local_repo.bump_spec(COMMENT)

-             local_repo.show_diff(local_repo.specfile)

- 

-             if pagure:

-                 # Create a fork before testing to avoid

-                 # waiting the fork to be ready to be pushed to.

-                 if not pagure_token or not pagure_user:

-                     raise Exception("Please provide both pagure user and token")

-                     break

- 

-                 pagure_fork.do_fork()

-                 fork_url = pagure_fork.get_ssh_git_url()

- 

-             # QA.

-             local_repo.create_srpm()

-             if not no_mock_build:

-                 test_in_mock(local_repo, logger, dnf_info=modifier.dnf_info)

- 

-             if not no_koji_build:

-                 test_in_koji(local_repo, logger)

-             modifier.check_for_other_fedora_branches()

- 

-             # All good at this point. Go ahead and push.

-             if pagure:

-                 # Use username as a name for fork remote if not provided.

-                 remote_name = pagure_user

- 

-                 local_repo.add_to_git_remotes(remote_name, fork_url)

-                 local_repo.git_add(local_repo.specfile)

-                 local_repo.git_commit(COMMIT)

-                 local_repo.git_push(remote_name, GIT_BRANCH)

-                 # If the PR is already there, then the script is being run

-                 # the second time, and just pushing to a fork is enough.

-                 pr = pagure_fork.has_upstream_pr_with(

-                     title=COMMIT, branch_from=GIT_BRANCH)

-                 if pr:

-                     logger.info(f"The PR was already opened: {pr}")

-                     continue

- 

-                 pagure_fork.create_pull_request(

-                     from_branch=GIT_BRANCH,

-                     to_branch='master',

-                     title=COMMIT,

-                     description=(

-                         f'{PR_DESCRIPTION}\n\n'

-                         f'{modifier.check_for_other_fedora_branches()}'

-                         '*This PR was opened automatically, for source code see '

-                         '[here](https://pagure.io/python-fixrequires)*'

-                     ),

-                     fas_user=fas_user, fas_password=fas_password)

- 

-                 pr = pagure_fork.has_upstream_pr_with(

-                     title=COMMIT, branch_from=GIT_BRANCH)

-                 opened_pr_links.append(pr)

- 

-         except Exception as err:

-             logger.error(f"Failed to fix a package {package_name}. Error: {err}")

-             problem_packages.append(package_name)

-             continue

- 

-         fixed_packages.append(package_name)

- 

-     result = (

-         '\n\nRESULTS:\n'

-         f'The following {len(problem_packages)} packages had problem'

-         f' while testing and were not pushed:\n{problem_packages}\n.'

-         f'The following {len(fixed_packages)} packages were successfully fixed:\n'

-         f'{fixed_packages}\n.'

-     )

-     logger.info(result)

- 

-     if pagure:

-         logger.info('Opened PRs:')

-         for pr_link in opened_pr_links:

-             print(pr_link)

+ 

+     prs_generator = PagurePRsGenerator(

+         packages_filename=packages,

+         output_dirname=dirname,

+         dry_run=not pagure,

+         pagure_token=pagure_token,

+         pagure_user=pagure_user,

+         logger=logger,

+         fas_user=fas_user,

+         fas_password=fas_password)

+ 

+     prs_generator.configure(

+         git_branch=GIT_BRANCH,

+         changelog_entry=COMMENT,

+         commit_message=COMMIT,

+         pr_title=COMMIT,

+         pr_description=PR_DESCRIPTION)

+ 

+     prs = prs_generator.run(

+         do_mock_build=not no_mock_build,

+         do_koji_scratch_build=not no_koji_build)

+     for pr in prs:

+         print(pr)

  

  

  if __name__ == '__main__':

file modified
-5
@@ -1,5 +0,0 @@ 

- """

- """

- 

- from .local_repo import LocalRepo

- from .pagure_fork import PagureFork

file removed
-157
@@ -1,157 +0,0 @@ 

- """

- """

- 

- import subprocess

- import time

- 

- 

- class LocalRepoException(Exception):

- 

-     """Base exception class for LocalRepo.

-     """

- 

- 

- class LocalRepo(object):

- 

-     """

-     """

- 

-     def __init__(self, package_name, dirname, logger):

-         self.package_name = package_name

-         self.dirname = dirname

-         self.logger = logger

- 

-         self._specfile = None

- 

-     @property

-     def specfile(self):

-         if not self._specfile:

-             self._specfile, = self.dirname.glob('*.spec')

-         return self._specfile

- 

-     # Fedpkg commands.

-     def clone(self):

-         self.logger.debug(f'Cloning {self.package_name} into {self.dirname}')

-         try:

-             subprocess.check_output(

-                 ['fedpkg', 'clone', self.package_name, self.dirname],

-                 stderr=subprocess.STDOUT)

-         except subprocess.CalledProcessError as err:

-             if 'already exists and is not an empty directory' in str(err.output):

-                 # The repo was already cloned, so just reset it to master

-                 # and pull recent changes.

-                 subprocess.check_output(

-                     ['git', 'reset', '--hard', 'master'],

-                     cwd=self.dirname)

-                 subprocess.check_output(

-                     ['git', 'pull', '--rebase', 'origin', 'master'],

-                     cwd=self.dirname)

- 

-     def create_srpm(self):

-         subprocess.call(

-             ['fedpkg', '--release', 'master', 'srpm'],

-             cwd=self.dirname, stdout=subprocess.PIPE)

-         srpm_name = self.dirname.glob('*.src.rpm')

-         return srpm_name

- 

-     def run_mockbuild(self):

-         try:

-             output = subprocess.check_output(

-                 ['fedpkg', '--release', 'master', 'mockbuild'],

-                 cwd=self.dirname, stderr=subprocess.STDOUT)

-         except subprocess.CalledProcessError as err:

-             self.logger.error(

-                 f'Mock build did not pass for {self.package_name}. '

-                 f'Error: {err.output}')

-             raise err

-         return output

- 

-     def run_koji_scratch_build(self):

-         try:

-             output = subprocess.check_output(

-                 ['fedpkg', '--release', 'master', 'build', '--scratch', '--srpm'], cwd=self.dirname)

-         except subprocess.CalledProcessError as err:

-             self.logger.error(

-                 f'Failed to run Koji scratch build for {self.dirname}. '

-                 f'Error: {err.output}')

-             raise err

-         return output

- 

-     def bump_spec(self, comment):

-         subprocess.check_call(

-             ['rpmdev-bumpspec', '-c', comment, self.specfile])

- 

-     def apply_patch(self, patch_path):

-         subprocess.check_output(

-             ['git', 'apply', patch_path],

-             cwd=self.dirname, stderr=subprocess.STDOUT)

- 

-     # Git commands.

-     def git_last_commit(self):

-         output = subprocess.check_output(

-             ['git', 'log', '-1', '--pretty=format:%ct'], cwd=self.dirname)

-         return output

- 

-     def create_branch(self, branch_name):

-         subprocess.check_output(

-             ['git', 'checkout', '-B', branch_name],

-             cwd=self.dirname)

- 

-     def show_diff(self, filename):

-         subprocess.call(

-             ['git', '--no-pager', 'diff', filename.relative_to(self.dirname)],

-             cwd=self.dirname)

- 

-     def has_branches(self, branches):

-         pass

- 

-     def add_to_git_remotes(self, remote_name, url):

-         try:

-             subprocess.check_output(

-                 ['git', 'remote', 'add', remote_name, url],

-                 cwd=self.dirname,

-                 stderr=subprocess.STDOUT)

-         except subprocess.CalledProcessError as err:

-             if 'already exists' in str(err.output):

-                 self.logger.info('Fork already added to remotes')

-                 pass

-             else:

-                 raise err

-         except Exception as err:

-             raise LocalRepoException(f"Failed to add to git remotes. Error: {err}")

- 

-     def git_add(self, filename):

-         subprocess.check_output(

-             ['git', 'add', filename.relative_to(self.dirname)],

-             cwd=self.dirname, stderr=subprocess.STDOUT)

- 

-     def git_commit(self, message):

-         subprocess.check_output(

-             ['git', 'commit', '-m', message],

-             cwd=self.dirname, stderr=subprocess.STDOUT)

- 

-     def git_push(self, remote_name, branch_name):

-         """Push local changes to a branch of fork.

-         """

-         # On Pagure you can not immediately push to the fork.

-         # And there is no api call to check that the fork is ready.

-         # So here is a hack: try to do it at least 4 times with an interval

-         # in 3 minutes. Oh well.

-         for attempt in range(4):

-             try:

-                 self.logger.debug(f'Trying to push changes to fork (Attempt {attempt})')

-                 subprocess.check_output(

-                     ['git', 'push', '-f', remote_name, branch_name],

-                     cwd=self.dirname,

-                     stderr=subprocess.STDOUT)

-                 break

-             except subprocess.CalledProcessError as err:

-                 if 'DENIED by fallthru' in str(err.output):

-                     self.logger.debug('Will sleep for 3 minutes')

-                     time.sleep(60 * 3)

-             except Exception as err:

-                 raise LocalRepoException(

-                     f"Failed to push to a fork. Error: {err}")

-         else:

-             raise LocalRepoException('Could not push to fork, it is still not available')

-         self.logger.debug("Successfully pushed to a fork")

file modified
+5 -19
@@ -8,24 +8,6 @@ 

  class ModifierException(Exception):

      """Base Exception for RequiresModifier."""

  

- 

- class BaseModifier(object):

- 

-     """A showcase modification class.

-     """

- 

-     def __init__(self, logger):

-         self.logger = logger

- 

-     def run(self, spec):

-         """Modify a spec file and return a modified version.

- 

-         :param spec: (str) spec file as a string

-         :return: (str) modified spec

-         """

-         return spec

- 

- 

  # Good luck reading the rest of the file.

  

  # All the various if clauses used in spec files.
@@ -112,6 +94,7 @@ 

      '%if 0%{?fedora} >= 22%else',

      '%if 0%{?fedora} >= 28%else',

      '%if 0%{?fedora} > 27%else',

+     '%if 0%{?fedora} >= 27%else',

      '%if 0%{?fedora} || 0%{?rhel} > 7%else',

      '%if 0%{?with_explicit_python27}%else',

      '%if 0%{?fedora} >= 24%else',
@@ -146,6 +129,9 @@ 

      'python2-fedmsg-consumers': 'python2-fedmsg-consumers',

      'python2-fedmsg-commands': 'python2-fedmsg-commands',

      'python-pillow-qt': 'python2-pillow-qt',

+     'python2-gexiv2': 'python2-gexiv2',

+     'python2-pillow-devel': 'python2-pillow-devel',

+     'python-imaging-devel': 'python2-imaging-devel',

  }

  

  REPLACEMENT_MACROS = (
@@ -158,7 +144,7 @@ 

  )

  

  

- class RequiresModifier(BaseModifier):

+ class RequiresModifier(object):

  

      def __init__(self, logger):

          self.logger = logger

@@ -1,97 +0,0 @@ 

- """

- """

- 

- from libpagure import Pagure

- from libpagure.exceptions import APIError

- 

- 

- PAGURE_INSTANCE = 'https://src.fedoraproject.org'

- 

- 

- class PagureForkException(Exception):

- 

-     """Base exception for PagureFork.

-     """

- 

- 

- class PagureFork(object):

- 

-     """

-     """

- 

-     def __init__(self, pagure_repo_name, pagure_token,

-                  pagure_user, logger, instance=PAGURE_INSTANCE):

-         self.package_name = pagure_repo_name

-         self.pagure_user = pagure_user

-         self.logger = logger

- 

-         self.pagure = Pagure(

-             pagure_repository=pagure_repo_name,

-             instance_url=instance,

-             pagure_token=pagure_token

-         )

- 

-         self.fork_api = f"{self.pagure.instance}/api/0/fork"

- 

-     def do_fork(self):

-         self.logger.debug(f"Creating fork of {self.package_name} for user {self.pagure_user}")

-         try:

-             payload = {'wait': True, 'namespace': 'rpms', 'repo': self.package_name}

-             response = self.pagure._call_api(self.fork_api, method='POST', data=payload)

-             self.logger.debug(f"Fork created: {response}")

-         except APIError as err:

-             if 'already exists' in str(err):

-                 self.logger.info(

-                     f'User {self.pagure_user} already has a fork of {self.package_name}')

-             else:

-                 raise err

-         except Exception as err:

-             raise PagureForkException(

-                 f"Failed to create a fork for {self.package_name}. Error: {err}")

- 

-     def get_ssh_git_url(self):

-         """

-         """

-         git_urls_api = f"{self.fork_api}/{self.pagure_user}/rpms/{self.package_name}/git/urls"

-         return_value = self.pagure._call_api(git_urls_api)

-         return return_value['urls']['ssh']

- 

-     def create_pull_request(self, from_branch, to_branch,

-                             title, description,

-                             fas_user, fas_password):

-         """Pagure API does not allow this yet.

-         https://pagure.io/pagure/issue/2803

-         """

-         from .pagure_pr import create_pull_request

-         url = (

-             f'{self.pagure.instance}/login/?next={self.pagure.instance}'

-             f'/fork/{self.pagure_user}/rpms/{self.package_name}/diff/{to_branch}..{from_branch}')

-         try:

-             create_pull_request(

-                 url, title, description,

-                 fas_user, fas_password)

-         except Exception as err:

-             raise PagureForkException(

-                 f"Failed to create a PR for {self.package_name}. Error: {err}")

- 

-     def upstream_pr_with(self, **kwargs):

-         request_url = (

-             f'{self.pagure.instance}/api/0/rpms/{self.package_name}'

-             '/pull-requests?status=All')

-         return_value = self.pagure._call_api(request_url)

-         for request in return_value['requests']:

-             if any(value == request[key] for key, value in kwargs.items()):

-                 return request

- 

-     def has_upstream_pr_with(self, **kwargs):

-         request = self.upstream_pr_with(**kwargs)

-         if request:

-             return f'{self.pagure.instance}/rpms/{self.package_name}/pull-request/{request["id"]}'

- 

-     def merge_upstream_pr(self, pr_id):

-         try:

-             self.pagure.merge_request(pr_id)

-             self.logger.debug(f"PR merged")

-         except Exception as err:

-             raise PagureForkException(

-                 f"Failed to merge a PR {pr_id}. Error: {err}")

file removed
-119
@@ -1,119 +0,0 @@ 

- from selenium import webdriver

- from selenium.webdriver.common.by import By

- from selenium.webdriver.support.ui import WebDriverWait

- from selenium.webdriver.support import expected_conditions as EC

- 

- 

- def create_pull_request(url, title, description,

-                         fas_user, fas_password):

-     """Create a pull request from a fork to upstream.

- 

-     Only if not created yet.

-     Pagure API does not allow this yet, so selenium it is.

-     """

-     driver = webdriver.Firefox()

-     driver.get(url)

- 

-     WebDriverWait(driver, 30).until(

-         EC.presence_of_element_located((By.CLASS_NAME, 'bodycontent'))

-     )

-     # Sometimes when the user is "kinited", the login page does not open,

-     # and you go directly to the PR page.

-     if driver.title == 'Login':

-         if not fas_user or not fas_password:

-             raise Exception('Please provide both FAS username and password')

- 

-         login_elem = driver.find_element_by_name('login_name')

-         login_elem.clear()

-         login_elem.send_keys(fas_user)

- 

-         password_elem = driver.find_element_by_name('login_password')

-         password_elem.clear()

-         password_elem.send_keys(fas_password)

- 

-         login_button = driver.find_element_by_id('loginbutton')

-         login_button.click()

- 

-     WebDriverWait(driver, 30).until(

-         EC.presence_of_element_located((By.NAME, 'title'))

-     )

- 

-     pr_title = driver.find_element_by_name('title')

-     pr_title_value = pr_title.get_attribute('value')

- 

-     if pr_title_value != title:

-         # This means the PR either contains more commits or is wrong.

-         # Needs to be checked manually.

-         raise Exception(

-             'Opening the PR did not go well. '

-             f'The PR title: {pr_title_value}')

- 

-     pr_init_comment = driver.find_element_by_id('initial_comment')

-     pr_init_comment.clear()

-     pr_init_comment.send_keys(description)

- 

-     create_button = driver.find_element_by_xpath(

-         "//input[@type='submit'][@value='Create']")

-     # import ipdb; ipdb.set_trace()

-     create_button.click()

- 

-     # TODO: check the page if it was a success.

-     driver.close()

- 

- 

- def merge_pull_request(url, fas_user, fas_password):

-     driver = webdriver.Firefox()

-     driver.get(url)

- 

-     WebDriverWait(driver, 30).until(

-         EC.presence_of_element_located((By.CLASS_NAME, 'bodycontent'))

-     )

-     # Sometimes when the user is "kinited", the login page does not open,

-     # and you go directly to the PR page.

-     login = driver.find_elements_by_xpath("//a[contains(text(), 'Log In')]")

-     if login:

-         login_url = login[0].get_attribute('href')

-         driver.get(login_url)

- 

-     WebDriverWait(driver, 30).until(

-         EC.presence_of_element_located((By.CLASS_NAME, 'bodycontent'))

-     )

- 

-     if driver.title == 'Login':

-         if not fas_user or not fas_password:

-             raise Exception('Please provide both FAS username and password')

- 

-         login_elem = driver.find_element_by_name('login_name')

-         login_elem.clear()

-         login_elem.send_keys(fas_user)

- 

-         password_elem = driver.find_element_by_name('login_password')

-         password_elem.clear()

-         password_elem.send_keys(fas_password)

- 

-         login_button = driver.find_element_by_id('loginbutton')

-         login_button.click()

- 

-     WebDriverWait(driver, 30).until(

-         EC.element_to_be_clickable((By.ID, 'merge_btn'))

-     )

- 

-     pr_message = driver.find_element_by_id('merge-alert-message')

-     if pr_message.text != 'The pull-request can be merged and fast-forwarded':

-         raise Exception(

-             'Failed to merge: conflicts maybe')

- 

-     merge_button = driver.find_element_by_id("merge_btn")

-     # import ipdb; ipdb.set_trace()

-     merge_button.click()

- 

-     WebDriverWait(driver, 30).until(EC.alert_is_present(), 'Waiting for alert timed out')

- 

-     alert = driver.switch_to_alert()

-     alert.accept()

-     WebDriverWait(driver, 30).until(

-         EC.presence_of_element_located((By.CLASS_NAME, 'bodycontent'))

-     )

- 

-     # TODO: check the page if it was a success.

-     driver.close()

file modified
+2 -2
@@ -1,8 +1,8 @@ 

  """

  """

  

- from .build_mock import test_in_mock

- from .build_koji import test_in_koji

+ from .build_mock import check_mock_build

+ from .build_koji import check_koji_build

  

  # def do_tests(package_repo, mock_build=True, koji_build=True):

  #     """Test the modified spec file:

file modified
+1 -5
@@ -6,11 +6,7 @@ 

  TASK_INFO_RE = r'Task info: (https://koji.fedoraproject.org/koji/taskinfo\?taskID=\d+)\\n'

  

  

- def test_in_koji(package_repo, logger):

-     logger.debug('Running a koji build')

-     output = package_repo.run_koji_scratch_build()

-     logger.debug(f'Koji scratch build completed. Output: {output}')

- 

+ def check_koji_build(output):

      if 'completed successfully' in str(output):

          # Yay Koji build completed. Find a link to a task.

          koji_scratch_build = re.search(TASK_INFO_RE, str(output)).group(1)

file modified
+1 -9
@@ -24,7 +24,7 @@ 

          name == 'python')

  

  

- def check_mockbuild_result(output, dnf_info):

+ def check_mock_build(output, dnf_info):

      # Check that no rpms require python-smth.

      result_dir_re = r'Results and/or logs in: (\S*)'

      result_dir_path = re.search(result_dir_re, output.decode('utf-8')).group(1)
@@ -40,11 +40,3 @@ 

                  if dnf_info and not dnf_info.requires_py2(require.decode('utf-8')):

                      return

                  raise Exception(f'{require} is still not versioned')

- 

- 

- def test_in_mock(package_repo, logger, check_requires=True, dnf_info=None):

-     logger.debug('Running mock build')

-     output = package_repo.run_mockbuild()

-     if check_requires:

-         logger.debug('Mock build completed. Checking resulting rpms')

-         check_mockbuild_result(output, dnf_info)

file modified
+1 -2
@@ -1,3 +1,2 @@ 

  click

- selenium

- libpagure

+ mass-pagure-prs