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

Issue11 #22

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Issue11 #22

wants to merge 4 commits into from

Conversation

shikuk
Copy link

@shikuk shikuk commented Oct 7, 2018

All variable declarations moved to the start of functions
Created sample cmd file to automatic build fastecdsa on windows

@AntonKueltz
Copy link
Owner

Please make sure all formatting is consistent (some of the C code is indented with 6 spaces rather than 4). Also, I don't think we need test.py, there's already a test suite in the python package folder. If you'd like to add tests please add them there.

Thanks!

@shikuk
Copy link
Author

shikuk commented Oct 8, 2018

If it will be commited, need to replace download address in windows_make.cmd
May be, it's better to write step-by-step instruction, than this ugly batch file?
More better way is to rewrite cmd in python, to exclude need in winrar/7zip installation and aria2c downloading... But I can't

@AntonKueltz
Copy link
Owner

I'll investigate if setuptools has a way to run OS dependent install scripts for packages. If so we can include it there. If not I can either update the README with how to build on Windows (which may be cleaner) or we can commit the make command. What do you think?

I can also write a python script to replace the windows script, shouldn't take too long but I'll need to test it on my Windows box.

@shikuk
Copy link
Author

shikuk commented Oct 8, 2018

Best way for users is "pip install fastecdsa", so it will be exellent, if setuptools just place OS depended prebuilded binaries. But in this way we have to repeat windows build for python 2.7 x64 and build for python 3.x x86/x64 with VC 2010/2014 or test, does builded x86 binaries works in python 2.7 x64 and 3.x x86/x64
Fast try to build for x64

  1. add 64bit yasm.exe to VC\bin\
  2. vcvarsall.bat x64
  3. configure.bat ABI 64
  4. copy mpir.lib VC\lib\amd64\gmp.lib
    fails at
  5. setup.py build --plat-name=win-amd64

curveMath.obj : error LNK2019: unresolved external symbol __imp_Py_BuildValue referenced in function curvemath_mul
curveMath.obj : error LNK2019: unresolved external symbol __imp_PyArg_ParseTuple referenced in function curvemath_mul
curveMath.obj : error LNK2019: unresolved external symbol __imp_Py_InitModule4_64 referenced in function initcurvemath
build\lib.win-amd64-2.7\fastecdsa\curvemath.pyd : fatal error LNK1120: 3 unresolved externals

So, all the ways (setuptools/cmd/python script) have to be checked a lot at all win platforms and python version. I have different machines for experiments, and will try some variants OS/python.
Perhaps, detailed instructions will be enough, but now I can write instruction for python27 x86 only. Need some time to test another variants.

@AntonKueltz
Copy link
Owner

AntonKueltz commented Oct 8, 2018

Sounds good, I can do some investigation into providing binary distributions via setuptools as well. It looks like cross compiling is supported by distutils so that may be something to look at so that we don't need 4 systems to generate binaries for all 32/64 bit python 2/3 combinations.

@shikuk
Copy link
Author

shikuk commented Oct 10, 2018

As I try, cross-compiling does not work. Fastecdsa (and MPIR) for python64 have to be compiled with 64bit cl/yasm.
Whl and zip packages made at one machine successfully works at another.
Reworked cmd files for 32 and 64 bit version - I don't found how to detect python version from cmd file, so make it different. It is for python 2.7 only, python 3.x wants VC 2010/2014

So, have I to write step-by-step instruction?
work.zip

@AntonKueltz
Copy link
Owner

I'll see if I can replicate your build steps and produce windows wheels this weekend. If I can then I'll set up VMs to build Windows wheels for all python version / architecture combinations and build them whenever a release occurs. I'll get back to you in a couple days.

Thanks,
Anton

@AntonKueltz
Copy link
Owner

AntonKueltz commented Oct 12, 2018

When I try to build using the windows_make64.cmd for 64bit Python2.7 I get the following error building the package -

LINK : warning LNK4098: defaultlib 'LIBCMT' conflicts with use of other libs; use /NODEFAULTLIB:library
gmp.lib(mul_basecase.obj) : error LNK2019: unresolved external symbol __gmpn_addmul_2 referenced in function __gmpn_mul_basecase
gmp.lib(sqr_basecase.obj) : error LNK2001: unresolved external symbol __gmpn_addmul_2
build\lib.win-amd64-2.7\fastecdsa\curvemath.pyd : fatal error LNK1120: 1 unresolved externals
error: command '<\\path\\to\\>link.exe' failed with exit status 1120

@shikuk
Copy link
Author

shikuk commented Oct 12, 2018

Can not reproduce.
Watching build logs at vary machines, have a think to replace
call configure.bat ABI 64
with
call configure.bat ABI=64 --cpu=x86_64
to remove platform specific optimisations. This variant make no warnings till building MPIR

@AntonKueltz
Copy link
Owner

Still working through some build issues on my end. I haven't forgotten about this, just moving very slowly. I should have more time to devote on the weekend again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants