OLD | NEW |
1 #!/usr/bin/python | 1 #!/usr/bin/python |
2 """ | 2 """ |
3 Script to verify errors on autotest code contributions (patches). | 3 Script to verify errors on autotest code contributions (patches). |
4 The workflow is as follows: | 4 The workflow is as follows: |
5 | 5 |
6 * Patch will be applied and eventual problems will be notified. | 6 * Patch will be applied and eventual problems will be notified. |
7 * If there are new files created, remember user to add them to VCS. | 7 * If there are new files created, remember user to add them to VCS. |
8 * If any added file looks like a executable file, remember user to make them | 8 * If any added file looks like a executable file, remember user to make them |
9 executable. | 9 executable. |
10 * If any of the files added or modified introduces trailing whitespaces, tabs | 10 * If any of the files added or modified introduces trailing whitespaces, tabs |
11 or incorrect indentation, report problems. | 11 or incorrect indentation, report problems. |
12 * If any of the files have problems during pylint validation, report failures. | 12 * If any of the files have problems during pylint validation, report failures. |
13 * If any of the files changed have a unittest suite, run the unittest suite | 13 * If any of the files changed have a unittest suite, run the unittest suite |
14 and report any failures. | 14 and report any failures. |
15 | 15 |
16 Usage: check_patch.py -p [/path/to/patch] | 16 Usage: check_patch.py -p [/path/to/patch] |
17 check_patch.py -i [patchwork id] | 17 check_patch.py -i [patchwork id] |
18 | 18 |
19 @copyright: Red Hat Inc, 2009. | 19 @copyright: Red Hat Inc, 2009. |
20 @author: Lucas Meneghel Rodrigues <lmr@redhat.com> | 20 @author: Lucas Meneghel Rodrigues <lmr@redhat.com> |
21 """ | 21 """ |
22 | 22 |
23 import os, stat, logging, sys, optparse, time | 23 import os, stat, logging, sys, optparse |
24 import common | 24 import common |
25 from autotest_lib.client.common_lib import utils, error, logging_config | 25 from autotest_lib.client.common_lib import utils, error, logging_config |
26 from autotest_lib.client.common_lib import logging_manager | 26 from autotest_lib.client.common_lib import logging_manager |
27 | 27 |
28 | 28 |
29 class CheckPatchLoggingConfig(logging_config.LoggingConfig): | 29 class CheckPatchLoggingConfig(logging_config.LoggingConfig): |
30 def configure_logging(self, results_dir=None, verbose=False): | 30 def configure_logging(self, results_dir=None, verbose=False): |
31 super(CheckPatchLoggingConfig, self).configure_logging(use_console=True, | 31 super(CheckPatchLoggingConfig, self).configure_logging(use_console=True, |
32 verbose=verbose) | 32 verbose=verbose) |
33 | 33 |
34 | 34 |
35 def ask(question, auto=False): | |
36 """ | |
37 Raw input with a prompt that emulates logging. | |
38 | |
39 @param question: Question to be asked | |
40 @param auto: Whether to return "y" instead of asking the question | |
41 """ | |
42 if auto: | |
43 logging.info("%s (y/n) y" % question) | |
44 return "y" | |
45 return raw_input("%s INFO | %s (y/n) " % | |
46 (time.strftime("%H:%M:%S", time.localtime()), question)) | |
47 | |
48 | |
49 class VCS(object): | 35 class VCS(object): |
50 """ | 36 """ |
51 Abstraction layer to the version control system. | 37 Abstraction layer to the version control system. |
52 """ | 38 """ |
53 def __init__(self): | 39 def __init__(self): |
54 """ | 40 """ |
55 Class constructor. Guesses the version control name and instantiates it | 41 Class constructor. Guesses the version control name and instantiates it |
56 as a backend. | 42 as a backend. |
57 """ | 43 """ |
58 backend_name = self.guess_vcs_name() | 44 backend_name = self.guess_vcs_name() |
(...skipping 52 matching lines...) Expand 10 before | Expand all | Expand 10 after Loading... |
111 return self.backend.update() | 97 return self.backend.update() |
112 | 98 |
113 | 99 |
114 class SubVersionBackend(object): | 100 class SubVersionBackend(object): |
115 """ | 101 """ |
116 Implementation of a subversion backend for use with the VCS abstraction | 102 Implementation of a subversion backend for use with the VCS abstraction |
117 layer. | 103 layer. |
118 """ | 104 """ |
119 def __init__(self): | 105 def __init__(self): |
120 logging.debug("Subversion VCS backend initialized.") | 106 logging.debug("Subversion VCS backend initialized.") |
121 self.ignored_extension_list = ['.orig', '.bak'] | |
122 | 107 |
123 | 108 |
124 def get_unknown_files(self): | 109 def get_unknown_files(self): |
125 status = utils.system_output("svn status --ignore-externals") | 110 status = utils.system_output("svn status --ignore-externals") |
126 unknown_files = [] | 111 unknown_files = [] |
127 for line in status.split("\n"): | 112 for line in status.split("\n"): |
128 status_flag = line[0] | 113 status_flag = line[0] |
129 if line and status_flag == "?": | 114 if line and status_flag == "?": |
130 for extension in self.ignored_extension_list: | 115 unknown_files.append(line[1:].strip()) |
131 if not line.endswith(extension): | |
132 unknown_files.append(line[1:].strip()) | |
133 return unknown_files | 116 return unknown_files |
134 | 117 |
135 | 118 |
136 def get_modified_files(self): | 119 def get_modified_files(self): |
137 status = utils.system_output("svn status --ignore-externals") | 120 status = utils.system_output("svn status --ignore-externals") |
138 modified_files = [] | 121 modified_files = [] |
139 for line in status.split("\n"): | 122 for line in status.split("\n"): |
140 status_flag = line[0] | 123 status_flag = line[0] |
141 if line and status_flag == "M" or status_flag == "A": | 124 if line and status_flag == "M" or status_flag == "A": |
142 modified_files.append(line[1:].strip()) | 125 modified_files.append(line[1:].strip()) |
(...skipping 48 matching lines...) Expand 10 before | Expand all | Expand 10 after Loading... |
191 utils.system("svn update", ignore_status=True) | 174 utils.system("svn update", ignore_status=True) |
192 except error.CmdError, e: | 175 except error.CmdError, e: |
193 logging.error("SVN tree update failed: %s" % e) | 176 logging.error("SVN tree update failed: %s" % e) |
194 | 177 |
195 | 178 |
196 class FileChecker(object): | 179 class FileChecker(object): |
197 """ | 180 """ |
198 Picks up a given file and performs various checks, looking after problems | 181 Picks up a given file and performs various checks, looking after problems |
199 and eventually suggesting solutions. | 182 and eventually suggesting solutions. |
200 """ | 183 """ |
201 def __init__(self, path, confirm=False): | 184 def __init__(self, path): |
202 """ | 185 """ |
203 Class constructor, sets the path attribute. | 186 Class constructor, sets the path attribute. |
204 | 187 |
205 @param path: Path to the file that will be checked. | 188 @param path: Path to the file that will be checked. |
206 @param confirm: Whether to answer yes to all questions asked without | |
207 prompting the user. | |
208 """ | 189 """ |
209 self.path = path | 190 self.path = path |
210 self.confirm = confirm | |
211 self.basename = os.path.basename(self.path) | 191 self.basename = os.path.basename(self.path) |
212 if self.basename.endswith('.py'): | 192 if self.basename.endswith('.py'): |
213 self.is_python = True | 193 self.is_python = True |
214 else: | 194 else: |
215 self.is_python = False | 195 self.is_python = False |
216 | 196 |
217 mode = os.stat(self.path)[stat.ST_MODE] | 197 mode = os.stat(self.path)[stat.ST_MODE] |
218 if mode & stat.S_IXUSR: | 198 if mode & stat.S_IXUSR: |
219 self.is_executable = True | 199 self.is_executable = True |
220 else: | 200 else: |
221 self.is_executable = False | 201 self.is_executable = False |
222 | 202 |
223 checked_file = open(self.path, "r") | 203 checked_file = open(self.path, "r") |
224 self.first_line = checked_file.readline() | 204 self.first_line = checked_file.readline() |
225 checked_file.close() | 205 checked_file.close() |
226 self.corrective_actions = [] | 206 self.corrective_actions = [] |
227 self.indentation_exceptions = ['job_unittest.py'] | 207 self.indentation_exceptions = ['cli/job_unittest.py'] |
228 | 208 |
229 | 209 |
230 def _check_indent(self): | 210 def _check_indent(self): |
231 """ | 211 """ |
232 Verifies the file with reindent.py. This tool performs the following | 212 Verifies the file with reindent.py. This tool performs the following |
233 checks on python files: | 213 checks on python files: |
234 | 214 |
235 * Trailing whitespaces | 215 * Trailing whitespaces |
236 * Tabs | 216 * Tabs |
237 * End of line | 217 * End of line |
238 * Incorrect indentation | 218 * Incorrect indentation |
239 | 219 |
240 For the purposes of checking, the dry run mode is used and no changes | 220 For the purposes of checking, the dry run mode is used and no changes |
241 are made. It is up to the user to decide if he wants to run reindent | 221 are made. It is up to the user to decide if he wants to run reindent |
242 to correct the issues. | 222 to correct the issues. |
243 """ | 223 """ |
244 reindent_raw = utils.system_output('reindent.py -v -d %s | head -1' % | 224 reindent_raw = utils.system_output('reindent.py -v -d %s | head -1' % |
245 self.path) | 225 self.path) |
246 reindent_results = reindent_raw.split(" ")[-1].strip(".") | 226 reindent_results = reindent_raw.split(" ")[-1].strip(".") |
247 if reindent_results == "changed": | 227 if reindent_results == "changed": |
248 if self.basename not in self.indentation_exceptions: | 228 if self.basename not in self.indentation_exceptions: |
| 229 logging.error("Possible indentation and spacing issues on " |
| 230 "file %s" % self.path) |
249 self.corrective_actions.append("reindent.py -v %s" % self.path) | 231 self.corrective_actions.append("reindent.py -v %s" % self.path) |
250 | 232 |
251 | 233 |
252 def _check_code(self): | 234 def _check_code(self): |
253 """ | 235 """ |
254 Verifies the file with run_pylint.py. This tool will call the static | 236 Verifies the file with run_pylint.py. This tool will call the static |
255 code checker pylint using the special autotest conventions and warn | 237 code checker pylint using the special autotest conventions and warn |
256 only on problems. If problems are found, a report will be generated. | 238 only on problems. If problems are found, a report will be generated. |
257 Some of the problems reported might be bogus, but it's allways good | 239 Some of the problems reported might be bogus, but it's allways good |
258 to look at them. | 240 to look at them. |
259 """ | 241 """ |
260 c_cmd = 'run_pylint.py %s' % self.path | 242 c_cmd = 'run_pylint.py %s' % self.path |
261 rc = utils.system(c_cmd, ignore_status=True) | 243 rc = utils.system(c_cmd, ignore_status=True) |
262 if rc != 0: | 244 if rc != 0: |
263 logging.error("Syntax issues found during '%s'", c_cmd) | 245 logging.error("Possible syntax problems on file %s", self.path) |
| 246 logging.error("You might want to rerun '%s'", c_cmd) |
264 | 247 |
265 | 248 |
266 def _check_unittest(self): | 249 def _check_unittest(self): |
267 """ | 250 """ |
268 Verifies if the file in question has a unittest suite, if so, run the | 251 Verifies if the file in question has a unittest suite, if so, run the |
269 unittest and report on any failures. This is important to keep our | 252 unittest and report on any failures. This is important to keep our |
270 unit tests up to date. | 253 unit tests up to date. |
271 """ | 254 """ |
272 if "unittest" not in self.basename: | 255 if "unittest" not in self.basename: |
273 stripped_name = self.basename.strip(".py") | 256 stripped_name = self.basename.strip(".py") |
274 unittest_name = stripped_name + "_unittest.py" | 257 unittest_name = stripped_name + "_unittest.py" |
275 unittest_path = self.path.replace(self.basename, unittest_name) | 258 unittest_path = self.path.replace(self.basename, unittest_name) |
276 if os.path.isfile(unittest_path): | 259 if os.path.isfile(unittest_path): |
277 unittest_cmd = 'python %s' % unittest_path | 260 unittest_cmd = 'python %s' % unittest_path |
278 rc = utils.system(unittest_cmd, ignore_status=True) | 261 rc = utils.system(unittest_cmd, ignore_status=True) |
279 if rc != 0: | 262 if rc != 0: |
280 logging.error("Unittest issues found during '%s'", | 263 logging.error("Problems during unit test execution " |
281 unittest_cmd) | 264 "for file %s", self.path) |
| 265 logging.error("You might want to rerun '%s'", unittest_cmd) |
282 | 266 |
283 | 267 |
284 def _check_permissions(self): | 268 def _check_permissions(self): |
285 """ | 269 """ |
286 Verifies the execution permissions, specifically: | 270 Verifies the execution permissions, specifically: |
287 * Files with no shebang and execution permissions are reported. | 271 * Files with no shebang and execution permissions are reported. |
288 * Files with shebang and no execution permissions are reported. | 272 * Files with shebang and no execution permissions are reported. |
289 """ | 273 """ |
290 if self.first_line.startswith("#!"): | 274 if self.first_line.startswith("#!"): |
291 if not self.is_executable: | 275 if not self.is_executable: |
292 self.corrective_actions.append("svn propset svn:executable ON %s
" % self.path) | 276 logging.info("File %s seems to require execution " |
| 277 "permissions. ", self.path) |
| 278 self.corrective_actions.append("chmod +x %s" % self.path) |
293 else: | 279 else: |
294 if self.is_executable: | 280 if self.is_executable: |
295 self.corrective_actions.append("svn propdel svn:executable %s" %
self.path) | 281 logging.info("File %s does not seem to require execution " |
| 282 "permissions. ", self.path) |
| 283 self.corrective_actions.append("chmod -x %s" % self.path) |
296 | 284 |
297 | 285 |
298 def report(self): | 286 def report(self): |
299 """ | 287 """ |
300 Executes all required checks, if problems are found, the possible | 288 Executes all required checks, if problems are found, the possible |
301 corrective actions are listed. | 289 corrective actions are listed. |
302 """ | 290 """ |
303 self._check_permissions() | 291 self._check_permissions() |
304 if self.is_python: | 292 if self.is_python: |
305 self._check_indent() | 293 self._check_indent() |
306 self._check_code() | 294 self._check_code() |
307 self._check_unittest() | 295 self._check_unittest() |
308 if self.corrective_actions: | 296 if self.corrective_actions: |
| 297 logging.info("The following corrective actions are suggested:") |
309 for action in self.corrective_actions: | 298 for action in self.corrective_actions: |
310 answer = ask("Would you like to execute %s?" % action, | 299 logging.info(action) |
311 auto=self.confirm) | 300 answer = raw_input("Would you like to apply it? (y/n) ") |
312 if answer == "y": | 301 if answer == "y": |
313 rc = utils.system(action, ignore_status=True) | 302 rc = utils.system(action, ignore_status=True) |
314 if rc != 0: | 303 if rc != 0: |
315 logging.error("Error executing %s" % action) | 304 logging.error("Error executing %s" % action) |
316 | 305 |
317 | 306 |
318 class PatchChecker(object): | 307 class PatchChecker(object): |
319 def __init__(self, patch=None, patchwork_id=None, confirm=False): | 308 def __init__(self, patch=None, patchwork_id=None): |
320 self.confirm = confirm | |
321 self.base_dir = os.getcwd() | 309 self.base_dir = os.getcwd() |
322 if patch: | 310 if patch: |
323 self.patch = os.path.abspath(patch) | 311 self.patch = os.path.abspath(patch) |
324 if patchwork_id: | 312 if patchwork_id: |
325 self.patch = self._fetch_from_patchwork(patchwork_id) | 313 self.patch = self._fetch_from_patchwork(patchwork_id) |
326 | 314 |
327 if not os.path.isfile(self.patch): | 315 if not os.path.isfile(self.patch): |
328 logging.error("Invalid patch file %s provided. Aborting.", | 316 logging.error("Invalid patch file %s provided. Aborting.", |
329 self.patch) | 317 self.patch) |
330 sys.exit(1) | 318 sys.exit(1) |
331 | 319 |
332 self.vcs = VCS() | 320 self.vcs = VCS() |
333 changed_files_before = self.vcs.get_modified_files() | 321 changed_files_before = self.vcs.get_modified_files() |
334 if changed_files_before: | 322 if changed_files_before: |
335 logging.error("Repository has changed files prior to patch " | 323 logging.error("Repository has changed files prior to patch " |
336 "application. ") | 324 "application. ") |
337 answer = ask("Would you like to revert them?", auto=self.confirm) | 325 answer = raw_input("Would you like to revert them? (y/n) ") |
338 if answer == "n": | 326 if answer == "n": |
339 logging.error("Not safe to proceed without reverting files.") | 327 logging.error("Not safe to proceed without reverting files.") |
340 sys.exit(1) | 328 sys.exit(1) |
341 else: | 329 else: |
342 for changed_file in changed_files_before: | 330 for changed_file in changed_files_before: |
343 self.vcs.revert_file(changed_file) | 331 self.vcs.revert_file(changed_file) |
344 | 332 |
345 self.untracked_files_before = self.vcs.get_unknown_files() | 333 self.untracked_files_before = self.vcs.get_unknown_files() |
346 self.vcs.update() | 334 self.vcs.update() |
347 | 335 |
(...skipping 27 matching lines...) Expand all Loading... |
375 add_to_vcs = [] | 363 add_to_vcs = [] |
376 for untracked_file in untracked_files_after: | 364 for untracked_file in untracked_files_after: |
377 if untracked_file not in self.untracked_files_before: | 365 if untracked_file not in self.untracked_files_before: |
378 add_to_vcs.append(untracked_file) | 366 add_to_vcs.append(untracked_file) |
379 | 367 |
380 if add_to_vcs: | 368 if add_to_vcs: |
381 logging.info("The files: ") | 369 logging.info("The files: ") |
382 for untracked_file in add_to_vcs: | 370 for untracked_file in add_to_vcs: |
383 logging.info(untracked_file) | 371 logging.info(untracked_file) |
384 logging.info("Might need to be added to VCS") | 372 logging.info("Might need to be added to VCS") |
385 answer = ask("Would you like to add them to VCS ?") | 373 logging.info("Would you like to add them to VCS ? (y/n/abort) ") |
| 374 answer = raw_input() |
386 if answer == "y": | 375 if answer == "y": |
387 for untracked_file in add_to_vcs: | 376 for untracked_file in add_to_vcs: |
388 self.vcs.add_untracked_file(untracked_file) | 377 self.vcs.add_untracked_file(untracked_file) |
389 modified_files_after.append(untracked_file) | 378 modified_files_after.append(untracked_file) |
390 elif answer == "n": | 379 elif answer == "n": |
391 pass | 380 pass |
| 381 elif answer == "abort": |
| 382 sys.exit(1) |
392 | 383 |
393 for modified_file in modified_files_after: | 384 for modified_file in modified_files_after: |
394 # Additional safety check, new commits might introduce | 385 file_checker = FileChecker(modified_file) |
395 # new directories | 386 file_checker.report() |
396 if os.path.isfile(modified_file): | |
397 file_checker = FileChecker(modified_file) | |
398 file_checker.report() | |
399 | 387 |
400 | 388 |
401 def check(self): | 389 def check(self): |
402 self.vcs.apply_patch(self.patch) | 390 self.vcs.apply_patch(self.patch) |
403 self._check_files_modified_patch() | 391 self._check_files_modified_patch() |
404 | 392 |
405 | 393 |
406 if __name__ == "__main__": | 394 if __name__ == "__main__": |
407 parser = optparse.OptionParser() | 395 parser = optparse.OptionParser() |
408 parser.add_option('-p', '--patch', dest="local_patch", action='store', | 396 parser.add_option('-p', '--patch', dest="local_patch", action='store', |
409 help='path to a patch file that will be checked') | 397 help='path to a patch file that will be checked') |
410 parser.add_option('-i', '--patchwork-id', dest="id", action='store', | 398 parser.add_option('-i', '--patchwork-id', dest="id", action='store', |
411 help='id of a given patchwork patch') | 399 help='id of a given patchwork patch') |
412 parser.add_option('--verbose', dest="debug", action='store_true', | 400 parser.add_option('--verbose', dest="debug", action='store_true', |
413 help='include debug messages in console output') | 401 help='include debug messages in console output') |
414 parser.add_option('-f', '--full-check', dest="full_check", | |
415 action='store_true', | |
416 help='check the full tree for corrective actions') | |
417 parser.add_option('-y', '--yes', dest="confirm", | |
418 action='store_true', | |
419 help='Answer yes to all questions') | |
420 | 402 |
421 options, args = parser.parse_args() | 403 options, args = parser.parse_args() |
422 local_patch = options.local_patch | 404 local_patch = options.local_patch |
423 id = options.id | 405 id = options.id |
424 debug = options.debug | 406 debug = options.debug |
425 full_check = options.full_check | |
426 confirm = options.confirm | |
427 | 407 |
428 logging_manager.configure_logging(CheckPatchLoggingConfig(), verbose=debug) | 408 logging_manager.configure_logging(CheckPatchLoggingConfig(), verbose=debug) |
429 | 409 |
430 ignore_file_list = ['common.py'] | 410 if local_patch: |
431 if full_check: | 411 patch_checker = PatchChecker(patch=local_patch) |
432 for root, dirs, files in os.walk('.'): | 412 elif id: |
433 if not '.svn' in root: | 413 patch_checker = PatchChecker(patchwork_id=id) |
434 for file in files: | |
435 if file not in ignore_file_list: | |
436 path = os.path.join(root, file) | |
437 file_checker = FileChecker(path, confirm=confirm) | |
438 file_checker.report() | |
439 else: | 414 else: |
440 if local_patch: | 415 logging.error('No patch or patchwork id specified. Aborting.') |
441 patch_checker = PatchChecker(patch=local_patch, confirm=confirm) | 416 sys.exit(1) |
442 elif id: | 417 |
443 patch_checker = PatchChecker(patchwork_id=id, confirm=confirm) | 418 patch_checker.check() |
444 else: | |
445 logging.error('No patch or patchwork id specified. Aborting.') | |
446 sys.exit(1) | |
447 patch_checker.check() | |
OLD | NEW |