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

include package data files in python package. #3357

Merged
merged 1 commit into from
Jun 16, 2021

Conversation

stefanseefeld
Copy link
Contributor

https://pypi.org/project/codechecker/ is missing package data files, resulting in errors when running

CodeChecker parse -e html ...

This PR makes sure the relevant static data files are included.

@stefanseefeld
Copy link
Contributor Author

Is there anything missing, or anything else I can do to help review this ?

@csordasmarton
Copy link
Contributor

@stefanseefeld Sorry, I am a little bit busy in these days. Thank you for this patch, we really appreciate it 😊. I will try to review / try it this week and give you feedback.

setup.py Outdated
@@ -157,6 +157,9 @@ def run(self):
},
data_files=data_files,
include_package_data=True,
package_data={
Copy link
Contributor

Choose a reason for hiding this comment

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

I checked it by running the make dist target, but unfortunately the zip file will not contain these static files. The problem is that when include_package_data option is specified it will ignore the package_data option. To solve this problem try to create a MANIFEST.in beside the setup.py file with the following content:

recursive-include build_dist/CodeChecker/lib/python3/plist_to_html/static *

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, adding MANIFEST.in as you suggest does indeed work as well. (I only tested with python3 setup.py bdist, for which both approaches work, curiously. But you are right, with my approach the source package was still lacking those files, while with the MANIFEST.in file both, source and binary packages are fixed.

Copy link
Contributor

@csordasmarton csordasmarton left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you very much for your patch 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants