coincurve
coincurve copied to clipboard
FEAT: Transition to scikit-build-core CMake-based build process
This was initally started to transition to Hatchling. However, as I learned more about scikit-build-core
, it seems that it handles all the steps required to build coincurve
. However, I do not know Hatchling, so let me know your thoughts on how we should integate it.
This PR itself changes some of the original features of coincurve. CMake provides a convenient way to download the SECP256K1 library, so currently, I did not add this feature (though the current UPSTREAM_REF is used)
The other major addition is that the flow creates the CFFI source headers directly from the downloaded SECP256K1 original headers, so it should be quite straightforward to follow SECP256K1 updates.
The rest is fairly similar, create the SECP256K1 library (using the CMake they provide), create the CFFI C-code (rather than compile it with CFFI), then compile the C-Code and link it to SECP256K1 library.
The feature of using SYSTEM_LIB is similar, although I have not tested it as well as for the CMake-based PR.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 84.64%. Comparing base (
07d332e
) to head (fe052a6
).
:exclamation: Current head fe052a6 differs from pull request most recent head 39d204f
Please upload reports for the commit 39d204f to get more accurate results.
Additional details and impacted files
@@ Coverage Diff @@
## master #162 +/- ##
==========================================
- Coverage 85.68% 84.64% -1.05%
==========================================
Files 11 12 +1
Lines 566 573 +7
Branches 67 68 +1
==========================================
Hits 485 485
- Misses 45 52 +7
Partials 36 36
Files | Coverage Δ | |
---|---|---|
src/coincurve/__about__.py | 0.00% <ø> (ø) |
@ofek So this is a scikit-build-core
backend build. I try to see how to use hatch
instead, but have not understood yet how to link scikit-built-core
to hatch
. Since this is a cmake
build, would it not be simpler to try to develop a hatch-cmake` hook?
Henry (the maintainer) is working on making a plugin for Hatchling. Until then this current approach is fine. Which PR should I review? I don't quite know the plan.
https://github.com/ofek/coincurve/pull/159 is ready, it only add verification for shared lib build. Or do you think it should not be added?
This PR still has issues, but I think it is more maintainable than the setuptools+CMake. So the plan was to move to CMake, then to use hatch back-end and create the hooks to setuptools - but then you pointed me to scikit-build-core ... so we can see this PR as a parallel dev and choose how to move forward - I am not so good at plan (or communication for that matter)
Also, I had not realized that you know so much more than me on this subject (looking at how hatch is written), so I admit I am myself a bit confused as to your plans for coincurve
- all I needed was to have coincurve on conda-forge, but I thought I could provide help in refactoring so that 1- we test the wheels on windows, 2 we upgrade the build to use CMake, which is where I think SECP256K1 is headed. As I was doing so, I also thought it'd be neat to reuse the framework to readily provide the experimental SECP256K1 that the issue was asking for, like coincurve[zkrp] of sorts
Do we need to better sync-up?
Definitely please move forward just with scikit-build-core!
So, if I undersand correctly, Henry will build a plugin for hatch which would use scikit-build-core and be able to interact with hatch, but I think what I thought we would have is a hatch plugin to interact/control scikit-build-core. With the current PR, do you see how coincurve woulb benefit from hatch
I am trying to update the config with something like:
[[tool.hatch.build.hooks.build-scripts.scripts]]
out_dir = "out"
commands = [
"mkdir -p build_with_cmake",
"cd build_with_cmake && cmake -S .. -B .",
"cd build_with_cmake && cmake --build .",
]
artifacts = [
to try to understand how hatch fits/improves the current status
The plugin would live at the build system layer i.e. Hatchling, which would invoke some logic within that other package. Hatch the CLI has nothing to do with this and we don't even have to use it if you don't want to.
Right, I meant hatchling
, but the backend would be scikit-build-core
, with a tools.scikit-built.hatchling.xxx
then what would we use it for? what is coincurve
missing without hatchling
in the current state
I am thinking to try to PR a update of the current master with hatchling to better understand the differences and benefits
The plugin would live at the build system layer i.e. Hatchling
I took a brief look and it seems to me that it is similar to the hooks/plugin of hatch. I think what I would prefer is a tools.hatch.hooks.scikit-build-core
framework
I was working a little on moving coincurve to using Cython and interestingly I see that Henry has a Cython_cmake project,
For more context, see this issue: https://github.com/PyO3/maturin/issues/1419
Basically, Hatchling will be the backend when there is a plugin and the only thing that other package would do is build the artifacts for Hatchling to consume.
Right, I think I understand, though the pyproject.toml should have hatchling as build-backend, then specify the configuration of scikit-build-core with a plug-in, correct?
Hatchling seems more tailored to something like testing a wheel within a pip-free conda environment (right, i don'tlike pip interfering with conda), the plugin would have to pass-through all hatchling config, rather than just the artifacts
Maybe I am still missing it, I'll start looking into a scikit-build-core plugin to try to better understand it
I mean, there's only one 'build-backend' spec in the pyproject.toml, or do you mean that we'll have to also provide a hatch.toml?
In the future scenario Hatchling would be the backend with a plugin configured that would call scikit-build-core. In the final future scenario after that, scikit-build-core would remove its ability to be a backend (i.e. strip out all of the code to build wheels and source distributions) and it would only be used as a plugin for any backend that supported the future plugin architecture.
For now, we have to use only scikit-build-core.
What do you think about scikit-build-cli
? It may not be too difficult to create a hatch plugin, this way we could move on to using hatchling and have a more finalized packaging, then once scikit-build-core has moved forward we'd use the more integrated interface
Do you mean a Hatchling build hook that invokes something as a subprocess to build the artifacts?
Yes, I thought that was the model you were looking for. I dug a bit more on scikit and I think the plugin is intended to be something likebuild-backend = scikit-build-core.hatchling.build_meta
, right, but it seems convoluted to me and more complex to engineer
No the future plugin would be a build hook like this https://github.com/ofek/hatch-mypyc so Hatchling would still be the backend, nothing else would be referenced in that section.
Sure if you can make that work that sounds good! You would want a custom build hook:
- https://hatch.pypa.io/latest/plugins/build-hook/custom/
- https://hatch.pypa.io/latest/plugins/build-hook/reference/#hatchling.builders.hooks.plugin.interface.BuildHookInterface.initialize
- https://hatch.pypa.io/latest/plugins/builder/wheel/#build-data
- https://github.com/ofek/hatch-mypyc/blob/5a198c0ba8660494d02716cfc9d79ce4adfb1442/hatch_mypyc/plugin.py#L340-L343
I am too superficial! After more digging, this is exactly what Henry is working on (last commit 4 days ago). He is developing it inside a branch, so I thought he was implementing it the way he wraps setuptools, but looking at his test suite and example, it is what I am looking for:
[tool.hatch.build.targets.wheel.hooks.scikit-build]
dependencies = ["scikit-build-core"]
wheel.install-dir = "hatchling_example"
cmake.source-dir = "cpp"
lOl ... 10s of back-and-forth for me to finally get it :'(
I didn't know it was that far along, nice!
Oh, you're not collaborating with him? So, I am copying his progress into a standalone plugin that I will install locally to test out
I am in general but not implementing the plugin directly. For that, I think one of his undergraduate researchers is assisting.
Hmmm ... I don't want to be ridiculed by an undergraduate student though ... maybe I should drop him a note before I get entangled
Some progress, though still some hiccups:
-- Install configuration: "Release"
-- Installing: //_libsecp256k1.cpython-311-x86_64-linux-gnu.so
CMake Error at /tmp/tmpyk6rr366/cmake_install.cmake:80 (file):
file INSTALL cannot copy file
"/tmp/tmpyk6rr366/cm_python_module/_libsecp256k1.cpython-311-x86_64-linux-gnu.so"
to "//_libsecp256k1.cpython-311-x86_64-linux-gnu.so": Permission denied.
from:
[tool.hatch.build.targets.wheel.hooks.hatch-scikit-build-core]
dependencies = ['scikit-build-core']
wheel.install-dir = 'build/scikit-build-core'
cmake.source-dir = '.'
# --- scikit-build-core ---
[tool.scikit-build]
# metadata.version.provider = "scikit_build_core.metadata.setuptools_scm"
cmake.verbose = true
# logging.level = 'INFO'
# wheel.packages = ['src/coincurve']
cmake.build-type = 'Release'
I had to extract the plugin from scikit-build-core, I could not figure out how to register a plugin from within another package. But we'll have to await until the 0.9 release since the plugin requires an update in scikit-build-core anyway I have a good working space though with locally installed scikit and plugin, I'll call it a day ... a successful day even
@ofek I invited you to the plugin repo. It is private for now. It is mostly from Henry, but I did not understand why he needed to update scikit-build-core. I implemented a version that seems to work with 0.8.2, but I'd like you to take a look. The build_dir test is failing because I don't quite understand the mypyc
that it comes from. But it builds the wheel.
I am trying to understand how to control exactly what files are built by scikit and passed to hatchling, but I could use a helping eye and brain ...
@ofek It looks like with the new plugin, scikit-build-core
is creating the wheel, the installed coincurve
in the tox directory only contains _libsecp256k1.xxx.so
.
How does hatch retrieve the artifact from scikit-build hook. The hook builds the library correctly:
-- Installing: /tmp/tmp17u80te9/wheel/platlib/coincurve/_libsecp256k1.cpython-311-x86_64-linux-gnu.so
Preparing editable metadata (pyproject.toml) ... done
but after that it seems that I am missing something. With the previous implementation, the artifacts were passed in the build_data
, I don't see it in this implementation - stuck for now
cc @henryiii
With python -m build
:
*** Installing project into wheel...
-- Install configuration: "Release"
-- Installing: /tmp/tmpzvu_i59_/wheel/platlib/coincurve/_libsecp256k1.cpython-311-x86_64-linux-gnu.so
Successfully built coincurve-19.0.2.tar.gz and coincurve-19.0.2-cp311-cp311-linux_x86_64.whl
content:
unzip -l dist/coincurve-19.0.2-cp311-cp311-linux_x86_64.whl
Archive: dist/coincurve-19.0.2-cp311-cp311-linux_x86_64.whl
Length Date Time Name
--------- ---------- ----- ----
179 2020-02-02 00:00 coincurve/__about__.py
297 2020-02-02 00:00 coincurve/__init__.py
1117 2020-02-02 00:00 coincurve/context.py
3985 2020-02-02 00:00 coincurve/ecdsa.py
433 2020-02-02 00:00 coincurve/flags.py
24235 2020-02-02 00:00 coincurve/keys.py
0 2020-02-02 00:00 coincurve/py.typed
310 2020-02-02 00:00 coincurve/types.py
4183 2020-02-02 00:00 coincurve/utils.py
1279192 2020-02-02 00:00 coincurve/_libsecp256k1.cpython-311-x86_64-linux-gnu.so
3977 2020-02-02 00:00 coincurve-19.0.2.dist-info/METADATA
99 2020-02-02 00:00 coincurve-19.0.2.dist-info/WHEEL
9748 2020-02-02 00:00 coincurve-19.0.2.dist-info/licenses/LICENSE-APACHE
1065 2020-02-02 00:00 coincurve-19.0.2.dist-info/licenses/LICENSE-MIT
1218 2020-02-02 00:00 coincurve-19.0.2.dist-info/RECORD
--------- -------
1330038 15 files
When I install that wheel and test it, it seems to work
Note that I could be missing something quite obvious
I'd recommend pushing the update so we can see the final failure message. Also if the wheel looks fine I don't quite understand what you mean about tox not working correctly. Could it be that editable installations aren't yet supported so we should remove that option in tox?
@ofek that was it! Pushing thru
Ready for review?