Make pip_kwargs consistent with async_mount_local_lib_path
Breaking change
Proposed change
The existing requirements.pip_kwargs function, seems like it intended, under specific circumstances (not virtualenv and not docker, so CORE installations?), to install additional dependencies into the directory mounted by bootstrap.async_mount_local_lib_path (/config/deps/..).
However, what actually ends up happening is that pip_kwargs sets target=/config/deps while async_mount_local_lib_path mounts /config/deps/python3.13/site-packages into sys.path.
This results in HASS installing packages into a directory it doesn't mount onto sys.path. Therefore, this PR is to correct that inconsistency by making pip_kwargs determine the target in the same way async_mount_local_lib_path does, ensuring that it will install additional dependencies into the same location it mounts.
Type of change
- [ ] Dependency upgrade
- [X] Bugfix (non-breaking change which fixes an issue)
- [ ] New integration (thank you!)
- [ ] New feature (which adds functionality to an existing integration)
- [ ] Deprecation (breaking change to happen in the future)
- [ ] Breaking change (fix/feature causing existing functionality to break)
- [ ] Code quality improvements to existing code or addition of tests
Additional information
- This PR fixes or closes issue: fixes #
- This PR is related to issue: https://github.com/home-assistant/core/issues/127966
- Link to documentation pull request:
Checklist
- [x] The code change is tested and works locally.
- [ ] Local tests pass. Your PR cannot be merged unless tests pass
- [x] There is no commented out code in this PR.
- [x] I have followed the development checklist
- [x] I have followed the perfect PR recommendations
- [x] The code has been formatted using Ruff (
ruff format homeassistant tests) - [ ] Tests have been added to verify that the new code works.
If user exposed functionality or configuration variables are added/changed:
- [ ] Documentation added/updated for www.home-assistant.io
If the code communicates with devices, web services, or third-party tools:
- [ ] The manifest file has all fields filled out correctly.
Updated and included derived files by running:python3 -m script.hassfest. - [ ] New or updated dependencies have been added to
requirements_all.txt.
Updated by runningpython3 -m script.gen_requirements_all. - [ ] For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
To help with the load of incoming pull requests:
- [ ] I have reviewed two other open pull requests in this repository.
Is this still needed? Haven't read the whole comment thread on linked bug
@CloCkWeRX under their "supported" deployment methods, you will almost never encounter any issues caused by this inconsistency.
I mostly worked around it by overwriting the run script in the container image.
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. If you are the author of this PR, please leave a comment if you want to keep it open. Also, please rebase your PR onto the latest dev branch to ensure that it's up to date with the latest changes. Thank you for your contribution!
Still think this is kind of a bug.
@CloCkWeRX under their "supported" deployment methods, you will almost never encounter any issues caused by this inconsistency.
If this is not a problem for our supported installations method, why should we fix it? Please come with a real use case
I mostly worked around it by overwriting the run script in the container image.
Why are you doing that?
Why are you doing that?
Because I want to run HASS, in a container, inside a venv, on K8S.
At the time I made this MR, HASS would, on detecting being run in a container, install dependencies to the system library path. And the issue was compounded by the linked issue where it would not correctly detect the containerized environment if it wasn't running in docker, thus running in a containerized environment, but using some code paths for CORE installations.
This is problematic for two reasons:
- It assumes the root filesystem is writeable.
- More importantly, the extra dependencies are not persisted and must be re-downloaded when the container sandbox is recreated.
This means that any deployment of the image, on an environment that does not persist the sandbox (read: K8S), will not be able to start without an internet connection.
Thus in the context of this MR, the modified start script was basically tricking HASS into properly detecting it's running in a container, which was fixed in that other issue. I'm still using a patched start script to manage instanced VENVs on a persistent mount point though, because I would prefer my HASS instance not redownload all the extra deps every time it's pod restarts.
If this is not a problem for our supported installations method, why should we fix it? Please come with a real use case
Because from what I can tell, this is a bug that was inadvertently introduced with the change to using uv for installing additional dependencies. And it's an inconsistency between where it installs extra deps, and where it mounts the extra deps under one specific scenario, as described in the MR above.
I can see this potentially still breaking if the containerized environment detection ever breaks again, and this code path is used while running in a containerized environment.
Also, I feel that the whole "if it doesn't happen on a supported installation method then it's not a problem" implication here is somewhat problematic.
It's understandable if this was reporting an issue that requires effort to troubleshoot, etc,. But this is an MR, with the issue already investigated, and a fix proposed.
There are many reasons why one might run it in an unsupported installation method, such as already having a K8S environment where everything else is deployed, and wanting to ensure that it can properly restart without an internet connection. And in light of that, not wanting the overhead of running an extra VM just for home assistant.
For reference, this is my modified start script that I'm mounting into the container on my environment https://github.com/ruifung/rfhome-infrastructure/blob/master/deployments/pathweb/workloads/home-network/home-assistant/venv-run
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. If you are the author of this PR, please leave a comment if you want to keep it open. Also, please rebase your PR onto the latest dev branch to ensure that it's up to date with the latest changes. Thank you for your contribution!
Not stale, waiting for review by @MartinHjelmare
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks :+1:
Ok wow, it's been quite a while, I'll see what I can do about fixing those tests then.
Ok wow, it's been quite a while, I'll see what I can do about fixing those tests then.
Thank you! ❤️
Based on what I saw of the automated tests that failed, it seems like the python 3.14 changes removed the asyncio loop variable from init, updated code to match it.
Also rebased on to latest-ish dev.