Add hook to process manifest by SylvainCorlay · Pull Request #476 · scikit-build/scikit-build

SylvainCorlay · 2020-04-07T22:36:29Z

This is a simple hook to enable custom processing of the cmake manifest before producing the wheel.

The goal for us is to drop static binaries of dependencies, as well as their headers, and other artefacts such as cmake files, as we want the wheel to only include what is required at runtime.

I am using this here: jupyter-xeus/xeus-python-wheel#36

Fixes #473

SylvainCorlay · 2020-04-07T23:08:43Z

It seems that the Python 2.7 builds need to be disabled as the installed version of setuptools requires Python>=3.5.

jcfr · 2020-04-09T12:26:22Z

Thanks for the contribution 🙏

I will be able to review tomorrow.

SylvainCorlay · 2020-04-09T21:19:20Z

I will be able to review tomorrow.

Thanks! If you are OK with it, I am happy to add a bit of documentation in the relevant section. I was able to use it successfully in jupyter-xeus/xeus-python-wheel#36.

If there is a release of scikit-build with that hook, that would be great.

thewtex · 2020-04-14T15:25:48Z

@jcfr why are the CI tests failing?

jcfr · 2020-04-14T15:49:42Z

why are the CI tests failing?

It is because the latest version of setuptools is not working with 2.7 ... I got distracted on Friday and didn't move forward with publishing a fix.

Stay tuned for an update. 🙏

SylvainCorlay · 2020-04-14T16:51:20Z

@jcfr I am happy to rebase this PR if you update master.

SylvainCorlay · 2020-04-16T08:01:36Z

If someone has the time to give this a look, it would be super appreciated.

jcfr · 2020-04-16T08:28:51Z

@SylvainCorlay I didn't forget ... it is definitely in the queue. Thanks for your patience 🙏

jcfr

In the context of the xeus-python-wheel project where all dependencies would be expected to support an option <projectname>_INSTALL_DEVELOPMENT allowing to skip the installation of development files when installing the project without specifying component, this makes sense.

Before we move forward, could you:

update documentation to describe the keyword and in which case it could be used
update CHANGES.rst
add a simple test

Thanks 🙏

SylvainCorlay · 2020-04-19T01:22:14Z

Before we move forward, could you:

update documentation to describe the keyword and in which case it could be used

update CHANGES.rst

add a simple test

Done! This is ready to go @jcfr

jcfr · 2020-04-19T03:10:09Z

Thanks, I will review later tomorrow or Monday. Have a good weekend.

SylvainCorlay · 2020-04-20T15:07:13Z

Hey, let me know if there is anything else needed. This is on our critical path to publish the xeus-python wheels.

We are currently using scikit-build from this branch, but this is not great for reproducibility.

thewtex · 2020-04-21T12:21:55Z

ping @jcfr

SylvainCorlay · 2020-04-21T20:22:45Z

Hey @jcfr is there anything we can do to help move this forward?

jcfr

Looks very good.

One last nitpick, let me know what you think

jcfr · 2020-04-21T21:27:42Z

+    # This hook enables custom processing of the cmake manifest
+    cmake_manifest = cmkr.install()
+    process_manifest = kw.get('cmake_process_manifest_hook')
+    if callable(process_manifest):


It may be sensible to report an error if it is not callable

jcfr · 2020-04-21T22:06:05Z

@SylvainCorlay I will add the error reporting and integrate. No further action needed.

As described in [1], support for python 2.7 was removed in setuptools >= 45 and it must be pinned. This commit fixes error like the following: ERROR: Package 'setuptools' requires a different Python: 2.7.17 not in '>=3.5' See scikit-build#478 [1] https://github.com/pypa/setuptools/blob/v46.1.3/.github/ISSUE_TEMPLATE/setuptools-warns-about-python-2-incompatibility.md

jcfr · 2020-04-23T09:30:33Z

The test tests/test_filter_manifest.py::test_bdist_wheel_command is failing on macOS (for all python versions) with the following error:

