Restore support for external Python package platform dependencies #108
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This will serve as a container for more in-depth documentation to supplement the fundamental reference provided in the readme.
Although Arduino boards platforms traditionally have bundled all dependencies, there are some cases where the user will need to use a preceeding workflow step to install external dependencies in the GitHub Actions runner environment. Since it is a "composite action", the action supports this. The action uses a virtual environment for its own Python package dependencies. This virtual environment might cause Python packages installed via the user's workflow (e.g., the pyserial dependency of the ESP32 platform) to not be accessible to the compilation commands. The added integration test will catch any regressions in the support for external Python package platform dependencies.
Although Arduino boards platforms traditionally have bundled all dependencies, there are some cases where the user will need to use a preceding workflow step to install external dependencies in the GitHub Actions runner environment. Since it is a "composite action", this is feasible and was previously supported. However, support for Python package dependencies is more complicated due to the action script running in a Python virtual environment which isolates it from packages installed globally. The workaround for this was to use the `--system-site-packages` flag in the venv invocation that created the action's Python virtual environment. This approach introduced a requirement that the project Python version match the minor version series of the runner system Python. That requirement was violated when the workflow's runner was updated to `ubuntu-22.04` which has system Python 3.10.x, whereas the action used 3.8. The breakage went unnoticed because the sole Python package platform dependency in real world usage, pyserial, had been preinstalled in the runner's Linux package manager "site-packages" folder, which caused it to be installed in the action's virtual environment even though it would not have if depending on the user installation of the package as was previously the case. The solution is to use a path configuration file to add the path of the system Python's "user site-packages" folder (where package dependencies installed via the workflow are located) to the module search paths used by the action's Python virtual environment. This is actually superior to the previous approach of using the `--system-site-packages` flag in the venv invocation (this flag is also supported by pipx and Poetry) because: - It avoids the chance of interference with the virtual environment through the unnecessary introduction of the global "site-packages" path into the module search paths along with the intended introduction of the "user site-packages" path. - It allows any version of Python to be used to run the action, rather than being forced to use the same minor version series as the runner's system Python (which can be updated at any time and also depends on the workflow configuration)
It is no longer necessary to specify the full path to the script in the invocation command now that the steps working directory has been set to the root folder of the action repository.
Previously, a script was used to install Python and venv for use by the action. The path of this script was added to the paths filter in the relevant workflows so that they would be triggered on any change to the script. The script was removed in favor of the use of the actions/setup-python script for installation of Python. The script path was not removed from the workflow at that time.
By default, pipx uses the runner's system Python installation to install Poetry. Although this works, it is best to use the controlled Python installation (installed via the `actions/setup-python` step) for all project operations.
Although Arduino boards platforms traditionally have bundled all dependencies, the ESP32 boards platform expects the pyserial Python package to be preinstalled on Linux systems. Although there is already test coverage and support for user installation of arbitrary Python package dependencies of a platform, the situation is more complex when it comes to this important boards platform. The problem is that there is an established use pattern that should be supported to avoid breaking user workflows. The added test checks for regressions in that support.
Although Arduino boards platforms traditionally have bundled all dependencies, the ESP32 boards platform expects the pyserial Python package to be preinstalled on Linux systems. Although there is already test coverage and support for user installation of arbitrary Python package dependencies of a platform, the situation is more complex when it comes to this important boards platform. The problem is that there is an established use pattern that should be supported to avoid breaking user workflows. The pattern is the use of a `pip install pyserial` command in the workflow to install the package. That command correctly installs the package in the "user site-packages" folder when the job runs in the `ubuntu-18.04` GitHub Actions runner, as was the case when the use pattern was established. However, pyserial is installed in the Linux package manager "site-packages" folder of the Ubuntu GitHub Actions runner starting from ubuntu-20.04 (most workflows run the arduino/compile-sketches job in the `ubuntu-latest` runner, which is currently `ubuntu-22.04`). This causes the established installation command to terminate without installing the package: ``` Requirement already satisfied: pyserial in /usr/lib/python3/dist-packages (3.5) ``` This did not cause a problem previously because the action was configured to include all system site-packages in the action's Python virtual environment. Because this approach resulted in the uncontrolled introduction of irrelevant packages into the action's Python virtual environment, the action was changed to only include the "user site-packages" in the virtual environment. That is fine if the now-recommended `--ignore-installed` flag is added to the pyserial installation command, but it breaks the established workflows. Although the goal was always to provide a flexible system that would allow the user installation of any arbitrary platform dependency, rather than hardcoding specific dependencies in the action itself, unfortunately I have not been able to find any way to progress in the improvement of the action without breaking the established workflows that compile for ESP32 boards. So as a workaround I am reluctantly adding the pyserial package to the action's virtual environment. This workaround will be reverted before the 2.0.0 release of the action so the pyserial installation command should be retained in workflows and occurrences of the historical `pip install pyserial` (or `pip3 install pyserial`) commands should be updated to the recommended command: ``` pip install --ignore-installed --user pyserial ```
per1234
added
type: imperfection
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## main #108 +/- ##
=======================================
Coverage 99.81% 99.81%
=======================================
Files 2 2
Lines 1610 1610
=======================================
Hits 1607 1607
Misses 3 3 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
Time and time again this aspect of Python comes back to bite the unsuspecting user. Great finding @per1234 and thanks for fixing this 🙇 |
|
@per1234 Instead of using deadsnake's PPA (which takes a long time to install), you could use actions/setup-python to not affect the runner's PATH and use the action's |
|
Thanks @2bndy5. I didn't do that at the time I migrated the action to the "composite" type because at that time the use of actions within composite actions were not supported. Since then GitHub added support for doing that and I have been meaning to take advantage of it ever since then. I finally got around to it last week and it works just fine: #105 |
|
After I posted that, I saw your changes. They work well, and the compile-sketches runtime is much much better. 🚀 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Background
Although Arduino boards platforms traditionally have bundled all dependencies, there are some cases where the user will need to use a preceding workflow step to install external dependencies in the GitHub Actions runner environment.
Since it is a "composite action", this is feasible and was previously supported. However, support for Python package dependencies is more complicated due to the action script running in a Python virtual environment which isolates it from packages installed globally. The workaround for this was to use the
--system-site-packagesflag in thevenvinvocation that created the action's Python virtual environment (#15).The previous approach to supporting user installed Python package dependencies of boards platforms introduced a requirement that the project Python version match the minor version series of the runner system Python. That requirement was violated when the workflow's runner was updated to
ubuntu-22.04which has system Python 3.10.x, whereas the action used 3.8.x. The breakage went unnoticed because the sole Python package platform dependency in real world usage, pyserial, had been preinstalled in the runner's Linux package manager "site-packages" folder, which caused it to be installed in the action's virtual environment even though it would not have if depending on the user installation of the package as was previously the case.Problem
The previous approach to supporting user installed Python package dependencies of boards platforms introduced a requirement that the project Python version match the minor version series of the runner system Python. That requirement was violated when the workflow's runner was updated to
ubuntu-22.04which has system Python 3.10.x, whereas the action used 3.8.x. The breakage went unnoticed because the sole Python package platform dependency in real world usage, pyserial, had been preinstalled in the runner's Linux package manager "site-packages" folder, which caused it to be installed in the action's virtual environment even though it would not have if depending on the user installation of the package as was previously the case.In addition to the historical loss of support for arbitrary user installed Python packages, the support for packages that happened to be pre-installed on the runner was lost when a new virtual environment system was introduced (#107).
Solution
The obvious solution would have been to use the same "system-site-packages" configuration for Poetry (which does have support for it). However, this was not possible because Poetry requires all packages to use PEP 440-compliant version numbers and the GitHub Actions runner contains a system installed Python package with a non-compliant version (python-poetry/poetry#7343).
The usable solution is to use a path configuration file to add the path of the system Python's "user site-packages" folder (where package dependencies installed via the workflow are located) to the module search paths used by the action's Python virtual environment.
This is actually superior to the "system-site-packages" in some ways:
Legacy Workflow Support
The ESP32 boards platform expects the pyserial Python package to be preinstalled on Linux systems.
The situation is more complex when it comes to this important boards platform. The problem is that there is an established use pattern that should be supported to avoid breaking user workflows. The pattern is the use of a
pip install pyserialcommand in the workflow to install the package (e.g.). That command correctly installs the package in the "user site-packages" folder when the job runs in theubuntu-18.04GitHub Actions runner, as was the case when the use pattern was established. However, pyserial is installed in the Linux package manager "site-packages" folder of the Ubuntu GitHub Actions runner starting fromubuntu-20.04(most workflows run the arduino/compile-sketches job in theubuntu-latestrunner, which is currentlyubuntu-22.04). This causes the established installation command to terminate without installing the package to the "user site-packages" folder:Now that only the user-installed packages from the "user site-packages" are added to the virtual environment, this would cause compilation for any ESP32 board to fail:
The problem of the terminated pyserial installation does not occur if the user's installation command is configured as recommended in the newly added FAQ item on installing Python package dependencies of boards platforms.
Although the goal was always to provide a flexible system that would allow the user installation of any arbitrary platform dependency, rather than hardcoding specific dependencies in the action itself, unfortunately I have not been able to find any way to progress in the improvement of the action while also supporting the existing workflows that compile for ESP32 boards. So as a workaround I am reluctantly adding the pyserial package to the action's virtual environment.
This workaround will be reverted before the 2.0.0 release of the action so the pyserial installation command should be retained in workflows (even though it is no longer required), and occurrences of the historical
pip install pyserial(orpip3 install pyserial) commands should be updated to the recommended form: