Skip to content
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

libp2p_swarm_derive: Add crate parameter to configure libp2p crate name #3006

Closed
wants to merge 2 commits into from
Closed

libp2p_swarm_derive: Add crate parameter to configure libp2p crate name #3006

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
11d8ee8
Allow crate parameter in NetworkBehaviour derive macro
stormshield-pj50 Oct 10, 2022
889073d
Merge branch 'master' into derive_libp2p_prefix
mxinden Oct 14, 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
54 changes: 38 additions & 16 deletions swarm-derive/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,21 +46,43 @@ fn build(ast: &DeriveInput) -> TokenStream {
fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream {
let name = &ast.ident;
let (_, ty_generics, where_clause) = ast.generics.split_for_impl();
let multiaddr = quote! {::libp2p::core::Multiaddr};
let trait_to_impl = quote! {::libp2p::swarm::NetworkBehaviour};
let either_ident = quote! {::libp2p::core::either::EitherOutput};
let network_behaviour_action = quote! {::libp2p::swarm::NetworkBehaviourAction};
let into_connection_handler = quote! {::libp2p::swarm::IntoConnectionHandler};
let connection_handler = quote! {::libp2p::swarm::ConnectionHandler};
let into_proto_select_ident = quote! {::libp2p::swarm::IntoConnectionHandlerSelect};
let peer_id = quote! {::libp2p::core::PeerId};
let connection_id = quote! {::libp2p::core::connection::ConnectionId};
let dial_errors = quote! {Option<&Vec<::libp2p::core::Multiaddr>>};
let connected_point = quote! {::libp2p::core::ConnectedPoint};
let listener_id = quote! {::libp2p::core::transport::ListenerId};
let dial_error = quote! {::libp2p::swarm::DialError};

let poll_parameters = quote! {::libp2p::swarm::PollParameters};

let user_provided_libp2p_prefix = ast
.attrs
.iter()
.filter_map(get_meta_items)
.flatten()
.filter_map(|meta_item| {
if let syn::NestedMeta::Meta(syn::Meta::NameValue(ref m)) = meta_item {
if m.path.is_ident("crate") {
if let syn::Lit::Str(ref s) = m.lit {
return Some(syn::parse_str::<syn::TypePath>(&s.value()).unwrap());
}
}
}
None
})
.next();
Comment on lines +50 to +65
Copy link
Member

Choose a reason for hiding this comment

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

This is mostly the same as the logic for out_event, right? What do you think of moving it into a function called in both places?


let libp2p_prefix = match user_provided_libp2p_prefix {
Some(pfx) => quote! {#pfx},
None => quote! {::libp2p},
};
let multiaddr = quote! {#libp2p_prefix::core::Multiaddr};
let trait_to_impl = quote! {#libp2p_prefix::swarm::NetworkBehaviour};
let either_ident = quote! {#libp2p_prefix::core::either::EitherOutput};
let network_behaviour_action = quote! {#libp2p_prefix::swarm::NetworkBehaviourAction};
let into_connection_handler = quote! {#libp2p_prefix::swarm::IntoConnectionHandler};
let connection_handler = quote! {#libp2p_prefix::swarm::ConnectionHandler};
let into_proto_select_ident = quote! {#libp2p_prefix::swarm::IntoConnectionHandlerSelect};
let peer_id = quote! {#libp2p_prefix::core::PeerId};
let connection_id = quote! {#libp2p_prefix::core::connection::ConnectionId};
let dial_errors = quote! {Option<&Vec<#libp2p_prefix::core::Multiaddr>>};
let connected_point = quote! {#libp2p_prefix::core::ConnectedPoint};
let listener_id = quote! {#libp2p_prefix::core::transport::ListenerId};
let dial_error = quote! {#libp2p_prefix::swarm::DialError};

let poll_parameters = quote! {#libp2p_prefix::swarm::PollParameters};

// Build the generics.
let impl_generics = {
Expand Down Expand Up @@ -624,7 +646,7 @@ fn build_struct(ast: &DeriveInput, data_struct: &DataStruct) -> TokenStream {
}

fn poll(&mut self, cx: &mut std::task::Context, poll_params: &mut impl #poll_parameters) -> std::task::Poll<#network_behaviour_action<Self::OutEvent, Self::ConnectionHandler>> {
use libp2p::futures::prelude::*;
use #libp2p_prefix::futures::prelude::*;
#(#poll_stmts)*
std::task::Poll::Pending
}
Expand Down
17 changes: 17 additions & 0 deletions swarm-derive/tests/test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,23 @@ fn custom_event() {
}
}

#[test]
fn custom_crate() {
use libp2p as mylibp2p;
#[allow(dead_code)]
#[derive(NetworkBehaviour)]
#[behaviour(crate = "mylibp2p")]
struct Foo {
ping: ping::Behaviour,
identify: identify::Behaviour,
}

#[allow(dead_code)]
fn foo() {
require_net_behaviour::<Foo>();
}
}
Comment on lines +136 to +151
Copy link
Contributor

Choose a reason for hiding this comment

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

This test also already passes with current master (if you remove line 141).
Correct me if I am wrong, but I believe the current ::libp2p prefix imports libp2p from the crate level, which means that aliases in a module should not be an issue? @mxinden?

I think it is only a problem if the crate is imported in the Cargo.toml with a different name.
serde on the other hand states for their crate attribute that:

This is normally only applicable when invoking re-exported Serde derives from a public macro in a different crate.

I am not against adding this parameter, but could we add a test that fails on current master and would be fixed with this? 🙂

Copy link
Contributor

Choose a reason for hiding this comment

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

As far as I understand, the issue is not clashing symbols but renaming the crate in your manifest.

To test that, we'd have to make a separate test crate with a renamed import.

This make me wonder, should we perhaps just require certain symbols to be present for the macro to work?

That would make #3023 easier to implement.

Or perhaps we could make a seperate "derive" prelude? Like libp2p::swarm_derive_prelude that includes all the required symbols. Then we can add a configuration knob here that allows you to change where that prelude is coming from.

Copy link
Contributor

Choose a reason for hiding this comment

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

This make me wonder, should we perhaps just require certain symbols to be present for the macro to work?

That would make #3023 easier to implement.

Or perhaps we could make a seperate "derive" prelude? Like libp2p::swarm_derive_prelude that includes all the required symbols. Then we can add a configuration knob here that allows you to change where that prelude is coming from.

Actually, don't worry about it, I already have a solution to that.


#[test]
fn custom_event_mismatching_field_names() {
#[allow(dead_code)]
Expand Down