lib_file = <_io.BufferedReader name='_skbuild/macosx-10.13-x86_64-3.7/setuptools/bdist.macosx-10.13-x86_64/wheel/hello/_swig_mwe.so'>
seek = 0
    def get_base_class_and_magic_number(lib_file, seek=None):
        if seek is None:
            seek = lib_file.tell()
        else:
            lib_file.seek(seek)
        magic_number = ctypes.c_uint32.from_buffer_copy(
>           lib_file.read(ctypes.sizeof(ctypes.c_uint32))).value
E       ValueError: Buffer size too small (0 instead of at least 4 bytes)
../../../.pyenv/versions/3.7.0/lib/python3.7/site-packages/wheel/macosx_libfile.py:224: ValueError

This is caused by the dummy .so file being an invalid library. In that case, the following function fails to extract macOS information: https://github.com/pypa/wheel/blob/a51977075740fda01b2c0e983a79bfe753567219/src/wheel/macosx_libfile.py#L219-L225

…m .so to .pyd This commit fixes the test by using .pyd extension on all platforms. This will avoid failure on macOS where wheel wheel/macosx_libfile.py attempts to extract shared library information.

SylvainCorlay · 2020-04-23T22:34:11Z

Maybe I should include a real compiled extension rather than an empty dummy file for this example - although the other tests that do the same will still be failing.

jcfr · 2020-04-24T14:30:46Z

No worry, I will finalize this morning. Look like I missed something.

…

On Thursday, April 23, 2020, Sylvain Corlay ***@***.***> wrote: Maybe I should include a real compiled extension rather than an empty dummy file for this example - although the other tests that do the same will still be failing. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#476 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABVPIZVDBYLVM5KGNCWUZLROC67BANCNFSM4MDPIRWQ> .

…dule extension from .so to .pyd This commit fixes the test by using .pyd extension on all platforms. This will avoid failure on macOS where wheel wheel/macosx_libfile.py attempts to extract shared library information.

This commit ensures that no signing message is displayed when running tests on workstation with commit signing enabled globally.

jcfr · 2020-04-25T05:03:03Z

CI is mostly green, after Travis completes I will integrate.

Linux_PyPy3 failures are unrelated and will be fixed later or even disabled.

SylvainCorlay · 2020-04-25T07:05:02Z

Great! Thanks for taking the time!

jcfr · 2020-04-25T14:29:09Z

if callable(process_manifest):

It may be sensible to report an error if it is not callable

I created an issue to track this. See #482

This would be a good first issue for anyone new to the code base

jcfr · 2020-04-25T14:29:51Z

Great! Thanks for taking the time!

Thanks to you for contributing 🙏

Have a good weekend

SylvainCorlay · 2020-04-27T23:13:24Z

@jcfr it would be fantastic if there was a scikit-build tag including this patch!

We have been looking into streamlining the release of xeus-python on PyPI for a few weeks and having a new release of scikit-build is the main blocker.

jcfr · 2020-04-27T23:18:59Z

Yes, I will do a release shortly. After integrating Pablo PR.

SylvainCorlay · 2020-04-27T23:21:06Z

Great thanks!

SylvainCorlay · 2020-04-29T15:25:26Z

Yes, I will do a release shortly. After integrating Pablo PR.

Is there any blocker on this ! I don't want to be nagging - but I am following up as we have been blocked on this for a few weeks!

jcfr · 2020-04-29T15:52:04Z

Fundamentally there are no blockers, time is just finite resource.

After #486 completes I will do release 🎉

SylvainCorlay mentioned this pull request Apr 7, 2020

Filter manifest jupyter-xeus/xeus-python-wheel#36

Merged

SylvainCorlay force-pushed the filter-manifest branch from 898fe37 to 2cf508c Compare April 8, 2020 20:34

jcfr added the Status: Triage Issues/PRs that need to be triaged label Apr 8, 2020

thewtex requested a review from jcfr April 9, 2020 02:50

jcfr requested changes Apr 16, 2020

View reviewed changes

Comment thread skbuild/setuptools_wrap.py Outdated

Comment thread skbuild/setuptools_wrap.py Outdated

jcfr added Status: In Progress This item is in progress. Type: Enhancement Improvement to functionality and removed Status: Triage Issues/PRs that need to be triaged labels Apr 16, 2020

jcfr mentioned this pull request Apr 16, 2020

Fix continuous integration requiring older setuptool for python 2.7 #478

Closed

SylvainCorlay force-pushed the filter-manifest branch from 2cf508c to 2aa0b8f Compare April 18, 2020 08:57

