Add --no-history option to fetch and gclient for shallow clones.

Add --no-history option to fetch and gclient for shallow clones.

Many people* have complained on chromium-dev about the long times
required to perform a full fetch over a DSL. This seems to be mostly
due to the huge size of chromium's history (~9 GB). On the other side,
not everybody is interested in downloading the full git history of
the projects. The size of git packs required to fetch a working HEAD
is one order of magnitude smaller (1.5 GB).
This change makes it possible to perform a shallow fetch (in a way
which is consistent with DEPS, leveraging git templates on clone),
reducing fetch times by 80% for those not interested in the history.

* See: 
[chromium-dev] "fetch chromium" keeps hanging/getting stuck on Windows 7
[chromium-dev] Initial checkout with git taking long
[chromium-dev] Trying to get latest source code fails when fetching
[chromium-dev] Gclient sync takes too long

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

[Primiano Tucci (use gerrit), 2014-08-02 07:22:35]

Can somebody PTAL?
I just tried on my 8M DSL in a land forgotten by the internet, and I could fetch
--no-history from scratch in 39 mins, which seems a pretty good win.

This could solve the issue of many people who want to get involved into chromium
and don't have a blazing fast bandwidth.

I tested this on a mac. After the --no-history fetch I'm still able to both sync
future changes and git cl upload commits.

[iannucci, 2014-08-02 18:27:19]

On 2014/08/02 07:26:15, Primiano Tucci wrote:

When you say 9GB, I'm assuming you mean chromium + deps + checkouts? Because the
chromium/src packfiles are only 2.9GB. I'm trying this patch locally on my mac
right now (on my 11Mbps DSL connection).

Also keep in mind that shallow git clones are really nasty in a lot of ways... I
personally think they're more trouble than they're worth and always do full
clones even when I'm over my mobile connection. It's worth the wait IMO.

[iannucci, 2014-08-02 18:31:00]

On 2014/08/02 18:27:19, iannucci wrote:
> On 2014/08/02 07:26:15, Primiano Tucci wrote:
> 
> When you say 9GB, I'm assuming you mean chromium + deps + checkouts? Because
the
> chromium/src packfiles are only 2.9GB. I'm trying this patch locally on my mac
> right now (on my 11Mbps DSL connection).
> 
> Also keep in mind that shallow git clones are really nasty in a lot of ways...
I
> personally think they're more trouble than they're worth and always do full
> clones even when I'm over my mobile connection. It's worth the wait IMO.

Also, the googlesource.com has a fast-path optimization for doing full clones v.
shallow clones, and we've observed the download speeds to be 2-4x faster for
full clones v. shallow. I expect that you'd need your DSL speed to be in the
4Mbps or slower range to actually see an overall benefit.

[Primiano Tucci (use gerrit), 2014-08-02 18:35:35]

On 2014/08/02 18:27:19, iannucci wrote:
> On 2014/08/02 07:26:15, Primiano Tucci wrote:
> 
> When you say 9GB, I'm assuming you mean chromium + deps + checkouts? 
yes, when I say 1.5 vs 9 Gb I mean the total of the .git folders (chromium +
deps)  which is a huge difference. 

> Also keep in mind that shallow git clones are really nasty in a lot of ways...

what do you mean specifically? AFAICT it was terrible before git 1.9. now it
should me more civilised (you can commit / diff on a shallow checkout) 

 I
> personally think they're more trouble than they're worth and always do full
> clones even when I'm over my mobile connection. It's worth the wait IMO.

as a matter of facts, for a lot of people downloading a single pack file of
several gb is a huge pain (git doesn't support recovery of interrupted
downloads)

[iannucci, 2014-08-02 18:38:27]

On 2014/08/02 18:35:35, Primiano Tucci wrote:
> On 2014/08/02 18:27:19, iannucci wrote:
> > On 2014/08/02 07:26:15, Primiano Tucci wrote:
> > 
> > When you say 9GB, I'm assuming you mean chromium + deps + checkouts? 
> yes, when I say 1.5 vs 9 Gb I mean the total of the .git folders (chromium +
> deps)  which is a huge difference. 
> 
> > Also keep in mind that shallow git clones are really nasty in a lot of
ways...
> 
> what do you mean specifically? AFAICT it was terrible before git 1.9. now it
> should me more civilised (you can commit / diff on a shallow checkout) 

