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

Side by Side Diff: utils/check_patch.py

Issue 6124004: Revert "Merge remote branch 'cros/upstream' into autotest-rebase" (Closed) Base URL: ssh://git@gitrw.chromium.org:9222/autotest.git@master
Patch Set: Created 9 years, 11 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
« no previous file with comments | « tko/parsers/version_1_unittest.py ('k') | utils/external_packages.py » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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, 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
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
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
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()
OLDNEW
« no previous file with comments | « 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