Histograms.xml python script housekeeping

tools/metrics/histograms/find_unmapped_histograms.py

Index: tools/metrics/histograms/find_unmapped_histograms.py
diff --git a/tools/metrics/histograms/find_unmapped_histograms.py b/tools/metrics/histograms/find_unmapped_histograms.py
index 31e41b5066fd2a4a4f5ac4b828af44b57dd743c1..a652c4b286bfdbdb78b6d17426ba8f91801cdaf1 100644
--- a/tools/metrics/histograms/find_unmapped_histograms.py
+++ b/tools/metrics/histograms/find_unmapped_histograms.py
@@ -10,15 +10,18 @@ trials are entirely ignored by this script.
 
 """
 
-import commands
-import extract_histograms
 import hashlib
 import logging
 import optparse
 import os
 import re
+import subprocess
 import sys
 
+sys.path.append(os.path.join(os.path.dirname(__file__), '..'))  # tools/metrics
+from common import path_util
+from histograms import extract_histograms
+
 
 ADJACENT_C_STRING_REGEX = re.compile(r"""
     ("      # Opening quotation mark
@@ -43,6 +46,17 @@ HISTOGRAM_REGEX = re.compile(r"""
     """, re.VERBOSE)
 
 
+def RunGit(command):
+  """Run a git subcommand, returning its output."""
+  # On Windows, use shell=True to get PATH interpretation.
+  command = ['git'] + command
+  logging.info(' '.join(command))
+  shell = (os.name == 'nt')
+  proc = subprocess.Popen(command, shell=shell, stdout=subprocess.PIPE)
+  out = proc.communicate()[0].strip()
+  return out
+
+
 class DirectoryNotFoundException(Exception):
   """Base class to distinguish locally defined exceptions from standard ones."""
   def __init__(self, msg):
@@ -52,27 +66,6 @@ class DirectoryNotFoundException(Exception):
     return self.msg
 
 
-def findDefaultRoot():
-  """Find the root of the chromium repo, in case the script is run from the
-  histograms dir.
-
-  Returns:
-    string: path to the src dir of the repo.
-
-  Raises:
-    DirectoryNotFoundException if the target directory cannot be found.
-  """
-  path = os.getcwd()
-  while path:
-    head, tail = os.path.split(path)
-    if tail == 'src':
-      return path
-    if path == head:
-      break
-    path = head
-  raise DirectoryNotFoundException('Could not find src/ dir')
-
-
 def collapseAdjacentCStrings(string):
   """Collapses any adjacent C strings into a single string.
 
@@ -130,7 +123,7 @@ def readChromiumHistograms():
   names that might vary during a single run of the app.
 
   Returns:
-    A set cotaining any found literal histogram names.
+    A set containing any found literal histogram names.
   """
   logging.info('Scanning Chromium source for histograms...')
 
@@ -138,7 +131,7 @@ def readChromiumHistograms():
   # Examples:
   #   'path/to/foo.cc:420:  UMA_HISTOGRAM_COUNTS_100("FooGroup.FooName",'
   #   'path/to/bar.cc:632:  UMA_HISTOGRAM_ENUMERATION('
-  locations = commands.getoutput('git gs UMA_HISTOGRAM').split('\n')
+  locations = RunGit(['gs', 'UMA_HISTOGRAM']).split('\n')
   filenames = set([location.split(':')[0] for location in locations])
 
   histograms = set()
@@ -152,7 +145,7 @@ def readChromiumHistograms():
       histogram = collapseAdjacentCStrings(histogram)
 
       # Must begin and end with a quotation mark.
-      if histogram[0] != '"' or histogram[-1] != '"':
+      if not histogram or histogram[0] != '"' or histogram[-1] != '"':

[Ilya Sherman, 2015/05/31 00:41:02]

Why did you need to add "not histogram" to this condition?  I'm not following
what change caused the need for that.

[ncarter (slow), 2015/06/01 17:04:12]

This comment caused this script to die because we got a null match in
HISTOGRAM_REGEX (the pattern matched but there was nothing in the capture group)
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/net...

The other way to fix this would be to change the * inside the first-parameter
capture group for HISTOGRAM_REGEX to be a +. [I considered trying to filter out
comment lines using lookaround but my regex foo isn't strong enough and it
seemed like a yak shave]

Let me know which you prefer.

[Ilya Sherman, 2015/06/01 20:25:45]

Okay, thanks for the explanation.  I was mostly just curious, and am fine with
either solution.  It sounds like Alexei would prefer to ignore comment lines
entirely.  Alexei, could you possibly help craft a regex that does this?  I'm
not sure offhand how to do that, and it sounds like Nick is also unsure.

         logNonLiteralHistogram(filename, histogram)
         continue
 
@@ -192,11 +185,11 @@ def hashHistogramName(name):
 
 def main():
   # Find default paths.
-  default_root = findDefaultRoot()
-  default_histograms_path = os.path.join(
-      default_root, 'tools/metrics/histograms/histograms.xml')
-  default_extra_histograms_path = os.path.join(
-      default_root, 'tools/histograms/histograms.xml')
+  default_root = path_util.GetInputFile('/')
+  default_histograms_path = path_util.GetInputFile(
+      'tools/metrics/histograms/histograms.xml')
+  default_extra_histograms_path = path_util.GetInputFile(
+      'tools/histograms/histograms.xml')
 
   # Parse command line options
   parser = optparse.OptionParser()