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

btrfs-progs: libbtrfsutil/python: fix all the warnings #896

Merged
merged 2 commits into from
Sep 21, 2024

Conversation

adam900710
Copy link
Collaborator

@adam900710 adam900710 commented Sep 21, 2024

There are the following warnings when building the python binding the
proper way (directly calling "setup.py build" is no longer recommended):

 $ python -m build btrfs-progs/libbtrfsutil/python
 * Creating isolated environment: venv+pip...
 * Installing packages in isolated environment:
   - setuptools >= 40.8.0
 * Getting build dependencies for sdist...
 /tmp/build-env-2u6q_udl/lib/python3.12/site-packages/setuptools/_distutils/extension.py:139: UserWarning: Unknown Extension options: 'headers'
   warnings.warn(msg)
 * Building sdist...
 /tmp/build-env-2u6q_udl/lib/python3.12/site-packages/setuptools/_distutils/extension.py:139: UserWarning: Unknown Extension options: 'headers'
   warnings.warn(msg)
 writing manifest file 'btrfsutil.egg-info/SOURCES.txt'
 warning: sdist: standard file not found: should have one of README, README.rst, README.txt, README.md

This patch fixes them by:

  • Use MANIFEST.in to properly include the "btrfsutilpy.h"
  • Remove unnecessary build options for C files
  • Reuse the existing README.md by a softlink
    So that even in a virtual environment the README.md will still be
    properly copied.

TODO: Migrate the "python setup.py build" system to "python -m build"
for building and tox for tests.

[BUG]
Currently with python3.12, the python bindding will always result the
following warning:

    [PY]     libbtrfsutil
/usr/lib/python3.12/site-packages/setuptools/_distutils/extension.py:134: UserWarning: Unknown Extension options: 'headers'
  warnings.warn(msg)

[CAUSE]
In the setup.py which specifies the files to be included into the package,
we use setuptools::Extension to specify the file lists and include paths.

But there is no handling of Extension::headers member, thus resulting the
above warning.

[FIX]
According to the docs of setuptools, MANIFEST.in is the file controlling
what files should be included.
So instead of the non-supported headers, use MANIFEST.in to include the
needed headers.

Signed-off-by: Qu Wenruo <[email protected]>
@adam900710 adam900710 self-assigned this Sep 21, 2024
@adam900710 adam900710 added build Changes in build system libbtrfsutil python Python binding and packaging labels Sep 21, 2024
@adam900710 adam900710 added this to the v6.11.1 milestone Sep 21, 2024
@adam900710 adam900710 force-pushed the python_rework branch 2 times, most recently from dc3beb2 to 498799b Compare September 21, 2024 03:23
…escription

Instead of copying the file during custom build commands, just use a
soft link to re-use the existing README.d from libbtrfsutil.

Issue: kdave#310
Signed-off-by: Qu Wenruo <[email protected]>
Copy link
Owner

@kdave kdave left a comment

Choose a reason for hiding this comment

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

Works for me, thanks.

@adam900710 adam900710 merged commit c2c922f into kdave:devel Sep 21, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Changes in build system libbtrfsutil python Python binding and packaging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants