Skip to content

Conversation

@sjaeckel
Copy link
Member

This is a follow-up of #489

It adds "automation" of the modifications to tommath.def and a travis job to check for this.

Additionally the proposed split-up of mp_rand.c is done and the modifications to s_mp_rand_platform.c are reverted.

@madebr
Copy link
Contributor

madebr commented Sep 13, 2020

FYI, this are the results of building libtommath on linux gcc, linux clang, macos and windows msvc: conan-io/conan-center-index#2904 (comment)
Don't mind the .la errors, I forgot to remove these.

Copy link
Contributor

@czurnieden czurnieden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't tell a lot about the Windows related changes, but the rest I like!

@madebr
Copy link
Contributor

madebr commented Sep 14, 2020

With the following changes, this pr allows building the tests linked to the dll.

--- a/appveyor.yml
+++ b/appveyor.yml
@@ -19,3 +19,4 @@ build_script:
         nmake -f makefile.msvc all
 test_script:
 - cmd: test.exe
+- cmd: test_dll.exe
--- a/makefile.msvc
+++ b/makefile.msvc
@@ -71,10 +71,15 @@ $(LIBMAIN_D) $(LIBMAIN_I): $(OBJECTS) tommath.def

 #Build test suite
 test.exe: $(LIBMAIN_S) demo/shared.obj demo/test.obj
-       cl $(LTM_CFLAGS) $(TOBJECTS) $(LIBMAIN_S) $(LTM_LDFLAGS) demo/shared.c demo/test.c /Fe$@
+       link $(TOBJECTS) $(LTM_LDFLAGS) $? /out:$@
        @echo NOTICE: start the tests by launching test.exe

-all: $(LIBMAIN_S) test.exe $(LIBMAIN_D)
+#Build test suite for dll
+test_dll.exe: $(LIBMAIN_I) demo/shared.obj demo/test.obj
+       link $(TOBJECTS) $(LTM_LDFLAGS) $? /out:$@
+       @echo NOTICE: start the tests by launching test_dll.exe
+
+all: $(LIBMAIN_S) test.exe $(LIBMAIN_D) test_dll.exe

 tune: $(LIBMAIN_S)
        $(MAKE) -C etc tune

@madebr
Copy link
Contributor

madebr commented Sep 15, 2020

The test is failing in test_mp_fread_fwrite.
It looks like we need to override the default runtime when building dll's.
By default static runtime is used, but that does not work because the mp_f* api calls pass FILE* handles.
So use the shared MD runtime by default instead.
People can override the default by setting it in CFLAGS.

--- a/makefile.msvc
+++ b/makefile.msvc
@@ -11,7 +11,7 @@

 #The following can be overridden from command line e.g. make -f makefile.msvc CC=gcc ARFLAGS=rcs
 PREFIX    = c:\devel
-CFLAGS    = /Ox
+CFLAGS    = /Ox /MD
 LDFLAGS   =

 #Compilation flags

@sjaeckel sjaeckel force-pushed the fix-488 branch 3 times, most recently from b401b16 to e798464 Compare September 15, 2020 16:55
sjaeckel and others added 3 commits September 20, 2020 18:15
* move jenkins' prng out of the library into the demo's.
* add CI test for shared library
@sjaeckel sjaeckel requested a review from czurnieden November 29, 2020 15:40
Copy link
Contributor

@czurnieden czurnieden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ubuntu's Bionic Beaver is version 18.04, you might change the comment above the distribution settings in .travis.yml otherwise: looks good for me!

@sjaeckel sjaeckel merged commit 5167f6c into develop Nov 30, 2020
@sjaeckel sjaeckel deleted the fix-488 branch November 30, 2020 15:03
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.

4 participants