New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 772843 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Oct 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression

Blocking:
issue 570091



Sign in to add a comment

roll_dep_svn.py doesn't work with new DEPS structure

Project Member Reported by machenb...@chromium.org, Oct 9 2017

Issue description

Repro:
Use this in a V8 checkout locally:
https://chromium-review.googlesource.com/c/v8/v8/+/707066

Run the script:
machenbach@malumi:~/v8/v8 testdep $ roll_dep_svn.py v8/third_party/android_tools ca9dc7245b888c75307f0619e4a39fb46a82de66
Traceback (most recent call last):
  File "/usr/local/google/home/machenbach/tools/depot_tools/roll_dep_svn.py", line 414, in <module>
    sys.exit(main(sys.argv[1:]))
  File "/usr/local/google/home/machenbach/tools/depot_tools/roll_dep_svn.py", line 409, in main
    return update_deps(deps_file, dep_path, dep_name, git_rev, comment)
  File "/usr/local/google/home/machenbach/tools/depot_tools/roll_dep_svn.py", line 331, in update_deps
    update_deps_entry(deps_lines, deps_ast, value_node, new_rev, comment)
  File "/usr/local/google/home/machenbach/tools/depot_tools/roll_dep_svn.py", line 306, in update_deps_entry
    line_idx = update_node(deps_lines, deps_ast, value_node, new_rev)
  File "/usr/local/google/home/machenbach/tools/depot_tools/roll_dep_svn.py", line 227, in update_node
    assert False, ast_err_msg(node)
AssertionError: ERROR: Undexpected DEPS file AST structure at line 22 column 34

 
Remark: V8 uses only the git portion of this script (I think svn appears only in the name). We wouldn't really need this script in particular if we had any other script that simply allows to set the a particular DEPS entry to a particular revision. Unfortunately also roll_dep doesn't just do that.

Would be really great if we had something like
gclient update v8/build deadbeef
-> modifies DEPS file and sets v8/build to deadbeef and that's it.
Blocking: 570091
Cc: phajdan@google.com
I couldn't reproduce this locally with up-to-date depot_tools (8db10a6fb1d4b86909b8cb32e3b53e35624c8979):

$ roll_dep_svn.py v8/third_party/android_tools ca9dc7245b888c75307f0619e4a39fb46a82de66
Pinning v8/third_party/android_tools
to revision ca9dc7245b888c75307f0619e4a39fb46a82de66
in ./DEPS
$ git diff
diff --git a/DEPS b/DEPS
index 2f89df2f2d..95f0e44d87 100644
--- a/DEPS
+++ b/DEPS
@@ -46,7 +46,7 @@ deps = {
 deps_os = {
   "android": {
     "v8/third_party/android_tools":
-      Var("chromium_url") + "/android_tools.git" + "@" + "3dd12d2ebe9ca670f637238ad86b3cba9da8c289",
+      Var("chromium_url") + "/android_tools.git" + "@" + "ca9dc7245b888c75307f0619e4a39fb46a82de66",
     "v8/third_party/catapult":
       Var('chromium_url') + "/catapult.git" + "@" + "a48a6afde0ff7eeb1c847744192977e412107d6a",
   },

We can discuss more via IM/VC.
Could you try with current V8 ToT? I think you might now have tested with the CL https://chromium-review.googlesource.com/c/v8/v8/+/707066 - it landed by now.
Correction comment 3, I meant of course CL https://chromium-review.googlesource.com/c/v8/v8/+/706995

Doesn't matter, should repro with ToT now...
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 10 2017

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

commit e83d02446d9e81ad3922f2f974326e766c0d84c5
Author: Paweł Hajdan, Jr <phajdan.jr@chromium.org>
Date: Tue Oct 10 10:22:26 2017

Update roll_dep_svn for new DEPS syntax

Bug:  772843 
Change-Id: I994f829cba7577e6b628d36d894f4c5d1f3c429c
Reviewed-on: https://chromium-review.googlesource.com/708656
Commit-Queue: Paweł Hajdan Jr. <phajdan.jr@chromium.org>
Reviewed-by: Michael Achenbach <machenbach@chromium.org>

[modify] https://crrev.com/e83d02446d9e81ad3922f2f974326e766c0d84c5/roll_dep_svn.py

Status: Fixed (was: Assigned)
Status: Verified (was: Fixed)
Thanks!

Sign in to add a comment