-
-
Notifications
You must be signed in to change notification settings - Fork 42
feat(venv): Static site-packages trees #551
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
e4f8f87
to
cdbe199
Compare
5672208
to
128642f
Compare
128642f
to
9cfdd0b
Compare
ed05c90
to
a5313f7
Compare
), | ||
}) | ||
|
||
def _python_version_transition_impl(_, attr): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can this transition live in a single place?
|
||
echo "Created virtualenv in ${VIRTUAL_ENV}" | ||
exec "$(rlocation "{{VENV}}")"/bin/python {{INTERPRETER_FLAGS}} "$@" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
previously this rule was used for IDE support via bazel run :venv
to create the .venv symlink in the source tree. we are probably breaking those people here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looking good, a few things, rust can be better but i think its good enough for now.
imports_depset = _py_library.make_imports_depset(ctx, extra_imports_depsets = virtual_resolution.imports) | ||
|
||
pth_lines = ctx.actions.args() | ||
pth_lines.use_param_file("%s", use_always = True) | ||
pth_lines.use_param_file("%s/", use_always = True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pth_lines.use_param_file("%s/", use_always = True) | |
pth_lines.use_param_file("%s", use_always = True) |
# which ends up in bazel_tools/tools/python/runfiles/runfiles.py, but there are no imports attrs that hint we | ||
# should be adding the root to the PYTHONPATH | ||
# Maybe in the future we can opt out of this? | ||
pth_lines.add(".") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets double check if this is breaking
VenvMode::StaticPth => { | ||
let venv = py::venv::create_empty_venv( | ||
&args.python, | ||
py::venv::PythonVersionInfo::from_str(&args.version.unwrap())?, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.unwrap() here sounds dangerous. do we want to check and print an error message?
LGTM, nice job! |
This PR introduces a new suite of virtualenv machinery building on the existing
py_binary
virtualenv tree builder to create such trees "statically" at build time rather than "dynamically" at runtime. The original reason this is difficult is that the$VENV/bin/python
file is usually a link to a static interpreter, and the interpreter specified inpyvenv.cfg
is also usually an absolute path to a real interpreter outside of the venv.The introduction of #546 or something like it makes it possible to create a virtualenv tree which does not refer to an interpreter with a fixed path. Instead an appropriate interpreter can be dynamically discovered by
$PATH
inspection. Support for other configuration ala/etc/pexrc
to facilitate interpreter discovery is possible but beyond the scope of an initial implementation.By using an interpreter shim, the
pyenv.cfg
file can use a relative path to./bin/python
as its interpreter reference, and the./bin/python3
and./bin/python3.X
links can be created statically pointing to the shim. As long as otherbin/
entrypoint scripts reference only#!/usr/bin/env python3
as is fairly conventional, the constructed virtual environment will contain no absolute references to a specific interpreter and will be fully relocatable.A relocatable virtualenv tree containing dependencies installed into the
site-packages
tree may be built using the new experimentalpy_venv
rule.Files are selected for inclusion into virtualenv based on import paths.
*.dist-info
child directory will have its contents included.site-packages
and for which there exist files in a siblingbin
or other conventional directory will have those assets included into the virtualenv.These heuristics are chosen to both handle how
rules_python
currently emulates package installation, and to avoid dependencies on external workspace specific behavior so that internal or vendored libraries can be "installed" into the venv.A
py_venv
is not intended to be a.runfiles
-style packaging of all source code, it is intended to be a packaging of dependency code which is compliant with being treated as ifpip install
ed. This may not be generally true of firstparty application code, depending on usage ofrunfiles.Runfiles
or repository-relative logics. Because of thispy_venv
targets provide files not relocated into the venv asrunfiles
dependencies.Applications wishing to leverage a static virtualenv should leverage the
py_venv_binary
rule.A
py_venv_binary
defines a more conventional.runfiles
-based Python application, but one which will use the.runfiles
embedded virtualenv as the source of thirdparty packages. Firstparty packages will receive the normal.runfiles
workspace structure preserving treatment.A
py_venv_test
is also provided which behaves the same as apy_venv_binary
.TODO
py_venv_binary
withrules_oci
together to create a static applicationFor follow-up work
Fix for [Bug]: py_venv doesn't include dependency executables in venv bin/ #423Not currently possible, but we do have support whenrules_python
catches uprules_pkg
Changes are visible to end-users: yes
Test plan