The following revision refers to this bug:
  https://chromium.googlesource.com/android_tools/+/9914c5704717424998c69e837be3631914d787cc

commit 9914c5704717424998c69e837be3631914d787cc
Author: John Budorick <jbudorick@chromium.org>
Date: Fri Nov 10 18:56:11 2017

Only check out the NDK in android checkouts.

Bug:  783607 
Change-Id: I35aa2457ad9304a6f3055d56d1cd68aebac258b6
Reviewed-on: https://chromium-review.googlesource.com/763868
Reviewed-by: agrieve <agrieve@chromium.org>

[modify] https://crrev.com/9914c5704717424998c69e837be3631914d787cc/DEPS

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4a36b481b54de3e27d20ea555f128790c11be276

commit 4a36b481b54de3e27d20ea555f128790c11be276
Author: John Budorick <jbudorick@chromium.org>
Date: Sat Nov 11 01:06:15 2017

Roll src/third_party/android_tools/ 4a9623af5..9914c5704 (1 commit)

https://chromium.googlesource.com/android_tools.git/+log/4a9623af5775..9914c5704717

$ git log 4a9623af5..9914c5704 --date=short --no-merges --format='%ad %ae %s'
2017-11-10 jbudorick Only check out the NDK in android checkouts.

Created with:
  roll-dep src/third_party/android_tools
R=agrieve@chromium.org
BUG= 783607 

No-Equivalent-Builders: true
Change-Id: I53dac6ccb5bb813596fd4f90e4f05f1e7126a325
Reviewed-on: https://chromium-review.googlesource.com/764389
Commit-Queue: John Budorick <jbudorick@chromium.org>
Reviewed-by: Michael Moss <mmoss@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515772}
[modify] https://crrev.com/4a36b481b54de3e27d20ea555f128790c11be276/DEPS

#1 and #2 updated the NDK to avoid this issue.

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/tools/buildspec/+/e2fca7b816f364e003e7688ff9c973ba3a370526

commit e2fca7b816f364e003e7688ff9c973ba3a370526
Author: Sylvain Defresne <sdefresne@google.com>
Date: Mon Nov 13 10:28:36 2017

/me suspects we should probably have conditional recursedeps; that would be another (and probably safer) way to make sure this works right.

Yep, I personally believe that recursedeps should be moved into the deps entries themselves, and that would let us easily add conditions like:

"src/third_party/v8": {
  "url": Var("chromium_git") + "/v8/v8.git@" + Var("v8_revision"),
  "recurse": 'checkout_android',
},

or something like that.

Yeah, I think something like #6 was the original plan, but just hasn't been implemented yet.

FWIW, I'm pretty sure this used to work properly when flattening deps_os entries, before those were switched to conditionals. For instance, if the recursedeps was for "foobar", and "foobar" was in "deps_os: android", then all the foobar/DEPS entries would be put into the flattened "deps_os: android" block.

In the mean time, this seems to just be a bug with "gclient flatten": when recursing, it should keep track of any conditions on the recursed dep, and apply them to everything that comes from the recursion.

I'll take a stab at this, as part of getting the checkout_*_internal conditions to work properly.

Not really part of this bug, but something which came up when I was messing around trying to create some corner case tests. It looks like gclient doesn't support the following type of configuration:

deps = {
  "foo": {
    "url": "git://example.com@refs/heads/android",
    "condition": "checkout_android"},
  "foo": {
    "url": "git://example.com@refs/heads/master",
    "condition": "not checkout_android"},
}

Logically, this should work, but gclient doesn't allow it because the DEPS is evaluated with exec(), which treats it as a normal python dict (which it basically is) and thus collapses duplicate keys, so only the last "foo" is ever seen. Should this type of thing be supported (which would require smarter gclient parsing and/or a different DEPS format), or are there other reasons we shouldn't allow it?

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools/+/42d02c27c1a008c5b847c533be58f134739a6525

