diff --git a/util/git-pre-commit.py b/util/git-pre-commit.py index 480bba1c9..276e21fc5 100755 --- a/util/git-pre-commit.py +++ b/util/git-pre-commit.py @@ -37,7 +37,9 @@ # # Authors: Andreas Sandberg +from tempfile import TemporaryFile import os +import subprocess import sys from style.repo import GitRepo @@ -62,6 +64,7 @@ ui = StdioUI() os.chdir(repo_base) failing_files = set() +staged_mismatch = set() for status, fname in git.status(filter="MA", cached=True): if args.verbose: @@ -73,18 +76,41 @@ for status, fname in git.status(filter="MA", cached=True): else: regions = all_regions + # Show they appropriate object and dump it to a file + status = git.file_from_index(fname) + f = TemporaryFile() + f.write(status) + verifiers = [ v(ui, opts, base=repo_base) for v in all_verifiers ] for v in verifiers: - if not v.skip(fname) and v.check(fname, regions): + f.seek(0) + # It is prefered that the first check is silent as it is in the + # staged file. If the check fails, then we will do it non-silently + # on the current file, reporting meaningful shortcomings + if not v.skip(fname) and v.check(fname, regions, fobj=f, silent=True): failing_files.add(fname) + if not v.check(fname, regions): + staged_mismatch.add(fname) + f.close() if failing_files: - print >> sys.stderr - print >> sys.stderr, "Style checker failed for the following files:" - for f in failing_files: - print >> sys.stderr, "\t%s" % f - print >> sys.stderr - print >> sys.stderr, \ + if len(failing_files) > len(staged_mismatch): + print >> sys.stderr + print >> sys.stderr, "Style checker failed for the following files:" + for f in failing_files: + if f not in staged_mismatch: + print >> sys.stderr, "\t%s" % f + print >> sys.stderr + print >> sys.stderr, \ "Please run the style checker manually to fix the offending files.\n" \ "To check your modifications, run: util/style.py -m" + + print >> sys.stderr + if staged_mismatch: + print >> sys.stderr, \ + "It looks like you have forgotten to stage your fixes for commit in\n"\ + "the following files: " + for f in staged_mismatch: + print >> sys.stderr, "\t%s" % f + print >> sys.stderr, "Please `git --add' them" sys.exit(1) diff --git a/util/style/verifiers.py b/util/style/verifiers.py index 6d7d581ea..1be453601 100644 --- a/util/style/verifiers.py +++ b/util/style/verifiers.py @@ -192,9 +192,20 @@ class Verifier(object): return False @abstractmethod - def check(self, filename, regions=all_regions): + def check(self, filename, regions=all_regions, fobj=None, silent=False): """Check specified regions of file 'filename'. + Given that it is possible that the current contents of the file + differ from the file as 'staged to commit', for those cases, and + maybe others, the argument fobj should be a file object open and reset + with the contents matching what the file would look like after the + commit. This is needed keep the messages using 'filename' meaningful. + + The argument silent is useful to prevent output when we run check in + the staged file vs the actual file to detect if the user forgot + staging fixes to the commit. This way, we prevent reporting errors + twice in stderr. + Line-by-line checks can simply provide a check_line() method that returns True if the line is OK and False if it has an error. Verifiers that need a multi-line view (like @@ -216,24 +227,29 @@ class Verifier(object): pass class LineVerifier(Verifier): - def check(self, filename, regions=all_regions): - f = self.open(filename, 'r') + def check(self, filename, regions=all_regions, fobj=None, silent=False): + close = False + if fobj is None: + fobj = self.open(filename, 'r') + close = True lang = lang_type(filename) assert lang in self.languages errors = 0 - for num,line in enumerate(f): + for num,line in enumerate(fobj): if num not in regions: continue line = line.rstrip('\n') if not self.check_line(line, language=lang): - self.ui.write("invalid %s in %s:%d\n" % \ - (self.test_name, filename, num + 1)) - if self.ui.verbose: - self.ui.write(">>%s<<\n" % line[:-1]) + if not silent: + self.ui.write("invalid %s in %s:%d\n" % \ + (self.test_name, filename, num + 1)) + if self.ui.verbose: + self.ui.write(">>%s<<\n" % line[:-1]) errors += 1 - f.close() + if close: + fobj.close() return errors @safefix @@ -329,12 +345,16 @@ class SortedIncludes(Verifier): super(SortedIncludes, self).__init__(*args, **kwargs) self.sort_includes = sort_includes.SortIncludes() - def check(self, filename, regions=all_regions): - f = self.open(filename, 'r') + def check(self, filename, regions=all_regions, fobj=None, silent=False): + close = False + if fobj is None: + fobj = self.open(filename, 'r') + close = True norm_fname = self.normalize_filename(filename) - old = [ l.rstrip('\n') for l in f.xreadlines() ] - f.close() + old = [ l.rstrip('\n') for l in fobj.xreadlines() ] + if close: + fobj.close() if len(old) == 0: return 0 @@ -345,10 +365,12 @@ class SortedIncludes(Verifier): modified = _modified_regions(old, new) & regions if modified: - self.ui.write("invalid sorting of includes in %s\n" % (filename)) - if self.ui.verbose: - for start, end in modified.regions: - self.ui.write("bad region [%d, %d)\n" % (start, end)) + if not silent: + self.ui.write("invalid sorting of includes in %s\n" + % (filename)) + if self.ui.verbose: + for start, end in modified.regions: + self.ui.write("bad region [%d, %d)\n" % (start, end)) return 1 return 0