Allow 'git cache fetch' to re-bootstrap.

Allow 'git cache fetch' to re-bootstrap.

With this change, 'git cache fetch' will automatically re-bootstrap
a repo from google storage if the local cache has too many pack
files.  The behavior can be disabled with the --no-bootstrap flag.

BUG=

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=296829

[szager1, 2015-09-08 20:12:25]

szager@chromium.org changed reviewers:
+ hinoka@chromium.org

[szager1, 2015-09-08 20:16:22]

This is intended to make it easier for developers to use git cache.  With this
change, 'git cache fetch' will automatically re-bootstrap from google storage
when there are too many pack files in the existing mirror.  When it does
re-bootstrap, it will preserve any custom fetch specs that may exists in the
existing mirror (e.g., refs/branch-heads/*).

[szager1, 2015-09-22 20:24:47]

szager@chromium.org changed reviewers:
+ agable@chromium.org, iannucci@chromium.org

[szager1, 2015-09-22 20:25:27]

ping, adding more reviewers.

With the blink merge imminent, it makes a lot of sense for developers to use a
git cache.

[iannucci, 2015-09-22 20:35:26]

lgtm though it makes me very nervous that there are no tests for this :(

https://codereview.chromium.org/1320383004/diff/1/git_cache.py
File git_cache.py (right):

https://codereview.chromium.org/1320383004/diff/1/git_cache.py#newcode150
git_cache.py:150: self.refs = set([ref.lstrip('+').rstrip('/') for ref in (refs
or [])])
can we have a 'parse_fetch_spec' function so the next person knows what's
happening here?

https://codereview.chromium.org/1320383004/diff/1/git_cache.py#newcode325
git_cache.py:325: self.refs.add(fetchspec.split(':')[0].lstrip('+').rstrip('/'))
and here too

[szager1, 2015-09-22 22:48:28]

Refactored a bit and added a test, PTAL.

https://codereview.chromium.org/1320383004/diff/1/git_cache.py
File git_cache.py (right):

https://codereview.chromium.org/1320383004/diff/1/git_cache.py#newcode150
git_cache.py:150: self.refs = set([ref.lstrip('+').rstrip('/') for ref in (refs
or [])])
On 2015/09/22 20:35:26, iannucci wrote:
> can we have a 'parse_fetch_spec' function so the next person knows what's
> happening here?

Done.

https://codereview.chromium.org/1320383004/diff/1/git_cache.py#newcode325
git_cache.py:325: self.refs.add(fetchspec.split(':')[0].lstrip('+').rstrip('/'))
On 2015/09/22 20:35:26, iannucci wrote:
> and here too

Done.

[iannucci, 2015-09-22 22:56:57]

https://codereview.chromium.org/1320383004/diff/20001/git_cache.py
File git_cache.py (right):

https://codereview.chromium.org/1320383004/diff/20001/git_cache.py#newcode326
git_cache.py:326: """Read and preserve remote.origin.fetch from an existing
mirror."""
I'd mention that this mutates self.fetch_specs as a side effect.

https://codereview.chromium.org/1320383004/diff/20001/git_cache.py#newcode336
git_cache.py:336: pass
This /seems/ like it could cause data loss on an exceptional condition (e.g.
where this fails, so never reads into self.fetch_specs, but the rest of the code
completes). What about the case where the user has 1 malformed fetch spec
followed by some good ones?

https://codereview.chromium.org/1320383004/diff/20001/tests/git_cache_test.py
File tests/git_cache_test.py (right):

https://codereview.chromium.org/1320383004/diff/20001/tests/git_cache_test.py...
tests/git_cache_test.py:20: class GitCacheTest(unittest.TestCase):
yay! :D

[szager1, 2015-09-22 23:11:56]

https://codereview.chromium.org/1320383004/diff/20001/git_cache.py
File git_cache.py (right):

https://codereview.chromium.org/1320383004/diff/20001/git_cache.py#newcode326
git_cache.py:326: """Read and preserve remote.origin.fetch from an existing
mirror."""
On 2015/09/22 22:56:57, iannucci wrote:
> I'd mention that this mutates self.fetch_specs as a side effect.

Done.

https://codereview.chromium.org/1320383004/diff/20001/git_cache.py#newcode336
git_cache.py:336: pass
On 2015/09/22 22:56:57, iannucci wrote:
> This /seems/ like it could cause data loss on an exceptional condition (e.g.
> where this fails, so never reads into self.fetch_specs, but the rest of the
code
> completes). What about the case where the user has 1 malformed fetch spec
> followed by some good ones?

Well, "data loss" has an interesting meaning here.  In a sense, there's always
100% data loss, because this code path only happens when we intend to blow away
an existing cache and repopulate it.  The only real "data loss" is the fetch
specs in the git config file.

... which could be problematic, but at this point, the only use case for this
code path is a developer running 'git cache fetch' after having manually edited
the config file in the old cache.  Which is to say, it could hamper a developer
in a non-permanent, recoverable way, but it should never affect the slaves
because they should never have manual changes in their config's.

I added a logging.warn for this case.

[iannucci, 2015-09-22 23:14:09]

lgtm

[szager1, 2015-09-22 23:16:29]

The CQ bit was checked by szager@chromium.org

[commit-bot: I haz the power, 2015-09-22 23:16:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1320383004/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1320383004/40001

[commit-bot: I haz the power, 2015-09-22 23:19:10]

Committed patchset #3 (id:40001) as
http://src.chromium.org/viewvc/chrome?view=rev&revision=296829