Proposing refactoring of build process

[MementoRC]

Final goal is to have a (hopefully) simplified build process to mitigate

• Relying on antiquated cffi_modules . It defines -lsecp256k1 which will fail with a CMake-based SECP256K1 build . It reduce the tailoring of CFFI build
• Revamp the CFFI build process to customize the _libsecp256k1.c file compilation

Hopefully, the commits will be self-explanatory and help verify a step-by-step transition

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 85.63%. Comparing base (b8e9dfe) to head (fd13ef8).

:exclamation: Current head fd13ef8 differs from pull request most recent head bc4efef. Consider uploading reports for the commit bc4efef to get more accurate results

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #141 +/- ## ======================================= Coverage 85.63% 85.63% ======================================= Files 11 11 Lines 564 564 Branches 66 66 ======================================= Hits 483 483 Misses 45 45 Partials 36 36 ```

[ofek]

I would be extremely interested in getting rid of CFFI and using a usual extension if possible without you having to invest a lot of your time.

[ofek]

Actually there might not be any benefit to that, forget me. Please continue doing what you were doing here before!

[MementoRC]

So in this proposal, CFFI is only used to prepare the _libsecp256k1.c interface and use the extension to build/link. One reason I am working on this is that when I tried to use the CMake to build libsecp256k1.a, it failed because they don't use -fPIC and gcc complains when building. So a more customized linking is needed, which is not possible with cffi_module But then ... MacOS decide to join the belligerent camp :\

[MementoRC]

My worry is that too many changes will be needed, I don't know see how to make incremental commit/merge to master in this case

[ofek]

I don't care what we merge as long as it passes tests and I will verify locally on my Windows machine using one of the built artifacts.

[MementoRC]

@ofek As I was struggling with the static lib, I ventured into the dynamic lib and wondered if it would not be simpler to provide libsecp256k1.so as part of coincurve build. Currently, it is the same only with SECP256K1 library hidden inside _libsecp245k1.

What I mean is that it is conceptually the same thing in terms of providing SECP256K1

I will propose to add:

coincurve
   |
   ---> lib
           |
            ---> libsecp256k1.so/dylib/dll  (might be a bit confusing for windows)

[MementoRC]

I don't care what we merge as long as it passes tests and I will verify locally on my Windows machine using one of the built artifacts.

Have you been able to reproduce the issue and verify that the current master resolved it? If so, we should probably release.

[ofek]

[MementoRC]

This seems to be the consequence of moving the dependencies into the pyproject.toml. I did that because when first tried the conda recipe, it seemed to need PEP517

I can take a look at it

[MementoRC]

@ofek https://github.com/ofek/coincurve/pull/142 seems to be resolving this issue

[MementoRC]

@ofek I continue to commit to this PR so that you can follow the updates if needed. If something requires a lot of debug, I'll switch over to my fork

[MementoRC]

Closing for debug purpose

[MementoRC]

@ofek This is close enough for your review and suggestions. Currently, the build uses a static libsecp256k1, but it should also work (tested on my branches) with a shared lib.

When built with a shared lib, coincurve will package the dynamic library, so it's unclear whether you'd want to provide a wheel for that build, it could be interesting if someone runs multiple instances of coincurve, it would then benefit from the dynamic link

[MementoRC]

Windows is such ... Do you spot something that I am missing:

  C:\Users\runneradmin\AppData\Local\Temp\cibw-run-g9acegp8\cp312-win_amd64\venv-test\Lib\site-packages\coincurve\__init__.py:31: UserWarning: DBG: libsecp256k1:EXTTERNAL
    load_secp256k1_conda_library()
  LoadLibrary \Device\HarddiskVolume2\Windows\System32\kernel.appcore.dll

  LoadLibrary \Device\HarddiskVolume2\Miniconda\envs\test\Library\bin\libsecp256k1-2.dll   <--- FOUND and loaded

  LoadLibrary \Device\HarddiskVolume2\Users\runneradmin\AppData\Local\pypa\cibuildwheel\Cache\nuget-cpython\python.3.12.1\tools\DLLs\_wmi.pyd
  LoadLibrary \Device\HarddiskVolume2\Windows\System32\propsys.dll

  LoadLibrary \Device\HarddiskVolume2\Users\runneradmin\AppData\Local\Temp\cibw-run-g9acegp8\cp312-win_amd64\venv-test\Lib\site-packages\coincurve\_libsecp256k1.cp312-win_amd64.pyd   <--- FOUND and loaded

  LoadLibrary \Device\HarddiskVolume2\Users\runneradmin\AppData\Local\Temp\cibw-run-g9acegp8\cp312-win_amd64\venv-test\Lib\site-packages\_cffi_backend.cp312-win_amd64.pyd
  LoadLibrary \Device\HarddiskVolume2\Users\runneradmin\AppData\Local\pypa\cibuildwheel\Cache\nuget-cpython\python.3.12.1\tools\DLLs\_hashlib.pyd
  LoadLibrary \Device\HarddiskVolume2\Users\runneradmin\AppData\Local\pypa\cibuildwheel\Cache\nuget-cpython\python.3.12.1\tools\DLLs\libcrypto-3.dll
  LoadLibrary \Device\HarddiskVolume2\Users\runneradmin\AppData\Local\pypa\cibuildwheel\Cache\nuget-cpython\python.3.12.1\tools\DLLs\unicodedata.pyd
  LoadLibrary \Device\HarddiskVolume2\Users\runneradmin\AppData\Local\pypa\cibuildwheel\Cache\nuget-cpython\python.3.12.1\tools\DLLs\_socket.pyd
  LoadLibrary \Device\HarddiskVolume2\Users\runneradmin\AppData\Local\pypa\cibuildwheel\Cache\nuget-cpython\python.3.12.1\tools\DLLs\select.pyd
  LoadLibrary \Device\HarddiskVolume2\Users\runneradmin\AppData\Local\pypa\cibuildwheel\Cache\nuget-cpython\python.3.12.1\tools\DLLs\_decimal.pyd

  *** This is in the __init__.py to preload when it is EXTERNAL
  C:\Users\runneradmin\AppData\Local\Temp\cibw-run-g9acegp8\cp312-win_amd64\venv-test\Lib\site-packages\coincurve\__init__.py:31: UserWarning: DBG: libsecp256k1:EXTTERNAL
    load_secp256k1_conda_library()

  Traceback (most recent call last):
    File "<string>", line 1, in <module>
    File "C:\Users\runneradmin\AppData\Local\Temp\cibw-run-g9acegp8\cp312-win_amd64\venv-test\Lib\site-packages\coincurve\__init__.py", line 33, in <module>
      from coincurve.context import GLOBAL_CONTEXT, Context  # noqa: E402
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "C:\Users\runneradmin\AppData\Local\Temp\cibw-run-g9acegp8\cp312-win_amd64\venv-test\Lib\site-packages\coincurve\context.py", line 5, in <module>
     from coincurve._libsecp256k1 import ffi, lib
  ImportError: DLL load failed while importing _libsecp256k1: The specified module could not be found.   <--- of course, no idea which one is not found

Starting to run out of ideas. I am pretty sure that this will work for the conda-forge recipe, but I would like to completely test the wheel built with a system lib, even though we don't release it

[ofek]

I'm not sure honestly

[MementoRC]

Thanks, case of too much desperation. In this case the issue was EXTTERNAL typo. Well, not only I don't like windows (so very biased), but I am frustrated with how often I mistype or write something silly

The good news is: It is finally working ... well, at least until I find that I actually didn't do it right ... but for a start of the weekend, I'll blissfully take it

Ok, now for the tough question ... how do we do this? There are lots of changes

[ofek]

First I would rebase to resolve conflicts and then get CI passing.

[MementoRC]

Oh no, I will close this PR (I thought I had done so already, absent-minded) - You said you did not want to change the download, which this PR included

I was referring to the significant changes in the flow that I finally completed (I will make a new PR, to hide all the shameful craziness I went through)