New issue
Advanced search Search tips

Issue 858978 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 3
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue webrtc:9470



Sign in to add a comment

gclient setdep for "gn/gn/${{platform}}" and "git_revision:"

Project Member Reported by oprypin@chromium.org, Jun 29 2018

Issue description

Recently there was quite an unusual addition to Chromium's DEPS file:

  'src/third_party/gn': {
      'packages': [
          {
              'package': 'gn/gn/${{platform}}',
              'version': 'git_revision:f30b5738e20fdd2f00eba6298c536d66c13b09e3',
          },
      ],
      'dep_type': 'cipd',
  },

https://chromium.googlesource.com/chromium/src/+/d5f76375ecb13c302d5edad0614ce638abd883ba/DEPS#654


WebRTC needs to autoroll this dependency somehow, but
`gclient setdep` cannot deal with this, here's what it says:

$ gclient setdep --revision 'src/third_party/gn:gn/gn/${{platform}}@f30b5738e20fdd2f00eba6298c536d66c13b09e3'
Traceback (most recent call last):
  File "/usr/local/google/home/oprypin/depot_tools/gclient.py", line 2934, in <module>
    sys.exit(main(sys.argv[1:]))
  File "/usr/local/google/home/oprypin/depot_tools/gclient.py", line 2920, in main
    return dispatcher.execute(OptionParser(), argv)
  File "/usr/local/google/home/oprypin/depot_tools/subcommand.py", line 252, in execute
    return command(parser, args[1:])
  File "/usr/local/google/home/oprypin/depot_tools/gclient.py", line 2791, in CMDsetdep
    gclient_eval.SetCIPD(local_scope, name, package, value)
  File "/usr/local/google/home/oprypin/depot_tools/gclient_eval.py", line 719, in SetCIPD
    "%s were found." % (package_name, len(packages)))
ValueError: There must be exactly one package with the given name (gn/gn/${{platform}}), 0 were found.


Even if I bypass the error by changing '${{platform}' to an arbitrary string, the result is still incorrect because `setdep` forces the prefix to be 'version:':

$ gclient setdep --revision 'src/third_party/gn:gn/gn/zzz@f30b5738e20fdd2f00eba6298c536d66c13b09e3'
$ git diff
diff --git a/DEPS b/DEPS
index f625e63c0..b7a153fb8 100644
--- a/DEPS
+++ b/DEPS
@@ -107,8 +107,8 @@ deps = {
   'src/third_party/gn': {
       'packages': [
           {
-              'package': 'gn/gn/${{platform}}',
-              'version': 'git_revision:a30b5738e20fdd2f00eba6298c536d66c13b09e3',
+              'package': 'gn/gn/zzz',
+              'version': 'version:f30b5738e20fdd2f00eba6298c536d66c13b09e3',
           },
       ],
       'dep_type': 'cipd',

 
Cc: mbonadei@chromium.org
Does that really work? It shouldn't...
$ gclient setdep --revision 'src/third_party/gn:gn/gn/zzz@f30b5738e20fdd2f00eba6298c536d66c13b09e3'

The right way to refer to it is
$ gclient setdep --revision 'src/third_party/gn:gn/gn/${platform}@f30b5738e20fdd2f00eba6298c536d66c13b09e3'

Since 'gn/gn/${platform}' is what it becomes after variable expansion.
Similarly, if the DEPS file was like 

  vars = {
    'foo_root': 'foo',
  }
  deps = {
    '{foo_root}/bar': ...,
  }

The right way to refer to the dependency is 'foo/bar'.

I'm uploading a CL to support prefixes other than 'version:'.
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 3

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

commit a92b961190ff4a2fefda10a89bd95c07d5d42c7a
Author: Edward Lemur <ehmaldonado@chromium.org>
Date: Tue Jul 03 02:34:32 2018

gclient setdep: Add support for CIPD versions

gclient setdep used to assume that the version of a CIPD package was always
of the form 'version:<something>'.
However, the following are all valid:
  * SHA1 hashes that specify a single instance
  * tags (tagname:tagvalue)
  * References (e.g. 'latest') that are dynamically resolved by the CIPD server.

This change adds support for all of them:
  * GetCIPD simply returns whatever the 'version' entry is, without assuming
    it begins with 'version:'.
  * SetCIPD sets 'version' ta whatever it is passed, without prepending 'version:'
    at the beginning.

Bug:  858978 
Change-Id: I53669c5df7fb51cde42e0af271139b5719a47622
Reviewed-on: https://chromium-review.googlesource.com/1120943
Reviewed-by: Aaron Gable <agable@chromium.org>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>

[modify] https://crrev.com/a92b961190ff4a2fefda10a89bd95c07d5d42c7a/gclient_eval.py
[modify] https://crrev.com/a92b961190ff4a2fefda10a89bd95c07d5d42c7a/tests/gclient_eval_unittest.py

Project Member

Comment 4 by bugdroid1@chromium.org, Jul 3

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

commit ffaf80849c15b94a30ba0360d6a07796de479b97
Author: depot-tools-chromium-autoroll <depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Tue Jul 03 04:28:30 2018

Roll src/third_party/depot_tools 90cac980d26c..43033409f481 (3 commits)

https://chromium.googlesource.com/chromium/tools/depot_tools.git/+log/90cac980d26c..43033409f481


git log 90cac980d26c..43033409f481 --date=short --no-merges --format='%ad %ae %s'
2018-07-03 tandrii@chromium.org bot_update: remove legacy Gerrit patch property support.
2018-07-03 ehmaldonado@chromium.org gclient setdep: Add support for CIPD versions
2018-07-03 recipe-roller@chromium.org Roll recipe dependencies (trivial).


Created with:
  gclient setdep -r src/third_party/depot_tools@43033409f481

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.



BUG= chromium:858978 
TBR=agable@chromium.org

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

Owner: ehmaldonado@chromium.org
Status: Fixed (was: Untriaged)
> Does that really work? It shouldn't...
> $ gclient setdep --revision 'src/third_party/gn:gn/gn/zzz@f30b5738e20fdd2f00eba6298c536d66c13b09e3'

Sorry, I meant when I manually changed it to zzz in the file and then called this.

> The right way to refer to it is
> $ gclient setdep --revision 'src/third_party/gn:gn/gn/${platform}@f30b5738e20fdd2f00eba6298c536d66c13b09e3'

That's true, thanks. I used `pkg['package'].format()` to replace `{{` with `{`. If the deps file starts using variables, it will require some thought to duplicate the expansion functionality.

>   * SetCIPD sets 'version' ta whatever it is passed, without prepending 'version:'
>     at the beginning.

Seems like this is a backwards incompatible change. But it helped. Thanks.
Project Member

Comment 6 by bugdroid1@chromium.org, Jul 3

The following revision refers to this bug:
  https://webrtc.googlesource.com/src.git/+/a8eb1e619eb9c13a21158885e0763f52e3881023

commit a8eb1e619eb9c13a21158885e0763f52e3881023
Author: Oleh Prypin <oprypin@webrtc.org>
Date: Tue Jul 03 09:41:53 2018

roll_deps: Accept any prefix (like 'git_revision:'), not only 'version:' for CIPD

`gclient setdep` was changed in https://chromium-review.googlesource.com/1123940
to support any prefix as well, but note that that was a backwards incompatible
change, because it now requires the prefix to be passed. So we just stop stripping
the prefix in this CL.

Also clarify the error when a CIPD dep is present in WebRTC and missing in Chromium.

No-Try: True
TBR: phoglund@webrtc.org
Bug:  webrtc:9470 ,  chromium:858978 
Change-Id: I5e42bbda04db37a628a0ac1de69667b9a30dd793
Reviewed-on: https://webrtc-review.googlesource.com/86280
Commit-Queue: Oleh Prypin <oprypin@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23814}
[modify] https://crrev.com/a8eb1e619eb9c13a21158885e0763f52e3881023/tools_webrtc/autoroller/roll_deps.py
[modify] https://crrev.com/a8eb1e619eb9c13a21158885e0763f52e3881023/tools_webrtc/autoroller/unittests/roll_deps_test.py

Project Member

Comment 7 by bugdroid1@chromium.org, Jul 3

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

commit c8e2b4d44e6b772cc1d3a098f44e5549fe28720a
Author: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Tue Jul 03 12:04:08 2018

Roll src/third_party/webrtc 0ea751539e3a..a8eb1e619eb9 (5 commits)

https://webrtc.googlesource.com/src.git/+log/0ea751539e3a..a8eb1e619eb9


git log 0ea751539e3a..a8eb1e619eb9 --date=short --no-merges --format='%ad %ae %s'
2018-07-03 oprypin@webrtc.org roll_deps: Accept any prefix (like 'git_revision:'), not only 'version:' for CIPD
2018-07-03 ishankhot@fb.com Make ReceiveSendsFromThread use Dispatch
2018-07-03 sprang@webrtc.org Add sprang@ as owner of simulcast.cc/h
2018-07-03 buildbot@webrtc.org Roll chromium_revision a1981d69db..f6935ecdd2 (571936:572058)
2018-07-02 emircan@webrtc.org Add VP9 profile negotiation to SDP


Created with:
  gclient setdep -r src/third_party/webrtc@a8eb1e619eb9

The AutoRoll server is located here: https://webrtc-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.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:linux_chromium_archive_rel_ng;master.tryserver.chromium.mac:mac_chromium_archive_rel_ng

BUG= chromium:858978 ,chromium:None,chromium:None,chromium:None
TBR=webrtc-chromium-sheriffs-robots@google.com

Change-Id: Ifef1fa58091510e43a491c29a74d787c9667284e
Reviewed-on: https://chromium-review.googlesource.com/1124419
Reviewed-by: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#572175}
[modify] https://crrev.com/c8e2b4d44e6b772cc1d3a098f44e5549fe28720a/DEPS

Sign in to add a comment