Skip to content

Comments

fix(venv.relocatable): script entrypoints should work if symlinked to#8079

Merged
charliermarsh merged 1 commit intoastral-sh:mainfrom
paveldikov:relocatable
Oct 10, 2024
Merged

fix(venv.relocatable): script entrypoints should work if symlinked to#8079
charliermarsh merged 1 commit intoastral-sh:mainfrom
paveldikov:relocatable

Conversation

@paveldikov
Copy link
Contributor

@paveldikov paveldikov commented Oct 10, 2024

Fixes: #8058

Test Plan

Integration test (but only for Unix, because symlinks on Windows require admin privs. Plus, they are not really all that idiomatic on Windows)


© 2024 Morgan Stanley.

THIS SOFTWARE IS CONTRIBUTED SUBJECT TO THE TERMS OF THE APACHE LICENSE V.2.0.

THIS SOFTWARE IS LICENSED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE AND ANY WARRANTY OF NON-INFRINGEMENT, ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. THIS SOFTWARE MAY BE REDISTRIBUTED TO OTHERS ONLY BY EFFECTIVELY USING THIS OR ANOTHER EQUIVALENT DISCLAIMER IN ADDITION TO ANY OTHER REQUIRED LICENSE TERMS.

@charliermarsh
Copy link
Member

This seems reasonable to me based on your prior comments. What other testing would you want to see prior to merging?

@paveldikov
Copy link
Contributor Author

This seems reasonable to me based on your prior comments. What other testing would you want to see prior to merging?

It's MacOS that I worry about primarily, but I expect the automated test suite to cover that?

@charliermarsh
Copy link
Member

We do have a concept of "system tests" that test across a variety of operating systems... You could add a test to scripts/check_system_python.py to get coverage there.

@charliermarsh
Copy link
Member

Do you have any concerns about other POSIX systems, e.g., rare Linux distros etc.?

@charliermarsh charliermarsh self-assigned this Oct 10, 2024
@charliermarsh charliermarsh added the bug Something isn't working label Oct 10, 2024
@paveldikov
Copy link
Contributor Author

We do have a concept of "system tests" that test across a variety of operating systems... You could add a test to scripts/check_system_python.py to get coverage there.

Turns out past-me did in fact write an integration test to ensure that the entrypoint works. So I just added another check in the same test case to ensure that the entrypoint keeps working if symlinked to (in an arbitrary directory with no python inside)

Do you have any concerns about other POSIX systems, e.g., rare Linux distros etc.?

Yes, which is why I used realpath instead of readlink -f. From my current research it seems that realpath is ubiquitous enough (thanks to MacOS 12's recent EOL) but I could certainly do with a second check.

Right now the hardest part is that I am developing from my home PC which is Windows :-( so I am fully reliant on the CI run + ad hoc experimentation in a separate Unix environment (which, OTOH, has no Rust)

@paveldikov
Copy link
Contributor Author

paveldikov commented Oct 10, 2024

Ok, tests are green and my research w.r.t. realpath availability suggests that:

  • RedHat and friends: available from el7+. <=el6 and below are EOL anyway.
  • MacOS: available from 13+. <=12 are EOL.
  • Debian and derivatives appear to have had realpath for decades.
  • FreeBSD appears to have had realpath for decades.
  • Busybox/Alpine appears to have realpath.
  • Cygwin/MINGW has realpath.

Also of note that GNU coreutils has had realpath since 2011. So realistically all recent GNU/Linux (yes, the 'GNU' part is important here), beyond perhaps a few relict enterprise near-EOL versions, should have realpath.

readlink -f appears to remain non-portable.

For the record, I am still not 100% dead certain about compatibility, but I am inclined to make the risk calculation that:

  • relocatable venvs are a fairly recent feature, therefore not likely to be used together with EOL operating systems
  • common off-the-shelf OSs appear to ship realpath in their currently-supported versions
  • less common OSs are far less end-user-centric, so we'd have all the more reason to expect coreutils installed.

@charliermarsh charliermarsh merged commit bf15ca9 into astral-sh:main Oct 10, 2024
@charliermarsh
Copy link
Member

Okay thanks for the thorough research here. Appreciate it!

tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Oct 15, 2024
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [astral-sh/uv](https://github.com/astral-sh/uv) | patch | `0.4.20` -> `0.4.21` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>astral-sh/uv (astral-sh/uv)</summary>

### [`v0.4.21`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#0421)

[Compare Source](astral-sh/uv@0.4.20...0.4.21)

##### Enhancements

-   Add support for managed installations of free-threaded Python ([#&#8203;8100](astral-sh/uv#8100))
-   Add note about `uvx` to `uv tool run` short help ([#&#8203;7695](astral-sh/uv#7695))
-   Enable HTTP/2 requests ([#&#8203;8049](astral-sh/uv#8049))
-   Support `uv tree --no-dev` ([#&#8203;8109](astral-sh/uv#8109))
-   Support PEP 723 metadata with `uv run -` ([#&#8203;8111](astral-sh/uv#8111))
-   Support `pip install --exact` ([#&#8203;8044](astral-sh/uv#8044))
-   Support `uv export --no-header` ([#&#8203;8096](astral-sh/uv#8096))
-   ADd Python 3.13 images to Docker publish ([#&#8203;8105](astral-sh/uv#8105))
-   Support remote (`https://`) scripts in `uv run` ([#&#8203;6375](astral-sh/uv#6375))
-   Allow comma value-delimited arguments in `uv run --with` ([#&#8203;7909](astral-sh/uv#7909))

##### Configuration

-   Support wildcards in `UV_INSECURE_HOST` ([#&#8203;8052](astral-sh/uv#8052))

##### Performance

-   Use shared index when fetching metadata in lock satisfaction routine ([#&#8203;8147](astral-sh/uv#8147))

##### Bug fixes

-   Add prerelease compatibility check to `uv python` CLI ([#&#8203;8020](astral-sh/uv#8020))
-   Avoid deleting a project environment directory if we cannot tell if a `pyvenv.cfg` file exists ([#&#8203;8012](astral-sh/uv#8012))
-   Avoid excluding valid wheels for exact `requires-python` bounds ([#&#8203;8140](astral-sh/uv#8140))
-   Bump `netrc` crate to latest commit ([#&#8203;8021](astral-sh/uv#8021))
-   Fix `uv python pin 3.13t` failure when parsing version for project requires check ([#&#8203;8056](astral-sh/uv#8056))
-   Fix handling of != intersections in `requires-python` ([#&#8203;7897](astral-sh/uv#7897))
-   Remove the newly created tool environment if sync failed ([#&#8203;8038](astral-sh/uv#8038))
-   Respect dynamic extras in `uv lock` and `uv sync` ([#&#8203;8091](astral-sh/uv#8091))
-   Treat resolver failures as fatal in lockfile validation ([#&#8203;8083](astral-sh/uv#8083))
-   Use `git config --get` for author information for improved backwards compatibility ([#&#8203;8101](astral-sh/uv#8101))
-   Use comma-separated values for `UV_FIND_LINKS` ([#&#8203;8061](astral-sh/uv#8061))
-   Use shared resolver state between add and lock to avoid double Git update ([#&#8203;8146](astral-sh/uv#8146))
-   Make `--relocatable` entrypoints robust to symlinking ([#&#8203;8079](astral-sh/uv#8079))
-   Improve compatibility with VSCode PS1 prompt ([#&#8203;8006](astral-sh/uv#8006))
-   Fix "Stream did not contain valid UTF-8" failures in Windows ([#&#8203;8120](astral-sh/uv#8120))
-   Use `--with-requirements` in `uvx` error hint ([#&#8203;8112](astral-sh/uv#8112))

##### Documentation

-   Include `uvx` installation in Docker examples ([#&#8203;8179](astral-sh/uv#8179))
-   Make the instructions for the Windows standalone installer consistent across README and documentation ([#&#8203;8125](astral-sh/uv#8125))
-   Update pip compatibility guide to note transitive URL dependency support ([#&#8203;8081](astral-sh/uv#8081))
-   Document `--reinstall` with `--exclude-newer` to ensure downgrades ([#&#8203;6721](astral-sh/uv#6721))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40NDAuNyIsInVwZGF0ZWRJblZlciI6IjM3LjQ0MC43IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
charliermarsh added a commit to astral-sh/python-build-standalone that referenced this pull request Dec 3, 2024
## Summary

The current shebang seems to fail when the path itself contains spaces. For example:

```
❯ "/Users/crmarsh/Library/Application Support/uv/python/cpython-3.13.0-macos-aarch64-none/bin/pydoc3"
/Users/crmarsh/Library/Application Support/uv/python/cpython-3.13.0-macos-aarch64-none/bin/pydoc3: line 2: /Users/crmarsh/Library
Support/uv/python/cpython-3.13.0-macos-aarch64-none/bin/python3.13: No such file or directory
/Users/crmarsh/Library/Application Support/uv/python/cpython-3.13.0-macos-aarch64-none/bin/pydoc3: line 2: exec: /Users/crmarsh/Library
Support/uv/python/cpython-3.13.0-macos-aarch64-none/bin/python3.13: cannot execute: No such file or directory
```

This PR replaces it with the shebang that we use in uv for relocatable environments, which is outlined in the following series of PRs and issues:

- https://github.com/astral-sh/uv/pull/5515/files#r1694358328
- astral-sh/uv#5640
- astral-sh/uv#8079

Closes #394.

Closes astral-sh/uv#9348.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

relocatable entrypoints stop working if symlinked to

2 participants