From fa00835328c6f220dfa6f7b7e4ca29c50b1952da Mon Sep 17 00:00:00 2001 From: Iryna Shcherbina Date: Jun 04 2018 15:30:17 +0000 Subject: [PATCH 1/2] Update modifier --- diff --git a/fixrequires/modifier.py b/fixrequires/modifier.py index 4f68721..60922de 100644 --- a/fixrequires/modifier.py +++ b/fixrequires/modifier.py @@ -112,6 +112,7 @@ F_IGNORE_ELSE_CLAUSES = ( '%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 +147,9 @@ SPECIAL = { '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 = ( From 23ca3fab48cb667de7e6203407c4a55ce0d5a37e Mon Sep 17 00:00:00 2001 From: Iryna Shcherbina Date: Jun 04 2018 15:31:59 +0000 Subject: [PATCH 2/2] Use move possibly reusable code into mass-pagure-prs --- diff --git a/fix_requires.py b/fix_requires.py index fa07b37..79f2b0e 100755 --- a/fix_requires.py +++ b/fix_requires.py @@ -27,14 +27,14 @@ Notes: # 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 @@ PR_DESCRIPTION = ( ) -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 @@ def main(packages, dirname, no_mock_build, no_koji_build, pagure, 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__': diff --git a/fixrequires/__init__.py b/fixrequires/__init__.py index ccf6c13..e69de29 100644 --- a/fixrequires/__init__.py +++ b/fixrequires/__init__.py @@ -1,5 +0,0 @@ -""" -""" - -from .local_repo import LocalRepo -from .pagure_fork import PagureFork diff --git a/fixrequires/local_repo.py b/fixrequires/local_repo.py deleted file mode 100644 index 636e1a7..0000000 --- a/fixrequires/local_repo.py +++ /dev/null @@ -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") diff --git a/fixrequires/modifier.py b/fixrequires/modifier.py index 60922de..e4d6caf 100644 --- a/fixrequires/modifier.py +++ b/fixrequires/modifier.py @@ -8,24 +8,6 @@ from .dnf_info import DNFInfo 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. @@ -162,7 +144,7 @@ REPLACEMENT_MACROS = ( ) -class RequiresModifier(BaseModifier): +class RequiresModifier(object): def __init__(self, logger): self.logger = logger diff --git a/fixrequires/pagure_fork.py b/fixrequires/pagure_fork.py deleted file mode 100644 index a637757..0000000 --- a/fixrequires/pagure_fork.py +++ /dev/null @@ -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}") diff --git a/fixrequires/pagure_pr.py b/fixrequires/pagure_pr.py deleted file mode 100644 index d72bb2a..0000000 --- a/fixrequires/pagure_pr.py +++ /dev/null @@ -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() diff --git a/qa/__init__.py b/qa/__init__.py index cd51feb..ea4d7a3 100644 --- a/qa/__init__.py +++ b/qa/__init__.py @@ -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: diff --git a/qa/build_koji.py b/qa/build_koji.py index 14fb801..5172b6b 100644 --- a/qa/build_koji.py +++ b/qa/build_koji.py @@ -6,11 +6,7 @@ import re 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) diff --git a/qa/build_mock.py b/qa/build_mock.py index d8661e7..bea8bc1 100644 --- a/qa/build_mock.py +++ b/qa/build_mock.py @@ -24,7 +24,7 @@ def is_unversioned(name): 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 @@ def check_mockbuild_result(output, dnf_info): 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) diff --git a/requirements.txt b/requirements.txt index c725a66..dd3ab04 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,3 +1,2 @@ click -selenium -libpagure +mass-pagure-prs