From 3c04c8317fc98aa64f9be18042a7255842e951c0 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Tue, 19 Sep 2017 10:10:38 -0400
Subject: [PATCH] Make check-spaces part of the standard "make check" process

To do this, we had to make sure it passes when the changes directory
is empty.  I also tried to improve the quality of the output, and
fix some false-positive cases.  Let's see how this goes!

Closes ticket 23564.
---
 Makefile.am                  |  4 +--
 scripts/maint/lintChanges.py | 60 +++++++++++++++++++++++++++---------
 ticket23564                  |  4 +++
 3 files changed, 51 insertions(+), 17 deletions(-)
 create mode 100644 ticket23564

diff --git a/Makefile.am b/Makefile.am
index 3982f1e8a4..7c454f79c6 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -96,7 +96,7 @@ doxygen:
 test: all
 	$(top_builddir)/src/test/test
 
-check-local: check-spaces
+check-local: check-spaces check-changes
 
 need-chutney-path:
 	@if test ! -d "$$CHUTNEY_PATH"; then \
@@ -217,7 +217,7 @@ check-logs:
 .PHONY: check-changes
 check-changes:
 	@if test -d "$(top_srcdir)/changes"; then \
-		$(PYTHON) $(top_srcdir)/scripts/maint/lintChanges.py $(top_srcdir)/changes/*; \
+		$(PYTHON) $(top_srcdir)/scripts/maint/lintChanges.py $(top_srcdir)/changes; \
 		fi
 
 .PHONY: update-versions
diff --git a/scripts/maint/lintChanges.py b/scripts/maint/lintChanges.py
index bf06064fa8..b418065ffb 100755
--- a/scripts/maint/lintChanges.py
+++ b/scripts/maint/lintChanges.py
@@ -20,8 +20,19 @@ KNOWN_GROUPS = set([
     "Testing",
     "Documentation",
     "Code simplification and refactoring",
-    "Removed features"])
+    "Removed features",
+    "Deprecated features"])
 
+NEEDS_SUBCATEGORIES = set([
+    "Minor bugfix",
+    "Minor bugfixes",
+    "Major bugfix",
+    "Major bugfixes",
+    "Minor feature",
+    "Minor features",
+    "Major feature",
+    "Major features",
+    ])
 
 def lintfile(fname):
     have_warned = []
@@ -46,13 +57,11 @@ def lintfile(fname):
 
     m = re.match(r'^[ ]{2}o ([^\(:]*)([^:]*):', contents)
     if not m:
-        warn("header not in format expected")
+        warn("Header not in format expected. ('  o Foo:' or '  o Foo (Bar):')")
     elif m.group(1).strip() not in KNOWN_GROUPS:
-        warn("Weird header: %r" % m.group(1))
-    elif (("bugfix" in m.group(1) or "feature" in m.group(1)) and
-          ("Removed" not in m.group(1)) and
-          '(' not in m.group(2)):
-        warn("Missing subcategory on %s" % m.group(1))
+        warn("Unrecognized header: %r" % m.group(1))
+    elif (m.group(1) in NEEDS_SUBCATEGORIES and '(' not in m.group(2)):
+        warn("Missing subcategory on %r" % m.group(1))
 
     if m:
         isBug = ("bug" in m.group(1).lower() or "fix" in m.group(1).lower())
@@ -62,25 +71,46 @@ def lintfile(fname):
     contents = " ".join(contents.split())
 
     if re.search(r'\#\d{2,}', contents):
-        warn("don't use a # before ticket numbers")
+        warn("Don't use a # before ticket numbers. ('bug 1234' not '#1234')")
 
     if isBug and not re.search(r'(\d+)', contents):
-        warn("bugfix does not mention a number")
+        warn("Ticket marked as bugfix, but does not mention a number.")
     elif isBug and not re.search(r'Fixes ([a-z ]*)bug (\d+)', contents):
-        warn("bugfix does not say 'Fixes bug XXX'")
+        warn("Ticket marked as bugfix, but does not say 'Fixes bug XXX'")
 
     if re.search(r'[bB]ug (\d+)', contents):
         if not re.search(r'[Bb]ugfix on ', contents):
-            warn("bugfix does not say 'bugfix on X.Y.Z'")
+            warn("Bugfix does not say 'bugfix on X.Y.Z'")
         elif not re.search('[fF]ixes ([a-z ]*)bug (\d+); bugfix on ',
                            contents):
-            warn("bugfix incant is not semicoloned")
+            warn("Bugfix does not say 'Fixes bug X; bugfix on Y'")
         elif re.search('tor-([0-9]+)', contents):
-            warn("do not prefix versions with 'tor-'")
+            warn("Do not prefix versions with 'tor-'. ('0.1.2', not 'tor-0.1.2'.)")
 
+    return have_warned != []
+
+def files(args):
+    """Walk through the arguments: for directories, yield their contents;
+       for files, just yield the files. Only search one level deep, because
+       that's how the changes directory is laid out."""
+    for f in args:
+        if os.path.isdir(f):
+            for item in os.listdir(f):
+                if item.startswith("."): #ignore dotfiles
+                    continue
+                yield os.path.join(f, item)
+        else:
+            yield f
 
 if __name__ == '__main__':
-    for fname in sys.argv[1:]:
+    problems = 0
+    for fname in files(sys.argv[1:]):
         if fname.endswith("~"):
             continue
-        lintfile(fname)
+        if lintfile(fname):
+            problems += 1
+
+    if problems:
+        sys.exit(1)
+    else:
+        sys.exit(0)
diff --git a/ticket23564 b/ticket23564
new file mode 100644
index 0000000000..c493caf359
--- /dev/null
+++ b/ticket23564
@@ -0,0 +1,4 @@
+  o Minor features (build):
+    - The "check-changes" feature is now part of the "make check"
+      tests; we'll use it to try to prevent misformed changes files
+      from accumulating.  Closes ticket 23564.