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

Fix build for 64bit field under OSX #70

Merged
merged 1 commit into from
Nov 3, 2014

Conversation

peterdettman
Copy link
Contributor

  • caused by 8881212
  • OSX's ar tool doesn't work for empty archives ("ar: no archive members specified")
  • repaired by moving entire commit under USE_ASM

@sipa
Copy link
Contributor

sipa commented Oct 26, 2014

Looks good to me; comments, @theuni?

@evoskuil
Copy link
Contributor

Verified compile on OSX 10.10

@theuni
Copy link
Contributor

theuni commented Oct 27, 2014

Well, the intention for the _common lib was for it to contain anything necessary for bench/test/lib. If we're going to change its meaning to just mean the asm bits, let's be explicit about that. Also, it can be stuffed into a var to avoid adding all the extra if's. Something like:

diff --git a/Makefile.am b/Makefile.am
index a9df7d5..9207688 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1,8 +1,16 @@
 ACLOCAL_AMFLAGS = -I m4
 INCLUDES = $(SECP_INCLUDES)
 lib_LTLIBRARIES = libsecp256k1.la
-noinst_LTLIBRARIES = libsecp256k1_common.la
-include_HEADERS = include/secp256k1.h
+noinst_LTLIBRARIES=
+
+if USE_ASM
+ASM_LIB=libsecp256k1_asm.la
+noinst_LTLIBRARIES += $(ASM_LIB)
+libsecp256k1_asm_la_SOURCES = src/field_5x52_asm.asm
+else
+ASM_LIB=
+endif
+
 noinst_HEADERS =
 noinst_HEADERS += src/group.h
 noinst_HEADERS += src/group_impl.h
@@ -33,14 +41,9 @@ noinst_HEADERS += src/field_impl.h
 pkgconfigdir = $(libdir)/pkgconfig
 pkgconfig_DATA = libsecp256k1.pc

-libsecp256k1_common_la_SOURCES =
-if USE_ASM
-libsecp256k1_common_la_SOURCES += src/field_5x52_asm.asm
-endif
-
 libsecp256k1_la_SOURCES = src/secp256k1.c
 libsecp256k1_la_CPPFLAGS = -I$(top_srcdir)/include $(SECP_INCLUDES)
-libsecp256k1_la_LIBADD = libsecp256k1_common.la $(SECP_LIBS)
+libsecp256k1_la_LIBADD = $(ASM_LIB) $(SECP_LIBS)

 noinst_PROGRAMS =
 if USE_BENCHMARK
@@ -54,7 +57,7 @@ if USE_TESTS
 noinst_PROGRAMS += tests
 tests_SOURCES = src/tests.c
 tests_CPPFLAGS = -DVERIFY $(SECP_TEST_INCLUDES)
-tests_LDADD = libsecp256k1_common.la $(SECP_LIBS) $(SECP_TEST_LIBS)
+tests_LDADD = $(ASM_LIB) $(SECP_LIBS) $(SECP_TEST_LIBS)
 tests_LDFLAGS = -static
 TESTS = tests
 endif

@peterdettman
Copy link
Contributor Author

@theuni I don't think the definition of common was changed; it's just that it currently only contains asm stuff, and so when not using asm, it would be empty (causing OSX's ar to choke). Adding COMMON_LIB variable would be fine.

@peterdettman peterdettman force-pushed the osx-64bit branch 2 times, most recently from a2d9872 to 93e118b Compare October 31, 2014 16:26
@peterdettman
Copy link
Contributor Author

COMMON_LIB added and rebased

- caused by bitcoin-core@8881212
- OSX's ar tool doesn't work for empty archives ("ar: no archive members specified")
- introduce COMMON_LIB variable; leave empty when not using asm
@sipa sipa merged commit e2d66a2 into bitcoin-core:master Nov 3, 2014
sipa added a commit that referenced this pull request Nov 3, 2014
e2d66a2 Fix build for 64bit field under OSX (Peter Dettman)
@peterdettman peterdettman deleted the osx-64bit branch November 6, 2014 13:18
ysangkok pushed a commit to ysangkok/secp256k1 that referenced this pull request Mar 19, 2020
…count

surjectionproof: fix malleability in surjection proof parsing
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