Add AffectedFile.IsTextFile() and deprecate AffectedTextFiles().

Make DepotToLocalPath() and LocalToDepotPath() not static. It is necessary for a subsequent change since I need to factor out it's SCM assumption.

Review URL: http://codereview.chromium.org/113933

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@17091 0039d316-1c4b-4281-b951-d872f2087c98

presubmit_support.py

@@ -27,6 +27,7 @@ import sys  # Parts exposed through API.
 import tempfile  # Exposed through the API.
 import types
 import urllib2  # Exposed through the API.
+import warnings
 
 # Local imports.
 # TODO(joi) Would be cleaner to factor out utils in gcl to separate module, but
@@ -54,6 +55,21 @@ def normpath(path):
   return os.path.normpath(path)
 
 
+def deprecated(func):
+  """This is a decorator which can be used to mark functions as deprecated.
+
+  It will result in a warning being emmitted when the function is used."""
+  def newFunc(*args, **kwargs):
+    warnings.warn("Call to deprecated function %s." % func.__name__,
+                  category=DeprecationWarning,
+                  stacklevel=2)
+    return func(*args, **kwargs)
+  newFunc.__name__ = func.__name__
+  newFunc.__doc__ = func.__doc__
+  newFunc.__dict__.update(func.__dict__)
+  return newFunc
+
+
 class OutputApi(object):
   """This class (more like a module) gets passed to presubmit scripts so that
   they can specify various types of results.
@@ -177,8 +193,7 @@ class InputApi(object):
     """
     return self._current_presubmit_path
 
-  @staticmethod
-  def DepotToLocalPath(depot_path):
+  def DepotToLocalPath(self, depot_path):
     """Translate a depot path to a local path (relative to client root).
 
     Args:
@@ -191,13 +206,10 @@ class InputApi(object):
       Remember to check for the None case and show an appropriate error!
     """
     local_path = gclient.CaptureSVNInfo(depot_path).get('Path')
-    if not local_path:
-      return None
-    else:
+    if local_path:
       return local_path
 
-  @staticmethod
-  def LocalToDepotPath(local_path):
+  def LocalToDepotPath(self, local_path):
     """Translate a local path to a depot path.
 
     Args:
@@ -207,9 +219,7 @@ class InputApi(object):
       The depot path (SVN URL) of the file if mapped, otherwise None.
     """
     depot_path = gclient.CaptureSVNInfo(local_path).get('URL')
-    if not depot_path:
-      return None
-    else:
+    if depot_path:
       return depot_path
 
   @staticmethod
@@ -260,6 +270,7 @@ class InputApi(object):
     """Returns server paths of input_api.AffectedFiles()."""
     return [af.ServerPath() for af in self.AffectedFiles(include_dirs)]
 
+  @deprecated
   def AffectedTextFiles(self, include_deletes=True):
     """Same as input_api.change.AffectedTextFiles() except only lists files
     in the same directory as the current presubmit script, or subdirectories
@@ -287,7 +298,8 @@ class InputApi(object):
         the contents of the line as a string.
     """
     return InputApi._RightHandSideLinesImpl(
-        self.AffectedTextFiles(include_deletes=False))
+        filter(lambda x: x.IsTextFile(),
+               self.AffectedFiles(include_deletes=False)))
 
   @staticmethod
   def _RightHandSideLinesImpl(affected_files):
@@ -348,6 +360,10 @@ class AffectedFile(object):
     """
     return self.properties.get(property_name, None)
 
+  def IsTextFile(self):
+    """Returns True if the file is a text file and not a binary file."""
+    raise NotImplementedError()  # Implement when needed
+
   def NewContents(self):
     """Returns an iterator over the lines in the new version of file.
 
@@ -405,6 +421,15 @@ class SvnAffectedFile(AffectedFile):
           self.AbsoluteLocalPath(), property_name)
     return self.properties[property_name]
 
+  def IsTextFile(self):
+    if self.Action() == 'D':
+      return False
+    mime_type = gcl.GetSVNFileProperty(self.AbsoluteLocalPath(),
+                                       'svn:mime-type')
+    if not mime_type or mime_type.startswith('text/'):
+      return True
+    return False
+
 
 class GclChange(object):
   """Describe a change.
@@ -496,6 +521,7 @@ class GclChange(object):
     else:
       return filter(lambda x: x.Action() != 'D', affected)
 
+  @deprecated
   def AffectedTextFiles(self, include_deletes=True):
     """Return a list of the text files in a change.
 
@@ -535,7 +561,8 @@ class GclChange(object):
         the contents of the line as a string.
     """
     return InputApi._RightHandSideLinesImpl(
-        self.AffectedTextFiles(include_deletes=False))
+        filter(lambda x: x.IsTextFile(),
+               self.AffectedFiles(include_deletes=False)))
 
 
 def ListRelevantPresubmitFiles(files):
@@ -574,6 +601,7 @@ class PresubmitExecuter(object):
       change_info: The ChangeInfo object for the change.
       committing: True if 'gcl commit' is running, False if 'gcl upload' is.
     """
