PR#16: Adding package.cfg file to epel branches - fedscm-admin

fedscm-admin

#16 Adding package.cfg file to epel branches

Merged 4 years ago by mohanboddu. Opened 4 years ago by mohanboddu.

mohanboddu/fedscm-admin epel-pkg-cfg into master

Adding package.cfg file to epel branches

Mohan Boddu • 4 years ago

7194a46

fedscm_admin/git.py

file modified

+44 -1

		`@@ -20,7 +20,7 @@`
		`import tempfile`
		`import shutil`
		`import os`
		`-`
		`+ import sys`
		`import yaml`
		`import click`

		`@@ -220,6 +220,16 @@`
		`push_cmd = ['git', 'push']`
		`self._run_git_cmd(push_cmd)`

		`+ def fetch(self):`
		`+ """`
		`+ Fetches from the Git server`
		`+ :return: None or GitException`
		`+ """`
		`+ self._assert_cloned()`
		`+ click.echo('- Fetching from {0}'.format(self.git_url))`
		`+ push_cmd = ['git', 'fetch']`
		`+ self._run_git_cmd(push_cmd)`
		`+`
		`def set_monitoring_on_repo(self, namespace, repo, monitoring_level):`
		`"""`
		`Set the monitoring level of a repo in config.yml`
		`@@ -256,3 +266,36 @@`
		`self.add(config_yml_path)`
		`self.commit('Adding monitoring for {0}/{1}'.format(namespace, repo))`
		`self.push()`
		`+`
		`+ def create_epel_package_cfg(self, namespace, repo, branch):`
		`+ """`
		`+ Create package.cfg file in epel branch`
		`+ :param namespace: the dist-git namespace of the package in question.`
		`+ :param repo: the name of the dist-git repo in question.`
		`+ :param branch: the name of the branch`
		`+ :return: None or GitException`
		`+ """`
		`+ self._assert_cloned()`
		`+`
		`+ if self.current_branch != branch:`
		`+ self.fetch()`
		`+ self.checkout_branch(branch)`
		`+ try:`
		`+ # Open the package.cfg file in the repo`
		`+ # Write the config needed with epel\d and epel\d-playground`
		`+ # build targets`
		`+ # Close the file`
		`+ # Add the file, commit the changes and push`
		`+ with open(os.path.join(self.clone_dir, 'package.cfg'), 'w') as pkg_cfg:`
		`+ pkg_cfg.write("[koji]\n")`
		`+ pkg_cfg.write("targets = {0} {0}-playground".format(branch))`
		`+ pkg_cfg.close()`
		`+ self.add(pkg_cfg.name)`
		`+ self.commit("Adding package.cfg file")`
		`+ self.push()`
		`+ except IOError:`
		`+ print("Couldn't create package.cfg file in {0} of {1}/{2} repo, "`
		`+ "please file a releng ticket".format(branch, namespace, repo))`
		`+ sys.exit(1)`
		`+ finally:`
		`+ pkg_cfg.close()`

fedscm_admin/pagure.py

file modified

+9 -3

		`@@ -140,9 +140,15 @@`
		`url = rv_json['urls'].get(url_type)`

		`# Insert the username if relevant.`
		`- if username is not None and url.startswith('ssh://') and '@' not in url:`
		`- url = 'ssh://{username}@{rest}'.format(`
		`- username=username, rest=url[6:])`
		`+ if username is not None and url.startswith('ssh://'):`
		`+ # Use the FAS username of the person who is processing the tickets.`
		`+ # username from git url returns '{username}'`
		`+ # For ex: https://src.fedoraproject.org/api/0/rpms/fedora-repos/git/urls`
		`+ if '{username}' in url:`
		`+ url = url.replace('{username}', username)`
		`+ if '@' not in url:`
		`+ url = 'ssh://{username}@{rest}'.format(`
		`+ username=username, rest=url[6:])`
		`return url`

fedscm_admin/utils.py

file modified

+10 -3

		`@@ -768,10 +768,11 @@`
		`fedscm_admin.pagure.new_branch(`
		`namespace, repo, branch, from_branch='master')`
		`else:`
		`- # Use the git url and not ssh since we only need read-only access. The`
		`- # new branch will be created using the Pagure API and not git directly.`
		`+ # Even though the branches are created using pagure api which dont`
		`+ # require ssh, but for epel\d branches we add package.cfg file.`
		`+ # This should be pushed using ssh.`
		`git_url = fedscm_admin.pagure.get_project_git_url(`
		`- namespace, repo, url_type='git',`
		`+ namespace, repo, url_type='ssh',`
		`username=FAS_CLIENT.client.username)`
		`git_obj = fedscm_admin.git.GitRepo(git_url)`
		`git_obj.clone_repo()`
		`@@ -780,6 +781,12 @@`
		`'git branch can\'t be created.')`
		`fedscm_admin.pagure.new_branch(`
		`namespace, repo, branch, from_commit=git_obj.first_commit)`
		`+ # If epel\d branch, then create package.cfg file in that branch`
		`+ # Remove the check for epel version >= 7 when playground is enabled`
		`+ # for epel7`
		`+ if (bool(re.match(r'^(?:epel)\d+$', branch)) and`
		`+ int(''.join([i for i in branch if re.match(r'\d', i)])) >= 7):`
		`+ git_obj.create_epel_package_cfg(namespace, repo, branch)`


		`def ticket_requires_approval(issue_type, issue):`

