| 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 |