Create "git cache" command.

git_cache.py

+#!/usr/bin/env python
+# Copyright 2014 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.
+
+"""A git command for managing a local cache of git repositories."""
+
+import errno
+import logging
+import optparse
+import os
+import subprocess
+import sys
+
+import gclient_utils
+import subcommand
+
+
+def DieWithError(message):
+  print >> sys.stderr, message
+  sys.exit(1)
+
+
+def UrlToCacheDir(url):
+  """Converts a git url to a normalized form for the cache dir path."""
+  idx = url.find('://')

[hinoka, 2014/02/13 23:01:29]

szager mentioned this in a different CL, but lets use urlparse for url
parsing/segmentation.

[agable, 2014/02/14 00:28:31]

Done.

+  if idx != -1:
+    url = url[idx+3:]
+  if not url.endswith('.git'):
+    url += '.git'
+  return url.replace('-', '--').replace('/', '-')
+
+
+def RunGit(cmd, **kwargs):
+  """Runs git in a subprocess."""
+  kwargs.setdefault('cwd', os.getcwd)

[hinoka, 2014/02/13 23:01:29]

os.getcwd()

[agable, 2014/02/14 00:28:31]

Done.

+  if kwargs.get('filter_fn'):
+    kwargs['filter_fn'] = gclient_utils.GitFilter(kwargs.get('filter_fn'))
+    kwargs.setdefault('print_stdout', False)
+    env = kwargs.get('env') or kwargs.setdefault('env', os.environ.copy())
+    env.setdefault('GIT_ASKPASS', 'true')
+    env.setdefault('SSH_ASKPASS', 'true')
+  else:
+    kwargs.setdefault('print_stdout', True)
+  stdout = kwargs.get('stdout', sys.stdout)
+  stdout.write(

[hinoka, 2014/02/13 23:01:29]

print >>stdout, "..." ?
\n usage might be flaky on win

[agable, 2014/02/19 02:25:51]

Done.

+      'running \'git %s\' in \'%s\'\n' % (' '.join(cmd), kwargs['cwd']))
+  gclient_utils.CheckCallAndFilter(['git'] + cmd, **kwargs)
+
+
+class LockError(Exception):
+  pass
+
+
+class Lockfile(object):
+  """Class to represent a cross-platform process-specific lockfile."""
+  def __init__(self, path):
+    self.path = os.path.abspath(path)
+    self.lockfile = self.path + ".lock"
+    self.pid = os.getpid()
+
+  def _read_pid(self):

[iannucci, 2014/02/14 01:43:08]

This method only really makes sense when stealing. Otherwise:
  * You don't have the lock and you can't trust the value you get from this
function
  * You DO have the lock, and so you know pid == self.pid

[agable, 2014/02/19 02:25:51]

I seriously considered having _read_pid() and steal() lock the lockfile. But
decided that was a bit too meta and we didn't care enough.

This method isn't meant to be 100% correct. It's used for informational purposes
when stealing to print the pid of the old locking process (or eventually check
if that process is still running, if windows cooperates).

Leaving as is, with updated docstring.

+    """Reads the pid stored in the lockfile."""
+    try:
+      with open(self.lockfile, 'r') as f:
+        pid = int(f.readline().strip())
+    except (IOError, ValueError):
+      pid = None
+    return pid
+
+  def _make_lockfile(self):
+    """Safely creates a lockfile containing the current pid."""
+    open_flags = (os.O_CREAT | os.O_EXCL | os.O_WRONLY)
+    open_mode = 0o644

[iannucci, 2014/02/14 01:43:08]

Not sure what the value in having this be a extra variable is. It's pretty
obviously an open mode.

[agable, 2014/02/19 02:25:51]

Done.

+    fd = os.open(self.lockfile, open_flags, open_mode)
+    f = os.fdopen(fd, 'w')

[hinoka, 2014/02/13 23:01:29]

with os.fdopen(fd, 'w') as f: ?

[agable, 2014/02/14 00:28:31]

AFAIK only builtin open() is a context manager, os.fdopen isn't.

[iannucci, 2014/02/14 01:43:08]

