Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(371)

Side by Side Diff: utils/check_patch.py

Issue 6246035: Merge remote branch 'cros/upstream' into master (Closed) Base URL: ssh://git@gitrw.chromium.org:9222/autotest.git@master
Patch Set: patch Created 9 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch | Annotate | Revision Log
OLDNEW
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
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
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
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()
OLDNEW
« cli/job.py ('K') | « tko/parsers/version_1_unittest.py ('k') | utils/external_packages.py » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698