Fix gen_context/ASM build on ARM #935
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This passes $(DEFS) (which should literally be "-DHAVE_CONFIG_H") to the compiler and $(COMMON_LIB) (which is necessary when using external ASM to the linker) when building gen_context.
real-or-random
force-pushed
the
202105-gen-context-defs
branch
from
2928fe1
to
4427d57
Compare
|
Ok, CI answered my question: https://cirrus-ci.com/task/5313033309257728 The idea of the first commit is of course stupid and does not work when cross-compiling. We can't simply link the The above CI failure is different but when I saw it, I still recognized my mistakes. So CI was a good teddy bear here. I'll open another PR with only the second commit and a rephrased comment. |
sipa
added a commit
that referenced
this pull request
May 4, 2021
c848352 Makefile.am: Don't pass a variable twice (Tim Ruffing) 2161f31 Makefile.am: Honor config when building gen_context (Tim Ruffing) 99f47c2 gen_context: Don't use external ASM because it complicates the build (Tim Ruffing) Pull request description: Obsoletes #935. ACKs for top commit: gmaxwell: ACK c848352 looks good and works here. Undefign is kinda yuck, but it is already doing it and it's cleaner than the obvious alternatives. sipa: utACK c848352. I verified that building still works on ARM64, but without asm of course. Tree-SHA512: fc5500688b2aecc4238e21c32f65559bcbfd1e83d1ae4d2c8e15573e94613667731064d8b5f2b9e4209016d88118263802ff4b9a73c1f37c224ccf2a4a1d6536
real-or-random
added a commit
to real-or-random/secp256k1
that referenced
this pull request
Dec 2, 2021
This was necessary because we used to cross-compile the library but compile the precomputation programs for the build host. Now it's no longer necessary and we can cleanly link even the external ASM (which was the intent of bitcoin-core#935). On the way, remove an obsolete "-I" parameter.
real-or-random
added a commit
to real-or-random/secp256k1
that referenced
this pull request
Dec 2, 2021
This was necessary because we used to cross-compile the library but compile the precomputation programs for the build host. Now it's no longer necessary and we can cleanly link even the external ASM (which was the intent of bitcoin-core#935). On the way, remove an obsolete "-I" parameter.
fanquake
pushed a commit
to fanquake/secp256k1
that referenced
this pull request
Dec 3, 2021
This was necessary because we used to cross-compile the library but compile the precomputation programs for the build host. Now it's no longer necessary and we can cleanly link even the external ASM (which was the intent of bitcoin-core#935). On the way, remove an obsolete "-I" parameter.
real-or-random
added a commit
to real-or-random/secp256k1
that referenced
this pull request
Dec 3, 2021
This was necessary because we used to cross-compile the library but compile the precomputation programs for the build host. Now it's no longer necessary and we can cleanly link even the external ASM (which was the intent of bitcoin-core#935). On the way, remove an obsolete "-I" parameter.
real-or-random
added a commit
to real-or-random/secp256k1
that referenced
this pull request
Dec 5, 2021
This was necessary because we used to cross-compile the library but compile the precomputation programs for the build host. Now it's no longer necessary and we can cleanly link even the external ASM (which was the intent of bitcoin-core#935). On the way, remove an obsolete "-I" parameter.
real-or-random
added a commit
to real-or-random/secp256k1
that referenced
this pull request
Dec 9, 2021
This was necessary because we used to cross-compile the library but compile the precomputation programs for the build host. Now it's no longer necessary and we can cleanly link even the external ASM (which was the intent of bitcoin-core#935). On the way, remove an obsolete "-I" parameter.
dderjoel
pushed a commit
to dderjoel/secp256k1
that referenced
this pull request
May 23, 2023
This was necessary because we used to cross-compile the library but compile the precomputation programs for the build host. Now it's no longer necessary and we can cleanly link even the external ASM (which was the intent of bitcoin-core#935). On the way, remove an obsolete "-I" parameter.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These are two attempts to fix #931.
@gmaxwell Can you test if either if the first commits solves the issue?
(The third commit is just a minor cleanup thing that I discovered on the way).
If yes, I'd probably prefer the first commit, which is a real fix. But we could still apply the second commit if people think that's a good idea.