Skip to content

Fix background-processor doc build for futures #1744

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.

Sign up for GitHub

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 2 commits into from
Oct 10, 2022
Merged

Fix background-processor doc build for futures #1744

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d3c369d
Fix `background-processor` doc build for `futures`
tnull Sep 26, 2022
0038a3f
Add `futures` CI check
tnull Oct 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 29 additions & 8 deletions .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,42 +12,51 @@ jobs:
beta,
# 1.41.1 is MSRV for Rust-Lightning, lightning-invoice, and lightning-persister
1.41.1,
# 1.45.2 is MSRV for lightning-net-tokio, lightning-block-sync, and coverage generation
# 1.45.2 is MSRV for lightning-net-tokio, lightning-block-sync, lightning-background-processor, and coverage generation
1.45.2,
# 1.47.0 will be the MSRV for no-std builds using hashbrown once core2 is updated
1.47.0]
include:
- toolchain: stable
build-net-tokio: true
build-no-std: true
build-futures: true
- toolchain: stable
platform: macos-latest
build-net-tokio: true
build-no-std: true
build-futures: true
- toolchain: beta
platform: macos-latest
build-net-tokio: true
build-no-std: true
build-futures: true
- toolchain: stable
platform: windows-latest
build-net-tokio: true
build-no-std: true
build-futures: true
- toolchain: beta
platform: windows-latest
build-net-tokio: true
build-no-std: true
build-futures: true
- toolchain: beta
build-net-tokio: true
build-no-std: true
build-futures: true
- toolchain: 1.41.1
build-no-std: false
test-log-variants: true
build-futures: false
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not test on MSRV? In fact, why not test on all builds?

Copy link
Contributor Author

@tnull tnull Sep 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Ah, I wasn't sure whether we wanted to give it that much resources. Now building on all platforms/toolchains with 1a55192.

Copy link
Contributor Author

@tnull tnull Sep 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Yikes, this just got a bit more relevant, since it fails to build under MSRV. To quote the futures repo README: "The current futures requires Rust 1.45 or later."

So 0.0.111 had an effective MSRV of 1.45 it seems ...

Copy link
Contributor Author

@tnull tnull Sep 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Now 'solved' by pinning the futures crates' versions to the last one with MSRV 1.41 (c367f71). Let me know if that's acceptable, or if we're okay with an MSRV of 1.45.2 for the lightning-background-processor crate.

I'm however unsure why macos-latest and windows-latest stable targets are hanging with "Waiting for status to be reported". Any ideas?

- toolchain: 1.45.2
build-net-old-tokio: true
build-net-tokio: true
build-no-std: false
build-futures: true
coverage: true
- toolchain: 1.47.0
build-futures: true
build-no-std: true
runs-on: ${{ matrix.platform }}
steps:
Expand Down Expand Up @@ -109,7 +118,7 @@ jobs:
- name: Test on Rust ${{ matrix.toolchain }} with net-tokio and full code-linking for coverage generation
if: matrix.coverage
run: RUSTFLAGS="-C link-dead-code" cargo test --verbose --color always
- name: Test on no-std bullds Rust ${{ matrix.toolchain }}
- name: Test no-std builds on Rust ${{ matrix.toolchain }}
if: "matrix.build-no-std && !matrix.coverage"
shell: bash # Default on Winblows is powershell
run: |
Expand Down Expand Up @@ -140,15 +149,26 @@ jobs:
run: |
cd lightning
RUSTFLAGS="-C link-dead-code" cargo test --verbose --color always --no-default-features --features no-std
cd ..
- name: Test futures builds on Rust ${{ matrix.toolchain }}
if: "matrix.build-futures && !matrix.coverage"
shell: bash # Default on Winblows is powershell
run: |
cd lightning-background-processor
cargo test --verbose --color always --no-default-features --features futures
- name: Test futures builds on Rust ${{ matrix.toolchain }} and full code-linking for coverage generation
if: "matrix.build-futures && matrix.coverage"
shell: bash # Default on Winblows is powershell
run: |
cd lightning-background-processor
RUSTFLAGS="-C link-dead-code" cargo test --verbose --color always --no-default-features --features futures
- name: Test on Rust ${{ matrix.toolchain }}
if: "! matrix.build-net-tokio"
run: |
cargo test --verbose --color always -p lightning
cargo test --verbose --color always -p lightning-invoice
cargo test --verbose --color always -p lightning-rapid-gossip-sync
cargo build --verbose --color always -p lightning-persister
cargo build --verbose --color always -p lightning-background-processor
cargo test --verbose --color always -p lightning
cargo test --verbose --color always -p lightning-invoice
cargo test --verbose --color always -p lightning-rapid-gossip-sync
cargo test --verbose --color always -p lightning-persister
cargo test --verbose --color always -p lightning-background-processor
- name: Test C Bindings Modifications on Rust ${{ matrix.toolchain }}
if: "! matrix.build-net-tokio"
run: |
Expand Down Expand Up @@ -299,6 +319,7 @@ jobs:
run: |
cargo check --release
cargo check --no-default-features --features=no-std --release
cargo check --no-default-features --features=futures --release
cargo doc --release

