Skip to content

For package installations, handle version mismatches with higher version winning, and store installed packages to handle downgrades #78

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

Merged

Conversation

raman325
Copy link
Contributor

@raman325 raman325 commented Nov 4, 2020

As mentioned in #70 the original implementation of installing packages from requirements.txt had some limitations, primarily around version mismatches. The previous code didn't do anything to address the case where different pyscript requirements.txt's had different version requirements for the same package, it would simply tell HA to install them in the order they were found, resulting in the last version found being installed.

This change does several things:

  1. Tracks packages installed by pyscript - This is necessary to handle scenarios where users want to downgrade a package they installed through pyscript. We want to let them downgrade through pyscript but not if it was installed via another source.
  2. Requires version pinning if a version is required (if not, it can remain unpinned) - This makes conflict resolution simpler because it makes it possible to do the next item.
  3. Always defer to the highest version defined in requirements.txt. Pinned versions will always take precedence over unpinned versions
  4. Install any package version that is either not already installed or was installed by pyscript (and this handles if it was originally installed by pyscript but another actor came in after the fact and changed the version) - this would allow users to downgrade packages as needed but would protect Home Assistant in case it takes over package management for a package installation that pyscript used to manage

There is complexity introduced by this change, but I tried to be comprehensive in my tests to account for that. I also moved the package logic into a separate module to keep __init__.py clean and to make it easier to write tests, so the number of lines changed looks bigger than it actually is.

As a note, you must either leave a package unpinned or pin it using the == specifier. This reduces the already quite complex complexity of the package version resolution logic.

@raman325 raman325 changed the title Handle unpinned versions in requirements.txt For package installations, handle version mismatches with higher version winning, and store installed packages to handle downgrades Nov 4, 2020
@raman325
Copy link
Contributor Author

raman325 commented Nov 4, 2020

As you may notice, I also added a block that prevents packages from being installed if allow_all_imports is False since they wouldn't be usable in that case

@raman325
Copy link
Contributor Author

raman325 commented Nov 4, 2020

Please hold off on merging this for a bit. I am adding at least one additional test and just checking on one of the scenarios

EDIT: This can be merged now, I'm satisfied with my test coverage

@craigbarratt
Copy link
Member

This is great - thanks!

@craigbarratt craigbarratt merged commit b6f4e4d into custom-components:master Nov 5, 2020
@raman325 raman325 deleted the handle_unpinned_versions branch November 5, 2020 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants