New issue
Advanced search Search tips

Issue 749720 link

Starred by 1 user

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

gclient's new Var() impl no longer supports "Var('foo') % bar" due to placeholders

Reported by kwy...@suitabletech.com, Jul 27 2017

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/59.0.3071.115 Safari/537.36

Steps to reproduce the problem:
1. download depot_tools
2. try to fetch something (ex. breakpad)
3. 

What is the expected behavior?
it fetches breakpad

What went wrong?
doesnt fetch breakpad

Did this work before? N/A 

Chrome version: 59.0.3071.115  Channel: stable
OS Version: Ubuntu 16.04.2 LTS
Flash Version: 

This is the same output as mac

breakpad$ fetch breakpad
Running: gclient root
Running: gclient config --spec 'solutions = [
  {
    "url": "https://chromium.googlesource.com/breakpad/breakpad.git",
    "managed": False,
    "name": "src",
    "custom_deps": {},
  },
]
'
Running: gclient sync

src/src/tools/gyp (ERROR)
----------------------------------------
[0:00:03] Started.
[0:00:03]

________ running 'git -c core.deltaBaseCacheLimit=2g clone --no-checkout --progress https://chromium.googlesource.com/external/gyp /opt/repos/k/breakpad/src/src/tools/_gclient_gyp_FJKipw' in '/opt/repos/k/breakpad'
[0:00:03] Cloning into '/opt/repos/k/breakpad/src/src/tools/_gclient_gyp_FJKipw'...
[0:00:04] remote: Sending approximately 11.77 MiB ...
[0:00:04] Receiving objects:   0% (1/17918)
[0:00:04] Receiving objects:  10% (1792/17918)
[0:00:05] remote: Total 17918 (delta 11145), reused 17918 (delta 11145)
[0:00:05] Receiving objects: 100% (17918/17918), 6.13 MiB | 12.10 MiB/s
[0:00:05] Resolving deltas:   0% (0/11145)
[0:00:05] Resolving deltas:  11% (1234/11145)
[0:00:05] Resolving deltas: 100% (11145/11145)
[0:00:05] Checking connectivity... done.
[0:00:05] Checked out e8ab0833a42691cd2184bd4c45d779e43821d3e0 to a detached HEAD. Before making any commits
in this repo, you should use 'git checkout <branch>' to switch to
an existing branch or use 'git checkout origin -b <branch>' to
create a new branch for your work.
[0:00:05]
----------------------------------------
Traceback (most recent call last):
  File "/opt/repos/k/depot_tools/gclient.py", line 2495, in <module>
    sys.exit(main(sys.argv[1:]))
  File "/opt/repos/k/depot_tools/gclient.py", line 2481, in main
    return dispatcher.execute(OptionParser(), argv)
  File "/opt/repos/k/depot_tools/subcommand.py", line 252, in execute
    return command(parser, args[1:])
  File "/opt/repos/k/depot_tools/gclient.py", line 2242, in CMDsync
    ret = client.RunOnDeps('update', args)
  File "/opt/repos/k/depot_tools/gclient.py", line 1431, in RunOnDeps
    work_queue.flush(revision_overrides, command, args, options=self._options)
  File "/opt/repos/k/depot_tools/gclient_utils.py", line 1050, in run
    self.item.run(*self.args, **self.kwargs)
  File "/opt/repos/k/depot_tools/gclient.py", line 884, in run
    self.ParseDepsFile()
  File "/opt/repos/k/depot_tools/gclient.py", line 689, in ParseDepsFile
    gclient_eval.Exec(deps_content, global_scope, local_scope, filepath)
  File "/opt/repos/k/depot_tools/gclient_eval.py", line 209, in Exec
    _visit_in_module(stmt)
  File "/opt/repos/k/depot_tools/gclient_eval.py", line 193, in _visit_in_module
    value = _gclient_eval(node.value, global_scope, filename=filename)
  File "/opt/repos/k/depot_tools/gclient_eval.py", line 173, in _gclient_eval
    return _convert(node_or_string)
  File "/opt/repos/k/depot_tools/gclient_eval.py", line 146, in _convert
    for k, v in zip(node.keys, node.values))
  File "/usr/lib/python2.7/collections.py", line 69, in __init__
    self.__update(*args, **kwds)
  File "/usr/lib/python2.7/_abcoll.py", line 571, in update
    for key, value in other:
  File "/opt/repos/k/depot_tools/gclient_eval.py", line 146, in <genexpr>
    for k, v in zip(node.keys, node.values))
  File "/opt/repos/k/depot_tools/gclient_eval.py", line 146, in _convert
    for k, v in zip(node.keys, node.values))
  File "/usr/lib/python2.7/collections.py", line 69, in __init__
    self.__update(*args, **kwds)
  File "/usr/lib/python2.7/_abcoll.py", line 571, in update
    for key, value in other:
  File "/opt/repos/k/depot_tools/gclient_eval.py", line 146, in <genexpr>
    for k, v in zip(node.keys, node.values))
  File "/opt/repos/k/depot_tools/gclient_eval.py", line 165, in _convert
    return _convert(node.left) + _convert(node.right)
  File "/opt/repos/k/depot_tools/gclient_eval.py", line 167, in _convert
    return _convert(node.left) % _convert(node.right)
