Merge lp:~mvo/click/multiple-frameworks into lp:click

Proposed by Michael Vogt on 2014-05-05
Status: Merged
Approved by: Colin Watson on 2014-05-08
Approved revision: 420
Merged at revision: 421
Proposed branch: lp:~mvo/click/multiple-frameworks
Merge into: lp:click
Diff against target: 370 lines (+221/-59)
6 files modified
click/build.py (+10/-22)
click/framework.py (+135/-0)
click/install.py (+8/-28)
click/tests/helpers.py (+27/-0)
click/tests/test_build.py (+39/-5)
click/tests/test_install.py (+2/-4)
To merge this branch: bzr merge lp:~mvo/click/multiple-frameworks
Reviewer Review Type Date Requested Status
Colin Watson 2014-05-05 Approve on 2014-05-08
Review via email: mp+218280@code.launchpad.net

Description of the change

This branch adds support for multiple frameworks in a click manifest.

To support multiple frameworks there is now a "click/framework.py" file that contains a new "validate_framework()" method that is used by build.py and install.py to validate the given framework string.

validate_framework() will ensure that:
- the base version is the same for all frameworks used
- no additional relations like >= or similar is used in the framework declaration string
- no or dependencies (foo|bar) is used in the framework declaration string

It is possible to override if missing_frameworks is a error or a warning. Logging is used when this is overriden. I'm happy to use a different mechanism if logging is inappropriate.

The test helper gets a new click.test.helpers.TestCase._create_mock_framework_{dir,file} that is used in both test_build.py and test_install.py

I currently use the pure-python version of click_framework_get_base_version() - we could use the native version when available but I wonder if that is really needed given that both build and install are already in python so the overhead of using the python version seems to be not a issue here.

If there is anything else you would like me to change or do differently, please just let me know, happy to do it.

Thanks!
 Michael

To post a comment you must log in.
Colin Watson (cjwatson) wrote :

We ought to avoid exposing the CLICK_FRAMEWORKS_DIR environment variable interface when it's only for tests; tests should be able to install mock functions as appropriate to override the frameworks directory. And as you say the default framework directory should be generated by configure - I suspect that undoing the change to click/paths.py.in from r372.1.1 would be enough for this.

