virtualize the trait Reid was stuck on

[myrrlyn]

Code review per request

[myrrlyn]

this can be stripped. I wrote it when I first made the real trait virtualizable, so that call sites would continue to compile. The gist is that the blanket impl on 510 makes the .path<_, _>() method continue to exist on everything that had it, and because that method has a different typesig and fully qualified name (<T as PthEntryHandlerExt>::plan<_, _>(...) vs <T as PthEntryHandler>::path(...), it's not actually an ambiguous symbol to call. But now that all the callers are using the monomorph form rather than the generic, it doesn't do anything.

[myrrlyn]

By switching from a match with one arm acting and the other arm doing-work-then-matching-again to if let with a return, I took out one level of indentation with no other code changes. The second match still needs the unreachable! in it because rustc doesn't have the ability to statically strip enum variants in cases like these. Annoying but c'est la vie. Beats writing Haskal

[myrrlyn]

quit calling Option::expect() in -> Result functions. that is what .ok_or()? and .ok_or_else()? are for.

[myrrlyn]

you have to annotate the virtual type here, because if you don't, the match will type-hint off the first real type it returns. The first arm types as !, which isn't real. The second arm is Box<py::venv::PthStrategy>, which means all other arms need to type as that concrete instance. ! types that way, but Box<py::venv::FirstPartyThirdPartyStrategy> does not.

You can dodge this by typing the binding site (let strat: ...) or by as-casting the box. Box::new(...) as Box<dyn ...> is more annoying, imo

[myrrlyn]

goofy but it works. Unfortunately, &dyn Trait does not actually count as an implementor of Trait unless you explicitly write an impl block for it. You Typically Do Not Want To Do This. There are exceptions in libstd but it's really not common.

[aspect-workflows]

Test

14 test targets passed

Targets

//examples/multi_version:py_version_default_test [k8-fastbuild]                         1s
//examples/multi_version:py_version_test [k8-fastbuild-ST-494921797612]                 1s
//examples/pytest:pytest_test [k8-fastbuild]                                            2s
//examples/pytest:sharded/test [k8-fastbuild]                                           3s
//examples/virtual_deps:pytest_test [k8-fastbuild]                                      1s
//py/tests/cc-deps:test_smoke [k8-fastbuild]                                            451ms
//py/tests/external-deps:test_can_import_runfiles_helper [k8-fastbuild]                 552ms
//py/tests/internal-deps:assert [k8-fastbuild]                                          611ms
//py/tests/py-binary:runfiles_from_pip_test [k8-fastbuild]                              808ms
//py/tests/py-test:test_env_vars [k8-fastbuild]                                         451ms
//py/tests/py-venv-disable-usersite:test [k8-fastbuild]                                 162ms
//py/tests/py-venv-enable-site:test [k8-fastbuild]                                      157ms
//py/tests/py_image_layer:py_image_test [k8-fastbuild]                                  5s
//py/tests/repo_relative_imports/test:test [k8-fastbuild]                               499ms

Total test execution time was 18s. 31 tests (68.9%) were fully cached saving 1m 8s.

[github-actions]

e2e/use_release folder: LCOV of commit bc2ba9a during CI #1987

Summary coverage rate:
  lines......: 100.0% (2 of 2 lines)
  functions..: 100.0% (1 of 1 function)
  branches...: no data found

Files changed coverage rate: n/a