TypeError: not all arguments converted during string formatting
Traceback (most recent call last):
  File "/opt/repos/k/depot_tools/fetch.py", line 299, in <module>
    sys.exit(main())
  File "/opt/repos/k/depot_tools/fetch.py", line 294, in main
    return run(options, spec, root)
  File "/opt/repos/k/depot_tools/fetch.py", line 288, in run
    return checkout.init()
  File "/opt/repos/k/depot_tools/fetch.py", line 131, in init
    self.run_gclient(*sync_cmd)
  File "/opt/repos/k/depot_tools/fetch.py", line 78, in run_gclient
    return self.run(cmd_prefix + cmd, **kwargs)
  File "/opt/repos/k/depot_tools/fetch.py", line 68, in run
    return subprocess.check_output(cmd, **kwargs)
  File "/usr/lib/python2.7/subprocess.py", line 574, in check_output
    raise CalledProcessError(retcode, cmd, output=output)
subprocess.CalledProcessError: Command '('gclient', 'sync')' returned non-zero exit status 1
 
Looks like this is a problem with breakpad specifically. I am able to fetch webrtc
Labels: Needs-Milestone
Cc: kkaluri@chromium.org
Labels: TE-NeedsTriageHelp
Unable to triage this issue from TE-End, hence adding "TE-NeedsTriageHelp" for further triage.

Comment 4 by bcl...@mozilla.com, Aug 3 2017

There is a typo/thinko in gclient_eval.py where % was used instead of +. This fixes it for me.
gclient_eval.patch
628 bytes Download
Dartino sync also craches.

The issue was introduced with:

commit b7e533360cc46f05cd35432d9cb64880b89eb176
Author: Pawe<C5><82> Hajdan, Jr <phajdan.jr@chromium.org>
Date:   Tue May 23 16:57:37 2017 +0200

    gclient validate: schema fixes (patch #1)
    
    - allow None in deps_os values (for chromeos buildspecs)
    - allow string formatting (%) in expressions (for dart)
    - allow empty lists/dicts (for build slaves)
    - allow target_os (for internal android)
    
    Bug:  570091 , 722206, 725422
    Change-Id: I7f3d822a1c4aac16fddd96ef2f9f365e05b2d0e8
    Reviewed-on: https://chromium-review.googlesource.com/512724
    Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
    Commit-Queue: Pawe<C5><82> Hajdan Jr. <phajdan.jr@chromium.org>
Cc: phajdan.jr@chromium.org
Components: Infra>SDK
Cc: -phajdan.jr@chromium.org
Labels: -OS-Linux -TE-NeedsTriageHelp -Via-Wizard-Other -Needs-Milestone
Owner: phajdan.jr@chromium.org
Status: Assigned (was: Unconfirmed)
Summary: gclient's new Var() impl no longer supports "Var('foo') % bar" due to placeholders (was: depot_tools crashes with latest changeset)
The patch in Comment 4 is wrong; the % (mod) operator is in fact the correct operator to use when the node.op is ast.Mod. Replacing that with + makes it identical to the case above, when the operator is ast.Add.

This problem was fixed for Breakpad by updating their gyp dependency past 4ff48eb18307. The relevant contents of that commit are:

     "src/third_party/pefile":
-      (Var("googlecode_url") % "pefile") + "/trunk@63",
+       Var("chromium_git") + "external/pefile@72c6ae4",
   },

The problem here is that Var("googlecode_url") doesn't get evaluated down to the expected string "http://%s.googlecode.com/svn". It is instead being evaluated down to "{googlecode_url}" (which doesn't have a %s format specifier).

This is caused by https://chromium-review.googlesource.com/c/586594/, which changed the way the Var() function works, by having it emit placeholders rather than evaluating all the way down.

@milos: You can fix this by updating dartino's GYP dependency past 4ff48eb18307, or by finding whatever other dependency uses "Var('foo') % bar" and fixing that.

@pawel: I'm updating the summary and assigning this to you. If you think the best course of action is simply to update all DEPS files which use "Var('foo') % bar" to... not do that, then go ahead and do that instead of fixing gclient. Either way, this should be addressed.
Owner: ----
Status: Untriaged (was: Assigned)
Removing myself from bugs because of team transfer, back to re-triage.

See https://goto.google.com/phajdan-goodbye-chrome (Google-internal) and  issue 783662  .

In case of any questions, feel free to ask - use phajdan@google.com for a faster response.

Sign in to add a comment