Makefile: add `-I$(top_srcdir)/{include,src}` to `CPPFLAGS` for precomputed #1160

pull whitslack wants to merge 1 commits into bitcoin-core:master from whitslack:out-of-source-tree changing 1 files +3 −1
  1. whitslack commented at 3:19 AM on November 22, 2022: contributor

    When performing an out-of-source-tree build, regenerating the source files for the precomputed ecmult tables places them outside the source tree. Then, when they are to be compiled, they cannot find the headers they need because the source tree is absent from their include search path. This appears to have been an oversight, as the relevant -I options are present in libsecp256k1_la_CPPFLAGS but were missing from libsecp256k1_precomputed_la_CPPFLAGS. This PR adds them.

  2. whitslack force-pushed on Nov 22, 2022
  3. in Makefile.am:76 in 54e290ddaf outdated 
      72 | @@ -73,7 +73,7 @@ noinst_HEADERS += examples/random.h
  73 |  PRECOMPUTED_LIB = libsecp256k1_precomputed.la
  74 |  noinst_LTLIBRARIES = $(PRECOMPUTED_LIB)
  75 |  libsecp256k1_precomputed_la_SOURCES =  src/precomputed_ecmult.c src/precomputed_ecmult_gen.c
  76 | -libsecp256k1_precomputed_la_CPPFLAGS = $(SECP_INCLUDES)
  77 | +libsecp256k1_precomputed_la_CPPFLAGS = -I$(top_srcdir)/include -I$(top_srcdir)/src $(SECP_INCLUDES)
    real-or-random commented at 11:11 AM on November 22, 2022: 
    # We need `-I$(top_srcdir)/src` in VPATH builds if libsecp256k1_precomputed_la_SOURCES have been recreated in the build tree.
# This helps users and packagers who insist on recreating the precomputed files (e.g., Gentoo).
libsecp256k1_precomputed_la_CPPFLAGS = -I$(top_srcdir)/src $(SECP_INCLUDES)

    -I$(top_srcdir)/src suffices. We use relative includes throughout the library, and since src/precomputed_ecmult*.c believe they live in -I$(top_srcdir)/src, everything they include is reachable from there.

    hebasto commented at 12:09 PM on November 30, 2022:

    libsecp256k1_precomputed_la_SOURCES have been cleaned from the source tree

    This is not required to trigger the error. Having them in the build tree is enough for it.

    Therefore, the comment could be adjusted.

    real-or-random commented at 12:32 PM on November 30, 2022:

    This is not required to trigger the error. Having them in the build tree is enough for it.

    Hm strange. What are the exact steps you've run to reproduce this?

    hebasto commented at 12:55 PM on November 30, 2022:

    Hm strange. What are the exact steps you've run to reproduce this?

    $ ./autogen.sh
$ mkdir ../build && cd ../build
$ ../secp256k1/configure
$ make precompute_ecmult precompute_ecmult_gen
$ ./precompute_ecmult
$ ./precompute_ecmult_gen
$ make
    real-or-random commented at 2:31 PM on November 30, 2022:

    Oh indeed, I edited my suggestion.

  4. real-or-random commented at 11:13 AM on November 22, 2022: contributor

    Okay, I think this is not how we intend users to build the library, but if this one-line patch makes it work for your use case, there's probably no reason not to accept this patch.

  5. hebasto commented at 12:06 PM on November 30, 2022: member

    I can reproduce the bug when building out-of-source:

    src/precomputed_ecmult.c:8:10: fatal error: ../include/secp256k1.h: No such file or directory
    8 | #include "../include/secp256k1.h"
      |          ^~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.

    src/precomputed_ecmult_gen.c:6:10: fatal error: ../include/secp256k1.h: No such file or directory
    6 | #include "../include/secp256k1.h"
      |          ^~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.

    Tested that adding -I$(top_srcdir)/src is enough to fix it, as @real-or-random suggested.

    as the relevant -I options are present in libsecp256k1_la_CPPFLAGS but were missing from libsecp256k1_precomputed_la_CPPFLAGS.

    I'm not sure about it:

    https://github.com/bitcoin-core/secp256k1/blob/751c4354d51fb5b10a80764df627b84e1a5ccd4f/Makefile.am#L95

    https://github.com/bitcoin-core/secp256k1/blob/751c4354d51fb5b10a80764df627b84e1a5ccd4f/Makefile.am#L76

  6. real-or-random commented at 4:29 PM on December 19, 2022: contributor

    @whitslack Friendly ping. :)

  7. whitslack commented at 10:46 PM on January 6, 2023: contributor

    Okay, I think this is not how we intend users to build the library

    The reason is to build for multiple ABIs (e.g., x86 and x86_64) without needing to make an extra copy of the sources.

    -I$(top_srcdir)/src suffices.

    Confirmed.

  8. whitslack force-pushed on Jan 6, 2023
  9. real-or-random commented at 11:04 PM on January 6, 2023: contributor

    @whitslack

    Nice! Can you also add the comments I suggested above? #1160 (review)

  10. whitslack referenced this in commit 2a6b89448c on Jan 6, 2023
  11. Makefile: add -I$(top_srcdir)/src to CPPFLAGS for precomputed
    When performing an out-of-source-tree build, regenerating the source