Add hook to process cmake install manifest

66d7a32

SylvainCorlay force-pushed the filter-manifest branch 3 times, most recently from ebd4c2f to ef9369d Compare April 19, 2020 01:09

Add test

76cca46

SylvainCorlay force-pushed the filter-manifest branch from ef9369d to 76cca46 Compare April 19, 2020 01:15

jcfr reviewed Apr 21, 2020

View reviewed changes

jcfr linked an issue Apr 23, 2020 that may be closed by this pull request

Fix continuous integration requiring older setuptool for python 2.7 #478

Closed

jcfr force-pushed the filter-manifest branch from 589b626 to b0ae814 Compare April 23, 2020 08:18

jcfr force-pushed the filter-manifest branch from b0ae814 to a0cbced Compare April 23, 2020 08:21

test_filter_manifest: fix test on macOS changing module extension fro…

44098a2

…m .so to .pyd This commit fixes the test by using .pyd extension on all platforms. This will avoid failure on macOS where wheel wheel/macosx_libfile.py attempts to extract shared library information.

jcfr force-pushed the filter-manifest branch from e475e78 to 11540d2 Compare April 23, 2020 17:04

jcfr added 2 commits April 25, 2020 00:15

test: Update initialize_git_repo_and_commit to disable commit signing

bef4146

This commit ensures that no signing message is displayed when running tests on workstation with commit signing enabled globally.

jcfr force-pushed the filter-manifest branch from 11540d2 to bef4146 Compare April 25, 2020 04:31

CHANGES: Acknowledge contributor

6428231

jcfr merged commit ccbd20f into scikit-build:master Apr 25, 2020

SylvainCorlay deleted the filter-manifest branch April 25, 2020 21:59

jcfr removed the Status: In Progress This item is in progress. label Apr 27, 2020

phcerdan mentioned this pull request Apr 27, 2020

Add cmake_target to change the target used by CMake #477

Merged

phcerdan mentioned this pull request Jul 27, 2020

fix: Working build with static library scikit-build/scikit-build-sample-projects#6

Merged

Conversation

SylvainCorlay commented Apr 7, 2020 • edited by jcfr Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SylvainCorlay commented Apr 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jcfr commented Apr 9, 2020

Uh oh!

SylvainCorlay commented Apr 9, 2020

Uh oh!

thewtex commented Apr 14, 2020

Uh oh!

jcfr commented Apr 14, 2020

Uh oh!

SylvainCorlay commented Apr 14, 2020

Uh oh!

SylvainCorlay commented Apr 16, 2020

Uh oh!

jcfr commented Apr 16, 2020

Uh oh!

jcfr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

SylvainCorlay commented Apr 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jcfr commented Apr 19, 2020

Uh oh!

SylvainCorlay commented Apr 20, 2020

Uh oh!

thewtex commented Apr 21, 2020

Uh oh!

SylvainCorlay commented Apr 21, 2020

Uh oh!

jcfr left a comment

Choose a reason for hiding this comment

Uh oh!

jcfr Apr 21, 2020

Choose a reason for hiding this comment

Uh oh!

jcfr commented Apr 21, 2020

Uh oh!

jcfr commented Apr 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SylvainCorlay commented Apr 23, 2020

Uh oh!

jcfr commented Apr 24, 2020 via email

Uh oh!

jcfr commented Apr 25, 2020

Uh oh!

SylvainCorlay commented Apr 25, 2020

Uh oh!

jcfr commented Apr 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jcfr commented Apr 25, 2020

Uh oh!

SylvainCorlay commented Apr 27, 2020

Uh oh!

jcfr commented Apr 27, 2020

Uh oh!

SylvainCorlay commented Apr 27, 2020

Uh oh!

SylvainCorlay commented Apr 29, 2020

Uh oh!

jcfr commented Apr 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

SylvainCorlay commented Apr 7, 2020 •

edited by jcfr

Loading

SylvainCorlay commented Apr 7, 2020 •

edited

Loading

SylvainCorlay commented Apr 19, 2020 •

edited

Loading

jcfr commented Apr 23, 2020 •

edited

Loading

jcfr commented Apr 25, 2020 •

edited

Loading

jcfr commented Apr 29, 2020 •

edited

Loading