review-tools

Merge ~jslarraz/review-tools:use-lint-error-on-missing-desktop-file into review-tools:master

  1. Git
  2. lp:~jslarraz/review-tools
  3. use-lint-error-on-missing-desktop-file
  4. Merge into master
Proposed by Jorge Sancho Larraz
Status: Merged
Merged at revision: 836a7647b261b0fef0868a1804aa40fa99cc0e05
Proposed branch: ~jslarraz/review-tools:use-lint-error-on-missing-desktop-file
Merge into: review-tools:master
Diff against target: 384 lines (+302/-23)
2 files modified
reviewtools/sr_lint.py (+12/-9)
tests/test.sh.expected (+290/-14)
Reviewer Review Type Date Requested Status
Alex Murray Approve
Review via email: mp+466748@code.launchpad.net

Commit message

Originally review-tools called common.error if meta/gui/*.desktop file could not be found. It makes the review to exit immediately.

This change is intended to treat missing meta/gui/*.desktop as a lint error

To post a comment you must log in.
Revision history for this message
Alex Murray (alexmurray) wrote :

Nice - LGTM!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/reviewtools/sr_lint.py b/reviewtools/sr_lint.py
2index dcf8ad1..b330ac6 100644
3--- a/reviewtools/sr_lint.py
4+++ b/reviewtools/sr_lint.py
5@@ -20,7 +20,6 @@ from reviewtools.containers.base_container import ContainerException
6 from reviewtools.sr_common import SnapReview
7 from reviewtools.common import (
8 find_external_symlinks,
9- error,
10 )
11 from reviewtools.overrides import (
12 redflagged_snap_types_overrides,
13@@ -1897,10 +1896,7 @@ class SnapReviewLint(SnapReview):
14 else:
15 appnames.append("%s.%s" % (self.snap_yaml["name"], app))
16
17- try:
18- file = self.snap.get_file(fn)
19- except ContainerException as e:
20- error(str(e))
21+ file = self.snap.get_file(fn)
22 lines = file.decode().split("\n")
23
24 # For now, just check Exec=, Icon= and bools since:
25@@ -2041,11 +2037,21 @@ class SnapReviewLint(SnapReview):
26 ):
27 default_provider_is_mir = True
28
29+ t = "info"
30+ n = self._get_check_name("meta_gui_desktop")
31+ s = "OK"
32+
33 has_desktop_files = False
34 for f in self.pkg_files:
35 fn = os.path.relpath(f, self._get_unpack_dir())
36 if fn.startswith("meta/gui/") and fn.endswith(".desktop"):
37- self._verify_desktop_file(fn)
38+ try:
39+ self._verify_desktop_file(fn)
40+ except ContainerException as e:
41+ t = "error"
42+ s = str(e)
43+ self._add_result(t, n, s)
44+ return
45 has_desktop_files = True
46 break
47
48@@ -2057,9 +2063,6 @@ class SnapReviewLint(SnapReview):
49 if len(desktop_interfaces_specified) < 1 and not has_desktop_files:
50 return
51
52- t = "info"
53- n = self._get_check_name("meta_gui_desktop")
54- s = "OK"
55 if len(desktop_interfaces_specified) > 0 and not has_desktop_files:
56 if self.snap_yaml["name"] in desktop_file_exception:
57 t = "info"
58diff --git a/tests/test.sh.expected b/tests/test.sh.expected
59index 6c66e85..1c726c3 100644
60--- a/tests/test.sh.expected
61+++ b/tests/test.sh.expected
62@@ -18270,32 +18270,308 @@ test-bad-desktop-file-icon_1_all.snap: FAIL
63 }
64
65 = test-bad-desktop-file_1_all.snap =
66-ERROR: Could not find 'meta/gui/env.desktop'
67+Errors
68+------
69+ - lint-snap-v2:meta_gui_desktop
70+ Could not find 'meta/gui/env.desktop'
71+test-bad-desktop-file_1_all.snap: FAIL
72
73 = --sdk test-bad-desktop-file_1_all.snap =
74+= snap.v2_declaration =
75 {
76- "runtime-errors": {
77- "error": {
78- "msg": {
79- "manual_review": true,
80- "text": "Could not find 'meta/gui/env.desktop'"
81- }
82+ "error": {},
83+ "info": {
84+ "declaration-snap-v2:plugs:env:x11": {
85+ "manual_review": false,
86+ "text": "OK"
87+ }
88+ },
89+ "warn": {}
90+}
91+= snap.v2_functional =
92+{
93+ "error": {},
94+ "info": {
95+ "functional-snap-v2:execstack": {
96+ "manual_review": false,
97+ "text": "OK"
98+ }
99+ },
100+ "warn": {}
101+}
102+= snap.v2_lint =
103+{
104+ "error": {
105+ "lint-snap-v2:meta_gui_desktop": {
106+ "manual_review": false,
107+ "text": "Could not find 'meta/gui/env.desktop'"
108+ }
109+ },
110+ "info": {
111+ "lint-snap-v2:app_plugs:env": {
112+ "manual_review": false,
113+ "text": "OK"
114 },
115- "info": {},
116- "warn": {}
117- }
118+ "lint-snap-v2:app_plugs_plug_reference:env:x11": {
119+ "manual_review": false,
120+ "text": "OK"
121+ },
122+ "lint-snap-v2:apps": {
123+ "manual_review": false,
124+ "text": "OK"
125+ },
126+ "lint-snap-v2:apps_entry:env": {
127+ "manual_review": false,
128+ "text": "OK"
129+ },
130+ "lint-snap-v2:apps_present": {
131+ "manual_review": false,
132+ "text": "OK"
133+ },
134+ "lint-snap-v2:apps_required:env": {
135+ "manual_review": false,
136+ "text": "OK"
137+ },
138+ "lint-snap-v2:apps_unknown:env": {
139+ "manual_review": false,
140+ "text": "OK"
141+ },
142+ "lint-snap-v2:assumes_valid": {
143+ "manual_review": false,
144+ "text": "OK (assumes not specified)"
145+ },
146+ "lint-snap-v2:cli_required:env": {
147+ "manual_review": false,
148+ "text": "OK"
149+ },
150+ "lint-snap-v2:command:env": {
151+ "manual_review": false,
152+ "text": "OK"
153+ },
154+ "lint-snap-v2:confinement_valid": {
155+ "manual_review": false,
156+ "text": "OK"
157+ },
158+ "lint-snap-v2:daemon_required:env": {
159+ "manual_review": false,
160+ "text": "OK"
161+ },
162+ "lint-snap-v2:external_symlinks": {
163+ "manual_review": false,
164+ "text": "OK"
165+ },
166+ "lint-snap-v2:grade_valid": {
167+ "manual_review": false,
168+ "text": "OK"
169+ },
170+ "lint-snap-v2:hooks_present": {
171+ "manual_review": false,
172+ "text": "OK (optional hooks field not specified)"
173+ },
174+ "lint-snap-v2:iffy_files": {
175+ "manual_review": false,
176+ "text": "OK"
177+ },
178+ "lint-snap-v2:snap_type_redflag": {
179+ "manual_review": false,
180+ "text": "OK"
181+ },
182+ "lint-snap-v2:unknown_field": {
183+ "manual_review": false,
184+ "text": "OK"
185+ },
186+ "lint-snap-v2:unknown_install_mode": {
187+ "manual_review": false,
188+ "text": "OK"
189+ },
190+ "lint-snap-v2:unknown_refresh_mode": {
191+ "manual_review": false,
192+ "text": "OK"
193+ },
194+ "lint-snap-v2:unknown_stop_mode": {
195+ "manual_review": false,
196+ "text": "OK"
197+ },
198+ "lint-snap-v2:valid_contents_for_architecture": {
199+ "manual_review": false,
200+ "text": "OK"
201+ },
202+ "lint-snap-v2:valid_unicode": {
203+ "manual_review": false,
204+ "text": "ok"
205+ },
206+ "lint-snap-v2:vcs_files": {
207+ "manual_review": false,
208+ "text": "OK"
209+ }
210+ },
211+ "warn": {}
212+}
213+= snap.v2_security =
214+{
215+ "error": {},
216+ "info": {
217+ "security-snap-v2:profile_name_length:env": {
218+ "manual_review": false,
219+ "text": "OK"
220+ },
221+ "security-snap-v2:squashfs_files": {
222+ "manual_review": false,
223+ "text": "OK"
224+ },
225+ "security-snap-v2:squashfs_repack_checksum": {
226+ "manual_review": false,
227+ "text": "OK"
228+ }
229+ },
230+ "warn": {}
231 }
232
233 = --json test-bad-desktop-file_1_all.snap =
234 {
235- "runtime-errors": {
236+ "snap.v2_declaration": {
237+ "error": {},
238+ "info": {
239+ "declaration-snap-v2:plugs:env:x11": {
240+ "manual_review": false,
241+ "text": "OK"
242+ }
243+ },
244+ "warn": {}
245+ },
246+ "snap.v2_functional": {
247+ "error": {},
248+ "info": {
249+ "functional-snap-v2:execstack": {
250+ "manual_review": false,
251+ "text": "OK"
252+ }
253+ },
254+ "warn": {}
255+ },
256+ "snap.v2_lint": {
257 "error": {
258- "msg": {
259- "manual_review": true,
260+ "lint-snap-v2:meta_gui_desktop": {
261+ "manual_review": false,
262 "text": "Could not find 'meta/gui/env.desktop'"
263 }
264 },
265- "info": {},
266+ "info": {
267+ "lint-snap-v2:app_plugs:env": {
268+ "manual_review": false,
269+ "text": "OK"
270+ },
271+ "lint-snap-v2:app_plugs_plug_reference:env:x11": {
272+ "manual_review": false,
273+ "text": "OK"
274+ },
275+ "lint-snap-v2:apps": {
276+ "manual_review": false,
277+ "text": "OK"
278+ },
279+ "lint-snap-v2:apps_entry:env": {
280+ "manual_review": false,
281+ "text": "OK"
282+ },
283+ "lint-snap-v2:apps_present": {
284+ "manual_review": false,
285+ "text": "OK"
286+ },
287+ "lint-snap-v2:apps_required:env": {
288+ "manual_review": false,
289+ "text": "OK"
290+ },
291+ "lint-snap-v2:apps_unknown:env": {
292+ "manual_review": false,
293+ "text": "OK"
294+ },
295+ "lint-snap-v2:assumes_valid": {
296+ "manual_review": false,
297+ "text": "OK (assumes not specified)"
298+ },
299+ "lint-snap-v2:cli_required:env": {
300+ "manual_review": false,
301+ "text": "OK"
302+ },
303+ "lint-snap-v2:command:env": {
304+ "manual_review": false,
305+ "text": "OK"
306+ },
307+ "lint-snap-v2:confinement_valid": {
308+ "manual_review": false,
309+ "text": "OK"
310+ },
311+ "lint-snap-v2:daemon_required:env": {
312+ "manual_review": false,
313+ "text": "OK"
314+ },
315+ "lint-snap-v2:external_symlinks": {
316+ "manual_review": false,
317+ "text": "OK"
318+ },
319+ "lint-snap-v2:grade_valid": {
320+ "manual_review": false,
321+ "text": "OK"
322+ },
323+ "lint-snap-v2:hooks_present": {
324+ "manual_review": false,
325+ "text": "OK (optional hooks field not specified)"
326+ },
327+ "lint-snap-v2:iffy_files": {
328+ "manual_review": false,
329+ "text": "OK"
330+ },
331+ "lint-snap-v2:snap_type_redflag": {
332+ "manual_review": false,
333+ "text": "OK"
334+ },
335+ "lint-snap-v2:unknown_field": {
336+ "manual_review": false,
337+ "text": "OK"
338+ },
339+ "lint-snap-v2:unknown_install_mode": {
340+ "manual_review": false,
341+ "text": "OK"
342+ },
343+ "lint-snap-v2:unknown_refresh_mode": {
344+ "manual_review": false,
345+ "text": "OK"
346+ },
347+ "lint-snap-v2:unknown_stop_mode": {
348+ "manual_review": false,
349+ "text": "OK"
350+ },
351+ "lint-snap-v2:valid_contents_for_architecture": {
352+ "manual_review": false,
353+ "text": "OK"
354+ },
355+ "lint-snap-v2:valid_unicode": {
356+ "manual_review": false,
357+ "text": "ok"
358+ },
359+ "lint-snap-v2:vcs_files": {
360+ "manual_review": false,
361+ "text": "OK"
362+ }
363+ },
364+ "warn": {}
365+ },
366+ "snap.v2_security": {
367+ "error": {},
368+ "info": {
369+ "security-snap-v2:profile_name_length:env": {
370+ "manual_review": false,
371+ "text": "OK"
372+ },
373+ "security-snap-v2:squashfs_files": {
374+ "manual_review": false,
375+ "text": "OK"
376+ },
377+ "security-snap-v2:squashfs_repack_checksum": {
378+ "manual_review": false,
379+ "text": "OK"
380+ }
381+ },
382 "warn": {}
383 }
384 }

Subscribers

People subscribed via source and target branches