files for the precomputed ecmult tables places them outside the source
tree. Then, when they are to be compiled, they cannot find the headers
they need because the source tree is absent from their include search
path. This appears to have been an oversight, as the relevant -I options
are present in libsecp256k1_la_CPPFLAGS but were missing from
libsecp256k1_precomputed_la_CPPFLAGS. This commit adds them.
    e862c4af0c
  12. whitslack force-pushed on Jan 6, 2023
  13. whitslack referenced this in commit b84b7ba908 on Jan 6, 2023
  14. whitslack referenced this in commit 6705fb2cc0 on Jan 6, 2023
  15. whitslack referenced this in commit 59e4e53602 on Jan 7, 2023
  16. real-or-random approved
  17. real-or-random commented at 9:25 PM on January 7, 2023: contributor

    ACK e862c4af0c5a7300129700d38eff499a836a108d

  18. real-or-random requested review from hebasto on Jan 9, 2023
  19. sipa commented at 9:16 PM on January 11, 2023: contributor

    utACK e862c4af0c5a7300129700d38eff499a836a108d

  20. real-or-random merged this on Jan 12, 2023
  21. real-or-random closed this on Jan 12, 2023
  22. real-or-random commented at 10:04 AM on January 12, 2023: contributor

    @whitslack note that we've created a release in the meanwhile : https://github.com/bitcoin-core/secp256k1/releases

  23. hebasto commented at 2:22 PM on January 13, 2023: member

    Post-merge ACK e862c4af0c5a7300129700d38eff499a836a108d.

  24. dhruv referenced this in commit 4d33046ce3 on Feb 1, 2023
  25. dhruv referenced this in commit 55e7f2cf2b on Feb 2, 2023
  26. stratospher referenced this in commit 647f63669e on Feb 6, 2023
  27. dhruv referenced this in commit a4351c0df6 on Feb 20, 2023
  28. stratospher referenced this in commit 23f825fc8b on Feb 21, 2023
  29. hebasto referenced this in commit 7c0cc5d976 on Mar 7, 2023
  30. dhruv referenced this in commit a5df79db12 on Mar 7, 2023
  31. dhruv referenced this in commit 77b510d84c on Mar 7, 2023
  32. sipa referenced this in commit 763079a3f1 on Mar 8, 2023
  33. div72 referenced this in commit 945b094575 on Mar 14, 2023
  34. real-or-random cross-referenced this on Apr 11, 2023 from issue Update src/secp256k1 subtree to release v0.3.1 by sipa
  35. vmta referenced this in commit e1120c94a1 on Jun 4, 2023
  36. vmta referenced this in commit 8f03457eed on Jul 1, 2023
  37. jonasnick cross-referenced this on Jul 20, 2023 from issue Upstream PRs 1160, 1193, 1169, 1190, 1192, 1194, 1196, 1195, 1170, 1172, 1200, 1199, 1203, 1201, 1206, 1078, 1209, 979, 1212, 1218, 1217, 1221, 1222 by jonasnick
  38. delta1 referenced this in commit 3f32c20932 on Aug 8, 2023
  39. delta1 referenced this in commit 31ac0c1081 on Aug 31, 2023
  40. janus referenced this in commit 4816e2a921 on Sep 3, 2023
  41. whitslack deleted the branch on Nov 16, 2023
Contributors
whitslackreal-or-randomhebastosipa
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin-core/secp256k1. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-30 15:15 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me