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
1=== modified file 'click/build.py'
2--- click/build.py 2014-01-23 17:19:09 +0000
3+++ click/build.py 2014-05-05 13:23:45 +0000
4@@ -50,6 +50,11 @@
5 from click.preinst import static_preinst
6 from click.versions import spec_version
7
8+from click.framework import (
9+ validate_framework,
10+ ClickFrameworkInvalid,
11+)
12+
13
14 @contextlib.contextmanager
15 def make_temp_dir():
16@@ -196,30 +201,13 @@
17 package.add_file("control.tar.gz", control_tar_path)
18 package.add_file("data.tar.gz", data_tar_path)
19
20- def _validate_framework(self, framework):
21+ def _validate_framework(self, framework_string):
22 """Apply policy checks to framework declarations."""
23 try:
24- apt_pkg
25- except NameError:
26- return
27-
28- try:
29- parsed_framework = apt_pkg.parse_depends(framework)
30- except ValueError:
31- raise ClickBuildError('Could not parse framework "%s"' % framework)
32- if len(parsed_framework) > 1:
33- raise ClickBuildError(
34- 'Multiple dependencies in framework "%s" not yet allowed' %
35- framework)
36- for or_dep in parsed_framework:
37- if len(or_dep) > 1:
38- raise ClickBuildError(
39- 'Alternative dependencies in framework "%s" not yet '
40- 'allowed' % framework)
41- if or_dep[0][1] or or_dep[0][2]:
42- raise ClickBuildError(
43- 'Version relationship in framework "%s" not yet allowed' %
44- framework)
45+ validate_framework(
46+ framework_string, ignore_missing_frameworks=True)
47+ except ClickFrameworkInvalid as e:
48+ raise ClickBuildError(str(e))
49
50 def build(self, dest_dir, manifest_path="manifest.json"):
51 with make_temp_dir() as temp_dir:
52
53=== added file 'click/framework.py'
54--- click/framework.py 1970-01-01 00:00:00 +0000
55+++ click/framework.py 2014-05-05 13:23:45 +0000
56@@ -0,0 +1,135 @@
57+# Copyright (C) 2014 Canonical Ltd.
58+# Author: Michael Vogt <michael.vogt@canonical.com>
59+
60+# This program is free software: you can redistribute it and/or modify
61+# it under the terms of the GNU General Public License as published by
62+# the Free Software Foundation; version 3 of the License.
63+#
64+# This program is distributed in the hope that it will be useful,
65+# but WITHOUT ANY WARRANTY; without even the implied warranty of
66+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
67+# GNU General Public License for more details.
68+#
69+# You should have received a copy of the GNU General Public License
70+# along with this program. If not, see <http://www.gnu.org/licenses/>.
71+
72+"""Pure python click framework handling support."""
73+
74+import logging
75+import os
76+import re
77+
78+try:
79+ import apt_pkg
80+except:
81+ pass
82+
83+
84+class ClickFrameworkInvalid(Exception):
85+ pass
86+
87+
88+# FIXME: use native lib if available
89+#from gi.repository import Click
90+#click_framework_get_base_version = Click.framework_get_base_version
91+#click_framework_has_framework = Click.has_framework
92+
93+
94+# python version of the vala parse_deb822_file()
95+def parse_deb822_file(filename):
96+ data = {}
97+ with open(filename) as f:
98+ for line in f:
99+ line = line.strip()
100+ # from deb822.vala
101+ field_re_posix = r'^([^:[:space:]]+)[[:space:]]*:[[:space:]]'\
102+ '([^[:space:]].*?)[[:space:]]*$'
103+ # python does not do posix char classes
104+ field_re = field_re_posix.replace("[:space:]", "\s")
105+ blank_re_posix = r'^[[:space:]]*$'
106+ blank_re = blank_re_posix.replace("[:space:]", "\s")
107+ if re.match(blank_re, line):
108+ break
109+ match = re.match(field_re, line)
110+ if match and match.group(1) and match.group(2):
111+ data[match.group(1).lower()] = match.group(2)
112+ return data
113+
114+
115+# python version of vala get_frameworks_dir
116+def get_frameworks_dir(framework_name):
117+ # FIXME: get via configure.in etc?
118+ frameworks_dir = os.environ.get(
119+ "CLICK_FRAMEWORKS_DIR", "/usr/share/click/frameworks")
120+ framework_path = os.path.join(
121+ frameworks_dir, framework_name+".framework")
122+ return framework_path
123+
124+
125+# python version of the vala click_framework_get_base_version()
126+def click_framework_get_base_version(framework_name):
127+ deb822 = parse_deb822_file(get_frameworks_dir(framework_name))
128+ return deb822.get("base-version", None)
129+
130+
131+# python version of the vala click_framework_has_framework
132+def click_framework_has_framework(framework_name):
133+ return os.path.exists(get_frameworks_dir(framework_name))
134+
135+
136+def validate_framework(framework_string, ignore_missing_frameworks=False):
137+ try:
138+ apt_pkg
139+ except NameError:
140+ logging.warning("No apt_pkg module, skipping validate_framework")
141+ return
142+
143+ try:
144+ parsed_framework = apt_pkg.parse_depends(framework_string)
145+ except ValueError:
146+ raise ClickFrameworkInvalid(
147+ 'Could not parse framework "%s"' % framework_string)
148+
149+ framework_base_versions = set()
150+ missing_frameworks = []
151+ for or_dep in parsed_framework:
152+ if len(or_dep) > 1:
153+ raise ClickFrameworkInvalid(
154+ 'Alternative dependencies in framework "%s" not yet '
155+ 'allowed' % framework_string)
156+ if or_dep[0][1] or or_dep[0][2]:
157+ raise ClickFrameworkInvalid(
158+ 'Version relationship in framework "%s" not yet allowed' %
159+ framework_string)
160+ # now verify that different base versions are not mixed
161+ framework_name = or_dep[0][0]
162+ if not click_framework_has_framework(framework_name):
163+ missing_frameworks.append(framework_name)
164+ continue
165+ framework_base_version = click_framework_get_base_version(
166+ framework_name)
167+ framework_base_versions.add(framework_base_version)
168+
169+ if not ignore_missing_frameworks:
170+ if len(missing_frameworks) > 1:
171+ raise ClickFrameworkInvalid(
172+ 'Frameworks %s not present on system (use '
173+ '--force-missing-framework option to override)' %
174+ ", ".join('"%s"' % f for f in missing_frameworks))
175+ elif missing_frameworks:
176+ raise ClickFrameworkInvalid(
177+ 'Framework "%s" not present on system (use '
178+ '--force-missing-framework option to override)' %
179+ missing_frameworks[0])
180+ else:
181+ if len(missing_frameworks) > 1:
182+ logging.warning("Ignoring missing frameworks %s" % (
183+ ", ".join('"%s"' % f for f in missing_frameworks)))
184+ elif missing_frameworks:
185+ logging.warning('Ignoring missing framework "%s"' % (
186+ missing_frameworks[0]))
187+
188+ if len(framework_base_versions) > 1:
189+ raise ClickFrameworkInvalid(
190+ 'Multiple frameworks with different base versions are not '
191+ 'allowed. Found: {0}'.format(framework_base_versions))
192
193=== modified file 'click/install.py'
194--- click/install.py 2014-04-03 08:52:02 +0000
195+++ click/install.py 2014-05-05 13:23:45 +0000
196@@ -49,6 +49,11 @@
197 from click.preinst import static_preinst_matches
198 from click.versions import spec_version
199
200+from click.framework import (
201+ validate_framework,
202+ ClickFrameworkInvalid,
203+)
204+
205
206 try:
207 _DebFile.close
208@@ -189,34 +194,9 @@
209 raise ClickInstallerAuditError(
210 'No "framework" entry in manifest')
211 try:
212- parsed_framework = apt_pkg.parse_depends(framework)
213- except ValueError:
214- raise ClickInstallerAuditError(
215- 'Could not parse framework "%s"' % framework)
216- for or_dep in parsed_framework:
217- if len(or_dep) > 1:
218- raise ClickInstallerAuditError(
219- 'Alternative dependencies in framework "%s" not yet '
220- 'allowed' % framework)
221- if or_dep[0][1] or or_dep[0][2]:
222- raise ClickInstallerAuditError(
223- 'Version relationship in framework "%s" not yet '
224- 'allowed' % framework)
225- if not self.force_missing_framework:
226- missing_frameworks = []
227- for or_dep in parsed_framework:
228- if not Click.Framework.has_framework(or_dep[0][0]):
229- missing_frameworks.append(or_dep[0][0])
230- if len(missing_frameworks) > 1:
231- raise ClickInstallerAuditError(
232- 'Frameworks %s not present on system (use '
233- '--force-missing-framework option to override)' %
234- ", ".join('"%s"' % f for f in missing_frameworks))
235- elif missing_frameworks:
236- raise ClickInstallerAuditError(
237- 'Framework "%s" not present on system (use '
238- '--force-missing-framework option to override)' %
239- missing_frameworks[0])
240+ validate_framework(framework, self.force_missing_framework)
241+ except ClickFrameworkInvalid as e:
242+ raise ClickInstallerAuditError(str(e))
243
244 if check_arch:
245 architecture = manifest.get("architecture", "all")
246
247=== modified file 'click/tests/helpers.py'
248--- click/tests/helpers.py 2014-03-10 14:49:27 +0000
249+++ click/tests/helpers.py 2014-05-05 13:23:45 +0000
250@@ -30,6 +30,7 @@
251
252 import contextlib
253 import os
254+import re
255 import shutil
256 import sys
257 import tempfile
258@@ -107,6 +108,32 @@
259 self.assertRaisesGError(
260 "click_user_error-quark", code, callableObj, *args, **kwargs)
261
262+ def _create_mock_framework_dir(self, frameworks_dir=None):
263+ if frameworks_dir is None:
264+ frameworks_dir = os.path.join(self.temp_dir, "frameworks")
265+ Click.ensuredir(frameworks_dir)
266+ os.environ["CLICK_FRAMEWORKS_DIR"] = frameworks_dir
267+ return frameworks_dir
268+
269+ def _create_mock_framework_file(self, framework_name):
270+ self.use_temp_dir()
271+ self._create_mock_framework_dir()
272+ r = r'(?P<name>[a-z]+-sdk)-(?P<ver>[0-9.]+)(-[a-z0-9-]+)?'
273+ match = re.match(r, framework_name)
274+ if match is None:
275+ name = "unknown"
276+ ver = "1.0"
277+ else:
278+ name = match.group("name")
279+ ver = match.group("ver")
280+ framework_filename = os.path.join(
281+ self.temp_dir, "frameworks",
282+ "{0}.framework".format(framework_name))
283+ with open(framework_filename, "w") as f:
284+ f.write("Base-Name: {0}\n".format(name))
285+ f.write("Base-Version: {0}\n".format(ver))
286+
287+
288
289 if not hasattr(mock, "call"):
290 # mock 0.7.2, the version in Ubuntu 12.04 LTS, lacks mock.ANY and
291
292=== modified file 'click/tests/test_build.py'
293--- click/tests/test_build.py 2014-01-23 17:19:09 +0000
294+++ click/tests/test_build.py 2014-05-05 13:23:45 +0000
295@@ -245,6 +245,7 @@
296 del target_json["installed-size"]
297 self.assertEqual(source_json, target_json)
298
299+ # FIXME: DRY violation with test_build_multiple_architectures etc
300 def test_build_multiple_frameworks(self):
301 self.use_temp_dir()
302 scratch = os.path.join(self.temp_dir, "scratch")
303@@ -259,11 +260,44 @@
304 "ubuntu-sdk-14.04-basic, ubuntu-sdk-14.04-webapps",
305 }, f)
306 self.builder.add_file(scratch, "/")
307- self.assertRaisesRegex(
308- ClickBuildError,
309- 'Multiple dependencies in framework "ubuntu-sdk-14.04-basic, '
310- 'ubuntu-sdk-14.04-webapps" not yet allowed',
311- self.builder.build, self.temp_dir)
312+ path = self.builder.build(self.temp_dir)
313+ control_path = os.path.join(self.temp_dir, "control")
314+ subprocess.check_call(["dpkg-deb", "-e", path, control_path])
315+ manifest_path = os.path.join(control_path, "manifest")
316+ with open(os.path.join(scratch, "manifest.json")) as source, \
317+ open(manifest_path) as target:
318+ source_json = json.load(source)
319+ target_json = json.load(target)
320+ del target_json["installed-size"]
321+ self.assertEqual(source_json, target_json)
322+
323+
324+class TestClickFrameworkValidation(TestCase):
325+ def setUp(self):
326+ super(TestClickFrameworkValidation, self).setUp()
327+ self.builder = ClickBuilder()
328+ for framework_name in ("ubuntu-sdk-13.10",
329+ "ubuntu-sdk-14.04-papi",
330+ "ubuntu-sdk-14.04-html"):
331+ self._create_mock_framework_file(framework_name)
332+
333+ def test_validate_framework_good(self):
334+ valid_framework_values = (
335+ "ubuntu-sdk-13.10",
336+ "ubuntu-sdk-14.04-papi, ubuntu-sdk-14.04-html",
337+ )
338+ for framework in valid_framework_values:
339+ self.builder._validate_framework(framework)
340+
341+ def test_validate_framework_bad(self):
342+ invalid_framework_values = (
343+ "ubuntu-sdk-13.10, ubuntu-sdk-14.04-papi",
344+ "ubuntu-sdk-13.10 (>= 13.10)",
345+ "ubuntu-sdk-13.10 | ubuntu-sdk-14.04",
346+ )
347+ for framework in invalid_framework_values:
348+ with self.assertRaises(ClickBuildError):
349+ self.builder._validate_framework(framework)
350
351
352 class TestClickSourceBuilder(TestCase, TestClickBuilderBaseMixin):
353
354=== modified file 'click/tests/test_install.py'
355--- click/tests/test_install.py 2014-04-03 08:52:02 +0000
356+++ click/tests/test_install.py 2014-05-05 13:23:45 +0000
357@@ -104,12 +104,10 @@
358 return package_path
359
360 def _setup_frameworks(self, preloads, frameworks_dir=None, frameworks=[]):
361- if frameworks_dir is None:
362- frameworks_dir = os.path.join(self.temp_dir, "frameworks")
363+ frameworks_dir = self._create_mock_framework_dir(frameworks_dir)
364 shutil.rmtree(frameworks_dir, ignore_errors=True)
365- Click.ensuredir(frameworks_dir)
366 for framework in frameworks:
367- touch(os.path.join(frameworks_dir, "%s.framework" % framework))
368+ self._create_mock_framework_file(framework)
369 preloads["click_get_frameworks_dir"].side_effect = (
370 lambda: self.make_string(frameworks_dir))
371

Subscribers

People subscribed via source and target branches

to all changes: