FEAT: Transition to scikit-build-core CMake-based build process #162

MementoRC · 2024-03-27T19:15:35Z

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 · 2024-03-27T19:18:41Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.64%. Comparing base (07d332e) to head (fe052a6).

❗ 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% <ø> (ø)`

…cikit-cmake # Conflicts: # setup_tools/build_clib.py # setup_tools/support.py

…kit-cmake

MementoRC · 2024-03-28T21:03:18Z

@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?

ofek · 2024-03-28T21:39:55Z

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.

MementoRC · 2024-03-28T22:17:51Z

#159 is ready, it only add verification for shared lib build. Or do you think it should not be added?

MementoRC · 2024-03-28T22:25:02Z

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?

ofek · 2024-03-28T22:41:54Z

Definitely please move forward just with scikit-build-core!

MementoRC · 2024-03-29T23:02:03Z

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

ofek · 2024-03-29T23:09:41Z

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.

MementoRC · 2024-04-22T16:49:11Z

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

ofek · 2024-04-22T17:03:01Z

pyproject.toml

+    'scikit-build-core>=0.9.0',
+    'hatchling',
+    'setuptools>=42.0.0',
+    'cffi>=1.3.0,


Suggested change

'cffi>=1.3.0,

'cffi>=1.3.0',

Right, Thanks, I got this one and corrected it at my end, but realized the tox test did not pass at my end either (I was using python -m build), so I stopped committing failing updates to minimize the emails

ofek · 2024-04-22T17:14:34Z

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?

MementoRC · 2024-04-22T19:11:24Z

@ofek that was it! Pushing thru

ofek · 2024-04-30T14:11:53Z

Ready for review?

MementoRC · 2024-04-30T19:17:59Z

Yes, it is ready - I got distracted with other projects - I am not very knowledgeable in CMake, so there maybe lots of improvements possible

MementoRC · 2024-05-20T19:18:16Z

@ofek Did you have some time to review? Perhaps finding someone knowledgeable in CMake to provide feedback would be good

ofek · 2024-06-01T00:04:12Z

Sorry for the wait! I just tried and am getting an error:

* Building wheel...
*** scikit-build-core 0.9.5 using CMake 3.29.3 (wheel)
*** Configuring CMake...
2024-05-31 20:02:10,042 - scikit_build_core - WARNING - Can't find a Python library, got libdir=None, ldlibrary=None, multiarch=None, masd=None
loading initial cache file C:\Users\ofek\AppData\Local\Temp\tmpz7b4bd21\build\CMakeInit.txt
-- Building for: Visual Studio 17 2022
-- Selecting Windows SDK version 10.0.19041.0 to target Windows 10.0.22631.
-- The C compiler identification is MSVC 19.33.31630.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: C:/Program Files (x86)/Microsoft Visual Studio/2022/BuildTools/VC/Tools/MSVC/14.33.31629/bin/Hostx64/x64/cl.exe - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Found Python: C:\Users\ofek\AppData\Local\Temp\build-env-ynqhppp2\Scripts\python.exe (found suitable version "3.11.7", minimum required is "3") found components: Interpreter Development.Module Development.SABIModule
CMake Error at C:/Users/ofek/AppData/Local/Temp/build-env-ynqhppp2/Lib/site-packages/cmake/data/share/cmake-3.29/Modules/FindPackageHandleStandardArgs.cmake:230 (message):
  Could NOT find PkgConfig (missing: PKG_CONFIG_EXECUTABLE)
Call Stack (most recent call first):
  C:/Users/ofek/AppData/Local/Temp/build-env-ynqhppp2/Lib/site-packages/cmake/data/share/cmake-3.29/Modules/FindPackageHandleStandardArgs.cmake:600 (_FPHSA_FAILURE_MESSAGE)
  C:/Users/ofek/AppData/Local/Temp/build-env-ynqhppp2/Lib/site-packages/cmake/data/share/cmake-3.29/Modules/FindPkgConfig.cmake:114 (find_package_handle_standard_args)
  CMakeLists.txt:35 (find_package)


-- Configuring incomplete, errors occurred!
Traceback (most recent call last):
  File "C:\Users\ofek\AppData\Local\Programs\Python\Python311\Lib\site-packages\pyproject_hooks\_in_process\_in_process.py", line 353, in <module>
    main()
  File "C:\Users\ofek\AppData\Local\Programs\Python\Python311\Lib\site-packages\pyproject_hooks\_in_process\_in_process.py", line 335, in main
    json_out['return_val'] = hook(**hook_input['kwargs'])
                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\ofek\AppData\Local\Programs\Python\Python311\Lib\site-packages\pyproject_hooks\_in_process\_in_process.py", line 251, in build_wheel
    return _build_backend().build_wheel(wheel_directory, config_settings,
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\ofek\AppData\Local\Temp\build-env-ynqhppp2\Lib\site-packages\hatchling\build.py", line 58, in build_wheel
    return os.path.basename(next(builder.build(directory=wheel_directory, versions=['standard'])))
                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\ofek\AppData\Local\Temp\build-env-ynqhppp2\Lib\site-packages\hatchling\builders\plugin\interface.py", line 147, in build
    build_hook.initialize(version, build_data)
  File "C:\Users\ofek\AppData\Local\Temp\build-env-ynqhppp2\Lib\site-packages\scikit_build_core\hatch\plugin.py", line 125, in initialize
    self._initialize(build_data=build_data)
  File "C:\Users\ofek\AppData\Local\Temp\build-env-ynqhppp2\Lib\site-packages\scikit_build_core\hatch\plugin.py", line 224, in _initialize
    builder.configure(
  File "C:\Users\ofek\AppData\Local\Temp\build-env-ynqhppp2\Lib\site-packages\scikit_build_core\builder\builder.py", line 235, in configure
    self.config.configure(
  File "C:\Users\ofek\AppData\Local\Temp\build-env-ynqhppp2\Lib\site-packages\scikit_build_core\cmake.py", line 221, in configure
    raise FailedLiveProcessError(msg) from None
scikit_build_core.errors.FailedLiveProcessError: CMake configuration failed

ERROR Backend subprocess exited when trying to invoke build_wheel

henryiii · 2024-06-01T01:20:32Z

No pkg-config it looks like.

henryiii · 2024-06-01T01:21:10Z

I wonder if there’s a way we can suppress those extra tracebacks that aren’t useful.

ofek · 2024-06-01T01:25:47Z

I was expecting it to Just Work ™️ ... is that a requirement?

CMakeLists.txt

.gitignore

pyproject.toml

Co-authored-by: Ofek Lev <[email protected]>

ofek · 2024-06-01T17:23:01Z

Can we drop Windows 32 bit? After that CI will pass (no pypkgconf wheel for that architecture).

ofek

This is awesome, thank you very much!

Please keep in mind that I will need assistance maintaining this because, although it's much better than before, I have no idea how any of this works... at all 😅 For example, it's unclear to me why we have multiple top-level directories, a script that does stuff to the header files and another for something else.

MementoRC · 2024-06-01T20:35:39Z

Well, that organization is just a matter of preference (or non-formal CS background) it could be organized differently. I am also not very knowledgeable with CMake, so I needed a lot of debugging, separating the different phase helped me navigate

MementoRC added 3 commits March 27, 2024 13:50

(feat) Add CMake framework

d675e4d

(broken) remove setup.py, setup_tools, _cffi_build

02a9e88

(ref) update flow, pyproject, reqs, MANIFEST

20a05a8

MementoRC requested a review from ofek March 27, 2024 19:34

MementoRC added 9 commits March 27, 2024 16:21

Merge remote-tracking branch 'upstream/master' into feat/transition-s…

476ba92

…cikit-cmake # Conflicts: # setup_tools/build_clib.py # setup_tools/support.py

(cos) update pyproject.toml

2cbf1be

(cos) update coverage.rc

118d1c9

(ref) integrate coverage in pyproject

5fa3bdc

(cos) cleanup

f32c83d

(fix) pkg-config search paths for libsecp256k1

75fae61

Merge remote-tracking branch 'origin/master' into feat/transition-sci…

ae4bbac

…kit-cmake

(fix) several major issue with pkg-config usage

c10194b

(fix) wrong library name

bb96d0d

(fix) SECP256K1_CONTEXT_NONE removed?

4582eaa

MementoRC added 8 commits March 28, 2024 17:47

(fix) generate errors when wrong input for CFFI headers

af44a89

Merge branch 'master' into feat/transition-scikit-cmake

e55f25b

(dbg) silly / typo

bd1291b

(ref) simplify top-level CMakeLists.txt

31bb2c8

(fix) typo in name

3ca6cc3

(ref) update version handling

fe052a6

(ref) update .gitignore

abbdf23

(ref) update .gitignore

d91e8aa

ofek reviewed Apr 22, 2024

View reviewed changes

(fix) develop not yet supported

aaac1b4

henryiii reviewed Jun 1, 2024

View reviewed changes

CMakeLists.txt Show resolved Hide resolved

(fix) pkgconfig as req

c56ab6f

ofek reviewed Jun 1, 2024

View reviewed changes

.gitignore Show resolved Hide resolved

ofek reviewed Jun 1, 2024

View reviewed changes

pyproject.toml Outdated Show resolved Hide resolved

MementoRC and others added 3 commits June 1, 2024 11:15

(fix) stie in .gitignore

7102a60

pypkgconf for win32

8bd4a34

Co-authored-by: Ofek Lev <[email protected]>

(fix) remove pkgconfiglite in CI

f58d6d9

MementoRC and others added 6 commits June 1, 2024 12:28

(fix) remove win x86

83e5dd3

cleanup

e4d2c6c

try

1c62582

oops

802fdbb

Update pyproject.toml

f237a9e

add license expression

39d204f

ofek approved these changes Jun 1, 2024

View reviewed changes

ofek merged commit 31a583c into ofek:master Jun 1, 2024
13 checks passed

MementoRC deleted the feat/transition-scikit-cmake branch June 2, 2024 14:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FEAT: Transition to scikit-build-core CMake-based build process #162

FEAT: Transition to scikit-build-core CMake-based build process #162

MementoRC commented Mar 27, 2024

codecov bot commented Mar 27, 2024 •

edited

Loading

MementoRC commented Mar 28, 2024

ofek commented Mar 28, 2024

MementoRC commented Mar 28, 2024

MementoRC commented Mar 28, 2024 •

edited

Loading

ofek commented Mar 28, 2024

MementoRC commented Mar 29, 2024

ofek commented Mar 29, 2024

MementoRC commented Apr 22, 2024

ofek Apr 22, 2024

MementoRC Apr 22, 2024

ofek commented Apr 22, 2024

MementoRC commented Apr 22, 2024

ofek commented Apr 30, 2024

MementoRC commented Apr 30, 2024

MementoRC commented May 20, 2024

ofek commented Jun 1, 2024

henryiii commented Jun 1, 2024

henryiii commented Jun 1, 2024

ofek commented Jun 1, 2024

ofek commented Jun 1, 2024

ofek left a comment

MementoRC commented Jun 1, 2024

FEAT: Transition to scikit-build-core CMake-based build process #162

FEAT: Transition to scikit-build-core CMake-based build process #162

Conversation

MementoRC commented Mar 27, 2024

codecov bot commented Mar 27, 2024 • edited Loading

Codecov Report

MementoRC commented Mar 28, 2024

ofek commented Mar 28, 2024

MementoRC commented Mar 28, 2024

MementoRC commented Mar 28, 2024 • edited Loading

ofek commented Mar 28, 2024

MementoRC commented Mar 29, 2024

ofek commented Mar 29, 2024

MementoRC commented Apr 22, 2024

ofek Apr 22, 2024

Choose a reason for hiding this comment

MementoRC Apr 22, 2024

Choose a reason for hiding this comment

ofek commented Apr 22, 2024

MementoRC commented Apr 22, 2024

ofek commented Apr 30, 2024

MementoRC commented Apr 30, 2024

MementoRC commented May 20, 2024

ofek commented Jun 1, 2024

henryiii commented Jun 1, 2024

henryiii commented Jun 1, 2024

ofek commented Jun 1, 2024

ofek commented Jun 1, 2024

ofek left a comment

Choose a reason for hiding this comment

MementoRC commented Jun 1, 2024

codecov bot commented Mar 27, 2024 •

edited

Loading

MementoRC commented Mar 28, 2024 •

edited

Loading