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