feat(venv): add relocatable flag #5515

paveldikov · 2024-07-28T14:19:01Z

Summary

Adds a --relocatable CLI arg to uv venv. This flag does two things:

ensures that the associated activation scripts do not rely on a hardcoded
absolute path to the virtual environment (to the extent possible; .csh and
.nu left as-is)
persists a relocatable flag in pyvenv.cfg.

The flag in pyvenv.cfg in turn instructs the wheel Installer to create script
entrypoints in a relocatable way (use exec trick + dirname $0 on POSIX;
use relative path to python[w].exe on Windows).

Fixes: #3863

Test Plan

Relocatable console scripts covered as additional scenarios in existing test cases.
Integration testing of boilerplate generation in venv.
Manual testing of uv venv with and without --relocatable

samypr100 · 2024-07-28T16:53:48Z

crates/uv-cli/src/lib.rs

+    /// Other scripts may be adjusted if they ship with a generic `#!python[w]` shebang,
+    /// and binaries are left as-is.
+    #[arg(long)]
+    pub relocatable: bool,


Likely worth marking this as a preview/experimental flag

would that be a simple hide = true, or is there a special annotation for that? (couldn't find one)

I was thinking using PreviewMode for this in venv_impl

Added warning (assuming that's what you meant?)

Yes, thanks!

charliermarsh · 2024-07-28T17:08:22Z

This has a little bit of overlap with #5509 which just does the installer piece and isn't exposed to users.

charliermarsh · 2024-07-28T17:13:44Z

One potential issue here is that users can't use relocatable with --target or --prefix, since those commands don't operate within a virtualenv 🤔

paveldikov · 2024-07-28T17:17:51Z

This has a little bit of overlap with #5509 which just does the installer piece and isn't exposed to users.

Yes, I see that! Feel free to cherry-pick pieces out of this as you see fit (and vice versa -- let me know if I am straying too far away from the direction you had in mind)

paveldikov · 2024-07-28T17:24:19Z

One potential issue here is that users can't use relocatable with --target or --prefix, since those commands don't operate within a virtualenv 🤔

I thought --python was vastly more recommended for this very reason?

charliermarsh · 2024-07-28T17:28:58Z

crates/install-wheel-rs/src/wheel.rs

@@ -131,6 +131,7 @@ fn copy_and_hash(reader: &mut impl Read, writer: &mut impl Write) -> io::Result<
 fn format_shebang(executable: impl AsRef<Path>, os_name: &str) -> String {
    // Convert the executable to a simplified path.
    let executable = executable.as_ref().simplified_display().to_string();
+    let is_relocatable = !executable.contains(['\\', '/']);


Does this assume that the relative path doesn't contain multiple segments? (That's true for virtual environments, but could be false in general, right?)

On second thought, this check is probably unnecessary and I might just pass the already-known is_relocatable as an argument, rather than trying to reverse-engineer it.

Refactored.

charliermarsh · 2024-07-28T17:29:18Z

Yes, I see that! Feel free to cherry-pick pieces out of this as you see fit (and vice versa -- let me know if I am straying too far away from the direction you had in mind)

Sounds good! Ultimately pretty similar, I'll likely merge yours first.

charliermarsh · 2024-07-28T17:32:44Z

We might want to add some tests by wiring this up through TestContext (perhaps with a new constructor to make it easiest).

paveldikov · 2024-07-28T17:35:41Z

We might want to add some tests by wiring this up through TestContext (perhaps with a new constructor to make it easiest).

I'll try to dig a little deeper tonight, but if you have examples of similar tests handy, that would be grand!

charliermarsh · 2024-07-28T17:37:28Z

@paveldikov -- A basic test would be like install_many in pip_sync.rs. I think we just want light coverage for, like: installing and importing a package; installing and running a script; moving the virtualenv (even just renaming) and doing the same.

charliermarsh · 2024-07-28T17:37:45Z

You can run cargo test -p uv --test pip_sync install_many to run an individual test.

Adds a `--relocatable` CLI arg to `uv venv`. This flag does two things: * ensures that the associated activation scripts do not rely on a hardcoded absolute path to the virtual environment (to the extent possible; `.csh` and `.nu` left as-is) * persists a `relocatable` flag in `pyvenv.cfg`. The flag in `pyvenv.cfg` in turn instructs the wheel `Installer` to create script entrypoints in a relocatable way (use `exec` trick + `dirname $0` on POSIX; use relative path to `python[w].exe` on Windows). Fixes: astral-sh#3863

paveldikov · 2024-07-28T19:06:50Z

Added a test case for the boilerplate on the venv side. Let me know if there are other areas in the critical path that you'd like to see covered -- I've mostly focussed on the two extremities (venv and installer) since this is where most of the action is happening.

edit: woops, saw that you'd asked for

covering package install scenario
check if renaming venv base dir makes entrypoint still work

Going to add those in. Does any of the already-existing stub packages in scripts/ contain meaningful console_scripts?

paveldikov · 2024-07-28T20:36:43Z

Added coverage for pip_install.

installing and importing a package; installing and running a script; moving the virtualenv (even just renaming) and doing the same.

Unless I'm reading this wrong, running a console script also implies importing the package, so I didn't handle that separately.

charliermarsh · 2024-07-28T20:46:17Z

Thank you! Haven't reviewed yet but per your last question, I believe black_editable has an entrypoint at least.

paveldikov · 2024-07-28T20:47:50Z

I believe black_editable has an entrypoint at least.

Yep, I did find it and used it specifically for its Hello World :-P

charliermarsh · 2024-07-28T23:24:24Z

Cool, this generally looks good to me! I'm going to make a few small tweaks to make it a bit easier to rebase #5509, but not anticipating any major changes. Thank you for the tests too.

charliermarsh · 2024-07-28T23:27:34Z

crates/install-wheel-rs/src/wheel.rs

+        // (note: the Windows trampoline binaries natively support relative paths to executable)
+        if shebang_length > 127 || executable.contains(' ') || is_relocatable {
+            let prefix = if is_relocatable {
+                r#""$(CDPATH= cd -- "$(dirname -- "$0")" && echo "$PWD")"/"#


What's the benefit of this over just dirname $0 alone as in https://github.com/astral-sh/uv/pull/5509/files#diff-c1686969b46b2c133e184a8c25069ada51363d91354e35fac208bb67239fcf2cR183? (I have no idea!)

Seems like there's a check if the directory exists (avoiding readlink/realpath friends) before substituting it with PWD which makes this a bit more cross-posix-platform and handle a couple error scenarios. Interestingly enough, I'd expect the / to be inside the echo though if this was the intention? Unless it's expected if cd fails that the output is "/" instead of "". Thoughts?

Seems like there's a check if the directory exists (avoiding readlink/realpath friends) before substituting it with PWD which makes this a bit more cross-posix-platform and handle a couple error scenarios.

Yes, that's exactly it, readlink -f and realpath are a real POSIX-feature-availability-matrix nightmare, so that's what the CDPATH= cd -- SOME_PATH && echo "$PWD" part is -- a cross-platform way of doing realpath SOME_PATH.

The -- bits in the middle are there to ensure that $0 is parsed as a positional argument (and never as an option), just in case decided to call their console script -myscript ;^)

Interestingly enough, I'd expect the / to be inside the echo though if this was the intention? Unless it's expected if cd fails that the output is "/" instead of "". Thoughts?

There was no intention behind the slash's positioning, to be honest -- if the echo fails we are in undefined territory -- I think I really just placed it according to my aesthetic whims! Perhaps a set -eo pipefail could further harden this boilerplate to ensure that it fails fast? (need to check if this is portable, though)

I guess one might argue that resolving the path down to absolute is not needed in the first place! I guess that would strip much of the complexity off, leaving you with r#""$(dirname -- "$0")/""#.

charliermarsh · 2024-07-29T00:04:12Z

crates/install-wheel-rs/src/linker.rs

@@ -37,6 +37,7 @@ pub struct Locks(Mutex<FxHashMap<PathBuf, Arc<Mutex<()>>>>);
 #[instrument(skip_all, fields(wheel = %filename))]
 pub fn install_wheel(
    layout: &Layout,
+    relocatable: bool,


I moved these next to each other because part of me wishes that it were on Layout (though I don't think creating layout should require querying the filesystem, so leaving them separate).

I did originally put this inside of Layout, but for some reason decided to back out of that.

(though I don't think creating layout should require querying the filesystem, so leaving them separate).

Could use the layout.as_relocatable() pattern? (only mark it as relocatable if specifically requested by the caller)

charliermarsh · 2024-07-29T00:11:05Z

Thanks!

samypr100 · 2024-07-29T03:08:48Z

crates/uv-virtualenv/src/activator/activate.bat

@@ -19,7 +19,7 @@
 @REM OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
 @REM WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.

-@set "VIRTUAL_ENV={{ VIRTUAL_ENV_DIR }}"
+@for %%i in ("{{ VIRTUAL_ENV_DIR }}") do @set "VIRTUAL_ENV=%%~fi"


This seems to be for expanding a relative path in command prompt since the previous command didn't, awesome!

chrisrodrigue · 2024-07-30T09:47:04Z

Please excuse my ignorance… does this mean that portable virtual environments are possible now?

paveldikov · 2024-07-30T20:59:18Z

Please excuse my ignorance… does this mean that portable virtual environments are possible now?

It depends -- a lot of people, myself included, would be wary of using the word 'portable'. (it could imply factors such as host OS or system architecture, in which case: no, these environments are defintely not portable.) But on a high level, yes, assuming a homogenised host environment, these virtual environments can be shipped around from one host to another with most (if not all) functionality being retained.

(also, this is a preview feature, so I imagine it is probably premature to talk of it as a done deal. Matter of fact, I just found a bug with the activate script on ksh, which I am now to fix!)

paveldikov mentioned this pull request Jul 28, 2024

Feature request: decouple "build" and "target" paths in install_wheel API #3863

Closed

paveldikov force-pushed the relocatable branch from 9fbabd1 to cd9e6ef Compare July 28, 2024 14:32

samypr100 reviewed Jul 28, 2024

View reviewed changes

paveldikov force-pushed the relocatable branch from cd9e6ef to 267edf9 Compare July 28, 2024 17:01

paveldikov force-pushed the relocatable branch from 267edf9 to 65d6483 Compare July 28, 2024 17:16

paveldikov marked this pull request as ready for review July 28, 2024 17:27

charliermarsh reviewed Jul 28, 2024

View reviewed changes

paveldikov force-pushed the relocatable branch from 65d6483 to 14ad5d6 Compare July 28, 2024 17:36

paveldikov force-pushed the relocatable branch from 14ad5d6 to fbbe0d9 Compare July 28, 2024 19:01

charliermarsh self-assigned this Jul 28, 2024

paveldikov force-pushed the relocatable branch from 46d02ff to c9748fc Compare July 28, 2024 20:34

paveldikov requested a review from charliermarsh July 28, 2024 20:39

paveldikov force-pushed the relocatable branch from c9748fc to 94b44e4 Compare July 28, 2024 20:44

Merge branch 'main' into relocatable

9eb8f8b

paveldikov force-pushed the relocatable branch from 94b44e4 to 9eb8f8b Compare July 28, 2024 21:02

charliermarsh reviewed Jul 28, 2024

View reviewed changes

charliermarsh added 3 commits July 28, 2024 19:43

Move some arguments around

a9b9bd3

Merge branch 'main' into relocatable

5a38a6c

Run cargo fmt

5508f23

charliermarsh approved these changes Jul 28, 2024

View reviewed changes

Invert order of path manipulations

19cef6c

charliermarsh force-pushed the relocatable branch from 2c80308 to 19cef6c Compare July 29, 2024 00:01

charliermarsh added the enhancement New feature or improvement to existing functionality label Jul 29, 2024

charliermarsh enabled auto-merge (squash) July 29, 2024 00:01

charliermarsh reviewed Jul 29, 2024

View reviewed changes

charliermarsh merged commit cb47aed into astral-sh:main Jul 29, 2024
55 checks passed

samypr100 reviewed Jul 29, 2024

View reviewed changes

BrewTestBot mentioned this pull request Jul 29, 2024

uv 0.2.31 Homebrew/homebrew-core#178890

Merged

nikaro mentioned this pull request Aug 31, 2024

Add support for --relocatable #3587

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(venv): add relocatable flag #5515

feat(venv): add relocatable flag #5515

paveldikov commented Jul 28, 2024 •

edited

Loading

samypr100 Jul 28, 2024

paveldikov Jul 28, 2024

samypr100 Jul 28, 2024

paveldikov Jul 28, 2024

samypr100 Jul 28, 2024

charliermarsh commented Jul 28, 2024

charliermarsh commented Jul 28, 2024

paveldikov commented Jul 28, 2024

paveldikov commented Jul 28, 2024

charliermarsh Jul 28, 2024

paveldikov Jul 28, 2024

paveldikov Jul 28, 2024

charliermarsh commented Jul 28, 2024

charliermarsh commented Jul 28, 2024

paveldikov commented Jul 28, 2024

charliermarsh commented Jul 28, 2024

charliermarsh commented Jul 28, 2024

paveldikov commented Jul 28, 2024 •

edited

Loading

paveldikov commented Jul 28, 2024

charliermarsh commented Jul 28, 2024

paveldikov commented Jul 28, 2024

charliermarsh commented Jul 28, 2024

charliermarsh Jul 28, 2024

samypr100 Jul 29, 2024

paveldikov Jul 29, 2024 •

edited

Loading

charliermarsh Jul 29, 2024

paveldikov Jul 29, 2024

charliermarsh commented Jul 29, 2024

samypr100 Jul 29, 2024 •

edited

Loading

chrisrodrigue commented Jul 30, 2024 •

edited

Loading

paveldikov commented Jul 30, 2024 •

edited

Loading

feat(venv): add relocatable flag #5515

feat(venv): add relocatable flag #5515

Conversation

paveldikov commented Jul 28, 2024 • edited Loading

Summary

Test Plan

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

charliermarsh commented Jul 28, 2024

charliermarsh commented Jul 28, 2024

paveldikov commented Jul 28, 2024

paveldikov commented Jul 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

charliermarsh commented Jul 28, 2024

charliermarsh commented Jul 28, 2024

paveldikov commented Jul 28, 2024

charliermarsh commented Jul 28, 2024

charliermarsh commented Jul 28, 2024

paveldikov commented Jul 28, 2024 • edited Loading

paveldikov commented Jul 28, 2024

charliermarsh commented Jul 28, 2024

paveldikov commented Jul 28, 2024

charliermarsh commented Jul 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paveldikov Jul 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

charliermarsh commented Jul 29, 2024

samypr100 Jul 29, 2024 • edited Loading

Choose a reason for hiding this comment

chrisrodrigue commented Jul 30, 2024 • edited Loading

paveldikov commented Jul 30, 2024 • edited Loading

paveldikov commented Jul 28, 2024 •

edited

Loading

paveldikov commented Jul 28, 2024 •

edited

Loading

paveldikov Jul 29, 2024 •

edited

Loading

samypr100 Jul 29, 2024 •

edited

Loading

chrisrodrigue commented Jul 30, 2024 •

edited

Loading

paveldikov commented Jul 30, 2024 •

edited

Loading