[copr] master: [backend][frontend] removing code related to multiply source rpms in build. Build.pkgs now expected to have exactly one src.rpm. (55bcab9)
by vgologuz@fedoraproject.org
Repository : http://git.fedorahosted.org/cgit/copr.git
On branch : master
>---------------------------------------------------------------
commit 55bcab972af332461351f04abac17356a6c4789b
Author: Valentin Gologuzov <vgologuz(a)redhat.com>
Date: Mon Feb 16 17:55:36 2015 +0100
[backend][frontend] removing code related to multiply source rpms in build. Build.pkgs now expected to have exactly one src.rpm.
>---------------------------------------------------------------
backend/backend/actions.py | 27 ++++++++-----------
backend/backend/job.py | 2 +-
backend/tests/test_action.py | 8 ++---
.../coprs_frontend/coprs/logic/builds_logic.py | 17 +++++++-----
frontend/coprs_frontend/coprs/models.py | 2 +-
.../coprs/templates/coprs/detail/build.html | 4 +--
.../tests/test_logic/test_builds_logic.py | 22 +++++++++++++---
7 files changed, 45 insertions(+), 37 deletions(-)
diff --git a/backend/backend/actions.py b/backend/backend/actions.py
index f1d4557..6b82ebe 100644
--- a/backend/backend/actions.py
+++ b/backend/backend/actions.py
@@ -106,12 +106,10 @@ class Action(object):
projectname = ext_data["projectname"]
chroots_requested = set(ext_data["chroots"])
- packages = [os.path.basename(x).replace(".src.rpm", "")
- for x in ext_data["pkgs"].split()]
-
+ package_name = os.path.basename(ext_data["pkgs"]).replace(".src.rpm", "")
path = os.path.join(self.destdir, project)
- self.add_event("Packages to delete {0}".format(' '.join(packages)))
+ self.add_event("Deleting package {0}".format(package_name))
self.add_event("Copr path {0}".format(path))
try:
@@ -122,24 +120,21 @@ class Action(object):
chroots_to_do = chroot_list.intersection(chroots_requested)
if not chroots_to_do:
- self.add_event("Nothing to delete for delete action: packages {}, {}"
- .format(packages, ext_data))
+ self.add_event("Nothing to delete for delete action: package {}, {}"
+ .format(package_name, ext_data))
return
for chroot in chroots_to_do:
self.add_event("In chroot {0}".format(chroot))
altered = False
- for pkg in packages:
- pkg_path = os.path.join(path, chroot, pkg)
- if os.path.isdir(pkg_path):
- self.add_event("Removing build {0}".format(pkg_path))
- shutil.rmtree(pkg_path)
- altered = True
- else:
- self.add_event(
- "Package {0} dir not found in chroot {1}"
- .format(pkg, chroot))
+ pkg_path = os.path.join(path, chroot, package_name)
+ if os.path.isdir(pkg_path):
+ self.add_event("Removing build {0}".format(pkg_path))
+ shutil.rmtree(pkg_path)
+ altered = True
+ else:
+ self.add_event("Package {0} dir not found in chroot {1}".format(package_name, chroot))
if altered:
self.add_event("Running createrepo")
diff --git a/backend/backend/job.py b/backend/backend/job.py
index 232ec09..5f83ee1 100644
--- a/backend/backend/job.py
+++ b/backend/backend/job.py
@@ -47,7 +47,7 @@ class BuildJob(object):
key = str(key)
setattr(self, key, val)
- self.pkg = task_data["pkgs"].split(" ")[0] # just for now
+ self.pkg = task_data["pkgs"]
del self.pkgs # better to produce error, than use it blindly
self.repos = [r for r in task_data["repos"].split(" ") if r.strip()]
diff --git a/backend/tests/test_action.py b/backend/tests/test_action.py
index 3e7ce91..74a8e2b 100644
--- a/backend/tests/test_action.py
+++ b/backend/tests/test_action.py
@@ -55,8 +55,8 @@ class TestAction(object):
self.test_time = time.time()
- self.pkgs = ["foo.src.rpm", "bar.src.rpm"]
- self.pkgs_stripped = ["foo", "bar"]
+ self.pkgs = ["foo.src.rpm"]
+ self.pkgs_stripped = ["foo"]
self.ext_data_for_delete_build = json.dumps({
"pkgs": " ".join(self.pkgs),
@@ -385,20 +385,18 @@ class TestAction(object):
assert_what_from_queue(self.test_q, msg_list=[
"Action delete build",
- "Packages to delete " + " ".join(self.pkgs_stripped),
+ "Deleting package " + " ".join(self.pkgs_stripped),
"Copr path",
])
def assert_epel7():
assert_what_from_queue(self.test_q, msg_list=[
"Package foo dir not found in chroot epel7",
- "Package bar dir not found in chroot epel7",
])
def assert_fedora20():
assert_what_from_queue(self.test_q, msg_list=[
"Removing build ",
- "Package bar dir not found in chroot fedora20",
"Running createrepo",
])
diff --git a/frontend/coprs_frontend/coprs/logic/builds_logic.py b/frontend/coprs_frontend/coprs/logic/builds_logic.py
index 8283d5e..0e839b3 100644
--- a/frontend/coprs_frontend/coprs/logic/builds_logic.py
+++ b/frontend/coprs_frontend/coprs/logic/builds_logic.py
@@ -135,6 +135,10 @@ class BuildsLogic(object):
if not repos:
repos = copr.repos
+ if " " in pkgs or "\n" in pkgs or "\t" in pkgs or pkgs.strip() != pkgs:
+ raise exceptions.MalformedArgumentException("Trying to create a build using src_pkg "
+ "with bad characters. Forgot to split?")
+
build = models.Build(
user=user,
pkgs=pkgs,
@@ -301,14 +305,13 @@ class BuildsMonitorLogic(object):
for build in builds:
chroot_results = {chroot.name: chroot.state for chroot in build.build_chroots}
- for pkg_url in build.pkgs.split():
- pkg = os.path.basename(pkg_url)
- pkg_name = helpers.parse_package_name(pkg)
+ pkg = os.path.basename(build.pkgs)
+ pkg_name = helpers.parse_package_name(pkg)
- for chroot_name, state in chroot_results.items():
- # set only latest version/state
- if build_result_by_pkg_chroot[pkg_name][chroot_name] is None:
- build_result_by_pkg_chroot[pkg_name][chroot_name] = (build.id, build.pkg_version, state)
+ for chroot_name, state in chroot_results.items():
+ # set only latest version/state
+ if build_result_by_pkg_chroot[pkg_name][chroot_name] is None:
+ build_result_by_pkg_chroot[pkg_name][chroot_name] = (build.id, build.pkg_version, state)
# "transpose" data to present build status per package
packages = []
diff --git a/frontend/coprs_frontend/coprs/models.py b/frontend/coprs_frontend/coprs/models.py
index e2d48c6..ef8ad0e 100644
--- a/frontend/coprs_frontend/coprs/models.py
+++ b/frontend/coprs_frontend/coprs/models.py
@@ -258,7 +258,7 @@ class Build(db.Model, helpers.Serializer):
"""
id = db.Column(db.Integer, primary_key=True)
- # list of space separated urls of packages to build
+ # single url to the source rpm, should not contain " ", "\n", "\t"
pkgs = db.Column(db.Text)
# built packages
built_packages = db.Column(db.Text)
diff --git a/frontend/coprs_frontend/coprs/templates/coprs/detail/build.html b/frontend/coprs_frontend/coprs/templates/coprs/detail/build.html
index 9dc0bb9..3a720da 100644
--- a/frontend/coprs_frontend/coprs/templates/coprs/detail/build.html
+++ b/frontend/coprs_frontend/coprs/templates/coprs/detail/build.html
@@ -108,9 +108,7 @@
<dt> Source: </dt>
<dd>
{% if build.pkgs is not none %}
- {% for pkg in build.pkgs.split() %}
- <dd> <a href="{{ pkg }}">{{ pkg }}</a></dd>
- {% endfor %}
+ <dd> <a href="{{ pkg }}">{{ build.pkgs }}</a></dd>
{% endif %}
</dd>
</div>
diff --git a/frontend/coprs_frontend/tests/test_logic/test_builds_logic.py b/frontend/coprs_frontend/tests/test_logic/test_builds_logic.py
index 055978a..e8a375e 100644
--- a/frontend/coprs_frontend/tests/test_logic/test_builds_logic.py
+++ b/frontend/coprs_frontend/tests/test_logic/test_builds_logic.py
@@ -6,7 +6,7 @@ import time
from sqlalchemy.orm.exc import NoResultFound
from coprs import helpers
-from coprs.exceptions import ActionInProgressException, InsufficientRightsException
+from coprs.exceptions import ActionInProgressException, InsufficientRightsException, MalformedArgumentException
from coprs.logic.actions_logic import ActionsLogic
from coprs.logic.builds_logic import BuildsLogic
from coprs.logic.builds_logic import BuildsMonitorLogic
@@ -21,7 +21,7 @@ class TestBuildsLogic(CoprsTestCase):
self.mc2.is_active = False
self.db.session.commit()
- b = BuildsLogic.add(self.u2, "blah blah", self.c2)
+ b = BuildsLogic.add(self.u2, "blah", self.c2)
self.db.session.commit()
assert b.chroots[0].name == self.mc3.name
@@ -29,7 +29,7 @@ class TestBuildsLogic(CoprsTestCase):
f_actions, f_db):
with pytest.raises(ActionInProgressException):
- b = BuildsLogic.add(self.u1, "blah blah", self.c1)
+ b = BuildsLogic.add(self.u1, "blah", self.c1)
self.db.session.rollback()
def test_add_assigns_params_correctly(self, f_users, f_coprs,
@@ -37,7 +37,7 @@ class TestBuildsLogic(CoprsTestCase):
params = dict(
user=self.u1,
- pkgs="blah blah",
+ pkgs="blah",
copr=self.c1,
repos="repos",
memory_reqs=3000,
@@ -47,6 +47,20 @@ class TestBuildsLogic(CoprsTestCase):
for k, v in params.items():
assert getattr(b, k) == v
+ def test_add_error_on_multiply_src(self, f_users, f_coprs,
+ f_mock_chroots, f_db):
+
+ params = dict(
+ user=self.u1,
+ pkgs="blah blah",
+ copr=self.c1,
+ repos="repos",
+ memory_reqs=3000,
+ timeout=5000)
+
+ with pytest.raises(MalformedArgumentException):
+ BuildsLogic.add(**params)
+
def test_monitor_logic(self, f_users, f_coprs, f_mock_chroots_many, f_build_many_chroots, f_db):
copr = self.c1
md = BuildsMonitorLogic.get_monitor_data(copr)