-
Notifications
You must be signed in to change notification settings - Fork 2
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
Support building colcon extensions with no setup.py #14
Conversation
I gave a quick peek through https://pip.pypa.io/en/stable/reference/requirements-file-format/ in case there was any kind of reverse or override option and didn't see anything. The only other thing I could think of would be to make the package name an action parameter that is passed through from the per-repo action configuration rather than divined using the |
This change separates the installation of the package being tested from the installation of its dependencies. After initial installation, the package's name is found using `pip list` and excluded from the org-level constraints file that's used to ensure that HEAD is used for each of those repositories. The package name must be removed from constraints.txt or pip will fail to install the package and its dependencies due to the conflict. Note that pytest-cov still requires a setup.cfg to be present - storing coverage information in pyproject.toml is not yet supported by this action.
Alright, I took a different approach that I think will be more reliable. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This approach relies much less on happenstantial behavior of pip which is much easier to reason about and trust.
echo ::group::Install dependencies | ||
# Remove this package from constraints | ||
grep -v "^$(python setup.py --name)@" ${GITHUB_ACTION_PATH}/constraints.txt > constraints.txt | ||
PKG_NAME=$(pip list --local --editable --format json | jq '.[0].name' -r) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just thinking about the ways this could fail:
- For this to work,
pip list
must be omitting setuptools and pip (installed above) - The python setup action could also start installing stuff that gets listed although I suppose the
--local
filters those out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the combination of --local
and --editable
that really narrows it down. Only stuff in the venv should show up due to --local
, and there's only one thing we're installing with --editable
and that's our target.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(note that we created a fresh venv on line 11)
This change uses a pip "download" operation to read the name of the package being built so that it can be removed from the constraints.txt prior to downloading dependencies.This change separates the installation of the package being tested from the installation of its dependencies. After initial installation, the package's name is found using
pip list
and excluded from the org-level constraints file that's used to ensure that HEAD is used for each of those repositories.The package name must be removed from constraints.txt or pip will fail to install the package and its dependencies due to the conflict.
Note that pytest-cov still requires a setup.cfg to be present - storing coverage information in pyproject.toml is not yet supported by this action.
I could find no better way to get pip to output the local package name than to invoke a "dummy" download operation like this. If anyone has a better idea how to reliably get the package name or avoid the constraints.txt conflict, I'd love to see it.