That's super cool! I think we should reconsider our custom handling of these two libraries given this new development.

Is it feasible to use the vpython environment intended for the bots on developers' machines too? Is there documentation somewhere?

I definitely agree we should use this instead of the custom solution. Especially if we can use vpython in all environments.

> That's super cool! I think we should reconsider our custom handling of these two libraries given this new development.
> 
> Is it feasible to use the vpython environment intended for the bots on developers' machines too?

Yes, absolutely, that's exactly what the tool is for! "vpython" itself is available to all bots/builds automatically in PATH, and is available to all developers via a "depot_tools" bootstrap. Environments are specialized based on architecture and Python parameters. We build wheels for a subset of all possible combinations of those, but that subset matches all supported developer environments for *most* wheels.

>  Is there documentation somewhere?

Plenty! I wrote a general rundown here: https://chromium.googlesource.com/infra/infra/+/master/doc/users/vpython.md

That has links to more lower-level documentation about the tool itself and Infra's specialization of it. Finally, failing that, don't hesitate to ask me about anything.

I should also mention that Chromium has a default "root environment": https://chromium.googlesource.com/chromium/src/+/master/.vpython

If you use "vpython" to launch any scripts underneath "src/", and those don't supply a higher-level ".vpython" file, they will automatically use this environment. This can be trivially augmented or completely overridden as-needed.

kbr, could you find an owner for this? I see this is basically related to:
https://github.com/catapult-project/catapult/issues/2865

Ideally, we would like the vpython environment to specify the cv2/numpy dependencies required, and have it work on both bots and developer workstations.

I think this should also allow to write in scripts "import cv2" and assume that the correct dependencies are available. The current "external_modules.ImportRequiredModule('cv2')" makes things brittle and also just trickier to code-search.

perezju: kainino or I would potentially be happy to pick this up but would like to understand the current situation a little better first. Are all Telemetry runs on all bots (both Swarmed and non-Swarmed) using vpython at this point? And/or what is needed to get to that state?

Basically, can you please enumerate the steps that need to be taken to get there from here?

I've read both:
https://chromium.googlesource.com/infra/infra/+/master/doc/users/vpython.md
and studied Issue 762005, but have to admit I'm still a little confused about what needs to be done.

I also noticed in a recent build:
https://ci.chromium.org/buildbot/chromium.gpu/Linux%20Release%20%28NVIDIA%29/113916

specifically:
https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium.gpu%2FLinux_Release__NVIDIA_%2F113916%2F%2B%2Frecipes%2Fsteps%2Fcontext_lost_tests_on_NVIDIA_GPU_on_Linux_on_Ubuntu%2F0%2Fstdout