+    # TODO(maruel): Determine the SCM.
     self.change = GclChange(change_info, gcl.GetRepositoryRoot())
     self.committing = committing
 

tests/presubmit_unittest.py

@@ -9,6 +9,7 @@ import os
 import StringIO
 import sys
 import unittest
+import warnings
 
 # Local imports
 import gcl
@@ -20,6 +21,8 @@ import presubmit_canned_checks
 class PresubmitTestsBase(unittest.TestCase):
   """Setups and tear downs the mocks but doesn't test anything as-is."""
   def setUp(self):
+    self._warnings_stack = warnings.catch_warnings()
+    warnings.simplefilter("ignore", DeprecationWarning)
     self.original_IsFile = os.path.isfile
     def MockIsFile(f):
       dir = os.path.dirname(f)
@@ -92,6 +95,7 @@ def CheckChangeOnUpload(input_api, output_api):
     gcl.ReadFile = self.original_ReadFile
     gcl.GetRepositoryRoot = self.original_GetRepositoryRoot
     sys.stdout = self._sys_stdout
+    self._warnings_stack = None
 
   @staticmethod
   def MakeBasicChange(name, description):
@@ -117,10 +121,11 @@ class PresubmitUnittest(PresubmitTestsBase):
       'AffectedFile', 'DoPresubmitChecks', 'GclChange', 'InputApi',
       'ListRelevantPresubmitFiles', 'Main', 'NotImplementedException',
       'OutputApi', 'ParseFiles', 'PresubmitExecuter',
-      'ScanSubDirs', 'SvnAffectedFile', 'cPickle', 'cStringIO', 'exceptions',
+      'ScanSubDirs', 'SvnAffectedFile',
+      'cPickle', 'cStringIO', 'deprecated', 'exceptions',
       'fnmatch', 'gcl', 'gclient', 'glob', 'marshal', 'normpath', 'optparse',
       'os', 'pickle', 'presubmit_canned_checks', 're', 'subprocess', 'sys',
-      'tempfile', 'types', 'urllib2',
+      'tempfile', 'types', 'urllib2', 'warnings',
     ]
     # If this test fails, you should add the relevant test.
     self.compareMembers(presubmit, members)
@@ -449,15 +454,16 @@ class InputApiUnittest(PresubmitTestsBase):
     self.compareMembers(presubmit.InputApi(None, './.'), members)
 
   def testDepotToLocalPath(self):
-    path = presubmit.InputApi.DepotToLocalPath('svn:/foo/smurf')
+    path = presubmit.InputApi(None, './p').DepotToLocalPath('svn:/foo/smurf')
     self.failUnless(path == 'smurf')
-    path = presubmit.InputApi.DepotToLocalPath('svn:/foo/notfound/burp')
+    path = presubmit.InputApi(None, './p').DepotToLocalPath(
+        'svn:/foo/notfound/burp')
     self.failUnless(path == None)
 
   def testLocalToDepotPath(self):
-    path = presubmit.InputApi.LocalToDepotPath('smurf')
+    path = presubmit.InputApi(None, './p').LocalToDepotPath('smurf')
     self.failUnless(path == 'svn:/foo/smurf')
-    path = presubmit.InputApi.LocalToDepotPath('notfound-food')
+    path = presubmit.InputApi(None, './p').LocalToDepotPath('notfound-food')
     self.failUnless(path == None)
 
   def testInputApiConstruction(self):
@@ -524,7 +530,7 @@ class InputApiUnittest(PresubmitTestsBase):
     for line in api.RightHandSideLines():
       rhs_lines.append(line)
     self.failUnless(len(rhs_lines) == 2)
-    self.failUnless(rhs_lines[0][0].LocalPath() ==
+    self.assertEqual(rhs_lines[0][0].LocalPath(),
                     presubmit.normpath('foo/blat.cc'))
 
   def testGetAbsoluteLocalPath(self):
@@ -623,7 +629,8 @@ class OuputApiUnittest(PresubmitTestsBase):
 class AffectedFileUnittest(PresubmitTestsBase):
   def testMembersChanged(self):
     members = [
-      'AbsoluteLocalPath', 'Action', 'IsDirectory', 'LocalPath', 'NewContents',
+      'AbsoluteLocalPath', 'Action', 'IsDirectory', 'IsTextFile',
+      'LocalPath', 'NewContents',
       'OldContents', 'OldFileTempPath', 'Property', 'ServerPath', 'action',
       'is_directory', 'path', 'properties', 'repository_root', 'server_path',
     ]