setup.py

file modified

+1 -1

		`@@ -20,5 +20,5 @@`
		`packages=['fedscm_admin'],`
		`package_dir={'fedscm_admin': 'fedscm_admin'},`
		`url='https://pagure.io/fedscm_admin',`
		`- version='1.0.7',`
		`+ version='1.0.8',`
		`)`

mohanboddu commented 4 years ago

Signed-off-by: Mohan Boddu mboddu@bhujji.com

mohanboddu commented 4 years ago

@kevin @smooge @sgallagh

rebased onto 377523059812de880568cc4356b2d5319741f2d6

4 years ago

sgallagh commented on line 12 of fedscm_admin/git.py 4 years ago

This variable name strikes me as confusing. Should it be fetch_cmd instead?

sgallagh commented on line 49 of fedscm_admin/git.py 4 years ago

Shouldn't you catch the possible IOError here from the open() and write() calls?

sgallagh commented on line 44 of fedscm_admin/git.py 4 years ago

I think this is just supposed to be {0}-playground, not {0}-playground-candidate. This is essentially a branch name, not a target name. I'm not sure where fedpkg gets the association between branch and koji target, but it does it. (See for example that https://src.fedoraproject.org/rpms/libuv/blob/1/f/package.cfg builds for f29+ and epel7 by branch name, not by Koji target).

sgallagh commented on line 21 of fedscm_admin/utils.py 4 years ago

Do we want to be creating it for epel7?

smooge commented 4 years ago

Do we want to be creating it for epel7?

sgallagh this was meant originally for epel7 but added to epel8 to get it there before we got so far down the garden lane as we are in epel7. [This has been on the drawing books in one form or another before epel7.]

sgallagh commented 4 years ago

Do we want to be creating it for epel7?

sgallagh this was meant originally for epel7 but added to epel8 to get it there before we got so far down the garden lane as we are in epel7. [This has been on the drawing books in one form or another before epel7.]

Right, but are we committing to implementing this in EPEL 7 right now? Because dropping this file into new epel7 branches will probably cause chaos if we aren't also producing epel7-playground branches...

smooge commented 4 years ago

Ah ok. yeah.. that would cause problems. This needs to be conditionalized for epel8 only for the time being.

rebased onto 954b3ccc6d29080656c8567531e804df34344d2d

4 years ago

mohanboddu commented 4 years ago

This variable name strikes me as confusing. Should it be fetch_cmd instead?

I just named it as per the previous one's as there are push, commit, add....

I think this is just supposed to be {0}-playground, not {0}-playground-candidate. This is essentially a branch name, not a target name. I'm not sure where fedpkg gets the association between branch and koji target, but it does it. (See for example that https://src.fedoraproject.org/rpms/libuv/blob/1/f/package.cfg builds for f29+ and epel7 by branch name, not by Koji target).

It uses the target, as there is fxx build target as well. I can add epel8-playground target as well, but we need epel8-playground-candidate target as fedpkg uses that to identify which target it has to use if target is not provided in fedpkg build command.
https://pagure.io/fedpkg/blob/master/f/fedpkg/__init__.py#_179

Shouldn't you catch the possible IOError here from the open() and write() calls?

Fixed

Do we want to be creating it for epel7?

Fixed

Edited 4 years ago by mohanboddu

rebased onto 11589a36287a91a4fcdf79146044ae238040d107

4 years ago

sgallagh commented on line 51 of fedscm_admin/git.py 4 years ago

Hmm, I think we probably ought to fail outright here, ideally filing the releng ticket automatically.

Continuing and letting the user proceed could lead to things getting out of sync pretty fast.

mohanboddu commented 4 years ago

Hmm, I think we probably ought to fail outright here, ideally filing the releng ticket automatically.
Continuing and letting the user proceed could lead to things getting out of sync pretty fast.

I thought of it, ideally it should file the releng ticket and close the fedscm ticket that it was processing. I thought I will leave it for future changes as we are running short on time :disappointed:

rebased onto 5bea172513ef8375bf3a82cff8c3e27c218f4a2e

4 years ago

rebased onto 7194a46

4 years ago

Pull-Request has been merged by mohanboddu

4 years ago