weixin_39678304
2021-01-07 14:24 阅读 1

Improved venv inspection

  • [x] I have added an entry to docs/changelog.md

Summary of changes

This is the successor to (and based on) #588 . Closes #587. Closes #534. Closes #528 .

This integrates the purpose of venv_metadata_inspector.py into code that is part of the pipx package and is no longer stand-alone. The breakthrough is that importlib.metadata allows us to specify the path for it to search for package metadata. This means that the venv inspection code no longer needs to run in the venv environment itself, but simply specify what sys.path would be if the venv environment were active.

EDIT: We can also fetch the venv environment of environment markers and apply it when evaluating package requirements to determine if a dependency is a match for the environment (see packaging.markers.Marker.evaluate).

The new code does not need to be run in a subprocess nor exchange data via json, and benefits from logging and python3.6+ of being a native part of pipx.

There is only one tiny subprocess that runs inside the venv to be examined. It gets the venv python version and what sys.path is when the venv is operational. I used the entire sys.path of the activated venv because that is the existing behavior, and also in case the user includes options to use system libraries to fulfill dependencies. EDIT: the subprocess now also fetches a dict specifying the venv environment for purposes of matching environment markers.

Additionally a huge speedup of the inspection for certain packages has been accomplished. These two lines are responsible for up to a 2 second speedup in certain cases! https://github.com/pipxproject/pipx/blob/48f6fe60169ef65120dc374152ed5d40a3a36bf2/src/pipx/venv_inspect.py#L92-L93 get_apps() was taking a huge amount of time, and the loop that checks each file was taking all the time. This if-statement was a much faster way of knowing that the file is not an app than the pathlib checks in the loop. Some selected packages and venv inspection time (start to finish) are shown below. "Optimization" refers to the two lines in the code linked to above.

venv inspection times (less is better) | package | this PR | pipx v0.15.6.0 | this PR no optimization | | ----------------|-----------:|------------:|------------:| | ansible==2.9.13 | 259ms | 1738ms | 1689ms | | awscli | 176ms | 1140ms | 998ms | | beancount | 484ms | 1110ms | 820ms | | cloudtoken | 239ms | 697ms | 462ms | | kibitzr | 380ms | 838ms | 779ms | | kolibri | 270ms | 2456ms | 2455ms | | magic-wormhole | 536ms | 1410ms | 1028ms | | mkdocs | 153ms | 324ms | 413ms | | sphinx | 274ms | 795ms | 394ms |

After this PR merges I hope to release pipx 0.16.0.0!

60+ packages (pipx slow tests) pass fine https://github.com/itsayellow/pipx/actions/runs/446326073

Test plan

Tested by running


# command(s) to exercise these changes

该提问来源于开源项目:pipxproject/pipx

  • 点赞
  • 写回答
  • 关注问题
  • 收藏
  • 复制链接分享

6条回答 默认 最新

  • weixin_39678304 weixin_39678304 2021-01-07 14:24

    The "small subprocess code" has grown to be a bit larger than it started. I'm happy to break it out into a separate file if people think that's a good idea. Let me know if people are interested in that.

    点赞 评论 复制链接分享
  • weixin_39814378 weixin_39814378 2021-01-07 14:24

    Maybe name the script venv_metadata_inspector.py and restore the mechanism used to call it? :p

    点赞 评论 复制链接分享
  • weixin_39678304 weixin_39678304 2021-01-07 14:24

    Maybe name the script venv_metadata_inspector.py and restore the mechanism used to call it? :p

    Yes I guess so. :) That whole mechanism seemed to so tortured to me, but I can't argue with success.

    点赞 评论 复制链接分享
  • weixin_39678304 weixin_39678304 2021-01-07 14:24

    I can't tell how serious your request is with the :p emoji. 🙂

    Do you think it's ok to keep the command-string inline? I kind of like it as it is because it's so simple and obvious. And I remember having issues finding the old venv_metadata_inspector.py in the code when I was doing experiments freezing the code with PyInstaller.

    Plus right now it's not all that complicated code.

    点赞 评论 复制链接分享
  • weixin_39814378 weixin_39814378 2021-01-07 14:24

    I think at its current length it's okay to do this either way. If you decide to make this a standalone file, all previous mechanisms should return (of course), but I'd want the file to be named something different; venv_metadata_inspector.py is pretty vague and arguably does not reflect what your current script actually does.

    点赞 评论 复制链接分享
  • weixin_39678304 weixin_39678304 2021-01-07 14:24

    Ah ok, then I'm totally in agreement. I think since the code right now is about one function in length, I like leaving it where it is.

    点赞 评论 复制链接分享

相关推荐