Skip to content
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

parts,charm_builder: add charm-python-packages plugin property (CRAFT-552) #553

Merged
merged 1 commit into from
Sep 24, 2021

Conversation

cmatsuoka
Copy link
Contributor

Packages listed in the charm-python-packages property are installed
before dependencies listed in the requirements file. This mirrors
python-packages that exists in the python plugin.

Signed-off-by: Claudio Matsuoka claudio.matsuoka@canonical.com

@cmatsuoka cmatsuoka force-pushed the charm-python-packages branch from 4fd04e4 to b61dd2a Compare September 23, 2021 20:11
Packages listed in the `charm-python-packages` property are installed
before dependencies listed in the requirements file. This mirrors
`python-packages` that exists in the python plugin.

Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>
@cmatsuoka
Copy link
Contributor Author

@heitorPB, could you check if the jinja2 installation works correctly using charm-python-packages: [markupsafe]?

Copy link
Contributor

@facundobatista facundobatista left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

100% awesome!

@heitorPB
Copy link
Contributor

I wonder if the charm-python-packages entry should be in the bases instead of parts? We do support multiple OSes and we will eventually encounter a situation that we need to add a dependency to the build system for one OS but not for the other.

Same comment applies for charm.parts.build-packages.

@heitorPB
Copy link
Contributor

@heitorPB, could you check if the jinja2 installation works correctly using charm-python-packages: [markupsafe]?

Is this availabe in the latest/edge snap? How do I run it locally?

@cmatsuoka
Copy link
Contributor Author

I wonder if the charm-python-packages entry should be in the bases instead of parts? We do support multiple OSes and we will eventually encounter a situation that we need to add a dependency to the build system for one OS but not for the other.

For Python packages you can add a platform conditional such as:

$ pip install "markupsafe;sys_platform=='darwin'"
Ignoring markupsafe: markers 'sys_platform == "darwin"' don't match your environment

Same comment applies for charm.parts.build-packages.

In this case we need to add a more advanced grammar processing similar to the one used in Snapcraft.

@cmatsuoka
Copy link
Contributor Author

Is this availabe in the latest/edge snap? How do I run it locally?

Hmm, edge snaps are built from the main branch so you can either create a snap locally (by checking out the branch and running snapcraft) or run from the checked out tree in destructive mode. Creating a local snap should be easier and safer.

@heitorPB
Copy link
Contributor

Could we use this plugin to update a specific library? It appears that the problem with #551 is setuptools is not the latest version.

@cmatsuoka
Copy link
Contributor Author

Could we use this plugin to update a specific library? It appears that the problem with #551 is setuptools is not the latest version.

Ah, that's an interesting find. Yes, setuptools should be upgraded if listed in charm-python-packages.

@cmatsuoka
Copy link
Contributor Author

Landing it. It should be available in edge tomorrow.

@cmatsuoka cmatsuoka merged commit ccec8b7 into canonical:master Sep 24, 2021
facundobatista pushed a commit to facundobatista/charmcraft that referenced this pull request Sep 28, 2021
parts,charm_builder: add charm-python-packages plugin property (CRAFT-552)
@cmatsuoka cmatsuoka deleted the charm-python-packages branch October 7, 2021 22:17
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.

3 participants