that it looks like the hand-built cv2 isn't working on some of the Swarmed Linux bots anyway:
WARNING:root:Unable to import cv2 due to: /usr/lib/x86_64-linux-gnu/libstdc++.so.6: version `GLIBCXX_3.4.20' not found (required by /b/s/w/ir/third_party/catapult/telemetry/third_party/cv2/lib/cv2_linux_x86_64_85be3046d2fef651206d7daadbd1b34af2a005f5/cv2.so)

> Are all Telemetry runs on all bots (both Swarmed and non-Swarmed) using vpython at this point?

I believe the answer is "no".

> And/or what is needed to get to that state? Basically, can you please enumerate the steps that need to be taken to get there from here?

1) Identify all of the Python package dependencies in those builders and add them to Chromium's ".vpython" file.
2) Run as much locally as you can to validate that you caught all of those dependencies.
3) Identify call points to these tests in recipes and set "venv=True" in the "self.m.python" tool.
4) Observe fallout, if there are any breakages either repair or revert (3).
5) Repeat until everything works.

As far as I can remember, the only issues that have manifested turning to "vpython" are:
- System package dependencies that aren't present in the "vpython" specification file.
- Unsupported architectures (e.g., ppc64le)

You'll probably want some agility to respond to any missing packages that do show up in the form of infra support (+iannucci@?). This would likely align with an infra goal, so that seems reasonable.

> that it looks like the hand-built cv2 isn't working on some of the Swarmed Linux bots anyway

Sure does! It's worth noting Infra's OpenCV is also ... hand-built by me, but that one was built on a "manylinux1" Docker container, which is a build/linking environment designed to work across Linux OSes. I'm not an OpenCV build expert nor did I robustly test it, but at the very least if that wheel doesn't work somewhere we can fix it in one place for everyone.

Regarding step (3): is there a way to specify "venv: True" in the Chromium-side JSON files? Looking through https://cs.chromium.org/chromium/build/scripts/slave/recipe_modules/chromium_tests/steps.py it seems the answer is no.

Is this the way we'd want to add this support anyway? Or should we wire up one of the scripts like:
https://cs.chromium.org/chromium/src/testing/scripts/run_gpu_integration_test_as_googletest.py

to manually invoke vpython?

perezju@, where else has Telemetry been switched over to use vpython?

I don't think anyone's instrumented "venv=True" into Chromium-side JSON.

Up to you how invocation works. Just keep in mind that adding something like "#!/usr/bin/env vpython" to a script has a notable shortcoming: if the script is invoked explicitly via Python (e.g., recipe is ['python', '/path/to/script']), which most are, then it will skip the shebang line.

The few options I've implemented have been:
- Recipe-driven explicit "vpython" invocation.
- Shebang ("#!") line invocation in user-executed tooling with self-bootstrapping hack: https://chromium.googlesource.com/chromium/tools/build/+/master/scripts/slave/unittests/test_env.py#16
- Combination of recipe being explicit w/ a shebang line in there for users, maybe with an assertion message if not invoked through "vpython".

(As an aside, you can detect if you're running in a VirtualEnv by checking the "VIRTUAL_ENV" environment variable).

So something like:
assert os.getenv('VIRTUAL_ENV'), 'This script is designed to be run through "vpython".'

There have been chats about replacing the top-level "python" program with "vpython" and having it fall through to an underlying Python interpreter (the way we did with "git") if a specification couldn't be found. This *would* unify invocation, but is guaranteed to be a highly sensitive operation that will probably break the world, and so I wouldn't count on it in the near future.

> Are all Telemetry runs on all bots (both Swarmed and non-Swarmed) using vpython at this point?

I did a quick check now and, I think, only the downstream non-swarmed bots are running using vpython at the moment.

I'm now in the process of switching over the catapult tryjobs too:
https://crbug.com/777865

Ideally (if our test coverage is sufficient enough) much of the identification of what packages need to be included in the vpython environment should happen there.

I believe upstream perf bots running on swarming are *not* yet on vpython. E.g. on the log posted in #7, I don't find references to vpython (we use sys.executable to run tsproxy commands, so that should be a give away.)

So, if OpenCV is not working there today, it hadn't probably worked in a long time anyway.

Dan, Robbie, do you know when or how will the switch to vpython happen on swarmed bots?

Talking with nodir@ and dpranke@ I'm going to plumb a flag through to the JSON files in src/testing/buildbot allowing individual tests to choose to use vpython. Then we'll be able to send tryjobs through testing whether things work.

Bumping to P2 and blocking on a new bug. The native libraries for these packages are broken right now on both Windows and Linux as far as I can tell and we want to switch to vpython ASAP, but there's a problem doing so for Swarming tasks.

Talked more with iannucci@ about this in particular regarding  Issue 716241  which re-surfaced on one developer's machine today. It sounds like the Infra team is aiming to switch everything to use vpython, which would eliminate this problem categorically and avoid more work plumbing a use_vpython flag into the src-side JSON files. Going to wait to do anything more on this.

Going to wait for Issue 801339 to be resolved before doing anything further on this.

FWIW I think that I have already built custom numpy / cv2 wheel packages for vpython, so that part should be taken care of: https://chromium.googlesource.com/infra/infra/+/master/infra/tools/dockerbuild/wheels.md

They're currently in use by Chromium: https://chromium.googlesource.com/chromium/src/+/master/.vpython#37

Yes, thanks much dnj@. We just haven't switched these Telemetry based tests over to using vpython yet because there was no good way to do so.

https://chromium-review.googlesource.com/872205 up for review changing Telemetry's handling of these third_party directories. This should allow a clean switch-over to using vpython for the GPU tests -- and eventual deletion of Telemetry's custom-built cv2 and numpy.

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/d64eeb3568ff622bba744a3c2d8ef4e90ff814d1

commit d64eeb3568ff622bba744a3c2d8ef4e90ff814d1
Author: Kenneth Russell <kbr@chromium.org>
Date: Thu Jan 18 22:55:21 2018

Use vpython's wheels for the cv2 and numpy libraries.

If running on vpython (which can reliably be detected by
"VIRTUAL_ENV" being set in the environment), then don't add the
third_party directories for these to the Python path.

Verified manually that when invoked via vpython, the GPU tests pick up
vpython's versions of these libraries:

  vpython content/test/gpu/run_gpu_integration_test.py pixel \
    --browser=system \
    --test-filter=Pixel_ScissorTestWithPreserveDrawingBuffer

BUG= chromium:776804 
Change-Id: I56483e666e28e0a5154b864d265d7e3b924fb273
Reviewed-on: https://chromium-review.googlesource.com/872205
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Kenneth Russell <kbr@chromium.org>

[modify] https://crrev.com/d64eeb3568ff622bba744a3c2d8ef4e90ff814d1/telemetry/telemetry/__init__.py

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

commit db246834025692b4cea01d5614564b2a7cbd61e5
Author: catapult-deps-roller@chromium.org <catapult-deps-roller@chromium.org>
Date: Fri Jan 19 12:54:03 2018

Roll src/third_party/catapult/ b4706e732..51d9354bb (9 commits)

https://chromium.googlesource.com/catapult.git/+log/b4706e73204b..51d9354bb0d4

$ git log b4706e732..51d9354bb --date=short --no-merges --format='%ad %ae %s'
2018-01-19 perezju Fix Fakes for telemetry_gpu_unittests to pass.
2018-01-18 kbr Use vpython's wheels for the cv2 and numpy libraries.
2018-01-17 dtu [pinpoint] Shorten display name for patches.
2018-01-18 simonhatch Dashboard - Remove bisect fyi bot code.
2018-01-18 eakuefner [Dashboard] Generate old-style rows for histogram uploads
2018-01-18 charliea Log wmic exit code when it fails
2018-01-18 achuith [oobe]: Use --oobe-skip-to-login instead of Oobe.skipToLoginForTesting.
2018-01-17 perezju [Telemetry] Migrate cros_test_case.CrOSTestCase to new browser API
2018-01-18 perezju Reland "[Telemetry] Migrate SeriallyExecutedBrowserTestCase to new API"

Created with:
  roll-dep src/third_party/catapult
BUG= 801578 , 776804 , 753870 


The AutoRoll server is located here: https://catapult-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=master.tryserver.chromium.android:android_optional_gpu_tests_rel
TBR=sullivan@chromium.org

Change-Id: I0836e5381460acc83837bce9ed5e752e233c0344
Reviewed-on: https://chromium-review.googlesource.com/875904
Reviewed-by: <catapult-deps-roller@chromium.org>
Commit-Queue: <catapult-deps-roller@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530493}
[modify] https://crrev.com/db246834025692b4cea01d5614564b2a7cbd61e5/DEPS

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

commit aad276cbbcd9385c1426331a6d464a0ee2ca377f
Author: Kenneth Russell <kbr@chromium.org>
Date: Wed Jan 31 07:54:00 2018

Use vpython rather than python for GPU tests on the bots.

From talking with iannucci@, this is the easiest way to use vpython
with these tests on the bots. Once the LUCI conversion is done, this
will be a no-op, and can be removed.

Incorporate the newly deployed pyobjc wheels into Chromium's root
.vpython; they are needed by helper scripts in the GPU directory.

Bug:  776804 , 804174
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I1c720a9f7992e411aa85bcc7959de95ecc03c1f5
Reviewed-on: https://chromium-review.googlesource.com/869512
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Kenneth Russell <kbr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533226}
[modify] https://crrev.com/aad276cbbcd9385c1426331a6d464a0ee2ca377f/.vpython
[modify] https://crrev.com/aad276cbbcd9385c1426331a6d464a0ee2ca377f/testing/scripts/run_gpu_integration_test_as_googletest.py

From my standpoint, we could now delete Telemetry's numpy and cv2 third_party modules, if the Telemetry team agrees that we could do that. Ned, Juan, what do you think? Or can we simply close this as Fixed without doing that?

Turning around the blocked on / blocking relationship with Issue 801339.

+1 to remove numpy and cv2 from third_party, and all references to those particular versions in the code. Thanks for looking into this!

+1 for cleaning up cvs & numpy. I don't particularly love that hack either :)

Just filling in some info, but the build instructions for the numpy and cv2 packages used by vpython are part of a code repository, much like the telemetry numpy/cv2: https://chromium.googlesource.com/infra/infra/+/2faa1f2ac925ddb00cfe142db60fa0e3e8b184b8/infra/tools/dockerbuild/wheel.py#481

Builds are performed, where possible, in Docker containers for consistency, too.
 (If this mitigates any concerns)

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

commit fc1cd93287a7c939a7facf3cf2096cb167ae2896
Author: Balazs Engedy <engedy@chromium.org>
Date: Wed Jan 31 16:00:46 2018

Revert "Use vpython rather than python for GPU tests on the bots."

This reverts commit aad276cbbcd9385c1426331a6d464a0ee2ca377f.

Reason for revert: Suspected to be the culprit for breaking Mac browser_tests and unit_tests that use an local python test server, e.g.:

 RUN      ] CloudPolicyTest.FetchPolicy
[4513:779:0131/005831.599342:3098052568384:ERROR:local_test_server_posix.cc(81)] poll() timed out; bytes_read=0
[4513:779:0131/005831.599552:3098052743513:ERROR:local_test_server_posix.cc(168)] Could not read server_data_len
../../chrome/browser/policy/cloud/cloud_policy_browsertest.cc:190: Failure
Value of: test_server_->Start()
  Actual: false
Expected: true

Original change's description:
> Use vpython rather than python for GPU tests on the bots.
> 
> From talking with iannucci@, this is the easiest way to use vpython
> with these tests on the bots. Once the LUCI conversion is done, this
> will be a no-op, and can be removed.
> 
> Incorporate the newly deployed pyobjc wheels into Chromium's root
> .vpython; they are needed by helper scripts in the GPU directory.
> 
> Bug:  776804 , 804174
> Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
> Change-Id: I1c720a9f7992e411aa85bcc7959de95ecc03c1f5
> Reviewed-on: https://chromium-review.googlesource.com/869512
> Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
> Commit-Queue: Kenneth Russell <kbr@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#533226}

TBR=iannucci@chromium.org,kbr@chromium.org,dnj@chromium.org,nednguyen@google.com

Change-Id: Idb03a7a1176d79443b3a29267a16c7631fbf1be8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  776804 , 804174
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/894208
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Commit-Queue: Balazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533285}
[modify] https://crrev.com/fc1cd93287a7c939a7facf3cf2096cb167ae2896/.vpython
[modify] https://crrev.com/fc1cd93287a7c939a7facf3cf2096cb167ae2896/testing/scripts/run_gpu_integration_test_as_googletest.py

engedy@: yes, I'm sorry about that. That failure mode was seen during development but I and we thought that restricting vpython's pyobjc wheel to be used only on later macOS versions was sufficient.

For completeness, the first failing build on multiple bots was seen here:

https://ci.chromium.org/buildbot/chromium.mac/Mac10.10%20Tests/28680
https://ci.chromium.org/buildbot/chromium.mac/Mac10.11%20Tests/22890
https://ci.chromium.org/buildbot/chromium.mac/Mac10.12%20Tests/9680

I'll ask on Issue 804174 that this be investigated further.

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

commit 4eeae7ba49f935ac91e2eca183c34bc5095540cf
Author: Kenneth Russell <kbr@chromium.org>
Date: Sat Feb 17 01:26:28 2018

Invoke color_profile_manager via system Python on macOS.

Until a pyobjc wheel can be provided in Chrome Infra's vpython
environment, which will require a few more steps, manually run this
test with the system Python which has pyobjc built in.

This will allow vpython-as-python to be deployed everywhere, and then
more changes to be made on top.

TBR=ccameron@chromium.org

Bug:  776804 , 804174, 812693
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Ib6a3a19ff30f2ecb72f51a23ffa028b6789f3ddb
Reviewed-on: https://chromium-review.googlesource.com/924614
Commit-Queue: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537495}
[modify] https://crrev.com/4eeae7ba49f935ac91e2eca183c34bc5095540cf/content/test/gpu/gpu_tests/color_profile_manager.py
[add] https://crrev.com/4eeae7ba49f935ac91e2eca183c34bc5095540cf/content/test/gpu/utilities/color_profile_manager_impl.py
[rename] https://crrev.com/4eeae7ba49f935ac91e2eca183c34bc5095540cf/content/test/gpu/utilities/color_profile_manager_mac.py

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

commit c4782ec17d5208ed25efbbc3611893f090c7fc0d
Author: Kenneth Russell <kbr@chromium.org>
Date: Thu Mar 08 06:34:17 2018

Use vpython's pyobjc wheel for color_profile_manager_mac.

This reverts commit 4eeae7ba49f935ac91e2eca183c34bc5095540cf and adds
the pyobjc wheel to Chromium's root .vpython. At this point, vpython
is used on all of the bots for the system Python, so it should be
suitably warmed up before getting into any of Chromium's tests.

Bug:  776804 , 804174
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I35c602ca6e0368532685ae8afc3139692a6153fa
Reviewed-on: https://chromium-review.googlesource.com/954704
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Kenneth Russell <kbr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541740}
[modify] https://crrev.com/c4782ec17d5208ed25efbbc3611893f090c7fc0d/.vpython
[modify] https://crrev.com/c4782ec17d5208ed25efbbc3611893f090c7fc0d/content/test/gpu/gpu_tests/color_profile_manager.py
[rename] https://crrev.com/c4782ec17d5208ed25efbbc3611893f090c7fc0d/content/test/gpu/gpu_tests/color_profile_manager_mac.py
[delete] https://crrev.com/300cd0df1125cce946d77bf041da44a3e8082dff/content/test/gpu/utilities/color_profile_manager_impl.py

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

commit fc202566f4b6ad4b4d317284e2e2c3bb9453458b
Author: Kenneth Russell <kbr@chromium.org>
Date: Thu Mar 08 19:24:40 2018

Stop fetching Telemetry's binary dependencies on chromium.gpu.fyi.

The only reason for fetching them on these machines was to pull the
platform-specific cv2 and numpy binaries. These are now being provided
via vpython wheels.

This also implies that associated permissions for some cloud storage
buckets can now be removed.

Bug:  776804 , 804174
Change-Id: I1de969cc16b2e1a46d3ba4a84422fd4b8dd56be7
Reviewed-on: https://chromium-review.googlesource.com/954982
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Commit-Queue: Kenneth Russell <kbr@chromium.org>

[modify] https://crrev.com/fc202566f4b6ad4b4d317284e2e2c3bb9453458b/scripts/slave/recipes/chromium_trybot.expected/dont_deapply_patch.json
[modify] https://crrev.com/fc202566f4b6ad4b4d317284e2e2c3bb9453458b/scripts/slave/recipe_modules/chromium_tests/chromium_gpu_fyi.py

It turns out that there is another reason that Telemetry's binary dependencies and benchmark dependencies are being fetched – https://cs.chromium.org/chromium/build/scripts/slave/recipe_modules/chromium/gclient_config.py?type=cs&q=checkout_telemetry_dependencies&sq=package:chromium&l=24 .

While it would be ideal to only fetch the benchmark pages on perf bots, this is a cleanup for another day.

At this point we're using vpython's numpy and cv2 wheels to provide speedups of the GPU tests which take screenshots, like screenshot_sync_tests and pixel_test.

Builds like this one:
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac%20FYI%20Retina%20Release%20%28AMD%29/29

show that screenshot_sync_tests is running in ~45 seconds, where it would take many minutes if it were running without these Python packages.

Considering this fixed at this point. We can consider deleting Telemetry's built-in numpy and cv2 packages separately.

Awesome! Thanks for driving this work!

Nicely done! Would you mind filing a (P3?) cleanup bug for #38 so it doesn't get lost in the void?

Thanks, good idea. Filed  Issue 820504  to track it.