Skip to content

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Dec 14, 2022

Cherry-picked the first commit from #1142 and addressed a comment.

hebasto and others added 2 commits December 15, 2022 10:56
This change eases the use of alternate build systems by moving
the variables in `src/libsecp256k1-config.h` to compiler macros
for each invocation, preventing duplication of these variables
for each build system.

Co-authored-by: Ali Sherief <[email protected]>
@hebasto
Copy link
Member Author

hebasto commented Dec 15, 2022

Updated e129a39 -> ad8647f (pr1178.01 -> pr1178.02, diff):

@hebasto
Copy link
Member Author

hebasto commented Dec 15, 2022

Not sure about this AC_DEFINE macro

]])], [has_valgrind=yes; AC_DEFINE(HAVE_VALGRIND,1,[Define this symbol if valgrind is installed, and it supports the host platform])])

@real-or-random
Copy link
Contributor

Not sure about this AC_DEFINE macro

]])], [has_valgrind=yes; AC_DEFINE(HAVE_VALGRIND,1,[Define this symbol if valgrind is installed, and it supports the host platform])])

I suggest removing it, it's unused, and also confusing because we use the VALGRIND macro.

@hebasto
Copy link
Member Author

hebasto commented Dec 15, 2022

I suggest removing it, it's unused, and also confusing because we use the VALGRIND macro.

Done.

Copy link
Contributor

@real-or-random real-or-random left a comment

Choose a reason for hiding this comment

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

utACK 9c5a4d2

@sipa
Copy link
Contributor

sipa commented Dec 20, 2022

utACK 9c5a4d2

@@ -148,7 +148,7 @@ endif
if USE_EXAMPLES
noinst_PROGRAMS += ecdsa_example
ecdsa_example_SOURCES = examples/ecdsa.c
ecdsa_example_CPPFLAGS = -I$(top_srcdir)/include
ecdsa_example_CPPFLAGS = -I$(top_srcdir)/include $(SECP_CONFIG_DEFINES)
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't these be dropped for the example, because a user application shouldn't need our defines?

Copy link
Member Author

Choose a reason for hiding this comment

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

See #1185.

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