Hm, ok... maybe it's fine then.

> 
>  I
> > personally think they're more trouble than they're worth and always do full
> > clones even when I'm over my mobile connection. It's worth the wait IMO.
> 
> as a matter of facts, for a lot of people downloading a single pack file of
> several gb is a huge pain (git doesn't support recovery of interrupted
> downloads)

^^^ That is a huge issue. One thing that we do for the bots is we have
pre-indexed packfiles available for download on google storage. When bots do the
initial clone, they actually download a tarball of the .git folder instead of
fetching it via the git protocol. WDYT about enabling resumable clones with
something like that?

[Primiano Tucci (use gerrit), 2014-08-02 19:20:23]

> Also, the http://googlesource.com has a fast-path optimization for doing full
clones v.
> shallow clones, and we've observed the download speeds to be 2-4x faster for
> full clones v. shallow. I expect that you'd need your DSL speed to be in the
> 4Mbps or slower range to actually see an overall benefit.

Ah right, this reminds me crbug.com/394363 which I filed a while agoe (I also
posted on chromium-dev few weeks ago) :-)
*That* fast path is currently broken and GoB counts the object every time you do
a full clone (conversely to what was doing up to last month).

Anyways, if we're talking about the same thing, 2-4x is an arguable estimation,
let me explain:
IIRC, the optimisation is that GoB keeps a cache of the pack files required to
perform a full clone from ToT (and probably lkgr).
Essentially that brings down to 0 the time that a conventional git server would
spend in the "remote: counting objects / compressing objects" phase.
That phase takes 6-7 minutes for a project of the size of chromium (and
currently googlesource.com is spending that time for the main chromium project).
Now, if we are counting them w.r.t. the time for a full clone on a blazing fast
corp lan, true, that is a 2-4x improvement.
But if we are talking about a DSL, where the download speed is in the order of
few (or less) MB/s, 6-7 minutes becomes some percentile points vs. several hours
of download time.

Anyways, my point here is simple: what I am proposing is IMHO a very simple
solution (we are talking about a 38 LOC change) for a problem which seems to be
affecting an increasing number of people behind a DSL (the bigger the pack
become the highest the probability of hitting a network flakiness increases).
If we were talking about some massive refactoring / huge maintenance cost I can
see a lot of arguments against. But for a self contained change like this, IMHO
that's worth the benefit for the rest of the users.
Just to be clear, I am not proposing to make this the default options suggested
in Chromium docs. But we use this as a last resort suggestion for "fetching
chromium from a slow DSL".

[Primiano Tucci (use gerrit), 2014-08-02 19:31:19]

> ^^^ That is a huge issue. One thing that we do for the bots is we have
> pre-indexed packfiles available for download on google storage. When bots do
the
> initial clone, they actually download a tarball of the .git folder instead of
> fetching it via the git protocol. WDYT about enabling resumable clones with
> something like that?

I am not aware of the specific problems affecting the bots. Definitely we could
have a separate conversation on that (especially on this days of yellow lights).
My change here was motivated mostly by the complaints on chromium-dev, and most
of them seem legitimate. Not everybody has a gigabit corp lan. Not everybody is
*the* internet, sometimes we tend to forget this :-)

If GoB caching works properly (see my previous reply) git clone should just work
at blazing fast speed, as fast as http.
If the bots are still hitting network flakiness, thought, we might probably want
to take a look to git bundles [1]
Zipping the .git folder is a bit gross and prone to errors / inconsistencies
(also I have not idea how that would look like on GoB).
For instance Android, in the pre-googlesource.com days, was used to have a
clone.bundle file to bootstrap over http. If GoB is still not enough for the
bots, we might probably want to take a look back to that, and use curl / rsync
to bootstrap.
Let's have a separate discussion on this. :)

