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

Rust: update protobuf_codegen crate to rely on separately installed protoc #19620

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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
15 changes: 0 additions & 15 deletions rust/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -291,18 +291,3 @@ pkg_filegroup(
prefix = "libupb",
visibility = ["//rust/release_crates:__subpackages__"],
)

# Bundle only the linux-x86_64 protoc for testing.
pkg_cross_compiled_binaries(
name = "vendored_protocs_test",
cpus = [
"linux-x86_64",
],
prefix = "bin",
tags = ["manual"],
targets = [
"//upb_generator/minitable:protoc-gen-upb_minitable",
"//:protoc",
],
visibility = ["//rust/release_crates:__subpackages__"],
)
2 changes: 2 additions & 0 deletions rust/release_crates/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ sh_binary(
"//rust/release_crates/protobuf:protobuf_crate",
"//rust/release_crates/protobuf_codegen:protobuf_codegen_crate",
"//rust/release_crates/protobuf_example:protobuf_example_crate",
"//src/google/protobuf/compiler:protoc",
"//upb_generator/minitable:protoc-gen-upb_minitable",
],
tags = ["manual"],
deps = ["@bazel_tools//tools/bash/runfiles"],
Expand Down
6 changes: 5 additions & 1 deletion rust/release_crates/cargo_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,11 @@ mkdir $EXAMPLE_ROOT
EXAMPLE_TAR=$(rlocation com_google_protobuf/rust/release_crates/protobuf_example/protobuf_example_crate.tar)

echo "Expanding protobuf_example crate tar"
tar -xvf $EXAMPLE_TAR -C $EXAMPLE_ROOT
tar -xvf $EXAMPLE_TAR -C $EXAMPLE_ROOT

# Put the Bazel-built protoc and plugin at the beginning of $PATH
PATH=$(dirname $(rlocation com_google_protobuf/protoc)):$PATH
PATH=$(dirname $(rlocation com_google_protobuf/upb_generator/minitable/protoc-gen-upb_minitable)):$PATH

cd $CRATE_ROOT
CARGO_HOME=$CARGO_HOME cargo test
Expand Down
1 change: 0 additions & 1 deletion rust/release_crates/protobuf_codegen/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ pkg_tar(
srcs = [
":protobuf_codegen_files",
"//:LICENSE",
"//rust:vendored_protocs_test",
],
tags = ["manual"],
visibility = ["//rust:__subpackages__"],
Expand Down
102 changes: 37 additions & 65 deletions rust/release_crates/protobuf_codegen/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use std::fs::{self, OpenOptions};
use std::io::prelude::*;
use std::path::{Path, PathBuf};
use walkdir::WalkDir;
Expand All @@ -7,8 +6,6 @@ use walkdir::WalkDir;
pub struct CodeGen {
inputs: Vec<PathBuf>,
output_dir: PathBuf,
protoc_path: Option<PathBuf>,
protoc_gen_upb_minitable_path: Option<PathBuf>,
includes: Vec<PathBuf>,
}

Expand All @@ -19,8 +16,6 @@ impl CodeGen {
Self {
inputs: Vec::new(),
output_dir: std::env::current_dir().unwrap().join("src").join("protobuf_generated"),
protoc_path: None,
protoc_gen_upb_minitable_path: None,
includes: Vec::new(),
}
}
Expand All @@ -40,20 +35,6 @@ impl CodeGen {
self
}

pub fn protoc_path(&mut self, protoc_path: impl AsRef<Path>) -> &mut Self {
self.protoc_path = Some(protoc_path.as_ref().to_owned());
self
}

pub fn protoc_gen_upb_minitable_path(
&mut self,
protoc_gen_upb_minitable_path: impl AsRef<Path>,
) -> &mut Self {
self.protoc_gen_upb_minitable_path =
Some(protoc_gen_upb_minitable_path.as_ref().to_owned());
self
}

pub fn include(&mut self, include: impl AsRef<Path>) -> &mut Self {
self.includes.push(include.as_ref().to_owned());
self
Expand All @@ -64,6 +45,33 @@ impl CodeGen {
self
}

fn protoc_version() -> String {
let output = std::process::Command::new("protoc").arg("--version").output().unwrap().stdout;

// The output of protoc --version looks something like "libprotoc XX.Y", with a
// possible suffix starting with a dash. We want to return just the
// "XX.Y" part.
let mut s =
String::from_utf8(output).unwrap().strip_prefix("libprotoc ").unwrap().to_string();
let first_dash = s.find('-');
if let Some(i) = first_dash {
s.truncate(i);
}
s
}

fn expected_protoc_version() -> String {
let mut s = VERSION.to_string();
let first_dash = s.find('-');
if let Some(i) = first_dash {
s.truncate(i);
}
let mut v: Vec<&str> = s.split('.').collect();
assert_eq!(v.len(), 3);
v.remove(0);
v.join(".")
}

pub fn generate_and_compile(&self) -> Result<(), String> {
let upb_version = std::env::var("DEP_UPB_VERSION").expect("DEP_UPB_VERSION should have been set, make sure that the Protobuf crate is a dependency");
if VERSION != upb_version {
Expand All @@ -73,36 +81,30 @@ impl CodeGen {
);
}

let protoc_path = if let Some(path) = &self.protoc_path {
path.clone()
} else {
protoc_path().expect("To be a supported platform")
};
let mut cmd = std::process::Command::new(protoc_path);
let protoc_version = Self::protoc_version();
let expected_protoc_version = Self::expected_protoc_version();
if protoc_version != expected_protoc_version {
panic!(
"Expected protoc version {} but found {}",
expected_protoc_version, protoc_version
);
}

let mut cmd = std::process::Command::new("protoc");
for input in &self.inputs {
cmd.arg(input);
}
if !self.output_dir.exists() {
// Attempt to make the directory if it doesn't exist
let _ = std::fs::create_dir(&self.output_dir);
}
let protoc_gen_upb_minitable_path = if let Some(path) = &self.protoc_gen_upb_minitable_path
{
path.clone()
} else {
protoc_gen_upb_minitable_path().expect("To be a supported platform")
};

for include in &self.includes {
println!("cargo:rerun-if-changed={}", include.display());
}

cmd.arg(format!("--rust_out={}", self.output_dir.display()))
.arg("--rust_opt=experimental-codegen=enabled,kernel=upb")
.arg(format!(
"--plugin=protoc-gen-upb_minitable={}",
protoc_gen_upb_minitable_path.display()
))
.arg(format!("--upb_minitable_out={}", self.output_dir.display()));
for include in &self.includes {
cmd.arg(format!("--proto_path={}", include.display()));
Expand Down Expand Up @@ -138,33 +140,3 @@ impl CodeGen {
Ok(())
}
}

fn get_path_for_arch() -> Option<PathBuf> {
let mut path = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
path.push("bin");
match (std::env::consts::OS, std::env::consts::ARCH) {
("macos", "x86_64") => path.push("osx-x86_64"),
("macos", "aarch64") => path.push("osx-aarch_64"),
("linux", "aarch64") => path.push("linux-aarch_64"),
("linux", "powerpc64") => path.push("linux-ppcle_64"),
("linux", "s390x") => path.push("linux-s390_64"),
("linux", "x86") => path.push("linux-x86_32"),
("linux", "x86_64") => path.push("linux-x86_64"),
("windows", "x86") => path.push("win32"),
("windows", "x86_64") => path.push("win64"),
_ => return None,
};
Some(path)
}

pub fn protoc_path() -> Option<PathBuf> {
let mut path = get_path_for_arch()?;
path.push("protoc");
Some(path)
}

pub fn protoc_gen_upb_minitable_path() -> Option<PathBuf> {
let mut path = get_path_for_arch()?;
path.push("protoc-gen-upb_minitable");
Some(path)
}
2 changes: 1 addition & 1 deletion upb_generator/minitable/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ bootstrap_cc_binary(
visibility = [
"//editions/codegen_tests:__pkg__",
"//net/proto2/contrib/protoc_explorer:__pkg__",
"//rust:__pkg__",
"//rust:__subpackages__",
"//third_party/prototiller/transformer:__pkg__",
],
)
Expand Down
Loading