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.

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

Fixing dependency leaks #155

Merged
merged 3 commits into from
May 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Changed

- Code generated by roslibrust_codegen now depends only on libraries exposed by roslibrust_codegen. This means that
crates that were previously adding dependencies on serde, serde-big-array, and smart-default will no longer need to do so.

## 0.9.0 - May 13th, 2023

### Added
Expand Down
25 changes: 14 additions & 11 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

members = [
"example_package",
"example_package_macro",
"roslibrust",
"roslibrust_codegen",
"roslibrust_codegen_macro",
Expand Down
13 changes: 3 additions & 10 deletions example_package/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,12 @@ version = "0.1.0"
edition = "2021"

[dependencies]
# This is what we need for using this package in our mono-repo
roslibrust = { path = "../roslibrust" }
# Normally you would have: roslibrust = "0.7"
# TODO in the current state of this example, we don't actually need roslibrust
# These dependencies are needed by the code that roslibrust_codegen autogenerates
# They are silently "leaked dependencies of the crate"
# See https://github.com/Carter12s/roslibrust/issues/72 for resolution path
serde = "1.0"
smart-default = "0.7"
# The code generated by roslibrust_codegen has dependendencies
# We need to depend on the crate at build time so that the generate code has access to these dependencies
roslibrust_codegen = { path = "../roslibrust_codegen" }

[build-dependencies]
# We depend on codegen as a build dependency as we (should) only need it to generate our types
# We depend on codegen as a build dependency as we invoke it in build.rs
roslibrust_codegen = { path = "../roslibrust_codegen" }
# This crate is very helpful for build.rs files but not required
cargo-emit = "0.2"
1 change: 1 addition & 0 deletions example_package/README.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# Example RosLibRust Package

The point of this package is provide a good example of how to integrate roslibrust into a package using a build.rs file.

This package also serves as a testbed for maintainers of roslibrust to refine build.rs integration.
11 changes: 11 additions & 0 deletions example_package_macro/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
[package]
name = "example_package_macro"
version = "0.1.0"
edition = "2021"

[dependencies]
# The code generated by roslibrust_codegen has dependendencies
# We need to depend on the crate at build time so that the generate code has access to these dependencies
roslibrust_codegen = { path = "../roslibrust_codegen" }
# This crate contains the actual macro we will invoke
roslibrust_codegen_macro = { path = "../roslibrust_codegen_macro" }
5 changes: 5 additions & 0 deletions example_package_macro/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Example RosLibRust Package

The point of this package is provide a good example of how to integrate roslibrust into a package using the proc macro.

This package also serves as a testbed for maintainers of roslibrust to refine proc macro integration.
20 changes: 20 additions & 0 deletions example_package_macro/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
roslibrust_codegen_macro::find_and_generate_ros_messages!(
"assets/ros1_test_msgs",
"assets/ros1_common_interfaces"
);

// Example of 'use' pointing to code created by the include! macro
mod submodule {
#[allow(unused_imports)]
use crate::std_msgs::Header;
}

// Our actual "main" here doesn't do much, just shows the generate types
// are here and real.
fn main() {
// Note: within our assets there is a folder named ros1_test_msgs which contains a ros package
// The ros package in its package.xml refers to the name of that package as test_msgs
// The module that is generated will use the name in package.xml and not the directory
let data = test_msgs::Constants::TEST_STR;
println!("Hello World! {data}");
}
5 changes: 5 additions & 0 deletions roslibrust_codegen/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ syn = "1.0"
tokio = { version = "1.0", features = ["time", "signal"], optional = true}
walkdir = "2.3"
xml-rs = "0.8"
# Not a direct dependencies of the crate, but something generated code uses
# We include them as dependencies here and expose them with a `pub use`
# So that the generate code can find them, and users don't have to added dependencies themselves
smart-default = "0.7"
serde-big-array = "0.5"

[dev-dependencies]
env_logger = "0.10"
Expand Down
13 changes: 5 additions & 8 deletions roslibrust_codegen/src/gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,14 @@ use crate::{bail, Error};
use crate::{ConstantInfo, FieldInfo, MessageFile, RosLiteral, ServiceFile};

fn derive_attrs() -> Vec<syn::Attribute> {
// TODO we should look into using $crate here...
// The way we're currently doing it leaks a dependency on these crates to users...
// However using $crate breaks the generated code in non-macro usage
// Pass a flag in "if_macro"?
vec![
parse_quote! { #[derive(::serde::Deserialize)] },
parse_quote! { #[derive(::serde::Serialize)] },
parse_quote! { #[derive(::smart_default::SmartDefault)] },
parse_quote! { #[derive(::roslibrust_codegen::Deserialize)] },
parse_quote! { #[derive(::roslibrust_codegen::Serialize)] },
parse_quote! { #[derive(::roslibrust_codegen::SmartDefault)] },
parse_quote! { #[derive(Debug)] },
parse_quote! { #[derive(Clone)] },
parse_quote! { #[derive(PartialEq)] },
parse_quote! { #[serde(crate = "::roslibrust_codegen::serde")] },
]
}

Expand Down Expand Up @@ -179,7 +176,7 @@ fn generate_field_definition(
const MAX_FIXED_ARRAY_LEN: usize = 32;
let serde_line = match field.field_type.array_info {
Some(Some(fixed_array_len)) if fixed_array_len > MAX_FIXED_ARRAY_LEN => {
quote! { #[serde(with = "::serde_big_array::BigArray")] }
quote! { #[serde(with = "::roslibrust_codegen::BigArray")] }
}
_ => quote! {},
};
Expand Down
11 changes: 9 additions & 2 deletions roslibrust_codegen/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
use log::*;
use proc_macro2::TokenStream;
use quote::quote;
use serde::de::DeserializeOwned;
use serde::Serialize;
use simple_error::{bail, SimpleError as Error};
use std::collections::{BTreeMap, VecDeque};
use std::fmt::{Debug, Display};
Expand All @@ -19,6 +17,15 @@ use utils::RosVersion;
pub mod integral_types;
pub use integral_types::*;

// These pub use statements are here to be able to export the dependencies of the generated code
// so that crates using this crate don't need to add these dependencies themselves.
// Our generated code should find these exports.
// Modeled from: https://users.rust-lang.org/t/proc-macros-using-third-party-crate/42465/4
pub use ::serde;
pub use serde::{de::DeserializeOwned, Deserialize, Serialize};
pub use serde_big_array::BigArray;
pub use smart_default::SmartDefault;

/// Fundamental traits for message types this crate works with
/// This trait will be satisfied for any types generated with this crate's message_gen functionality
pub trait RosMessageType:
Expand Down
5 changes: 0 additions & 5 deletions roslibrust_test/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,6 @@ env_logger = "0.10"
roslibrust = { path = "../roslibrust" }
roslibrust_codegen = { path = "../roslibrust_codegen" }
lazy_static = "1.4"
# None of these three dependencies should have to be in here, they are leaking from generated code...
serde = { version = "1.0", features = ["derive"] }
serde_json = "1.0"
smart-default = "0.6"
serde-big-array = "0.5"

[dev-dependencies]
diffy = "0.3.0"
Loading
Loading