[1] http://git-scm.com/docs/git-bundle

[iannucci, 2014-08-02 21:22:15]

in general lgtm. I tried this patch on my laptop over 11Mbps DSL:
  * with --no-history: 26m for full initial clone + hooks
  * without --no-history: 1:11+hours. I ended up killing it because I got bored,
but it was 60% of the way though cloning the WebKit repo....

https://codereview.chromium.org/437903002/diff/1/gclient.py
File gclient.py (right):

https://codereview.chromium.org/437903002/diff/1/gclient.py#newcode1816
gclient.py:1816: 'the cost of no history. Requires Git 1.9+')
it may be beneficial to specify a very low amount of history (like 1000
commits). If you could time that, it may be a good compromise between no history
and full history, especially if people are interested in recently-changed files.

https://codereview.chromium.org/437903002/diff/1/gclient_scm.py
File gclient_scm.py (right):

https://codereview.chromium.org/437903002/diff/1/gclient_scm.py#newcode835
gclient_scm.py:835: # a template git dir which has a 'shallow' file pointing to
the sha.
sneaky :(. I'm not sure if this is guaranteed to work though... wouldn't git
claim that it already has <SHA> to the remote server?

more specifically, wouldn't this work the same way that `git fetch <sha>` works
(that is: sporadically supported and discouraged?). googlesource.com (AFAIK)
only supports fetching <sha>'s which were (recently) the HEAD of some ref, for
example.

[iannucci, 2014-08-02 21:36:32]

https://codereview.chromium.org/437903002/diff/1/fetch.py
File fetch.py (right):

https://codereview.chromium.org/437903002/diff/1/fetch.py#newcode227
fetch.py:227: while len(argv) >= 2:
this should really be using {arg,opt}parse...

[wtc, 2014-08-03 04:12:49]

Drive-by review comments on patch set 1:

Primiano: thank you very much for writing this CL. I have been trying to check
out a Chromium source tree on Windows at home, and it's taking more than 12
hours and still going! I am testing your CL now.

https://codereview.chromium.org/437903002/diff/1/gclient.py
File gclient.py (right):

https://codereview.chromium.org/437903002/diff/1/gclient.py#newcode1814
gclient.py:1814: parser.add_option('--no-history', action='store_true',

It is unfortunate that the use of '_' or '-' in option names for the "sync"
command is inconsistent.

https://codereview.chromium.org/437903002/diff/1/gclient_scm.py
File gclient_scm.py (right):

https://codereview.chromium.org/437903002/diff/1/gclient_scm.py#newcode864
gclient_scm.py:864: gclient_utils.rmtree(template_dir)

Should we add
  if os.listdir(template_dir):
    self.Print('_____ removing non-empty template dir %s' % template_dir)
?

See lines 860-861.

https://codereview.chromium.org/437903002/diff/1/gclient_utils.py
File gclient_utils.py (right):

https://codereview.chromium.org/437903002/diff/1/gclient_utils.py#newcode91
gclient_utils.py:91: def IsGitSha(text):

Nit: I know you listed IsGitSha after another IsXXX function, but it is now
between two functions related to DateRevision. Perhaps we should list IsGitSha
before IsDateRevision?

[iannucci, 2014-08-03 15:28:26]

On 2014/08/03 04:12:49, wtc wrote:
> Drive-by review comments on patch set 1:
> 
> Primiano: thank you very much for writing this CL. I have been trying to check
> out a Chromium source tree on Windows at home, and it's taking more than 12
> hours and still going! I am testing your CL now.
> 
> https://codereview.chromium.org/437903002/diff/1/gclient.py
> File gclient.py (right):
> 
> https://codereview.chromium.org/437903002/diff/1/gclient.py#newcode1814
> gclient.py:1814: parser.add_option('--no-history', action='store_true',
> 
> It is unfortunate that the use of '_' or '-' in option names for the "sync"
> command is inconsistent.

Infra had a recent, informal discussion about this on a different CL
(https://chromiumcodereview.appspot.com/436963005/). Basically, '-' is
significantly easier to type on non-US keyboards, and we're thinking that this
outweighs the greppability benefits of the underscore flavor. Additionally, it's
the style that chrome seems to use.

We need a proper PSA for this and some standardizing CLs, but it's looking like
we'll be converging towards '-', unless someone brings up a really good reason
not to.

> 
> https://codereview.chromium.org/437903002/diff/1/gclient_scm.py
> File gclient_scm.py (right):
> 
> https://codereview.chromium.org/437903002/diff/1/gclient_scm.py#newcode864
> gclient_scm.py:864: gclient_utils.rmtree(template_dir)
> 
> Should we add
>   if os.listdir(template_dir):
>     self.Print('_____ removing non-empty template dir %s' % template_dir)
> ?
> 
> See lines 860-861.
> 
> https://codereview.chromium.org/437903002/diff/1/gclient_utils.py
> File gclient_utils.py (right):
> 
> https://codereview.chromium.org/437903002/diff/1/gclient_utils.py#newcode91
> gclient_utils.py:91: def IsGitSha(text):
> 
> Nit: I know you listed IsGitSha after another IsXXX function, but it is now
> between two functions related to DateRevision. Perhaps we should list IsGitSha
> before IsDateRevision?

[Primiano Tucci (use gerrit), 2014-08-03 17:43:35]

Thanks all for the comments, see replies inline.

https://codereview.chromium.org/437903002/diff/1/fetch.py
File fetch.py (right):

https://codereview.chromium.org/437903002/diff/1/fetch.py#newcode227
fetch.py:227: while len(argv) >= 2:
On 2014/08/02 21:36:31, iannucci wrote:
> this should really be using {arg,opt}parse...

I stumbled upon this for a while as well. If I'm interpreting this right, I
think that the reason why this doesn't use optparse is because it is
intentionally striving of not swallowing the -- options after the recipe name,
to let the recipe script parse the remaining arguments on its own.
In other words, all this boilerplate seems to make it so that
$ fetch --foo recipe_name --bar
stops swallowing arguments @recipe_name, keeps --bar and passes it to the fetch
recipe. This is the reason (I think, I am just speculating here) why we can't
just use optparse here.

Having said that, I agree with you and looking at this code drives me nuts as
well.
My personal opinion is that we should rework this logic (here and in the fetch
recipes), use optparse just once (here) and pass all the parsed options to the
recipe (instead of having that re-run optparse  there). 

Rationale:
If I am interpreting everything correctly, all this logic here makes it so that:
$ fetch --nohooks chromium  # is valid
$ fetch chromium --nohooks # barfs with an error (because the chromium.py recipe
doesn't understand nohooks).

This is IMHO a terrible end-user experience. The only *benefit* that this brings
is decoupling the fetch vs. recipe arguments. At present state, a fetch recipe
could have its own --nohooks argument, with a completely different semantic,
allowing to invoke fetch --nohooks chromium --nohooks.

I'd just vote for simplicity, refactor recipes and fetch, use optparse only once
and rely on fetch recipes to not clash with the same global argument names used
by fetch. But I feel this should be part of another (slightly bigger) CL.

https://codereview.chromium.org/437903002/diff/1/gclient.py
File gclient.py (right):

https://codereview.chromium.org/437903002/diff/1/gclient.py#newcode1816
gclient.py:1816: 'the cost of no history. Requires Git 1.9+')
On 2014/08/02 21:22:15, iannucci wrote:
> it may be beneficial to specify a very low amount of history (like 1000
> commits). If you could time that, it may be a good compromise between no
history
> and full history, especially if people are interested in recently-changed
files.
This is in principle a very nice feature. In practice, however, I feel this
would bump up the complexity of this change and require some major refactoring.
Explanation: I can easily change the --depth of the main project, that's a 1
line change. However, I won't be able to do the same for the subprojects. In
order to do that I'd need to fetch DEPS for src/HEAD~1000, recurse its deps, and
get the sha's for the shallow file.

Even the approximated version (just take the last 1000 commits for WebKit,
regardless of following deps) is not trivial, because I'd have still to find a
way to get the the sha of pinned_sha~1000 to put in the shallow file.

As regards the size, I did some git analysis back in this thread on
chromium-dev: http://goo.gl/FFsCdP
A depth of 5000 commits (which at the main project peace is ~= 3 months) is 1 GB
bigger than the shallow one. 

In short: unless you feel beneficial having a relatively small number  ~500  of
history just for the main project (even if this would make the term "no history"
a small lie), I'd leave this as it is. Just 3 months of history adds up to an
extra gig, which is several minutes.
IMHO it would be nice having a configurable --depth option for gclient, but, as
I was saying above, I'm afraid this require definitely more than 20-30 lines :)

https://codereview.chromium.org/437903002/diff/1/gclient_scm.py
File gclient_scm.py (right):

https://codereview.chromium.org/437903002/diff/1/gclient_scm.py#newcode835
gclient_scm.py:835: # a template git dir which has a 'shallow' file pointing to
the sha.
+torne as he might have more insights on this topic.

On 2014/08/02 21:22:15, iannucci wrote:
> sneaky :(. I'm not sure if this is guaranteed to work though... wouldn't git
> claim that it already has <SHA> to the remote server?

Yes, I think this is precisely the way it works.
However, this is different than "git fetch SHA". IIRC "git fetch SHA" is a GoB
invention as is not part of the git wire protocol at all.
The mechanism that shallow uses (--depth/shallow is officially part of git-scm
docs even before 1.9), instead, should just leverage the standard protocol
adopted by fetch-pack.

https://codereview.chromium.org/437903002/diff/1/gclient_scm.py#newcode864
gclient_scm.py:864: gclient_utils.rmtree(template_dir)
On 2014/08/03 04:12:49, wtc wrote:
> 
> Should we add
>   if os.listdir(template_dir):
Hmm this I think should stay as it is. 
Conversely to tmp_dir, template_dir can be None if the user never specified
--no-history.
On the other side if it is not-None, it necessarily means that mkdtemp has
created a directory and we want to clean that up anyways, empty or not, no?

>     self.Print('_____ removing non-empty template dir %s' % template_dir)
Agree here instead.: added the print.

https://codereview.chromium.org/437903002/diff/1/gclient_utils.py
File gclient_utils.py (right):

https://codereview.chromium.org/437903002/diff/1/gclient_utils.py#newcode91
gclient_utils.py:91: def IsGitSha(text):
On 2014/08/03 04:12:49, wtc wrote:
> 
> Nit: I know you listed IsGitSha after another IsXXX function, but it is now
> between two functions related to DateRevision. Perhaps we should list IsGitSha
> before IsDateRevision?
Oh right, didn't realize that. Done.

[szager1, 2014-08-04 21:15:24]

lgtm, we need this.

https://codereview.chromium.org/437903002/diff/20002/gclient_utils.py
File gclient_utils.py (right):

https://codereview.chromium.org/437903002/diff/20002/gclient_utils.py#newcode89
gclient_utils.py:89: return re.match('^[a-fA-F0-9]{6,40}$', revision) != None
nit:

s/!= None/is not None/

[wtc, 2014-08-04 22:32:50]

https://codereview.chromium.org/437903002/diff/20002/gclient_scm.py
File gclient_scm.py (right):

https://codereview.chromium.org/437903002/diff/20002/gclient_scm.py#newcode865
gclient_scm.py:865: gclient_utils.rmtree(template_dir)

What I suggested was:

    if template_dir:
      if os.listdir(template_dir):
        self.Print('_____ removing non-empty template dir %s' % template_dir)
      gclient_utils.rmtree(template_dir)

So, if template_dir is not-None, we remove it, empty or not. But we print a
message only when template_dir is not empty.

Your new code is fine by me. Since you have thought about this code more than I
have, I'll let you decide which is better. I actually prefer your form and I
think lines 860-862 should also be changed accordinly:
    self.Print('_____ removing tmp dir %s' % tmp_dir)
    gclient_utils.rmtree(tmp_dir)

[Primiano Tucci (use gerrit), 2014-08-04 23:00:25]

https://codereview.chromium.org/437903002/diff/20002/gclient_scm.py
File gclient_scm.py (right):

https://codereview.chromium.org/437903002/diff/20002/gclient_scm.py#newcode865
gclient_scm.py:865: gclient_utils.rmtree(template_dir)
On 2014/08/04 22:32:50, wtc wrote:
> So, if template_dir is not-None, we remove it, empty or not. But we print a
> message only when template_dir is not empty.
But... template_dir will always be non-empty (Conversely to tmp_dir, which is
checked out from the remote, and its empty state depends on the outcome of the
clone).
That's why I removed also the "non-empty" from the message for template_dir

> 
> Your new code is fine by me. Since you have thought about this code more than
I
> have, I'll let you decide which is better. I actually prefer your form and I
> think lines 860-862 should also be changed accordinly:
>     self.Print('_____ removing tmp dir %s' % tmp_dir)
>     gclient_utils.rmtree(tmp_dir)

Actually looking closer at the code, I think we shouldn't probably add a Print
for template_dir but keep it as it is for tmp_dir. Rationale: in nominal
conditions (if everything goes right) tmp_dir should not exists at the end, as
line 853 renames it atomically to the final checkout dir. That tmp_dir stays
there in its original tmp location only if something goes wrong and we end up in
the finally block due to an exception. 
Essentially, it looks to me that the print on line 861 is there to say "I tried
to checked out/clone the remote repo, something went wrong, and now I'm deleting
the temporary directory where I attempted the checkout". In nominal condition
that print should never be hit.
Having said that, the situation is different for template_dir. That one is
supposed to be just a temporary scratch directory to be used for git clone.
After the clone (either successful or not), that directory will always be not
empty, and we always want to clean that up. Hence, probably a Print there would
be misleading (we'd print the message also in the case of success) and would add
too much noise.

I'd be tempted to remove lines 863-865 ad this point. WDYT?

https://codereview.chromium.org/437903002/diff/20002/gclient_utils.py
File gclient_utils.py (right):

https://codereview.chromium.org/437903002/diff/20002/gclient_utils.py#newcode89
gclient_utils.py:89: return re.match('^[a-fA-F0-9]{6,40}$', revision) != None
On 2014/08/04 21:15:24, szager1 wrote:
> nit:
> 
> s/!= None/is not None/

Done.

[Primiano Tucci (use gerrit), 2014-08-04 23:01:52]

https://codereview.chromium.org/437903002/diff/20002/gclient_scm.py
File gclient_scm.py (right):

https://codereview.chromium.org/437903002/diff/20002/gclient_scm.py#newcode865
gclient_scm.py:865: gclient_utils.rmtree(template_dir)
> I'd be tempted to remove lines 863-865 ad this point. WDYT?
Ehm, sorry, I actually meant: "I'd be tempted to remove line 864" (just the
print, not the rmtree)

[wtc, 2014-08-05 00:11:56]

https://codereview.chromium.org/437903002/diff/20002/gclient_scm.py
File gclient_scm.py (right):

https://codereview.chromium.org/437903002/diff/20002/gclient_scm.py#newcode865
gclient_scm.py:865: gclient_utils.rmtree(template_dir)

On 2014/08/04 23:01:52, Primiano Tucci wrote:
> I'd be tempted to remove line 864 (just the print, not the rmtree)

I agree. Thanks for the analysis.

[Primiano Tucci (use gerrit), 2014-08-05 07:45:30]

The CQ bit was checked by primiano@chromium.org

[commit-bot: I haz the power, 2014-08-05 07:46:29]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/primiano@chromium.org/437903002/110001

[commit-bot: I haz the power, 2014-08-05 07:48:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-05 07:48:22]

Presubmit check for 437903002-110001 failed and returned exit status 1.

Running presubmit commit checks ...
Checking out rietveld...
Running save-description-on-failure.sh
Running push-basic.sh
Running upstream.sh
Running submit-from-new-dir.sh
Running abandon.sh
Running submodule-merge-test.sh
Running upload-local-tracking-branch.sh
Running hooks.sh
Running post-dcommit-hook-test.sh
Running upload-stale.sh
Running patch.sh
Running basic.sh

** Presubmit ERRORS **
tests/gclient_scm_test.py (8.02s) failed
............................................EEEEEEEEEE.
======================================================================
ERROR: testUpdateClone (__main__.UnmanagedGitWrapperTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/gclient_scm_test.py", line 1430, in testUpdateClone
    scm.update(options, (), file_list)
  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient_scm.py", line
389, in update
    self._Clone(revision, url, options)
  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient_scm.py", line
830, in _Clone
    if options.no_history:
AttributeError: 'OptionsObject' object has no attribute 'no_history'

======================================================================
ERROR: testUpdateClone (__main__.UnmanagedGitWrapperTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/gclient_scm_test.py", line 913, in tearDown
    StdoutCheck.tearDown(self)
  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/testing_support/super_mox.py",
line 102, in tearDown
    self.assertEquals('', sys.stdout.getvalue())
AssertionError: '' != '[0:00:07] \n'

======================================================================
ERROR: testUpdateCloneOnBranch (__main__.UnmanagedGitWrapperTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/gclient_scm_test.py", line 1494, in testUpdateCloneOnBranch
    scm.update(options, (), file_list)
  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient_scm.py", line
389, in update
    self._Clone(revision, url, options)
  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient_scm.py", line
830, in _Clone
    if options.no_history:
AttributeError: 'OptionsObject' object has no attribute 'no_history'

======================================================================
ERROR: testUpdateCloneOnBranch (__main__.UnmanagedGitWrapperTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/gclient_scm_test.py", line 913, in tearDown
    StdoutCheck.tearDown(self)
  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/testing_support/super_mox.py",
line 102, in tearDown
    self.assertEquals('', sys.stdout.getvalue())
AssertionError: '' != '[0:00:07] \n'

======================================================================
ERROR: testUpdateCloneOnBranchHead (__main__.UnmanagedGitWrapperTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/gclient_scm_test.py", line 1556, in testUpdateCloneOnBranchHead
    scm.update(options, (), file_list)
  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient_scm.py", line
389, in update
    self._Clone(revision, url, options)
  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient_scm.py", line
830, in _Clone
    if options.no_history:
AttributeError: 'OptionsObject' object has no attribute 'no_history'

======================================================================
ERROR: testUpdateCloneOnBranchHead (__main__.UnmanagedGitWrapperTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/gclient_scm_test.py", line 913, in tearDown
    StdoutCheck.tearDown(self)
  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/testing_support/super_mox.py",
line 102, in tearDown
    self.assertEquals('', sys.stdout.getvalue())
AssertionError: '' != '[0:00:07] \n'

======================================================================
ERROR: testUpdateCloneOnCommit (__main__.UnmanagedGitWrapperTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/gclient_scm_test.py", line 1462, in testUpdateCloneOnCommit
    scm.update(options, (), file_list)
  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient_scm.py", line
389, in update
    self._Clone(revision, url, options)
  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient_scm.py", line
830, in _Clone
    if options.no_history:
AttributeError: 'OptionsObject' object has no attribute 'no_history'

======================================================================
ERROR: testUpdateCloneOnCommit (__main__.UnmanagedGitWrapperTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/gclient_scm_test.py", line 913, in tearDown
    StdoutCheck.tearDown(self)
  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/testing_support/super_mox.py",
line 102, in tearDown
    self.assertEquals('', sys.stdout.getvalue())
AssertionError: '' != '[0:00:07] \n'

======================================================================
ERROR: testUpdateCloneOnDetachedBranch (__main__.UnmanagedGitWrapperTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/gclient_scm_test.py", line 1524, in
testUpdateCloneOnDetachedBranch
    scm.update(options, (), file_list)
  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient_scm.py", line
389, in update
    self._Clone(revision, url, options)
  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/gclient_scm.py", line
830, in _Clone
    if options.no_history:
AttributeError: 'OptionsObject' object has no attribute 'no_history'

======================================================================
ERROR: testUpdateCloneOnDetachedBranch (__main__.UnmanagedGitWrapperTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/gclient_scm_test.py", line 913, in tearDown
    StdoutCheck.tearDown(self)
  File
"/b/infra_internal/commit_queue/workdir/tools/depot_tools/testing_support/super_mox.py",
line 102, in tearDown
    self.assertEquals('', sys.stdout.getvalue())
AssertionError: '' != '[0:00:07] \n'

----------------------------------------------------------------------
Ran 50 tests in 7.802s

FAILED (errors=10)


Presubmit checks took 106.8s to calculate.

Was the presubmit check useful? If not, run "git cl presubmit -v"
to figure out which PRESUBMIT.py was run, then run git blame
on the file to figure out who to ask for help.

[Primiano Tucci (use gerrit), 2014-08-05 19:19:43]

Sorry for the 2nd pass.
I added another small patchset. The original code itself is untouched (%
repositioning the option in the --help list) ,
I just figured out it might be worth having a smoke test for this ;-)

[iannucci, 2014-08-05 21:22:16]

lgtm, thanks for the smoke test :)

[Primiano Tucci (use gerrit), 2014-08-05 21:46:45]

The CQ bit was checked by primiano@chromium.org

[commit-bot: I haz the power, 2014-08-05 21:47:35]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/primiano@chromium.org/437903002/230001

[commit-bot: I haz the power, 2014-08-05 21:50:46]

Change committed as 287606

[Primiano Tucci (use gerrit), 2014-08-05 23:14:48]

A revert of this CL has been created in
https://codereview.chromium.org/440263002/ by primiano@chromium.org.

The reason for reverting is: Broke the WebRTC waterfall:
http://build.chromium.org/p/tryserver.webrtc/builders/win/builds/3958/steps/g...


Traceback (most recent call last):
  File "E:\b\depot_tools\\gclient.py", line 2067, in <module>
    sys.exit(Main(sys.argv[1:]))
  File "E:\b\depot_tools\\gclient.py", line 2055, in Main
    return dispatcher.execute(OptionParser(), argv)
  File "E:\b\depot_tools\subcommand.py", line 245, in execute
    return command(parser, args[1:])
  File "E:\b\depot_tools\\gclient.py", line 1891, in CMDrevert
    return client.RunOnDeps('revert', args)
  File "E:\b\depot_tools\\gclient.py", line 1342, in RunOnDeps
    work_queue.flush(revision_overrides, command, args, options=self._options)
  File "E:\b\depot_tools\gclient_utils.py", line 859, in flush
    self._run_one_task(self.queued.pop(i), args, kwargs)
  File "E:\b\depot_tools\gclient_utils.py", line 951, in _run_one_task
    task_item.run(*args, **kwargs)
  File "E:\b\depot_tools\\gclient.py", line 744, in run
    file_list)
  File "E:\b\depot_tools\gclient_scm.py", line 160, in RunCommand
    return getattr(self, command)(options, args, file_list)
  File "E:\b\depot_tools\gclient_scm.py", line 656, in revert
    return self.update(options, [], file_list)
  File "E:\b\depot_tools\gclient_scm.py", line 389, in update
    self._Clone(revision, url, options)
  File "E:\b\depot_tools\gclient_scm.py", line 830, in _Clone
Sending crash report ...
  args: ['E:\\b\\depot_tools\\\\gclient.py', 'revert', '-v', '-v', '-v',
'--nohooks', '--upstream']
  cwd: E:\b\build\slave\win\build
  exception: Values instance has no attribute 'no_history'
  host: VM207-M4.golo.chromium.org
  stack:   File "E:\b\depot_tools\\gclient.py", line 2067, 
  user: chrome-bot
  version: 2.7.6 (default, Nov 10 2013, 19:24:18) [MSC v.1500
    if options.no_history:
AttributeError: Values instance has no attribute 'no_history'
A stack trace has been sent to the maintainers..

[Primiano Tucci (use gerrit), 2014-08-08 11:29:07]

Relanded in https://codereview.chromium.org/440273002