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

Issue 821199 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Evaluate vars in gclient_eval

Project Member Reported by ehmaldonado@chromium.org, Mar 12 2018

Issue description

When calling gclient_eval.Exec, Var('x') is replaced by '{x}' and a further pass is necessary to convert '{x}' into whatever 'x' was declared to be in the 'vars' dict.

I think gclient_eval should take care of it.

We could do this by saying something like 
global_scope = {'Var': lambda v: local_scope['vars'][v]}
Which would fail if Var is used before the vars dict was declared, but that would be an easy fix in the offending DEPS file.
 

Comment 1 by whesse@google.com, Mar 13 2018

I thought that the change to very late evaluation of variables was intentional, so that one of the export formats (GN?) or one of the late gclient stages could get the variables as variables, and override them, rather than having them expanded.

Would this still preserve whatever behavior was needed, to avoid expanding the variables until late in the process?
`gn flatten` needs the variables preserved where possible; the format can and should be normalized to the '{}' version, though, i.e., Var() should not be present in the output. Otherwise it should be safe to evaluate the vars when parsing the file.
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 22 2018

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

commit 88f9c40e0ccefb491790888a703c857b0c3ec6d1
Author: Edward Lesmes <ehmaldonado@chromium.org>
Date: Thu Mar 22 20:56:06 2018

gclient eval: Expand vars while parsing DEPS files

Introduce a Parse function that takes care of expanding vars while parsing
the DEPS file.

It wraps Exec and exec calls, and supports deferring the expansion until
later, so gclient flatten gets access to the unexpanded version.

Bug:  821199 
Change-Id: I943b021cc4474c9cda67b3816b841dd8ada3f5b2
Reviewed-on: https://chromium-review.googlesource.com/973749
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Aaron Gable <agable@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>

[modify] https://crrev.com/88f9c40e0ccefb491790888a703c857b0c3ec6d1/gclient_eval.py
[modify] https://crrev.com/88f9c40e0ccefb491790888a703c857b0c3ec6d1/gclient.py
[modify] https://crrev.com/88f9c40e0ccefb491790888a703c857b0c3ec6d1/tests/gclient_eval_unittest.py

Running "gclient sync" is throwing this error:

Traceback (most recent call last):
  File "/usr/local/google/home/prasadv/depot_tools/gclient.py", line 103, in <module>
    import gclient_eval
  File "/usr/local/google/home/prasadv/depot_tools/gclient_eval.py", line 359
    exec(content, global_scope, local_scope)
SyntaxError: unqualified exec is not allowed in function 'Parse' it contains a nested function with free variables

Looks like this is related to this change.
Project Member

Comment 5 by bugdroid1@chromium.org, Mar 22 2018

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

commit 728244f70a8b9d21757c6c24159ee41fe9f32ddd
Author: Benjamin Pastene <bpastene@chromium.org>
Date: Thu Mar 22 21:31:37 2018

Revert "gclient eval: Expand vars while parsing DEPS files"

This reverts commit 88f9c40e0ccefb491790888a703c857b0c3ec6d1.

Reason for revert: suspected to be causing update scripts failures and prevent gce bots from connecting
https://build.chromium.org/deprecated/tryserver.chromium.win/builders/win7_chromium_rel_ng/builds/128479/steps/update_scripts/logs/stdio

Original change's description:
> gclient eval: Expand vars while parsing DEPS files
> 
> Introduce a Parse function that takes care of expanding vars while parsing
> the DEPS file.
> 
> It wraps Exec and exec calls, and supports deferring the expansion until
> later, so gclient flatten gets access to the unexpanded version.
> 
> Bug:  821199 
> Change-Id: I943b021cc4474c9cda67b3816b841dd8ada3f5b2
> Reviewed-on: https://chromium-review.googlesource.com/973749
> Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
> Reviewed-by: Aaron Gable <agable@chromium.org>
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>

TBR=agable@chromium.org,dpranke@chromium.org,ehmaldonado@chromium.org

Change-Id: Ib9b84c13ef9b56735fd8f714b74a6601c61715e5
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  821199 
Reviewed-on: https://chromium-review.googlesource.com/976721
Reviewed-by: Benjamin Pastene <bpastene@chromium.org>
Commit-Queue: Benjamin Pastene <bpastene@chromium.org>

[modify] https://crrev.com/728244f70a8b9d21757c6c24159ee41fe9f32ddd/gclient_eval.py
[modify] https://crrev.com/728244f70a8b9d21757c6c24159ee41fe9f32ddd/gclient.py
[modify] https://crrev.com/728244f70a8b9d21757c6c24159ee41fe9f32ddd/tests/gclient_eval_unittest.py

Project Member

Comment 6 by bugdroid1@chromium.org, Mar 23 2018

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

commit f311c119ab164b10b083c09fe4ade377cd348c18
Author: depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Thu Mar 22 22:49:15 2018

Roll src/third_party/depot_tools/ c621b21ad..88f9c40e0 (3 commits)

https://chromium.googlesource.com/chromium/tools/depot_tools.git/+log/c621b21ad4bf..88f9c40e0cce

$ git log c621b21ad..88f9c40e0 --date=short --no-merges --format='%ad %ae %s'
2018-03-22 ehmaldonado gclient eval: Expand vars while parsing DEPS files
2018-03-22 iannucci Trigger recipe roll to pick up 5025893cb6061b8a219750155cccd67b55079311.
2018-03-19 iannucci [presubmit_support] Prevent depot_tools' virtualenv from leaking into other repos.

Created with:
  roll-dep src/third_party/depot_tools