fuzz:
Expand Down
5 changes: 4 additions & 1 deletion lightning-background-processor/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,14 @@ edition = "2018"
all-features = true
rustdoc-args = ["--cfg", "docsrs"]

[features]
futures = [ "futures-util" ]

[dependencies]
bitcoin = "0.29.0"
lightning = { version = "0.0.111", path = "../lightning", features = ["std"] }
lightning-rapid-gossip-sync = { version = "0.0.111", path = "../lightning-rapid-gossip-sync" }
futures = { version = "0.3", optional = true }
futures-util = { version = "0.3", default-features = false, features = ["async-await-macro"], optional = true }

[dev-dependencies]
lightning = { version = "0.0.111", path = "../lightning", features = ["_test_utils"] }
Expand Down
8 changes: 5 additions & 3 deletions lightning-background-processor/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ use std::time::{Duration, Instant};
use std::ops::Deref;

#[cfg(feature = "futures")]
use futures::{select, future::FutureExt};
use futures_util::{select_biased, future::FutureExt};

/// `BackgroundProcessor` takes care of tasks that (1) need to happen periodically to keep
/// Rust-Lightning running properly, and (2) either can or should be run in the background. Its
Expand Down Expand Up @@ -378,14 +378,15 @@ pub async fn process_events_async<
Descriptor: 'static + SocketDescriptor + Send + Sync,
CMH: 'static + Deref + Send + Sync,
RMH: 'static + Deref + Send + Sync,
OMH: 'static + Deref + Send + Sync,
EH: 'static + EventHandler + Send,
PS: 'static + Deref + Send,
M: 'static + Deref<Target = ChainMonitor<Signer, CF, T, F, L, P>> + Send + Sync,
CM: 'static + Deref<Target = ChannelManager<Signer, CW, T, K, F, L>> + Send + Sync,
PGS: 'static + Deref<Target = P2PGossipSync<G, CA, L>> + Send + Sync,
RGS: 'static + Deref<Target = RapidGossipSync<G, L>> + Send,
UMH: 'static + Deref + Send + Sync,
PM: 'static + Deref<Target = PeerManager<Descriptor, CMH, RMH, L, UMH>> + Send + Sync,
PM: 'static + Deref<Target = PeerManager<Descriptor, CMH, RMH, OMH, L, UMH>> + Send + Sync,
S: 'static + Deref<Target = SC> + Send + Sync,
SC: WriteableScore<'a>,
SleepFuture: core::future::Future<Output = bool>,
Expand All @@ -405,14 +406,15 @@ where
L::Target: 'static + Logger,
P::Target: 'static + Persist<Signer>,
CMH::Target: 'static + ChannelMessageHandler,
OMH::Target: 'static + OnionMessageHandler,
RMH::Target: 'static + RoutingMessageHandler,
UMH::Target: 'static + CustomMessageHandler,
PS::Target: 'static + Persister<'a, Signer, CW, T, K, F, L, SC>,
{
let mut should_continue = true;
define_run_body!(persister, event_handler, chain_monitor, channel_manager,
gossip_sync, peer_manager, logger, scorer, should_continue, {
select! {
select_biased! {
Comment on lines -415 to +417
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be good to have the reason for this change in the commit message or in a separate commit if unrelated.

Copy link
Contributor Author

@tnull tnull Oct 10, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Added more context in commit message with 0038a3f. Let me know if it is sufficient.

_ = channel_manager.get_persistable_update_future().fuse() => true,
cont = sleeper(Duration::from_millis(100)).fuse() => {
should_continue = cont;
Expand Down
2 changes: 1 addition & 1 deletion lightning-block-sync/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ rpc-client = [ "serde", "serde_json", "chunked_transfer" ]
[dependencies]
bitcoin = "0.29.0"
lightning = { version = "0.0.111", path = "../lightning" }
futures = { version = "0.3" }
futures-util = { version = "0.3" }
tokio = { version = "1.0", features = [ "io-util", "net", "time" ], optional = true }
serde = { version = "1.0", features = ["derive"], optional = true }
serde_json = { version = "1.0", optional = true }
Expand Down
2 changes: 1 addition & 1 deletion lightning-block-sync/src/rest.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::http::{BinaryResponse, HttpEndpoint, HttpClient, JsonResponse};
use bitcoin::hash_types::BlockHash;
use bitcoin::hashes::hex::ToHex;

use futures::lock::Mutex;
use futures_util::lock::Mutex;

use std::convert::TryFrom;
use std::convert::TryInto;
Expand Down
2 changes: 1 addition & 1 deletion lightning-block-sync/src/rpc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::http::{HttpClient, HttpEndpoint, HttpError, JsonResponse};
use bitcoin::hash_types::BlockHash;
use bitcoin::hashes::hex::ToHex;

use futures::lock::Mutex;
use futures_util::lock::Mutex;

use serde_json;

Expand Down