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

feat: Use package.links trick for Protobuf compilation #32

Merged
10 changes: 4 additions & 6 deletions node/Cargo.lock

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

2 changes: 0 additions & 2 deletions node/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,7 @@ proc-macro2 = "1.0.66"
prost = "0.12.0"
prost-build = "0.12.0"
prost-reflect = { version = "0.12.0", features = ["serde"] }
prost-types = "0.12.0"
protox = "0.5.0"
protox-parse = "0.5.0"
prettyplease = "0.2.6"
pretty_assertions = "1.4.0"
quick-protobuf = "0.8.1"
Expand Down
2 changes: 1 addition & 1 deletion node/actors/executor/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,4 @@ vise.workspace = true
tokio.workspace = true

[build-dependencies]
zksync_protobuf.workspace = true
zksync_protobuf_build.workspace = true
3 changes: 2 additions & 1 deletion node/actors/executor/build.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
//! Generates rust code from protobufs.
fn main() {
zksync_protobuf::build::Config {
zksync_protobuf_build::Config {
input_root: "src/config/proto".into(),
proto_root: "zksync/executor/config".into(),
dependencies: vec![],
protobuf_crate: "::zksync_protobuf".parse().unwrap(),
is_public: false,
}
.generate()
.expect("generate(config)");
Expand Down
3 changes: 1 addition & 2 deletions node/actors/network/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,4 @@ test-casing.workspace = true
tokio.workspace = true

[build-dependencies]
zksync_consensus_roles.workspace = true
zksync_protobuf.workspace = true
zksync_protobuf_build.workspace = true
16 changes: 6 additions & 10 deletions node/actors/network/build.rs
Original file line number Diff line number Diff line change
@@ -1,28 +1,24 @@
//! Generates rust code from protobufs.
fn main() {
zksync_protobuf::build::Config {
zksync_protobuf_build::Config {
input_root: "src/proto".into(),
proto_root: "zksync/network".into(),
dependencies: vec![
(
"::zksync_protobuf::proto".parse().unwrap(),
&zksync_protobuf::proto::DESCRIPTOR,
),
(
"::zksync_consensus_roles::proto".parse().unwrap(),
&zksync_consensus_roles::proto::DESCRIPTOR,
),
"::zksync_protobuf::proto".parse().unwrap(),
"::zksync_consensus_roles::proto".parse().unwrap(),
],
protobuf_crate: "::zksync_protobuf".parse().unwrap(),
is_public: false,
}
.generate()
.expect("generate()");

zksync_protobuf::build::Config {
zksync_protobuf_build::Config {
input_root: "src/mux/tests/proto".into(),
proto_root: "zksync/network/mux/tests".into(),
dependencies: vec![],
protobuf_crate: "::zksync_protobuf".parse().unwrap(),
is_public: false,
}
.generate()
.expect("generate()");
Expand Down
4 changes: 2 additions & 2 deletions node/libs/protobuf/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ authors.workspace = true
homepage.workspace = true
license.workspace = true

links = "zksync_protobuf_proto"

[[bin]]
name = "conformance_test"

Expand All @@ -28,5 +30,3 @@ tracing-subscriber.workspace = true

[build-dependencies]
zksync_protobuf_build.workspace = true

anyhow.workspace = true
3 changes: 3 additions & 0 deletions node/libs/protobuf/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ fn main() {
proto_root: "zksync".into(),
dependencies: vec![],
protobuf_crate: "crate".parse().expect("protobuf_crate"),
is_public: true,
}
.generate()
.expect("generate(std)");
Expand All @@ -14,6 +15,7 @@ fn main() {
proto_root: "zksync/protobuf/tests".into(),
dependencies: vec![],
protobuf_crate: "crate".parse().expect("protobuf_crate"),
is_public: false,
}
.generate()
.expect("generate(test)");
Expand All @@ -23,6 +25,7 @@ fn main() {
proto_root: "zksync/protobuf/conformance_test".into(),
dependencies: vec![],
protobuf_crate: "::zksync_protobuf".parse().expect("protobuf_crate"),
is_public: false,
}
.generate()
.expect("generate(conformance)");
Expand Down
85 changes: 85 additions & 0 deletions node/libs/protobuf/src/build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
//! Build-related functionality. This should only be used by the code generated by the `protobuf_build` crate.

pub use once_cell::sync::Lazy;
pub use prost;
use prost::Message as _;
pub use prost_reflect;
use prost_reflect::prost_types;
use std::sync::RwLock;

/// Global descriptor pool.
static POOL: Lazy<RwLock<prost_reflect::DescriptorPool>> = Lazy::new(RwLock::default);

/// Protobuf descriptor + info about the mapping to rust code.
#[derive(Debug)]
pub struct Descriptor(());

impl Descriptor {
/// Constructs a descriptor and adds it to the global pool.
pub fn new(_dependencies: &[&'static Self], descriptor_bytes: &[u8]) -> Self {
Copy link
Collaborator

Choose a reason for hiding this comment

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

if unused, please remove

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Having dependency refs allows ensuring via type system that these dependencies are added in the global pool. Would it be better to force dependencies to get initialized via Lazy::force(), or in some other way?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Type system doesn't ensure any relation between the dependencies and the newly constructed descriptor, so this is as good as Lazy::force (or just dereferencing the lazy descriptors).

let descriptor = prost_types::FileDescriptorSet::decode(descriptor_bytes).unwrap();
let pool = &mut POOL.write().unwrap();

// Dependencies are already loaded to the global pool on their initialization.
// The fact that we have their refs is sufficient to prove that they are already in the global pool.
Self::load(pool, descriptor).expect("failed loading descriptor into global pool");
Self(())
}

/// Loads the descriptor to the pool, if not already loaded.
fn load(
pool: &mut prost_reflect::DescriptorPool,
descriptor: prost_types::FileDescriptorSet,
) -> anyhow::Result<()> {
let pool_has_all_files = descriptor
.file
.iter()
.all(|file| pool.get_file_by_name(file.name()).is_some());
if pool_has_all_files {
return Ok(());
}
pool.add_file_descriptor_set(descriptor)?;
Ok(())
}

/// Returns a descriptor by a fully qualified message name.
pub fn get_message_by_name(&self, name: &str) -> Option<prost_reflect::MessageDescriptor> {
POOL.read().unwrap().get_message_by_name(name)
// ^ This works because this descriptor must have been loaded into the global pool
// when the instance was constructed.
}
}

/// Expands to a descriptor declaration.
#[macro_export]
macro_rules! declare_descriptor {
($name:ident => $descriptor_path:expr, $($rust_deps:path),*) => {
pub static $name: $crate::build::Lazy<$crate::build::Descriptor> =
$crate::build::Lazy::new(|| {
$crate::build::Descriptor::new(
&[$({ use $rust_deps as dep; &dep::DESCRIPTOR }),*],
::std::include_bytes!($descriptor_path),
)
});
}
}

/// Implements `ReflectMessage` for a type based on a provided `Descriptor`.
#[macro_export]
macro_rules! impl_reflect_message {
($ty:ty, $descriptor:expr, $proto_name:expr) => {
impl $crate::build::prost_reflect::ReflectMessage for $ty {
fn descriptor(&self) -> $crate::build::prost_reflect::MessageDescriptor {
static INIT: $crate::build::Lazy<$crate::build::prost_reflect::MessageDescriptor> =
$crate::build::Lazy::new(|| {
$crate::build::Descriptor::get_message_by_name($descriptor, $proto_name)
.unwrap()
});
INIT.clone()
}
}
};
}

pub use declare_descriptor;
pub use impl_reflect_message;
5 changes: 3 additions & 2 deletions node/libs/protobuf/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
//! Code generated from protobuf schema files and
//! utilities for serialization.

#[doc(hidden)] // should only be used by code generated by `protobuf_build` crate
pub mod build;
pub mod proto;
mod proto_fmt;
mod std_conv;
pub mod testonly;

pub use proto_fmt::*;
pub use zksync_protobuf_build as build;
pub use self::proto_fmt::*;

#[cfg(test)]
mod tests;
7 changes: 0 additions & 7 deletions node/libs/protobuf_build/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,9 @@ license.workspace = true
[dependencies]
anyhow.workspace = true
heck.workspace = true
once_cell.workspace = true
prettyplease.workspace = true
proc-macro2.workspace = true
prost.workspace = true
prost-build.workspace = true
prost-types.workspace = true
prost-reflect.workspace = true
protox.workspace = true
protox-parse.workspace = true
quote.workspace = true
rand.workspace = true
syn.workspace = true

1 change: 1 addition & 0 deletions node/libs/protobuf_build/src/canonical.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Checks whether messages in the given file descriptor set support canonical encoding.
use crate::syntax::extract_message_names;
use anyhow::Context as _;
use protox::prost_reflect::{self, prost_types};
slowli marked this conversation as resolved.
Show resolved Hide resolved
use std::collections::HashSet;

#[derive(Default)]
Expand Down
Loading
Loading