BUG= chromium:821199 , chromium:821669 , chromium:821669 


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: I8471b021376c10b08dd3241f5083c1be87ea6c85
Reviewed-on: https://chromium-review.googlesource.com/976625
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@{#545276}
[modify] https://crrev.com/f311c119ab164b10b083c09fe4ade377cd348c18/DEPS

Project Member

Comment 7 by bugdroid1@chromium.org, Mar 23 2018

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

commit b8f69bfd811f571fb552d8274ac1db445f0723df
Author: depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Fri Mar 23 01:12:09 2018

Roll src/third_party/depot_tools/ 88f9c40e0..728244f70 (1 commit)

https://chromium.googlesource.com/chromium/tools/depot_tools.git/+log/88f9c40e0cce..728244f70a8b

$ git log 88f9c40e0..728244f70 --date=short --no-merges --format='%ad %ae %s'
2018-03-22 bpastene Revert "gclient eval: Expand vars while parsing DEPS files"

Created with:
  roll-dep src/third_party/depot_tools
BUG= chromium:821199 


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: I0b62df44a0c0e601200632a1c52d81475e1f04a4
Reviewed-on: https://chromium-review.googlesource.com/976874
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@{#545320}
[modify] https://crrev.com/b8f69bfd811f571fb552d8274ac1db445f0723df/DEPS

Project Member

Comment 8 by bugdroid1@chromium.org, Mar 23 2018

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

commit 0d9ecc925d389c0ae72014d15a021788b2e824f2
Author: Edward Lesmes <ehmaldonado@chromium.org>
Date: Fri Mar 23 21:41:09 2018

Reland "gclient eval: Expand vars while parsing DEPS files"

This is a reland of 88f9c40e0ccefb491790888a703c857b0c3ec6d1

Original change's description:
> gclient eval: Expand vars while parsing DEPS files
> 
> Introduce a Parse function that takes care of expanding vars while parsing
> the DEPS file.
> 
> It wraps Exec and exec calls, and supports deferring the expansion until
> later, so gclient flatten gets access to the unexpanded version.
> 
> Bug:  821199 
> Change-Id: I943b021cc4474c9cda67b3816b841dd8ada3f5b2
> Reviewed-on: https://chromium-review.googlesource.com/973749
> Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
> Reviewed-by: Aaron Gable <agable@chromium.org>
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>

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

[modify] https://crrev.com/0d9ecc925d389c0ae72014d15a021788b2e824f2/gclient_eval.py
[modify] https://crrev.com/0d9ecc925d389c0ae72014d15a021788b2e824f2/gclient.py
[modify] https://crrev.com/0d9ecc925d389c0ae72014d15a021788b2e824f2/tests/gclient_eval_unittest.py

Project Member

Comment 9 by bugdroid1@chromium.org, Mar 23 2018

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

commit 325503ce9c14a4685fb36be0c7df9053e7b5ab9b
Author: depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Fri Mar 23 23:25:39 2018

Roll src/third_party/depot_tools/ 728244f70..0d9ecc925 (3 commits)

https://chromium.googlesource.com/chromium/tools/depot_tools.git/+log/728244f70a8b..0d9ecc925d38

$ git log 728244f70..0d9ecc925 --date=short --no-merges --format='%ad %ae %s'
2018-03-23 ehmaldonado Reland "gclient eval: Expand vars while parsing DEPS files"
2018-03-23 sergiyb Correct documentation for the --no-referenced-issues argument
2018-03-23 iannucci Clear $VPYTHON_CLEAR_PYTHONPATH when setting $PYTHONPATH.

Created with:
  roll-dep src/third_party/depot_tools
BUG= chromium:821199 , chromium:825290 , chromium:825174 


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: I58cbb0ca469176bb99a5a86d7504c986fa9b162b
Reviewed-on: https://chromium-review.googlesource.com/978865
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@{#545622}
[modify] https://crrev.com/325503ce9c14a4685fb36be0c7df9053e7b5ab9b/DEPS

Project Member

Comment 10 by bugdroid1@chromium.org, Mar 28 2018

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

commit 6c24d37fe9e9650a9c42fa7796cf443d2f2deb1f
Author: Edward Lesmes <ehmaldonado@chromium.org>
Date: Wed Mar 28 17:11:53 2018

Reland "gclient eval: Expand vars while parsing DEPS files"

This is a reland of 88f9c40e0ccefb491790888a703c857b0c3ec6d1

We no longer need the vars dict to be declared before vars can be used.
This was causing problems because gclient flatten printed vars after deps,
breaking the above assumption.

Original change's description:
> gclient eval: Expand vars while parsing DEPS files
>
> Introduce a Parse function that takes care of expanding vars while parsing
> the DEPS file.
>
> It wraps Exec and exec calls, and supports deferring the expansion until
> later, so gclient flatten gets access to the unexpanded version.
>
> Bug:  821199 
> Change-Id: I943b021cc4474c9cda67b3816b841dd8ada3f5b2
> Reviewed-on: https://chromium-review.googlesource.com/973749
> Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
> Reviewed-by: Aaron Gable <agable@chromium.org>
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>

Bug:  821199 
Change-Id: I583df23558f91871e1a2aa2574c20d35e54635f6
Reviewed-on: https://chromium-review.googlesource.com/981086
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Michael Moss <mmoss@chromium.org>
Reviewed-by: Aaron Gable <agable@chromium.org>

[modify] https://crrev.com/6c24d37fe9e9650a9c42fa7796cf443d2f2deb1f/gclient_eval.py
[modify] https://crrev.com/6c24d37fe9e9650a9c42fa7796cf443d2f2deb1f/tests/gclient_smoketest.py
[modify] https://crrev.com/6c24d37fe9e9650a9c42fa7796cf443d2f2deb1f/gclient.py
[modify] https://crrev.com/6c24d37fe9e9650a9c42fa7796cf443d2f2deb1f/tests/gclient_eval_unittest.py

Project Member

Comment 11 by bugdroid1@chromium.org, Mar 28 2018

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

commit 390482f0a81336dcdb26310f36213ac1cfc4c4a1
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 Mar 28 20:10:51 2018

Roll src/third_party/depot_tools/ 96fc33383..6c24d37fe (1 commit)

https://chromium.googlesource.com/chromium/tools/depot_tools.git/+log/96fc33383b6d..6c24d37fe9e9

$ git log 96fc33383..6c24d37fe --date=short --no-merges --format='%ad %ae %s'
2018-03-28 ehmaldonado Reland "gclient eval: Expand vars while parsing DEPS files"

Created with:
  roll-dep src/third_party/depot_tools
BUG= chromium:821199 


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: I1b8339ece880aa2cc3fe7eb48572f69d671070c6
Reviewed-on: https://chromium-review.googlesource.com/984336
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@{#546580}
[modify] https://crrev.com/390482f0a81336dcdb26310f36213ac1cfc4c4a1/DEPS

Status: Fixed (was: Assigned)

Sign in to add a comment