Update vendored lib secp256k1 to v0.4.0 #653
Conversation
|
Hmm, not sure what to make of this CI failure. Will try kicking it tomorrow. |
|
ACK 102df63. I have a small preference that the last two commits be squashed (so that each one can be tested using the committed libsecp and the committed patchfiles). Will ACK for real once we sort out CI. |
|
Regarding the MIPS CI, I think it's the same as rust-lang/rust#115218. |
Davidson-Souza
force-pushed
the
update-libsecp
branch
from
102df63
to
dccaf41
Compare
|
Pushed dccaf41 I've fixed windows cross compiling error and squashed the last two commits. MIPS* will likely only be solved by removing MIPS at all, because it is now tier 3. Or making another job that builds with |
|
Ok, concept ACK from me just removing MIPs then from our tests :/. |
|
In dccaf41: You have not updated the version in secp256k1-sys/Cargo.toml to match the version put in the vendored symbols. |
Davidson-Souza
force-pushed
the
update-libsecp
branch
from
dccaf41
to
f622c65
Compare
|
CI is working now.
Wait, ac6d106 bumps secp256k1-sys to |
|
Ah, yes, I was reading my diff backward. Yes, please squash them -- that way when I run the revendoring script on each commit, I get a clean diff. |
|
I'm confused; how did you fix the linking errors I was getting if there are no additional changes other than the MIPs stuff? |
Davidson-Souza
force-pushed
the
update-libsecp
branch
from
f622c65
to
3f5b6d3
Compare
I think it's everything ok now.
I've updated some patch files, but I squashed it as suggested here |
|
I reviewed all the changes to files outside of |
|
In 3f5b6d3 the patchfiles are still updated in their own commit, meaning that the last several commits do not pass tests for me, since the vendored code does not match the output of the vendoring script. Can you squash the patchfile commit into the one that changes the vendored files? |
|
I think they are all in the same commit now. Sorry for the delay, I was traveling back from Azores. |
|
CI looks stuck. |
|
Restarted the jobs. Maybe a Github UI issue. |
|
@Davidson-Souza 1639f25 is still its own commit. |
Davidson-Souza
force-pushed
the
update-libsecp
branch
from
3f5b6d3
to
be48b75
Compare
|
Oh, sorry! I've pushed the wrong branch. Now it's squashed. |
|
I'm sorry to keep doing this to you, but could you
The former I could probably just patch my local checks to be more forgiving, but the latter is needed before I can merge this. |
Davidson-Souza
force-pushed
the
update-libsecp
branch
2 times, most recently
from
59ec754
to
ce1731a
Compare
upstream libsecp now has a CMakeLists.txt file. Many years ago we added some things to .gitignore which appear to be local developers committing the names of their own stray files, and now this is causing the revendoring script to lose track of vendored files.
This is just a bad test. It constructs a preallocated context object by starting from a non-preallocated context object, in a way that can't be done by users (since it directly constructs a `Secp256k1` struct) and a way that is very difficult to unwind, because you wind up with two pointers to the same underlying context object, one a "preallocated" one and one a normal one. If you then drop the preallocated one, it will call `secp256k1_context_destroy`, forcing you to manually deallocate the other one. If you drop the normally-allocated one, you need to mem::forget the preallocated one to avoid calling `secp256k1_context_destroy` twice. The whole thing is pretty fragile. There is another unit test, `test_raw_ctx`, which gets into the same situation but using the public API, and demonstrates a few ways to get out of it.
MIPS was recently downgraded to Tier 3, which means it won't be installable by rustup and may not work as expected. This commit removes all MIPS-related CI jobs.
Davidson-Souza
force-pushed
the
update-libsecp
branch
from
ce1731a
to
80b2a8d
Compare
Done! |
|
Ok, great :) I am still getting errors related to .orig files but I'll just patch my script to ignore those. I think we can finally merge this! Thanks so much for keeping with this despite so many iterations that were purely about making our CI machine happy. (Though FWIW we do get a lot of value in being able to machine-check that the vendored code is consistent.) |
Replaces #645 and #652. Precedes #627.
I'm basically using #652 but resolving the linking problems,
My local CI is erring on windows cross-test, but I can compile without issue with
cargo build --target x86_64-pc-windows-gnu. Some MIPS jobs failed before even installing cross, I think those aren't really related to this PR. Any ideas on what can be happening?