Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2336753002:
Roll ANGLE c955058..e79c2d1 (Closed)
DescriptionRoll ANGLE c955058..e79c2d1
https://chromium.googlesource.com/angle/angle.git/+log/c955058..e79c2d1
BUG=chromium:644610
TEST=bots
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Committed: https://crrev.com/65b5df92da95c574434beb322d9eb4c7e2658dd5
Cr-Commit-Position: refs/heads/master@{#418099}
Patch Set 1 #
Total comments: 1
Patch Set 2 : rebase #
Total comments: 2
Patch Set 3 : Don't change test_runner.pydeps #Messages
Total messages: 29 (7 generated)
ynovikov@chromium.org changed reviewers: + geofflang@chromium.org, jbudorick@chromium.org
https://codereview.chromium.org/2336753002/diff/1/build/android/test_runner.p... File build/android/test_runner.pydeps (left): https://codereview.chromium.org/2336753002/diff/1/build/android/test_runner.p... build/android/test_runner.pydeps:6: ../../third_party/catapult/common/py_utils/py_utils/lock.py I don't think this is right. When was the last time you updated your checkout?
On 2016/09/12 19:47:19, jbudorick wrote: > https://codereview.chromium.org/2336753002/diff/1/build/android/test_runner.p... > File build/android/test_runner.pydeps (left): > > https://codereview.chromium.org/2336753002/diff/1/build/android/test_runner.p... > build/android/test_runner.pydeps:6: > ../../third_party/catapult/common/py_utils/py_utils/lock.py > I don't think this is right. When was the last time you updated your checkout? About 5 minutes ago. I've just pulled and double checked.
On 2016/09/12 19:50:46, ynovikov wrote: > On 2016/09/12 19:47:19, jbudorick wrote: > > > https://codereview.chromium.org/2336753002/diff/1/build/android/test_runner.p... > > File build/android/test_runner.pydeps (left): > > > > > https://codereview.chromium.org/2336753002/diff/1/build/android/test_runner.p... > > build/android/test_runner.pydeps:6: > > ../../third_party/catapult/common/py_utils/py_utils/lock.py > > I don't think this is right. When was the last time you updated your checkout? > > About 5 minutes ago. I've just pulled and double checked. Maybe roll_angle behaves differently for me, since I'm doing it from clank checkout? I'll try chromium.
On 2016/09/12 19:52:03, ynovikov wrote: > On 2016/09/12 19:50:46, ynovikov wrote: > > On 2016/09/12 19:47:19, jbudorick wrote: > > > > > > https://codereview.chromium.org/2336753002/diff/1/build/android/test_runner.p... > > > File build/android/test_runner.pydeps (left): > > > > > > > > > https://codereview.chromium.org/2336753002/diff/1/build/android/test_runner.p... > > > build/android/test_runner.pydeps:6: > > > ../../third_party/catapult/common/py_utils/py_utils/lock.py > > > I don't think this is right. When was the last time you updated your > checkout? > > > > About 5 minutes ago. I've just pulled and double checked. > > Maybe roll_angle behaves differently for me, since I'm doing it from clank > checkout? I'll try chromium. Possibly. Regardless, rolling ANGLE shouldn't affect test_runner.pydeps at all...
On 2016/09/12 19:53:10, jbudorick wrote: > On 2016/09/12 19:52:03, ynovikov wrote: > > On 2016/09/12 19:50:46, ynovikov wrote: > > > On 2016/09/12 19:47:19, jbudorick wrote: > > > > > > > > > > https://codereview.chromium.org/2336753002/diff/1/build/android/test_runner.p... > > > > File build/android/test_runner.pydeps (left): > > > > > > > > > > > > > > https://codereview.chromium.org/2336753002/diff/1/build/android/test_runner.p... > > > > build/android/test_runner.pydeps:6: > > > > ../../third_party/catapult/common/py_utils/py_utils/lock.py > > > > I don't think this is right. When was the last time you updated your > > checkout? > > > > > > About 5 minutes ago. I've just pulled and double checked. > > > > Maybe roll_angle behaves differently for me, since I'm doing it from clank > > checkout? I'll try chromium. > > Possibly. Regardless, rolling ANGLE shouldn't affect test_runner.pydeps at > all... I've gclient sync'ed now. Is it better? I had to update test_runner.pydeps because of presubmit check: ynovikov@ynovikov-z620:~/work/clank/src$ ./tools/roll_angle.py Pinning src/third_party/angle to revision e79c2d1f3f08963ec3cf01f7c81e4a7d048e79bf in /usr/local/google/work/clank/src/DEPS ERROR:root:Command failed: ['git', 'cl', 'upload'] Using 50% similarity for rename/copy detection. Override with --similarity. Running presubmit upload checks ... ** Presubmit ERRORS ** File is stale: build/android/test_runner.pydeps To regenerate, run: build/print_python_deps.py --root build/android --output build/android/test_runner.pydeps build/android/test_runner.py Presubmit checks took 2.1s to calculate.
jbudorick@chromium.org changed reviewers: + nednguyen@google.com
+nednguyen
On 2016/09/12 19:56:39, ynovikov wrote: > On 2016/09/12 19:53:10, jbudorick wrote: > > On 2016/09/12 19:52:03, ynovikov wrote: > > > On 2016/09/12 19:50:46, ynovikov wrote: > > > > On 2016/09/12 19:47:19, jbudorick wrote: > > > > > > > > > > > > > > > https://codereview.chromium.org/2336753002/diff/1/build/android/test_runner.p... > > > > > File build/android/test_runner.pydeps (left): > > > > > > > > > > > > > > > > > > > > https://codereview.chromium.org/2336753002/diff/1/build/android/test_runner.p... > > > > > build/android/test_runner.pydeps:6: > > > > > ../../third_party/catapult/common/py_utils/py_utils/lock.py > > > > > I don't think this is right. When was the last time you updated your > > > checkout? > > > > > > > > About 5 minutes ago. I've just pulled and double checked. > > > > > > Maybe roll_angle behaves differently for me, since I'm doing it from clank > > > checkout? I'll try chromium. > > > > Possibly. Regardless, rolling ANGLE shouldn't affect test_runner.pydeps at > > all... > > I've gclient sync'ed now. Is it better? > I had to update test_runner.pydeps because of presubmit check: > ynovikov@ynovikov-z620:~/work/clank/src$ ./tools/roll_angle.py > Pinning src/third_party/angle > to revision e79c2d1f3f08963ec3cf01f7c81e4a7d048e79bf > in /usr/local/google/work/clank/src/DEPS > ERROR:root:Command failed: ['git', 'cl', 'upload'] > Using 50% similarity for rename/copy detection. Override with --similarity. > Running presubmit upload checks ... > > ** Presubmit ERRORS ** > File is stale: build/android/test_runner.pydeps > To regenerate, run: > > build/print_python_deps.py --root build/android --output > build/android/test_runner.pydeps build/android/test_runner.py > > Presubmit checks took 2.1s to calculate. Updating would've made a difference had you not updated today. Investigating.
lgtm
https://codereview.chromium.org/2336753002/diff/20001/build/android/test_runn... File build/android/test_runner.pydeps (left): https://codereview.chromium.org/2336753002/diff/20001/build/android/test_runn... build/android/test_runner.pydeps:5: ../../third_party/catapult/common/py_utils/py_utils/cloud_storage_global_lock Removing this will surely make the test fail the CQ. I think the fix here is probably edit print_python_deps.py script so that we can hardcode this path in. +John: thoughts?
https://codereview.chromium.org/2336753002/diff/20001/build/android/test_runn... File build/android/test_runner.pydeps (left): https://codereview.chromium.org/2336753002/diff/20001/build/android/test_runn... build/android/test_runner.pydeps:5: ../../third_party/catapult/common/py_utils/py_utils/cloud_storage_global_lock On 2016/09/12 20:42:41, nednguyen wrote: > Removing this will surely make the test fail the CQ. I think the fix here is > probably edit print_python_deps.py script so that we can hardcode this path in. > > +John: thoughts? I'm not sure that it will. I cleanly repro'd this...
On 2016/09/12 20:46:05, jbudorick wrote: > https://codereview.chromium.org/2336753002/diff/20001/build/android/test_runn... > File build/android/test_runner.pydeps (left): > > https://codereview.chromium.org/2336753002/diff/20001/build/android/test_runn... > build/android/test_runner.pydeps:5: > ../../third_party/catapult/common/py_utils/py_utils/cloud_storage_global_lock > On 2016/09/12 20:42:41, nednguyen wrote: > > Removing this will surely make the test fail the CQ. I think the fix here is > > probably edit print_python_deps.py script so that we can hardcode this path > in. > > > > +John: thoughts? > > I'm not sure that it will. I cleanly repro'd this... Let's check if this fails the CQ or not ...
The CQ bit was checked by ynovikov@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2016/09/12 20:51:09, ynovikov wrote: > On 2016/09/12 20:46:05, jbudorick wrote: > > > https://codereview.chromium.org/2336753002/diff/20001/build/android/test_runn... > > File build/android/test_runner.pydeps (left): > > > > > https://codereview.chromium.org/2336753002/diff/20001/build/android/test_runn... > > build/android/test_runner.pydeps:5: > > ../../third_party/catapult/common/py_utils/py_utils/cloud_storage_global_lock > > On 2016/09/12 20:42:41, nednguyen wrote: > > > Removing this will surely make the test fail the CQ. I think the fix here is > > > probably edit print_python_deps.py script so that we can hardcode this path > > in. > > > > > > +John: thoughts? > > > > I'm not sure that it will. I cleanly repro'd this... > > Let's check if this fails the CQ or not ... I stand corrected; it is indeed failing. Ned has a CL that should fix this in the short term.
On 2016/09/12 21:34:44, jbudorick wrote: > On 2016/09/12 20:51:09, ynovikov wrote: > > On 2016/09/12 20:46:05, jbudorick wrote: > > > > > > https://codereview.chromium.org/2336753002/diff/20001/build/android/test_runn... > > > File build/android/test_runner.pydeps (left): > > > > > > > > > https://codereview.chromium.org/2336753002/diff/20001/build/android/test_runn... > > > build/android/test_runner.pydeps:5: > > > > ../../third_party/catapult/common/py_utils/py_utils/cloud_storage_global_lock > > > On 2016/09/12 20:42:41, nednguyen wrote: > > > > Removing this will surely make the test fail the CQ. I think the fix here > is > > > > probably edit print_python_deps.py script so that we can hardcode this > path > > > in. > > > > > > > > +John: thoughts? > > > > > > I'm not sure that it will. I cleanly repro'd this... > > > > Let's check if this fails the CQ or not ... > > I stand corrected; it is indeed failing. > > Ned has a CL that should fix this in the short term. Sorry for your annoyance, you can update the DEPS of catapult to point to latest commit once https://codereview.chromium.org/2331033004/ lands & run build/print_python_deps.py script again
The CQ bit was unchecked by ynovikov@chromium.org
On 2016/09/12 21:37:09, nednguyen wrote: > On 2016/09/12 21:34:44, jbudorick wrote: > > On 2016/09/12 20:51:09, ynovikov wrote: > > > On 2016/09/12 20:46:05, jbudorick wrote: > > > > > > > > > > https://codereview.chromium.org/2336753002/diff/20001/build/android/test_runn... > > > > File build/android/test_runner.pydeps (left): > > > > > > > > > > > > > > https://codereview.chromium.org/2336753002/diff/20001/build/android/test_runn... > > > > build/android/test_runner.pydeps:5: > > > > > > ../../third_party/catapult/common/py_utils/py_utils/cloud_storage_global_lock > > > > On 2016/09/12 20:42:41, nednguyen wrote: > > > > > Removing this will surely make the test fail the CQ. I think the fix > here > > is > > > > > probably edit print_python_deps.py script so that we can hardcode this > > path > > > > in. > > > > > > > > > > +John: thoughts? > > > > > > > > I'm not sure that it will. I cleanly repro'd this... > > > > > > Let's check if this fails the CQ or not ... > > > > I stand corrected; it is indeed failing. > > > > Ned has a CL that should fix this in the short term. > > Sorry for your annoyance, you can update the DEPS of catapult to point to latest > commit once https://codereview.chromium.org/2331033004/ lands & run > build/print_python_deps.py script again Update: the fix is landed as https://github.com/catapult-project/catapult/commit/0f8747a99ab1380bf05b2276d...
On 2016/09/12 21:55:19, nednguyen wrote: > On 2016/09/12 21:37:09, nednguyen wrote: > > On 2016/09/12 21:34:44, jbudorick wrote: > > > On 2016/09/12 20:51:09, ynovikov wrote: > > > > On 2016/09/12 20:46:05, jbudorick wrote: > > > > > > > > > > > > > > > https://codereview.chromium.org/2336753002/diff/20001/build/android/test_runn... > > > > > File build/android/test_runner.pydeps (left): > > > > > > > > > > > > > > > > > > > > https://codereview.chromium.org/2336753002/diff/20001/build/android/test_runn... > > > > > build/android/test_runner.pydeps:5: > > > > > > > > > ../../third_party/catapult/common/py_utils/py_utils/cloud_storage_global_lock > > > > > On 2016/09/12 20:42:41, nednguyen wrote: > > > > > > Removing this will surely make the test fail the CQ. I think the fix > > here > > > is > > > > > > probably edit print_python_deps.py script so that we can hardcode this > > > path > > > > > in. > > > > > > > > > > > > +John: thoughts? > > > > > > > > > > I'm not sure that it will. I cleanly repro'd this... > > > > > > > > Let's check if this fails the CQ or not ... > > > > > > I stand corrected; it is indeed failing. > > > > > > Ned has a CL that should fix this in the short term. > > > > Sorry for your annoyance, you can update the DEPS of catapult to point to > latest > > commit once https://codereview.chromium.org/2331033004/ lands & run > > build/print_python_deps.py script again > > Update: the fix is landed as > https://github.com/catapult-project/catapult/commit/0f8747a99ab1380bf05b2276d... Could you also roll catapult and update test_runner.pydeps? Thanks!
The CQ bit was checked by ynovikov@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from jbudorick@chromium.org Link to the patchset: https://codereview.chromium.org/2336753002/#ps40001 (title: "Don't change test_runner.pydeps")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2016/09/12 21:58:11, ynovikov wrote: > On 2016/09/12 21:55:19, nednguyen wrote: > > On 2016/09/12 21:37:09, nednguyen wrote: > > > On 2016/09/12 21:34:44, jbudorick wrote: > > > > On 2016/09/12 20:51:09, ynovikov wrote: > > > > > On 2016/09/12 20:46:05, jbudorick wrote: > > > > > > > > > > > > > > > > > > > > > https://codereview.chromium.org/2336753002/diff/20001/build/android/test_runn... > > > > > > File build/android/test_runner.pydeps (left): > > > > > > > > > > > > > > > > > > > > > > > > > > > https://codereview.chromium.org/2336753002/diff/20001/build/android/test_runn... > > > > > > build/android/test_runner.pydeps:5: > > > > > > > > > > > > ../../third_party/catapult/common/py_utils/py_utils/cloud_storage_global_lock > > > > > > On 2016/09/12 20:42:41, nednguyen wrote: > > > > > > > Removing this will surely make the test fail the CQ. I think the fix > > > here > > > > is > > > > > > > probably edit print_python_deps.py script so that we can hardcode > this > > > > path > > > > > > in. > > > > > > > > > > > > > > +John: thoughts? > > > > > > > > > > > > I'm not sure that it will. I cleanly repro'd this... > > > > > > > > > > Let's check if this fails the CQ or not ... > > > > > > > > I stand corrected; it is indeed failing. > > > > > > > > Ned has a CL that should fix this in the short term. > > > > > > Sorry for your annoyance, you can update the DEPS of catapult to point to > > latest > > > commit once https://codereview.chromium.org/2331033004/ lands & run > > > build/print_python_deps.py script again > > > > Update: the fix is landed as > > > https://github.com/catapult-project/catapult/commit/0f8747a99ab1380bf05b2276d... > > Could you also roll catapult and update test_runner.pydeps? Thanks! Oh, I think I misunderstood your comment, I guess you wanted me to do it. I understood the comment as suggesting to make a local change. Anyway, I don't think rolling ANGLE and catapult in the same CL would be a good idea, so could you please do it separately?
On 2016/09/12 22:11:58, ynovikov wrote: > On 2016/09/12 21:58:11, ynovikov wrote: > > On 2016/09/12 21:55:19, nednguyen wrote: > > > On 2016/09/12 21:37:09, nednguyen wrote: > > > > On 2016/09/12 21:34:44, jbudorick wrote: > > > > > On 2016/09/12 20:51:09, ynovikov wrote: > > > > > > On 2016/09/12 20:46:05, jbudorick wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://codereview.chromium.org/2336753002/diff/20001/build/android/test_runn... > > > > > > > File build/android/test_runner.pydeps (left): > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://codereview.chromium.org/2336753002/diff/20001/build/android/test_runn... > > > > > > > build/android/test_runner.pydeps:5: > > > > > > > > > > > > > > > > ../../third_party/catapult/common/py_utils/py_utils/cloud_storage_global_lock > > > > > > > On 2016/09/12 20:42:41, nednguyen wrote: > > > > > > > > Removing this will surely make the test fail the CQ. I think the > fix > > > > here > > > > > is > > > > > > > > probably edit print_python_deps.py script so that we can hardcode > > this > > > > > path > > > > > > > in. > > > > > > > > > > > > > > > > +John: thoughts? > > > > > > > > > > > > > > I'm not sure that it will. I cleanly repro'd this... > > > > > > > > > > > > Let's check if this fails the CQ or not ... > > > > > > > > > > I stand corrected; it is indeed failing. > > > > > > > > > > Ned has a CL that should fix this in the short term. > > > > > > > > Sorry for your annoyance, you can update the DEPS of catapult to point to > > > latest > > > > commit once https://codereview.chromium.org/2331033004/ lands & run > > > > build/print_python_deps.py script again > > > > > > Update: the fix is landed as > > > > > > https://github.com/catapult-project/catapult/commit/0f8747a99ab1380bf05b2276d... > > > > Could you also roll catapult and update test_runner.pydeps? Thanks! > > Oh, I think I misunderstood your comment, I guess you wanted me to do it. > I understood the comment as suggesting to make a local change. > Anyway, I don't think rolling ANGLE and catapult in the same CL would be a good > idea, so could you please do it separately? Sure
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Roll ANGLE c955058..e79c2d1 https://chromium.googlesource.com/angle/angle.git/+log/c955058..e79c2d1 BUG=chromium:644610 TEST=bots CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel ========== to ========== Roll ANGLE c955058..e79c2d1 https://chromium.googlesource.com/angle/angle.git/+log/c955058..e79c2d1 BUG=chromium:644610 TEST=bots CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel Committed: https://crrev.com/65b5df92da95c574434beb322d9eb4c7e2658dd5 Cr-Commit-Position: refs/heads/master@{#418099} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/65b5df92da95c574434beb322d9eb4c7e2658dd5 Cr-Commit-Position: refs/heads/master@{#418099} |
