Skip to content

Whitelist cfg attrs and bump nightly version #2785

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 23, 2024

Conversation

apoelstra
Copy link
Member

rust-lang/rust#124800 has been fixed and we can update our nightly version by whitelisting all cfgs that are used.

There was one place where we had an old cfg(feature = "no-std") despite having removed the feature. By removing that cfg check we re-enabled a previously disabled test.

@github-actions github-actions bot added C-bitcoin PRs modifying the bitcoin crate C-hashes PRs modifying the hashes crate C-units PRs modifying the units crate C-internals PRs modifying the internals crate C-io PRs modifying the io crate test C-base58 labels May 21, 2024
@coveralls
Copy link

coveralls commented May 21, 2024

Pull Request Test Coverage Report for Build 9192394789

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 83.323%

Totals Coverage Status
Change from base Build 9175347631: 0.0%
Covered Lines: 19311
Relevant Lines: 23176

💛 - Coveralls

@apoelstra apoelstra force-pushed the 2024-05--cfg-attrs branch from e3b7833 to 9e7ac4e Compare May 21, 2024 17:18
@@ -79,3 +79,6 @@ required-features = ["std", "rand-std", "bitcoinconsensus"]

[[example]]
name = "sighash"

[lints.rust]
unexpected_cfgs = { level = "deny", check-cfg = ['cfg(bench)', 'cfg(fuzzing)', 'cfg(kani)', 'cfg(mutate)', 'cfg(rust_v_1_60)'] }
Copy link

Choose a reason for hiding this comment

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

rust_v_1_60

Shouldn't this be set in your build.rs where the cfg is defined?

Copy link
Member Author

Choose a reason for hiding this comment

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

@epage not a bad idea -- but I kinda like having everything defined here because I can easily pull stuff out of TOML files, and not-so-easily pull stuff out of .rs files, in case I want to generate a summary of all the cfg options or do some CI sanity-checks on them or something.

units/Cargo.toml Outdated
@@ -29,3 +29,6 @@ serde = { version = "1.0.103", default-features = false, features = ["derive"],
[dev-dependencies]
serde_test = "1.0"
serde_json = "1.0"

[lints.rust]
unexpected_cfgs = { level = "warn", check-cfg = ['cfg(kani)'] }
Copy link
Member

Choose a reason for hiding this comment

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

Just out of interest, why is this level = "warn" and the others are level = "deny"?

Copy link
Member Author

Choose a reason for hiding this comment

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

They should all be deny. I think I just messed one up.

Copy link
Member

Choose a reason for hiding this comment

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

Sweet, review for the win.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

@apoelstra apoelstra force-pushed the 2024-05--cfg-attrs branch from 9e7ac4e to 30a4825 Compare May 22, 2024 13:37
Copy link
Member

@tcharding tcharding left a comment

Choose a reason for hiding this comment

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

ACK 30a4825

@apoelstra apoelstra merged commit 1142d16 into rust-bitcoin:master May 23, 2024
23 checks passed
apoelstra added a commit to apoelstra/rust-bitcoin that referenced this pull request May 24, 2024
In rust-bitcoin#2785 I modified fuzz/Cargo.toml without updating the Cargo.toml
generating script. Oops. Fix that.
tcharding added a commit to tcharding/rust-jsonrpc that referenced this pull request May 31, 2024
As we did in rust-bitcoin/rust-bitcoin#2785.

rust-lang/rust#124800 has been fixed and we can update our nightly
version by whitelisting all cfgs that are used.
apoelstra added a commit that referenced this pull request Oct 15, 2024
d99b256 Remove explicit borrow (Tobin C. Harding)
7965e72 Fix rustdoc lint warnings (Tobin C. Harding)
e75e7f4 Remove deprecated legacy numeric methods (Divyansh Gupta)
87e353b Update nightly version (Tobin C. Harding)
753972d cargo: whitelist all cfgs used in this repo (Andrew Poelstra)
62eca12 crypto: enable and fix accidentally disabled unit test (Andrew Poelstra)

Pull request description:

  Backport a bunch of patches to get the clippy job passing.

  - the first two patches from #2785
  - Update `nightly-toolchain`
  - the single patch from #2667

  After rebase I just manually fixed all remaining lint warnings/errors

ACKs for top commit:
  apoelstra:
    ACK d99b256 successfully ran local tests; nice! especially updating the rust-nightly version, which probably was not strictly necessary. will one-ack merge

Tree-SHA512: 895807dc9402f88f7d2207799c50ab96841eaeb6ee1aaf8cd1a735365cc9728025c80c8c9fae0d28d4c8de6d3685121b162225a63ea64e72b749d9960a28f10e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-base58 C-bitcoin PRs modifying the bitcoin crate C-hashes PRs modifying the hashes crate C-internals PRs modifying the internals crate C-io PRs modifying the io crate C-units PRs modifying the units crate test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants