fix: add flag to use runtime venv creation when using bootstrap=script

[rickeylev]

The bootstrap=script implementation was changed to use declare_symlink() to create explicit
symlinks so its venv works. Unfortunately, this broke packaging rules, which would treat
the symlinks as regular files.

To fix, introduce a flag that stops using declare_symlink() and instead creates the venv
at runtime. Creating a venv at runtime is problematic for various reasons, but this should
work well enough until packaging rules are able to handle these raw symlinks.

The location of the venv can be somewhat controlled by setting the RULES_PYTHON_VENVS_ROOT
environment variable. This is to better accommodate cases where using /tmp is problematic.

Along the way, sort the environment variable docs by their name.

Fixes #2489

[rickeylev]

Ah doh, i forgot to undo some of the diffs. I need to revert .bazelversion and remove the rules_pkg stuff -- that was just debug code.

[rickeylev]

PTAL. Ci is happy, tests added, cleaned up.

[aignas]

LGTM in general with a few questions.

I think that #2598 should be merged first. I am not 100% sure if the two PRs are related or not - my current understanding is that we cannot have the venv being set by the features introduced here and build a zip at the same time, but I would like CI to verify that.

The only question is the env variable name. The _VENVS_ plurality is probably the only thing I would like to decide on before we set that in stone.

[rickeylev]

merge #2598 first

SGTM.

I am not 100% sure if the two PRs are related or not - my current understanding is that we cannot have the venv being set by the features introduced here and build a zip at the same time, but I would like CI to verify that.

They're not related -- the problematic case was trying to use e.g. rules_pkg to take a non-zip py_binary and "naively" put it into a zipfile. When a zip py_binary is created, it doesn't use declare_symlink (it instead recreates just the interpreter symlinks).

The confusion is understandable, though -- there's just too many permuations of outputs a py_binary produces: build_python_zip={yes,no} x bootstrap={script,system_python}, plus how that affects the python_zip_file output group, and now this flag (which only affects build_python_zip=no, bootstrap=script).

The only question is the env variable name. The _VENVS_ plurality is probably the only thing I would like to decide on before we set that in stone.

I picked a plural name to capture that there could be multiple venvs in the directory. This could occur if the binary has another binary in the data dependencies.

Ideas for a new name? I agree the name could be a bit confusing. It basically acts as the directory that any temporary stuff will be created into. Re-using it for e.g. where a zip-py_binary extracts into seems like a reasonable thing to do (another PR for that, though).

RULES_PYTHON_EXTRACT_ROOT ?

[aignas]

I like the suggestion of RULES_PYTHON_EXTRACT_ROOT.

Re confusion about the zips - thanks for the eexplanation. I guess people could workaround the current issues in the tar rules by taring the zipapp and rules_python would do the right thing when decompressing the zip file and executing it. I would not call it the best or most intuitive solution, but it is one.

[rickeylev]

Bah, I don't like that relative_venv_symlinks flag name anymore. I think I'll rename it.

It's not really about whether a relative path is used in the venv. It's about whether declare_symlink() with a "raw" path is used, which requires the runfiles to have a particular layout.

--build_venv_at_runtime=yes|no ; --raw_venv_symlinks=yes|no ?

re: RULES_PYTHON_EXTRACT_ROOT -- SGTM, I'll rename it.

re: tar a zipapp: Yes, that'd work. The zipapps are a bit annoying because they always extract to a tmp dir. This adds startup overhead and /tmp files can get cleaned up (which long lived processes don't like).

[rickeylev]

PTAL

Renamed to RULES_PYTHON_EXTRACT_ROOT

Renamed to --venvs_use_declare_symlink

[rickeylev]

Ok, done changes and fixed some doc types i saw along the way. PTAL

[aignas]

Thanks!