and also
print >> f, self.pid

:)

[agable, 2014/02/19 02:25:51]

Done.

+    f.write('%s\n' % self.pid)
+    f.close()
+
+  def _remove_lockfile(self):
+    """Deletes the lockfile. Does nothing if it doesn't exist."""
+    try:
+      os.remove(self.lockfile)
+    except OSError as exc:
+      if exc.errno == errno.ENOENT:
+        pass
+      else:
+        raise
+
+  def lock(self):
+    """Acquire the lock."""
+    try:
+      self._make_lockfile()
+    except OSError as e:
+      if e.errno == errno.EEXIST:
+        raise LockError("%s is already locked" % self.path)
+      else:
+        raise LockError("failed to create %s" % self.path)

[hinoka, 2014/02/13 23:01:29]

s/failed/Failed/
Also might as well include e.errno in the message.

[agable, 2014/02/14 00:28:31]

Done.

+
+  def unlock(self):
+    """Release the lock."""
+    if not self.is_locked():
+      raise LockError("%s is not locked" % self.path)
+    if not self.i_am_locking():
+      raise LockError("%s is locked, but not by me" % self.path)
+    self._remove_lockfile()
+
+  def steal(self):

[hinoka, 2014/02/13 23:01:29]

This isn't used anywhere? Need a CMDwipeoutallthelocks() for bothammer to call

[agable, 2014/02/14 00:28:31]

Heh. Yeah, adding CMDmineallmine() in this patchset.

+    """Break a lock and replace it with one of our own.
+
+    Note that this is potentially racy (someone else could put a lockfile in
+    place between breaking the lock and placing our own). This is fine,
+    since we only want to use this method when it is completely safe.
+    """
+    old_pid = self._read_pid()
+    if old_pid:
+      print 'Breaking lock on %s left behind by process %d' % (
+          self.path, old_pid)
+    self._remove_lockfile()
+    self._make_lockfile()
+
+  def is_locked(self):
+    """Test if the file is locked by anyone."""

[iannucci, 2014/02/14 01:43:08]

note, this is racy...

P1: lock
P2: is_locked?
P1: stuff
P1: unlock
P2: do stuff assuming it's locked  # <- :(

The only safe way to tell if it's locked is if YOU have the lock (i.e. the only
answers are "yes" and "maybe")

[agable, 2014/02/19 02:25:51]

I think you mean "yes" and "ask me again later" :P

But anyway, not really another way to do it. Noted in docstring.

+    return os.path.exists(self.lockfile)
+
+  def i_am_locking(self):
+    """Test if the file is locked by this process."""
+    return self.is_locked() and self.pid == self._read_pid()

[iannucci, 2014/02/14 01:43:08]

This should just be True iff make_lockfile didn't throw an exception.

[agable, 2014/02/19 02:25:51]

...and no one else has stolen the lock. Checking the pid is important.

+
+  def __enter__(self):
+    self.lock()
+    return self
+
+  def __exit__(self, *_exc):
+    self.unlock()
+
+
+@subcommand.usage('[url of repo to check for caching]')
+def CMDexists(parser, args):
+  """Checks to see if there already is a cache of the given repo."""

[hinoka, 2014/02/13 23:01:29]

Whats the purpose of this?  It looks like it returns:

0 (OK): Path exists
1 (FAIL): Path does not exist, or you passed in the wrong arguments.

If this is meant to be called externally, i'd prefer if it returns 0 for path
exists/not exists, and vary stdout accordingly, rather than exit with a failure
for not existing, otherwise its easy to conflate a "Path does not exist' signal
with a "The script failed unexpectedly" signal.

Just my opinion though, I realize most bash commands do the same thing.

[agable, 2014/02/14 00:28:31]

Its purpose is to test for existence, and print the path to the cache if it
exists (so that other things can use this to look up cache locations and e.g.
perform clones from there).

Good point about the return code, it will always return 0 on success, even if
that success is "doesn't exist".

[iannucci, 2014/02/14 01:43:08]

I disagree a bit. I think: 0 exists, 1 DNE, -1 bad args

[agable, 2014/02/19 02:25:51]

