Skip to content

Conversation

@Kixunil
Copy link
Collaborator

@Kixunil Kixunil commented Jul 3, 2024

Updated deprecated item and fixed cfg lints.

Kixunil added 4 commits July 3, 2024 05:46
This is a legacy constant and it's better to just use `i32::MAX`. Note
that one cannot `use` an associated constant so this just removed the
import. This is better anyway since it's only used once and it didn't
provide meaningful line length reduction.
Rust is now checking cfg attributes for typos but this interferes with
our cfgs that rustc/cargo don't recognize. This whitelists them so they
no longer produce warnings.
The newest nightly stabilized `PanicMessage` with a slightly different
API. This updates the API and removes the `#![feature()]` attribute.
Despite using the `#![feature()]` attribute rustc still warns about it
being unstable. Changing it to `libc::abort` gets rid of the annoying
message.
@apoelstra
Copy link
Member

CI failure looks like rust-lang/cc-rs#852 which we've fixed elsewhere by pinning cc to 1.0.79. I'm not sure why cc is not pinned in this part of CI.

@apoelstra
Copy link
Member

Ah, when we compile the no_std_test using cargo run --release --manifest-path=./no_std_test/Cargo.toml cargo creates a new lockfile independent of our pinned one.

Maybe the simplest thing is to commit no_std_test/Cargo.lock. Alternately we can add a cargo update -p cc --precise 1.0.79 --manifest-path=./no_std_test/Cargo.toml line to contrib/_test.sh which might be less fragile.

@Kixunil
Copy link
Collaborator Author

Kixunil commented Jul 4, 2024

They also suggest we use resolver v2. I believe that's a more principled approach and given we've bumped MSRV anyway we should use it.

@Kixunil
Copy link
Collaborator Author

Kixunil commented Jul 4, 2024

It looks like that's not it (though updating edition doesn't hurt).

Previously we had dependency problems that were resolved by resolver v2.
We want to activate it just in case it happens again but even better,
bump the edition.  This was probably forgotten when other crates were
migrated.
@Kixunil
Copy link
Collaborator Author

Kixunil commented Jul 4, 2024

After trying to enable unwind I got his message:

note: since the core library is usually precompiled with panic="unwind", rebuilding your crate with panic="abort" may not be enough to fix the problem

And indeed, it's referenced in core. I guess we need to recompile it.

The `no_std` test disables `std`, so unwinding is unsupported, so we use
`panic = "abort"` but the `core` library is compiled with unwind by
default which breaks the build. Xargo can handle this by recompiling
`core` with `panic = "abort"` so we use it.
This can help debug CI issues.
@Kixunil Kixunil force-pushed the fix-ci branch 3 times, most recently from f20c127 to b0fa740 Compare July 4, 2024 08:32
@Kixunil
Copy link
Collaborator Author

Kixunil commented Jul 4, 2024

Since downgrading isn't helping, I think the windows issue is caused by something using HashMap/HashSet causing it to link to bcryptprimitives to get RNG and the library is only available in wine 9.0+.

This can be solved by using a custom dockerfile which I'm not keen on doing.

See cross-rs/cross#1522

Cross uses an old image by default and there's a problem that is
resolved in the newest wine version, so this commit upgrades the
image.
@apoelstra
Copy link
Member

Thanks for digging into this! I had my dates confused and thought that our MSRV was at the beginning of 2018, not the beginning of 2021. But apparently three extra years have happened..

@apoelstra
Copy link
Member

I'll go ahead and ACK/merge this since it gets CI passing and is easy to whitelist on my end, but I see one more clippy nit:

error: doc list item missing indentation
   --> src/context.rs:252:13
    |
252 |         /// for `Secp256k1::gen_new()`).
    |             ^
    |
    = help: if this is supposed to be its own paragraph, add a blank line
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#doc_lazy_cont>
    = note: `-D clippy::doc-lazy-continuation` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::doc_lazy_continuation)]`
help: indent this line
    |
252 |         ///   for `Secp256k1::gen_new()`).
    |             ++

which we can address in another PR.

Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK 33a1893

@apoelstra apoelstra merged commit 30dda2c into rust-bitcoin:master Jul 5, 2024
@Kixunil Kixunil deleted the fix-ci branch July 5, 2024 13:43
chain-forgexcr45 added a commit to chain-forgexcr45/rust-secp256k1 that referenced this pull request Sep 28, 2025
33a1893c14f242ba0653bbb26a9dad6f83ea81c2 Upgrade cross image for windows (Martin Habovstiak)
24e81eeadb15b448657dd1b6a0c5e1d622acc306 Run cross with --verbose flag (Martin Habovstiak)
742c69f9757745e7e2ba8343f10bc79c5e788a78 Compile `no_std` test using xargo (Martin Habovstiak)
2572fb6ab0c9323d98b58c57c0ec9634f03caefa Migrate `no_std_test` to edition 2021 (Martin Habovstiak)
df0523a0a72c7e3a59abf48a097de6a8c1f840c4 Use `libc::abort` instead of `intrinsics::abort` (Martin Habovstiak)
924ba381c8541805fd1bf029213f6a1776fadb3b Update panic message handling (Martin Habovstiak)
614fe81708b86d8119de4706f379d6651f67a8c6 Whitelist known cfgs (Martin Habovstiak)
05a4e3963cbd41ef967796d2216fcdf6705affd7 Don't use `core::i32::MAX` (Martin Habovstiak)

Pull request description:

  Updated deprecated item and fixed cfg lints.

ACKs for top commit:
  apoelstra:
    ACK 33a1893c14f242ba0653bbb26a9dad6f83ea81c2

Tree-SHA512: 8b66f1f404d44916b2a18dbbe829b31ec1915d3fd084164127aa6e5f98ee5de3ea988f5b1ed05e9532c026890a769b4c54e175508fe472beaea5898a477d5c76
william2332-limf added a commit to william2332-limf/rust-secp256k1 that referenced this pull request Oct 2, 2025
33a1893c14f242ba0653bbb26a9dad6f83ea81c2 Upgrade cross image for windows (Martin Habovstiak)
24e81eeadb15b448657dd1b6a0c5e1d622acc306 Run cross with --verbose flag (Martin Habovstiak)
742c69f9757745e7e2ba8343f10bc79c5e788a78 Compile `no_std` test using xargo (Martin Habovstiak)
2572fb6ab0c9323d98b58c57c0ec9634f03caefa Migrate `no_std_test` to edition 2021 (Martin Habovstiak)
df0523a0a72c7e3a59abf48a097de6a8c1f840c4 Use `libc::abort` instead of `intrinsics::abort` (Martin Habovstiak)
924ba381c8541805fd1bf029213f6a1776fadb3b Update panic message handling (Martin Habovstiak)
614fe81708b86d8119de4706f379d6651f67a8c6 Whitelist known cfgs (Martin Habovstiak)
05a4e3963cbd41ef967796d2216fcdf6705affd7 Don't use `core::i32::MAX` (Martin Habovstiak)

Pull request description:

  Updated deprecated item and fixed cfg lints.

ACKs for top commit:
  apoelstra:
    ACK 33a1893c14f242ba0653bbb26a9dad6f83ea81c2

Tree-SHA512: 8b66f1f404d44916b2a18dbbe829b31ec1915d3fd084164127aa6e5f98ee5de3ea988f5b1ed05e9532c026890a769b4c54e175508fe472beaea5898a477d5c76
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.

2 participants