commit 42d02c27c1a008c5b847c533be58f134739a6525
Author: Michael Moss <mmoss@google.com>
Date: Tue Feb 06 19:22:04 2018

Make flattened recursedeps inherit parent conditionals.

BUG= 783607 
R=agable@google.com, dpranke@google.com

Change-Id: I6989203a9e560930cc53254c566275a30d3bead3
Reviewed-on: https://chromium-review.googlesource.com/902168
Reviewed-by: Aaron Gable <agable@chromium.org>
Commit-Queue: Michael Moss <mmoss@chromium.org>

[modify] https://crrev.com/42d02c27c1a008c5b847c533be58f134739a6525/testing_support/fake_repos.py
[modify] https://crrev.com/42d02c27c1a008c5b847c533be58f134739a6525/gclient.py
[modify] https://crrev.com/42d02c27c1a008c5b847c533be58f134739a6525/tests/gclient_smoketest.py

Re: #c11 - we intentionally do not want to support two different revisions for the same path, so I'd say that should *not* work. (We want it to be possible to check out dependency at once).

However, we don't use exec() any more, we parse and evaluate the file directly, and we should be detecting and rejecting the duplicate key.

Oh, I saw the exec() in:

https://chromium.googlesource.com/chromium/tools/depot_tools/+/42d02c27c1a008c5b847c533be58f134739a6525/gclient.py#768

and thought it was being used for everything but 'gclient validate'. Is 'validate_syntax' set for other operations as well, like 'gclient sync'? I was mostly testing 'gclient flatten', so maybe that's why I was seeing the duplicates just get collapsed, rather than rejected.

Actually, I just did a 'gclient sync' test with duplicate "src-internal" entries (one with condition "checkout_src_internal", and one with "not checkout_src_internal") and gclient didn't complain about it, but just used whichever one came last in the DEPS. 'gclient validate' doesn't complain either.

validation is on by default, but can be turned off. I think it's just a bug that we're not complaining about the duplicate keys.

OK. I guess validation isn't on for 'flatten', since that appears to go through the exec() path, but I don't really know if it matters. I'll create a separate bug about the dupe keys being allowed.

As for this bug, I think #12 takes care of it, so closing.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/40f270043c0d69d8d491cd8e8534321577a41278

commit 40f270043c0d69d8d491cd8e8534321577a41278
Author: depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Wed Feb 07 04:02:36 2018

Roll src/third_party/depot_tools/ e117e46a6..2b51c930e (9 commits; 2 trivial rolls)

https://chromium.googlesource.com/chromium/tools/depot_tools.git/+log/e117e46a6894..2b51c930e009

$ git log e117e46a6..2b51c930e --date=short --no-merges --format='%ad %ae %s'
2018-02-07 tandrii Revert "bot_update: allow rebasing the patch onto an older revision."
2018-02-06 iannucci [luci-auth] Add statically-linked luci-auth CLI tool.
2018-02-06 iannucci [prpc] Add pRPC CLI tool to depot_tools.
2018-02-06 tandrii bot_update: optimize checkout of solution's main repo.
2018-02-06 thomasanderson Add target_cpu builtin variable to gclient
2018-02-06 oprypin bot_update: allow rebasing the patch onto an older revision.
2018-02-05 mmoss Make flattened recursedeps inherit parent conditionals.

Created with:
  roll-dep src/third_party/depot_tools
BUG= 807986 , 783607 


The AutoRoll server is located here: https://depot-tools-chromium-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.


TBR=agable@chromium.org

Change-Id: I2db4866326042b36e2d166db3cfd0740dcb639ec
Reviewed-on: https://chromium-review.googlesource.com/905932
Commit-Queue: depot-tools-chromium-autoroll <depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Reviewed-by: depot-tools-chromium-autoroll <depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#534909}
[modify] https://crrev.com/40f270043c0d69d8d491cd8e8534321577a41278/DEPS