parser.error() automatically generates retcode 2 for bad args (not -1).

Upon further consideration, I'd like to keep non-existence as a non-zero return
code so that client code can 
try:
  cache_path = subprocess.check_output(['git', 'cache', 'exists', 'foo'])
except subprocess.CalledProcessError:
  # oh snap son, you done goofed

+  options, args = parser.parse_args(args)
+  if not len(args) == 1:
+    DieWithError('git cache exists only takes exactly one repo url.')

[hinoka, 2014/02/13 23:01:29]

lets use parser.error() instead

[agable, 2014/02/14 00:28:31]

Done.

+  url = args[0]
+  repo_dir = os.path.join(options.cache_dir, UrlToCacheDir(url))
+  flag_file = os.path.join(repo_dir, 'config')
+  if os.path.isdir(repo_dir) and os.path.isfile(flag_file):
+    print repo_dir
+    return 0
+  return 1
+
+
+@subcommand.usage('[url of repo to add to or update in cache]')
+def CMDpopulate(parser, args):
+  """Bare clones or updates a repository in the cache."""
+  parser.add_option('--local',
+      help='local repository to initialize from')

[hinoka, 2014/02/13 23:01:29]

nit: align at paren.  Only align at 4 spaces if first line is empty, eg
parse.add_option(
    '--local', ....)

[agable, 2014/02/14 00:28:31]

Done.

[iannucci, 2014/02/14 01:43:08]

should we auto-default to the current repo, if there is one?

[agable, 2014/02/19 02:25:51]

What does git clone do if --reference points to an object store that is
completely irrelevant? I'd prefer to keep it explicit -- none of the 'git cache'
commands so far have any dependency on being run from inside a git repo, so I'm
not really in favor of adding a secret default.

+  options, args = parser.parse_args(args)
+  if not len(args) == 1:
+    DieWithError('git cache populate only takes exactly one repo url.')

[hinoka, 2014/02/13 23:01:29]

lets use parser.error() instead

[agable, 2014/02/14 00:28:31]

Done.

+  url = args[0]
+
+  gclient_utils.safe_makedirs(options.cache_dir)
+  repo_dir = os.path.join(options.cache_dir, UrlToCacheDir(url))
+
+  # If we've been supplied with a local repository to help out,
+  # make sure that it is a full direct clone before relying on it.
+  local_objects = local_altfile = ''
+  if options.local:
+    local_objects = os.path.join(
+        os.path.abspath(options.local), '.git', 'objects')
+    local_altfile = os.path.join(local_objects, 'info', 'alternates')
+  use_reference = (
+      os.path.exists(local_objects) and not os.path.exists(local_altfile))
+  altfile = os.path.join(repo_dir, 'objects', 'info', 'alternates')
+
+  v = []
+  filter_fn = lambda l: '[up to date]' not in l
+  if options.verbose:
+    v = ['-v']
+    filter_fn = None
+
+  with Lockfile(repo_dir):

[hinoka, 2014/02/13 23:01:29]

"with Lock" generally means "wait for this lock", but the code suggests that it
fails if the lock is present.

I think this should be
(a):
if Lockfile.is_locked(repo_dir):
  DieWithError('%s is locked by someone else' % repo_dir)

(b):
Have "with Lockfile" be blocking.

(c): Add a comment saying its a non-blocking fail-fast lock and be done with it.

[agable, 2014/02/14 00:28:31]

Yeah, we already talked about this, but replying here for posterity:

Agreed about the semantics. Would be better to have it nicely wait until the
lock becomes available. But definitely don't want to have it always wait
arbitrarily long, since sometimes there mere existence of a previous lock is an
indicator of error. And finding a good default timeout for waiting for the lock
is hard, since some repos take 2 seconds to clone and some take 2 hours.

So I'm going to leave this as-is until Robbie pitches in and graces us with his
wisdom.

[iannucci, 2014/02/14 01:43:08]

