Add repo-managed checkout support to trychange.py

trychange.py

 #!/usr/bin/env python
 # Copyright (c) 2011 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """Client-side script to send a try job to the try server. It communicates to
 the try server by either writting to a svn repository or by directly connecting
 to the server by HTTP.
 """
 
@@ skipping 73 matching lines @@
     return self.args[0] + '\n' + HELP_STRING
 
 
 def Escape(name):
   """Escapes characters that could interfere with the file system or try job
   parsing.
   """
   return re.sub(r'[^\w#-]', '_', name)
 
 
+def _FindRepoCheckoutRoot(path):
+  """Returns the nearest higher-level repo dir from the specified path.
+
+  Args:
+    path: The path to use. Defaults to cwd.
+  """
+  path = os.path.abspath(path)
+  while path != '/':

[M-A Ruel, 2011/10/10 18:16:06]

I'd like it to be windows-compatible, so I'd use something like
gclient_utils.FindFileUpwards('.repo') instead.

[rcui1, 2011/10/11 19:42:34]

Done.

+    if os.path.isdir(os.path.join(path, '.repo')):
+      return path
+    path = os.path.dirname(path)
+  return None
+
+
 class SCM(object):
   """Simplistic base class to implement one function: ProcessOptions."""
   def __init__(self, options, path, file_list):
     items = path.split('@')
     assert len(items) <= 2
-    self.checkout_root = items[0]
+    self.checkout_root = os.path.abspath(items[0])
     items.append(None)
     self.diff_against = items[1]
     self.options = options
     # Lazy-load file list from the SCM unless files were specified in options.
     self._files = None
     self._file_tuples = None
     if file_list:
       self._files = file_list
       self._file_tuples = [('M', f) for f in self.files]
     self.options.files = None
     self.codereview_settings = None
     self.codereview_settings_file = 'codereview.settings'
-    self.gclient_root = None
+    self.toplevel_root = None
 
   def GetFileNames(self):
     """Return the list of files in the diff."""
     return self.files
 
   def GetCodeReviewSetting(self, key):
     """Returns a value for the given key for this repository.
 
     Uses gcl-style settings from the repository.
     """
@@ skipping 22 matching lines @@
       'project': self.GetCodeReviewSetting('TRYSERVER_PROJECT'),
       'root': self.GetCodeReviewSetting('TRYSERVER_ROOT'),
       'patchlevel': self.GetCodeReviewSetting('TRYSERVER_PATCHLEVEL'),
     }
     logging.info('\n'.join(['%s: %s' % (k, v)
                             for (k, v) in settings.iteritems() if v]))
     for (k, v) in settings.iteritems():
       if v and getattr(self.options, k) is None:
         setattr(self.options, k, v)
 
-  def _GclientStyleSettings(self):
-    """Find the root, assuming a gclient-style checkout."""
-    if not self.options.no_gclient and not self.options.root:
-      root = self.checkout_root
-      self.gclient_root = gclient_utils.FindGclientRoot(root)
-      if self.gclient_root:
-        logging.info('Found .gclient at %s' % self.gclient_root)
-        self.options.root = gclient_utils.PathDifference(self.gclient_root,
-                                                         root)
-
   def AutomagicalSettings(self):
     """Determines settings based on supported code review and checkout tools.
     """
-    self._GclientStyleSettings()
+    # If user did not provide toplevel root, try to find gclient root first.
+    if not self.toplevel_root and not self.options.no_gclient:
+      self.toplevel_root = gclient_utils.FindGclientRoot(self.checkout_root)
+      if self.toplevel_root:
+        logging.info('Found .gclient at %s' % self.toplevel_root)

[M-A Ruel, 2011/10/10 18:16:06]

If you want, add:
assert os.path.abspath(self.toplevel_root) == self.toplevel_root

[rcui1, 2011/10/11 19:42:34]

Done.

+
+    # If still no toplevel root found, try to find repo-managed chromium root
+    if not self.toplevel_root:
+      repo_root = _FindRepoCheckoutRoot(self.checkout_root)
+      if repo_root:
+        chromium_root = os.path.join(repo_root,

[M-A Ruel, 2011/10/10 18:16:06]

weird alignment, also I've refrained from hardcoding path like 'chromium' and
I'd rather to keep it this way.

Just generalize to:
1. use override if set.
2. find .gclient
3. find .repo

[rcui1, 2011/10/11 19:42:34]

Done.

+                                     'chromium')
+        if self.checkout_root.startswith(chromium_root):
+          self.toplevel_root = chromium_root
+          logging.info('Found chromium checkout root at %s'
+                       % self.toplevel_root)
+
+    if self.toplevel_root and not self.options.root:
+      self.options.root = gclient_utils.PathDifference(self.toplevel_root,
+                                                       self.checkout_root)
+
     self._GclStyleSettings()
 
   def ReadRootFile(self, filename):
-    if not self.options.root:
-      filepath = os.path.join(self.checkout_root, filename)
-      if os.path.isfile(filepath):
-        logging.info('Found %s at %s' % (filename, self.checkout_root))
-        return gclient_utils.FileRead(filepath)
-      return None
-    cur = os.path.abspath(self.checkout_root)
-    if self.gclient_root:
-      root = os.path.abspath(self.gclient_root)
-    else:
-      root = gclient_utils.FindGclientRoot(cur)
-    if not root:
-      root = cur
+    root = cur = os.path.abspath(self.checkout_root)

[M-A Ruel, 2011/10/10 18:16:06]

self.checkout_root is guaranteed to be abspath so no need to do it again.

[rcui1, 2011/10/11 19:42:34]

Done.

+    if self.toplevel_root:
+      root = os.path.abspath(self.toplevel_root)

[M-A Ruel, 2011/10/10 18:16:06]

If you also abspath toplevel_root, then you can do:
root = self.toplevel_root or self.checkout_root

[rcui1, 2011/10/11 19:42:34]

I think the logic is pretty clear as it is.

On 2011/10/10 18:16:06, Marc-Antoine Ruel wrote:

+
     assert cur.startswith(root), (root, cur)
     while cur.startswith(root):
       filepath = os.path.join(cur, filename)
       if os.path.isfile(filepath):
         logging.info('Found %s at %s' % (filename, cur))
         return gclient_utils.FileRead(filepath)
       cur = os.path.dirname(cur)
     logging.warning('Didn\'t find %s' % filename)
     return None
 
@@ skipping 117 matching lines @@
   if options.patchlevel:
     values['patchlevel'] = options.patchlevel
   if options.issue:
     values['issue'] = options.issue
   if options.patchset:
     values['patchset'] = options.patchset
   if options.target:
     values['target'] = options.target
   if options.project:
     values['project'] = options.project
+

[M-A Ruel, 2011/10/10 18:16:06]

bof, I'd revert this line

[rcui1, 2011/10/11 19:42:34]

Done.

   return values
 
 
 def _SendChangeHTTP(options):
   """Send a change to the try server using the HTTP protocol."""
   if not options.host:
     raise NoTryServerAccess('Please use the --host option to specify the try '
         'server host to connect to.')
   if not options.port:
     raise NoTryServerAccess('Please use the --port option to specify the try '
@@ skipping 477 matching lines @@
     return 1
   except (gclient_utils.Error, subprocess2.CalledProcessError), e:
     print >> sys.stderr, e
     return 1
   return 0
 
 
 if __name__ == "__main__":
   fix_encoding.fix_encoding()
   sys.exit(TryChange(None, None, False))