However, this doesn't need to block merging, and you can clean it up at your leisure. Other than that I see nothing to complain about here. Thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'click/build.py'
--- click/build.py 2014-01-23 17:19:09 +0000
+++ click/build.py 2014-05-05 13:23:45 +0000
@@ -50,6 +50,11 @@
50from click.preinst import static_preinst50from click.preinst import static_preinst
51from click.versions import spec_version51from click.versions import spec_version
5252
53from click.framework import (
54 validate_framework,
55 ClickFrameworkInvalid,
56)
57
5358
54@contextlib.contextmanager59@contextlib.contextmanager
55def make_temp_dir():60def make_temp_dir():
@@ -196,30 +201,13 @@
196 package.add_file("control.tar.gz", control_tar_path)201 package.add_file("control.tar.gz", control_tar_path)
197 package.add_file("data.tar.gz", data_tar_path)202 package.add_file("data.tar.gz", data_tar_path)
198203
199 def _validate_framework(self, framework):204 def _validate_framework(self, framework_string):
200 """Apply policy checks to framework declarations."""205 """Apply policy checks to framework declarations."""
201 try:206 try:
202 apt_pkg207 validate_framework(
203 except NameError:208 framework_string, ignore_missing_frameworks=True)
204 return209 except ClickFrameworkInvalid as e:
205210 raise ClickBuildError(str(e))
206 try:
207 parsed_framework = apt_pkg.parse_depends(framework)
208 except ValueError:
209 raise ClickBuildError('Could not parse framework "%s"' % framework)
210 if len(parsed_framework) > 1:
211 raise ClickBuildError(
212 'Multiple dependencies in framework "%s" not yet allowed' %
213 framework)
214 for or_dep in parsed_framework:
215 if len(or_dep) > 1:
216 raise ClickBuildError(
217 'Alternative dependencies in framework "%s" not yet '
218 'allowed' % framework)
219 if or_dep[0][1] or or_dep[0][2]:
220 raise ClickBuildError(
221 'Version relationship in framework "%s" not yet allowed' %
222 framework)
223211
224 def build(self, dest_dir, manifest_path="manifest.json"):212 def build(self, dest_dir, manifest_path="manifest.json"):
225 with make_temp_dir() as temp_dir:213 with make_temp_dir() as temp_dir:
226214
=== added file 'click/framework.py'
--- click/framework.py 1970-01-01 00:00:00 +0000
+++ click/framework.py 2014-05-05 13:23:45 +0000
@@ -0,0 +1,135 @@
1# Copyright (C) 2014 Canonical Ltd.
2# Author: Michael Vogt <michael.vogt@canonical.com>
3
4# This program is free software: you can redistribute it and/or modify
5# it under the terms of the GNU General Public License as published by
6# the Free Software Foundation; version 3 of the License.
7#
8# This program is distributed in the hope that it will be useful,
9# but WITHOUT ANY WARRANTY; without even the implied warranty of
10# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
11# GNU General Public License for more details.
12#
13# You should have received a copy of the GNU General Public License
14# along with this program. If not, see <http://www.gnu.org/licenses/>.
15
16"""Pure python click framework handling support."""
17
18import logging
19import os
20import re
21
22try:
23 import apt_pkg
24except:
25 pass
26
27
28class ClickFrameworkInvalid(Exception):
29 pass
30
31
32# FIXME: use native lib if available
33#from gi.repository import Click
34#click_framework_get_base_version = Click.framework_get_base_version
35#click_framework_has_framework = Click.has_framework
36
37
38# python version of the vala parse_deb822_file()
39def parse_deb822_file(filename):
40 data = {}
41 with open(filename) as f:
42 for line in f:
43 line = line.strip()
44 # from deb822.vala
45 field_re_posix = r'^([^:[:space:]]+)[[:space:]]*:[[:space:]]'\
46 '([^[:space:]].*?)[[:space:]]*$'
47 # python does not do posix char classes
48 field_re = field_re_posix.replace("[:space:]", "\s")
49 blank_re_posix = r'^[[:space:]]*$'
50 blank_re = blank_re_posix.replace("[:space:]", "\s")
51 if re.match(blank_re, line):
52 break
53 match = re.match(field_re, line)
54 if match and match.group(1) and match.group(2):
55 data[match.group(1).lower()] = match.group(2)
56 return data
57
58
59# python version of vala get_frameworks_dir
60def get_frameworks_dir(framework_name):
61 # FIXME: get via configure.in etc?
62 frameworks_dir = os.environ.get(
63 "CLICK_FRAMEWORKS_DIR", "/usr/share/click/frameworks")
64 framework_path = os.path.join(
65 frameworks_dir, framework_name+".framework")
66 return framework_path
67
68
69# python version of the vala click_framework_get_base_version()
70def click_framework_get_base_version(framework_name):
71 deb822 = parse_deb822_file(get_frameworks_dir(framework_name))
72 return deb822.get("base-version", None)
73
74
75# python version of the vala click_framework_has_framework
76def click_framework_has_framework(framework_name):
77 return os.path.exists(get_frameworks_dir(framework_name))
78
79
80def validate_framework(framework_string, ignore_missing_frameworks=False):
81 try:
82 apt_pkg
83 except NameError:
84 logging.warning("No apt_pkg module, skipping validate_framework")
85 return
86
87 try:
88 parsed_framework = apt_pkg.parse_depends(framework_string)
89 except ValueError:
90 raise ClickFrameworkInvalid(
91 'Could not parse framework "%s"' % framework_string)
92
93 framework_base_versions = set()
94 missing_frameworks = []
95 for or_dep in parsed_framework:
96 if len(or_dep) > 1:
97 raise ClickFrameworkInvalid(
98 'Alternative dependencies in framework "%s" not yet '
99 'allowed' % framework_string)
100 if or_dep[0][1] or or_dep[0][2]:
101 raise ClickFrameworkInvalid(
102 'Version relationship in framework "%s" not yet allowed' %
103 framework_string)
104 # now verify that different base versions are not mixed
105 framework_name = or_dep[0][0]
106 if not click_framework_has_framework(framework_name):
107 missing_frameworks.append(framework_name)
108 continue
109 framework_base_version = click_framework_get_base_version(
110 framework_name)
111 framework_base_versions.add(framework_base_version)
112
113 if not ignore_missing_frameworks:
114 if len(missing_frameworks) > 1:
115 raise ClickFrameworkInvalid(
116 'Frameworks %s not present on system (use '
117 '--force-missing-framework option to override)' %
118 ", ".join('"%s"' % f for f in missing_frameworks))
119 elif missing_frameworks:
120 raise ClickFrameworkInvalid(
121 'Framework "%s" not present on system (use '
122 '--force-missing-framework option to override)' %
123 missing_frameworks[0])
124 else:
125 if len(missing_frameworks) > 1:
126 logging.warning("Ignoring missing frameworks %s" % (
127 ", ".join('"%s"' % f for f in missing_frameworks)))
128 elif missing_frameworks:
129 logging.warning('Ignoring missing framework "%s"' % (
130 missing_frameworks[0]))
131
132 if len(framework_base_versions) > 1:
133 raise ClickFrameworkInvalid(
134 'Multiple frameworks with different base versions are not '
135 'allowed. Found: {0}'.format(framework_base_versions))
0136
=== modified file 'click/install.py'
--- click/install.py 2014-04-03 08:52:02 +0000
+++ click/install.py 2014-05-05 13:23:45 +0000
@@ -49,6 +49,11 @@
49from click.preinst import static_preinst_matches49from click.preinst import static_preinst_matches
50from click.versions import spec_version50from click.versions import spec_version
5151
52from click.framework import (
53 validate_framework,
54 ClickFrameworkInvalid,
55)
56
5257
53try:58try:
54 _DebFile.close59 _DebFile.close
@@ -189,34 +194,9 @@
189 raise ClickInstallerAuditError(194 raise ClickInstallerAuditError(
190 'No "framework" entry in manifest')195 'No "framework" entry in manifest')
191 try:196 try:
192 parsed_framework = apt_pkg.parse_depends(framework)197 validate_framework(framework, self.force_missing_framework)
193 except ValueError:198 except ClickFrameworkInvalid as e:
194 raise ClickInstallerAuditError(199 raise ClickInstallerAuditError(str(e))
195 'Could not parse framework "%s"' % framework)
196 for or_dep in parsed_framework:
197 if len(or_dep) > 1:
198 raise ClickInstallerAuditError(
199 'Alternative dependencies in framework "%s" not yet '
200 'allowed' % framework)
201 if or_dep[0][1] or or_dep[0][2]:
202 raise ClickInstallerAuditError(
203 'Version relationship in framework "%s" not yet '
204 'allowed' % framework)
205 if not self.force_missing_framework:
206 missing_frameworks = []
207 for or_dep in parsed_framework:
208 if not Click.Framework.has_framework(or_dep[0][0]):
209 missing_frameworks.append(or_dep[0][0])
210 if len(missing_frameworks) > 1:
211 raise ClickInstallerAuditError(
212 'Frameworks %s not present on system (use '
213 '--force-missing-framework option to override)' %
214 ", ".join('"%s"' % f for f in missing_frameworks))
215 elif missing_frameworks:
216 raise ClickInstallerAuditError(
217 'Framework "%s" not present on system (use '
218 '--force-missing-framework option to override)' %
219 missing_frameworks[0])
220200
221 if check_arch:201 if check_arch:
222 architecture = manifest.get("architecture", "all")202 architecture = manifest.get("architecture", "all")
223203
=== modified file 'click/tests/helpers.py'
--- click/tests/helpers.py 2014-03-10 14:49:27 +0000
+++ click/tests/helpers.py 2014-05-05 13:23:45 +0000
@@ -30,6 +30,7 @@
3030
31import contextlib31import contextlib
32import os32import os
33import re
33import shutil34import shutil
34import sys35import sys
35import tempfile36import tempfile
@@ -107,6 +108,32 @@
107 self.assertRaisesGError(108 self.assertRaisesGError(
108 "click_user_error-quark", code, callableObj, *args, **kwargs)109 "click_user_error-quark", code, callableObj, *args, **kwargs)
109110
111 def _create_mock_framework_dir(self, frameworks_dir=None):
112 if frameworks_dir is None:
113 frameworks_dir = os.path.join(self.temp_dir, "frameworks")
114 Click.ensuredir(frameworks_dir)
115 os.environ["CLICK_FRAMEWORKS_DIR"] = frameworks_dir
116 return frameworks_dir
117
118 def _create_mock_framework_file(self, framework_name):
119 self.use_temp_dir()
120 self._create_mock_framework_dir()
121 r = r'(?P<name>[a-z]+-sdk)-(?P<ver>[0-9.]+)(-[a-z0-9-]+)?'
122 match = re.match(r, framework_name)
123 if match is None:
124 name = "unknown"
125 ver = "1.0"
126 else:
127 name = match.group("name")
128 ver = match.group("ver")
129 framework_filename = os.path.join(
130 self.temp_dir, "frameworks",
131 "{0}.framework".format(framework_name))
132 with open(framework_filename, "w") as f:
133 f.write("Base-Name: {0}\n".format(name))
134 f.write("Base-Version: {0}\n".format(ver))
135
136
110137
111if not hasattr(mock, "call"):138if not hasattr(mock, "call"):
112 # mock 0.7.2, the version in Ubuntu 12.04 LTS, lacks mock.ANY and139 # mock 0.7.2, the version in Ubuntu 12.04 LTS, lacks mock.ANY and
113140
=== modified file 'click/tests/test_build.py'
--- click/tests/test_build.py 2014-01-23 17:19:09 +0000
+++ click/tests/test_build.py 2014-05-05 13:23:45 +0000
@@ -245,6 +245,7 @@
245 del target_json["installed-size"]245 del target_json["installed-size"]
246 self.assertEqual(source_json, target_json)246 self.assertEqual(source_json, target_json)
247247
248 # FIXME: DRY violation with test_build_multiple_architectures etc
248 def test_build_multiple_frameworks(self):249 def test_build_multiple_frameworks(self):
249 self.use_temp_dir()250 self.use_temp_dir()
250 scratch = os.path.join(self.temp_dir, "scratch")251 scratch = os.path.join(self.temp_dir, "scratch")
@@ -259,11 +260,44 @@
259 "ubuntu-sdk-14.04-basic, ubuntu-sdk-14.04-webapps",260 "ubuntu-sdk-14.04-basic, ubuntu-sdk-14.04-webapps",
260 }, f)261 }, f)
261 self.builder.add_file(scratch, "/")262 self.builder.add_file(scratch, "/")
262 self.assertRaisesRegex(263 path = self.builder.build(self.temp_dir)
263 ClickBuildError,264 control_path = os.path.join(self.temp_dir, "control")
264 'Multiple dependencies in framework "ubuntu-sdk-14.04-basic, '265 subprocess.check_call(["dpkg-deb", "-e", path, control_path])
265 'ubuntu-sdk-14.04-webapps" not yet allowed',266 manifest_path = os.path.join(control_path, "manifest")
266 self.builder.build, self.temp_dir)267 with open(os.path.join(scratch, "manifest.json")) as source, \
268 open(manifest_path) as target:
269 source_json = json.load(source)
270 target_json = json.load(target)
271 del target_json["installed-size"]
272 self.assertEqual(source_json, target_json)
273
274
275class TestClickFrameworkValidation(TestCase):
276 def setUp(self):
277 super(TestClickFrameworkValidation, self).setUp()
278 self.builder = ClickBuilder()
279 for framework_name in ("ubuntu-sdk-13.10",
280 "ubuntu-sdk-14.04-papi",
281 "ubuntu-sdk-14.04-html"):
282 self._create_mock_framework_file(framework_name)
283
284 def test_validate_framework_good(self):
285 valid_framework_values = (
286 "ubuntu-sdk-13.10",
287 "ubuntu-sdk-14.04-papi, ubuntu-sdk-14.04-html",
288 )
289 for framework in valid_framework_values:
290 self.builder._validate_framework(framework)
291
292 def test_validate_framework_bad(self):
293 invalid_framework_values = (
294 "ubuntu-sdk-13.10, ubuntu-sdk-14.04-papi",
295 "ubuntu-sdk-13.10 (>= 13.10)",
296 "ubuntu-sdk-13.10 | ubuntu-sdk-14.04",
297 )
298 for framework in invalid_framework_values:
299 with self.assertRaises(ClickBuildError):
300 self.builder._validate_framework(framework)
267301
268302
269class TestClickSourceBuilder(TestCase, TestClickBuilderBaseMixin):303class TestClickSourceBuilder(TestCase, TestClickBuilderBaseMixin):
270304
=== modified file 'click/tests/test_install.py'
--- click/tests/test_install.py 2014-04-03 08:52:02 +0000
+++ click/tests/test_install.py 2014-05-05 13:23:45 +0000
@@ -104,12 +104,10 @@
104 return package_path104 return package_path
105105
106 def _setup_frameworks(self, preloads, frameworks_dir=None, frameworks=[]):106 def _setup_frameworks(self, preloads, frameworks_dir=None, frameworks=[]):
107 if frameworks_dir is None:107 frameworks_dir = self._create_mock_framework_dir(frameworks_dir)
108 frameworks_dir = os.path.join(self.temp_dir, "frameworks")
109 shutil.rmtree(frameworks_dir, ignore_errors=True)108 shutil.rmtree(frameworks_dir, ignore_errors=True)
110 Click.ensuredir(frameworks_dir)
111 for framework in frameworks:109 for framework in frameworks:
112 touch(os.path.join(frameworks_dir, "%s.framework" % framework))110 self._create_mock_framework_file(framework)
113 preloads["click_get_frameworks_dir"].side_effect = (111 preloads["click_get_frameworks_dir"].side_effect = (
114 lambda: self.make_string(frameworks_dir))112 lambda: self.make_string(frameworks_dir))
115113

Subscribers

People subscribed via source and target branches

to all changes: