ci: automated update to rustc 1.83.0 by create-pr-actions[bot] · Pull Request #1776 · bitcoindevkit/bdk

create-pr-actions · 2024-12-15T01:15:06Z

Automated update to Github CI workflow cont_integration.yml by create-pull-request GitHub action

I manually fixed new clippy warnings.

I also changed CI to:
~~1. not build or run tests or test dependencies when building with MSRV. This reduced the number of dependencies we need to pin for MSRV and fixes 1398.~~ I removed these changes.
2. use the same version or rust as in the rust-version file for all jobs instead of just for clippy. I made this change to prevent CI breaks when stable changes before we have a chance to fix the auto created PR that bumps the rust-version file.

https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes rust-lang/rust-clippy#13286

…st 1.83 https://rust-lang.github.io/rust-clippy/master/index.html#empty_line_after_doc_comments rust-lang/rust-clippy#13091

https://rust-lang.github.io/rust-clippy/master/index.html#ptr_arg rust-lang/rust-clippy#13313

notmandatory · 2024-12-16T03:15:48Z

This doesn't need to go into the final 1.0.0, but if folks have time to review it I'd like to get it in so:

downstream projects using 1.83 don't get the new clippy warnings
~~2. CI will build with MSRV without so many pinned dependencies~~ I removed these changes.

oleonardolima

tACK 74a7f73

oleonardolima · 2024-12-16T03:48:22Z

crates/chain/Cargo.toml

 proptest = "1.2.0"
 bdk_testenv = { path = "../testenv", default-features = false }
-criterion = { version = "0.2" }
+criterion = { version = "0.5" }


I'm not aware of the reason it has been downgraded to 0.2 by #1670, but I guess there might be some #:thinking:

I manually ran the bench tests with criterion 0.5 and they still build and run so I think it's ok. @ValuedMammal do you know why we were using such an old version?

AFAICT on VM's initial PR it was using 0.5 🤔.

do you know why we were using such an old version?

I think it was only to get CI to pass using MSRV. I guess the issue didn't surface on #1735 because running the benches required a special invocation and so criterion was not automatically linked in with the build deps.

oleonardolima · 2024-12-16T03:51:06Z

.github/workflows/cont_integration.yml

+          cargo update -p indexmap --precise "2.5.0"
      - name: Build
-        run: cargo build --workspace --exclude 'example_*' ${{ matrix.features }}
+        run: cargo build --workspace --exclude 'example_*' --exclude 'bdk_testenv' ${{ matrix.features }}


I'm divided on this one, it surely helps not pinning on CI, but I'm unsure if we should test or not on MSRV toolchain 🤔.

Since we are keeping such an old MSRV and our test deps change so often I think this is the only way to manage it. But I'm open to move this to a different PR for after 1.0 if this is going to need more discussion.

oleonardolima · 2024-12-16T20:25:12Z

@notmandatory It's probably needed to update the docs nightly toolchain too, see https://github.com/bitcoindevkit/bdk/actions/runs/12360406444/job/34495287138#step:7:307

nymius

I've run the workflows with act tool locally, but the output is not clear.
Though the CI is green, and have reviewed the output there.
If there are better ways to check this, please let me know.

Besides that, I have to research more about best practices on MSRV, as it seems to be another deep rabbit hole. The best material I have found so far is this discussion in the api guidelines repository of rust-lang. The stance adopted there seems to be in favor of pinning dependencies but in an alternative Cargo.lock.min file, which I think is not very different from what what was done here before, and I think is a path we should consider, probably against the original motivation of this PR.
However, I think these changes doesn't go in the opposite direction the link I mentioned above goes, and considering the project is still checking it builds against MSRV, my only concern here is the lack of testing this introduces for MSRV, so I wouldn't remove them unless there are strong reasons to remove the pinning and remove the testing in the way.

notmandatory · 2024-12-17T02:15:08Z

@notmandatory It's probably needed to update the docs nightly toolchain too, see https://github.com/bitcoindevkit/bdk/actions/runs/12360406444/job/34495287138#step:7:307

Good call. I updated the docs job to use rust nightly since the #[doc(cfg(…))] attribute (feature: doc_cfg) still isn't stabilized, see tracking issue: rust-lang/rust#43781.

notmandatory · 2024-12-17T02:51:52Z

Besides that, I have to research more about best practices on MSRV, as it seems to be another deep rabbit hole.
...

I removed the commits that changed our MSRV testing and dependencies. I agree this is a rabbit hole that deserves more work to get right.

notmandatory · 2024-12-17T02:53:48Z

I've run the workflows with act tool locally, but the output is not clear. Though the CI is green, and have reviewed the output there. If there are better ways to check this, please let me know.

I haven't tried the act tool, to test CI stuff I either push the changes in my PR or if it's someone else's PR and I want to try something different I push to my own bdk fork and let the Github CI run there.

oleonardolima

tACK 272ce22

nymius

tACK 272ce22

ValuedMammal

ACK 272ce22

ci: automated update to rustc 1.83.0

2586e1e

notmandatory added the ci label Dec 16, 2024

notmandatory added this to the 1.0.0-beta milestone Dec 16, 2024

notmandatory self-assigned this Dec 16, 2024

notmandatory added 3 commits December 15, 2024 19:53

ci(clippy): fix new stricter needless_lifetime errors for rust 1.83

941bca0

https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes rust-lang/rust-clippy#13286

ci(clippy): fix new default warn empty_line_after_doc_comments for ru…

1d03db9

…st 1.83 https://rust-lang.github.io/rust-clippy/master/index.html#empty_line_after_doc_comments rust-lang/rust-clippy#13091

ci(clippy): fix updated ptr_arg check for rust 1.83

173cc42

https://rust-lang.github.io/rust-clippy/master/index.html#ptr_arg rust-lang/rust-clippy#13313

notmandatory force-pushed the create-pull-request/update-rust-version branch from 42f270a to ed88439 Compare December 16, 2024 01:54

notmandatory mentioned this pull request Dec 16, 2024

Stop pinning everything in CI bitcoindevkit/bdk_wallet#101

Open

notmandatory force-pushed the create-pull-request/update-rust-version branch 2 times, most recently from aa83307 to 74a7f73 Compare December 16, 2024 03:54

oleonardolima reviewed Dec 16, 2024

View reviewed changes

notmandatory mentioned this pull request Dec 16, 2024

RUSTSEC-2024-0375: atty is unmaintained #1769

Closed

nymius reviewed Dec 16, 2024

View reviewed changes

notmandatory force-pushed the create-pull-request/update-rust-version branch from d4dc092 to b287325 Compare December 17, 2024 02:20

notmandatory added 2 commits December 16, 2024 20:40

ci: use prepared rust-version instead of stable for all jobs

1755480

ci: update build_docs job to use rust nightly

272ce22

notmandatory force-pushed the create-pull-request/update-rust-version branch from b287325 to 272ce22 Compare December 17, 2024 02:46

This was referenced Dec 17, 2024

Make clippy happy #1780

Closed

ci: Fix build_docs job #1781

Closed

oleonardolima reviewed Dec 17, 2024

View reviewed changes

nymius approved these changes Dec 17, 2024

View reviewed changes

ValuedMammal approved these changes Dec 17, 2024

View reviewed changes

notmandatory merged commit 1a6a78a into master Dec 17, 2024

notmandatory mentioned this pull request Dec 17, 2024

transactions method should only return relevant transactions #1779

Merged

3 tasks

notmandatory mentioned this pull request Dec 19, 2024

Release wallet-1.0.0 FINAL #1783

Closed

40 tasks

Conversation

create-pr-actions bot commented Dec 15, 2024 • edited by notmandatory Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

notmandatory commented Dec 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oleonardolima left a comment

Choose a reason for hiding this comment

Uh oh!

oleonardolima Dec 16, 2024

Choose a reason for hiding this comment

Uh oh!

notmandatory Dec 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oleonardolima Dec 16, 2024

Choose a reason for hiding this comment

Uh oh!

ValuedMammal Dec 16, 2024

Choose a reason for hiding this comment

Uh oh!

oleonardolima Dec 16, 2024

Choose a reason for hiding this comment

Uh oh!

notmandatory Dec 16, 2024

Choose a reason for hiding this comment

Uh oh!

oleonardolima commented Dec 16, 2024

Uh oh!

nymius left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

notmandatory commented Dec 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

notmandatory commented Dec 17, 2024

Uh oh!

notmandatory commented Dec 17, 2024

Uh oh!

oleonardolima left a comment

Choose a reason for hiding this comment

Uh oh!

nymius left a comment

Choose a reason for hiding this comment

Uh oh!

ValuedMammal left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

create-pr-actions bot commented Dec 15, 2024 •

edited by notmandatory

Loading

notmandatory commented Dec 16, 2024 •

edited

Loading

notmandatory Dec 16, 2024 •

edited

Loading

nymius left a comment •

edited

Loading

notmandatory commented Dec 17, 2024 •

edited

Loading