Racy
Maybe, but you'd need a very different style of file lock for this to work
(http://linux.die.net/man/2/flock)
Probably the best option for now. Eventually we should aim to move to B, because
we can't guarantee that DEPS authors won't do silly things (unless we're relying
on in-process locks to prevent this i.e. in gclient/deps2git/botupdate)

[agable, 2014/02/19 02:25:51]

Yep.
Yeah, I spent a while trying to do this in a way that would leverage flock on
windows and some other api on windows. Turns out, a lot of other people have
done that already. And they're all pretty disappointing implementations, and
they all rely on win32 apis that we don't have installed on any of our windows
systems. So that was a show stopper.
Doing this.

+    # Do a full clone if the repo is new or is in a bad state.
+    if not os.path.exists(os.path.join(repo_dir, 'config')):
+      gclient_utils.rmtree(repo_dir)
+      cmd = ['clone'] + v + ['-c', 'core.deltaBaseCacheLimit=2g',
+                             '--progress', '--bare']
+
+      if use_reference:
+        cmd += ['--reference', os.path.abspath(options.local)]
+
+      RunGit(cmd + [url, repo_dir],
+             filter_fn=filter_fn, cwd=options.cache_dir, retry=True)
+
+    else:
+      if use_reference:
+        with open(altfile, 'w') as f:
+          f.write(os.path.abspath(local_objects))
+
+      RunGit(['fetch'] + v + ['--multiple', '--progress', '--all'],
+             filter_fn=filter_fn, cwd=repo_dir, retry=True)
+
+    # If the clone has an object dependency on the local repo, break it
+    # with repack and remove the linkage.

[iannucci, 2014/02/14 01:43:08]

may consider an optimization:
  * if the local repo only contains refs which are <= the mirror refs
  * add alternates file, nuke local objects/packfiles

repack is a pretty pricey operation.

[agable, 2014/02/19 02:25:51]

I think not worth it. In addition, repack -ad is pretty much *the* canonical way
to detach a repo from anywhere it has alternates pointing, so I'd rather stick
to that.

+    if os.path.exists(altfile):
+      RunGit(['repack', '-a'], cwd=repo_dir)
+      os.remove(altfile)
+
+
+class OptionParser(optparse.OptionParser):
+  """Wrapper class for OptionParser to handle global options."""
+  def __init__(self, *args, **kwargs):
+    optparse.OptionParser.__init__(self, *args, prog='git cache', **kwargs)
+    self.add_option('-c', '--cache-dir',
+        help='Path to the directory containing the cache.')
+    self.add_option('-v', '--verbose', action='count', default=0,
+        help='Increase verbosity (can be passed multiple times).')
+
+  def parse_args(self, args=None, values=None):
+    options, args = optparse.OptionParser.parse_args(self, args, values)
+
+    try:
+      global_cache_dir = subprocess.check_output(
+          ['git', 'config', '--global', 'cache.cachepath']).strip()
+      if options.cache_dir:

[hinoka, 2014/02/13 23:01:29]

The if/else doesn't need to be in the try block.

[agable, 2014/02/14 00:28:31]

Well, it kinda does. I can move it out but then I have to predeclare
global_cache_dir as None and add "and if global_cache_dir' to the if-clause. I
think this is more logical:
It's a 2x2 grid of global exists/not vs options exists/not. The global exists
case is in the try, the global doesn't exist case is in the except.

+        logging.warn('Overriding globally-configured cache directory.')
+      else:
+        options.cache_dir = global_cache_dir
+    except subprocess.CalledProcessError:
+      if not options.cache_dir:
+        DieWithError('No cache directory specified on command line '

[hinoka, 2014/02/13 23:01:29]

lets use parser.error() instead

[agable, 2014/02/14 00:28:31]

Done.

+                     'or in cache.cachepath.')
+    options.cache_dir = os.path.abspath(options.cache_dir)
+
+    levels = [logging.WARNING, logging.INFO, logging.DEBUG]
+    logging.basicConfig(level=levels[min(options.verbose, len(levels) - 1)])
+
+    return options, args
+
+
+def main(argv):
+  dispatcher = subcommand.CommandDispatcher(__name__)
+  return dispatcher.execute(OptionParser(), argv)
+
+
+if __name__ == '__main__':
+  sys.exit(main(sys.argv[1:]))