This is an automated email from the ASF dual-hosted git repository.
tustvold pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
The following commit(s) were added to refs/heads/master by this push:
new f3304965a Move protoc generation to binary crate, unpin prost/tonic
build (#3876) (#3927)
f3304965a is described below
commit f3304965a7bf662111253c98445c64173db3477a
Author: Raphael Taylor-Davies <[email protected]>
AuthorDate: Fri Mar 24 11:02:21 2023 +0000
Move protoc generation to binary crate, unpin prost/tonic build (#3876)
(#3927)
* Move protoc generation to binary crate (#3876)
* Review feedback
* Format
* Fix link
---
.github/workflows/arrow_flight.yml | 15 +++++-
Cargo.toml | 1 +
arrow-flight/CONTRIBUTING.md | 41 +++++++++++++++
arrow-flight/Cargo.toml | 7 ---
arrow-flight/build.rs | 102 -------------------------------------
arrow-flight/gen/Cargo.toml | 37 ++++++++++++++
arrow-flight/gen/src/main.rs | 86 +++++++++++++++++++++++++++++++
arrow-flight/regen.sh | 21 ++++++++
8 files changed, 199 insertions(+), 111 deletions(-)
diff --git a/.github/workflows/arrow_flight.yml
b/.github/workflows/arrow_flight.yml
index 7facf1719..5301a3f85 100644
--- a/.github/workflows/arrow_flight.yml
+++ b/.github/workflows/arrow_flight.yml
@@ -41,7 +41,6 @@ on:
- .github/**
jobs:
- # test the crate
linux-test:
name: Test
runs-on: ubuntu-latest
@@ -62,7 +61,19 @@ jobs:
- name: Test --examples
run: |
cargo test -p arrow-flight --features=flight-sql-experimental,tls
--examples
- - name: Verify workspace clean
+
+ vendor:
+ name: Verify Vendored Code
+ runs-on: ubuntu-latest
+ container:
+ image: amd64/rust
+ steps:
+ - uses: actions/checkout@v3
+ - name: Setup Rust toolchain
+ uses: ./.github/actions/setup-builder
+ - name: Run gen
+ run: ./arrow-flight/regen.sh
+ - name: Verify workspace clean (if this fails, run
./arrow-flight/regen.sh and check in results)
run: git diff --exit-code
clippy:
diff --git a/Cargo.toml b/Cargo.toml
index ebecc9eaf..64ce3166e 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -25,6 +25,7 @@ members = [
"arrow-csv",
"arrow-data",
"arrow-flight",
+ "arrow-flight/gen",
"arrow-integration-test",
"arrow-integration-testing",
"arrow-ipc",
diff --git a/arrow-flight/CONTRIBUTING.md b/arrow-flight/CONTRIBUTING.md
new file mode 100644
index 000000000..156a0b9ca
--- /dev/null
+++ b/arrow-flight/CONTRIBUTING.md
@@ -0,0 +1,41 @@
+<!---
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+-->
+
+# Flight
+
+## Generated Code
+
+The prost/tonic code can be generated by running, which in turn invokes the
Rust binary located in [gen](./gen)
+
+This is necessary after modifying the protobuf definitions or altering the
dependencies of [gen](./gen), and requires a
+valid installation of
[protoc](https://github.com/protocolbuffers/protobuf#protocol-compiler-installation).
+
+```bash
+./regen.sh
+```
+
+### Why Vendor
+
+The standard approach to integrating `prost-build` / `tonic-build` is to use a
`build.rs` script that automatically generates the code as part of the standard
build process.
+
+Unfortunately this caused a lot of friction for users:
+
+- Requires all users to have a protoc install in order to compile the crate -
[#2616](https://github.com/apache/arrow-rs/issues/2616)
+- Some distributions have very old versions of protoc that don't support
required functionality - [#1574](https://github.com/apache/arrow-rs/issues/1574)
+- Inconsistent support within IDEs for code completion of automatically
generated code
diff --git a/arrow-flight/Cargo.toml b/arrow-flight/Cargo.toml
index 5f839ca68..e8f57345e 100644
--- a/arrow-flight/Cargo.toml
+++ b/arrow-flight/Cargo.toml
@@ -64,13 +64,6 @@ tempfile = "3.3"
tokio-stream = { version = "0.1", features = ["net"] }
tower = "0.4.13"
-[build-dependencies]
-# Pin specific version of the tonic-build dependencies to avoid auto-generated
-# (and checked in) arrow.flight.protocol.rs from changing
-proc-macro2 = { version = "=1.0.53", default-features = false }
-prost-build = { version = "=0.11.8", default-features = false }
-tonic-build = { version = "=0.8.4", default-features = false, features =
["transport", "prost"] }
-
[[example]]
name = "flight_sql_server"
required-features = ["flight-sql-experimental", "tls"]
diff --git a/arrow-flight/build.rs b/arrow-flight/build.rs
deleted file mode 100644
index 3f50fa812..000000000
--- a/arrow-flight/build.rs
+++ /dev/null
@@ -1,102 +0,0 @@
-// Licensed to the Apache Software Foundation (ASF) under one
-// or more contributor license agreements. See the NOTICE file
-// distributed with this work for additional information
-// regarding copyright ownership. The ASF licenses this file
-// to you under the Apache License, Version 2.0 (the
-// "License"); you may not use this file except in compliance
-// with the License. You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing,
-// software distributed under the License is distributed on an
-// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-// KIND, either express or implied. See the License for the
-// specific language governing permissions and limitations
-// under the License.
-
-use std::{
- fs::OpenOptions,
- io::{Read, Write},
- path::Path,
-};
-
-fn main() -> Result<(), Box<dyn std::error::Error>> {
- // The current working directory can vary depending on how the project is
being
- // built or released so we build an absolute path to the proto file
- let path = Path::new("../format/Flight.proto");
- if path.exists() {
- // avoid rerunning build if the file has not changed
- println!("cargo:rerun-if-changed=../format/Flight.proto");
-
- let proto_dir = Path::new("../format");
- let proto_path = Path::new("../format/Flight.proto");
-
- tonic_build::configure()
- // protoc in unbuntu builder needs this option
- .protoc_arg("--experimental_allow_proto3_optional")
- .out_dir("src")
- .compile_with_config(prost_config(), &[proto_path], &[proto_dir])?;
-
- // read file contents to string
- let mut file = OpenOptions::new()
- .read(true)
- .open("src/arrow.flight.protocol.rs")?;
- let mut buffer = String::new();
- file.read_to_string(&mut buffer)?;
- // append warning that file was auto-generate
- let mut file = OpenOptions::new()
- .write(true)
- .truncate(true)
- .open("src/arrow.flight.protocol.rs")?;
- file.write_all("// This file was automatically generated through the
build.rs script, and should not be edited.\n\n".as_bytes())?;
- file.write_all(buffer.as_bytes())?;
- }
-
- // The current working directory can vary depending on how the project is
being
- // built or released so we build an absolute path to the proto file
- let path = Path::new("../format/FlightSql.proto");
- if path.exists() {
- // avoid rerunning build if the file has not changed
- println!("cargo:rerun-if-changed=../format/FlightSql.proto");
-
- let proto_dir = Path::new("../format");
- let proto_path = Path::new("../format/FlightSql.proto");
-
- tonic_build::configure()
- // protoc in ubuntu builder needs this option
- .protoc_arg("--experimental_allow_proto3_optional")
- .out_dir("src/sql")
- .compile_with_config(prost_config(), &[proto_path], &[proto_dir])?;
-
- // read file contents to string
- let mut file = OpenOptions::new()
- .read(true)
- .open("src/sql/arrow.flight.protocol.sql.rs")?;
- let mut buffer = String::new();
- file.read_to_string(&mut buffer)?;
- // append warning that file was auto-generate
- let mut file = OpenOptions::new()
- .write(true)
- .truncate(true)
- .open("src/sql/arrow.flight.protocol.sql.rs")?;
- file.write_all("// This file was automatically generated through the
build.rs script, and should not be edited.\n\n".as_bytes())?;
- file.write_all(buffer.as_bytes())?;
- }
-
- // Prost currently generates an empty file, this was fixed but then
reverted
- // https://github.com/tokio-rs/prost/pull/639
- let google_protobuf_rs = Path::new("src/sql/google.protobuf.rs");
- if google_protobuf_rs.exists() &&
google_protobuf_rs.metadata().unwrap().len() == 0 {
- std::fs::remove_file(google_protobuf_rs).unwrap();
- }
-
- // As the proto file is checked in, the build should not fail if the file
is not found
- Ok(())
-}
-
-fn prost_config() -> prost_build::Config {
- let mut config = prost_build::Config::new();
- config.bytes([".arrow"]);
- config
-}
diff --git a/arrow-flight/gen/Cargo.toml b/arrow-flight/gen/Cargo.toml
new file mode 100644
index 000000000..c3b9cbd8c
--- /dev/null
+++ b/arrow-flight/gen/Cargo.toml
@@ -0,0 +1,37 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+[package]
+name = "gen"
+description = "Code generation for arrow-flight"
+version = "0.1.0"
+edition = "2021"
+rust-version = "1.62"
+authors = ["Apache Arrow <[email protected]>"]
+homepage = "https://github.com/apache/arrow-rs";
+repository = "https://github.com/apache/arrow-rs";
+license = "Apache-2.0"
+publish = false
+
+# See more keys and their definitions at
https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]
+# Pin specific version of the tonic-build dependencies to avoid auto-generated
+# (and checked in) arrow.flight.protocol.rs from changing
+proc-macro2 = { version = "=1.0.53", default-features = false }
+prost-build = { version = "=0.11.8", default-features = false }
+tonic-build = { version = "=0.8.4", default-features = false, features =
["transport", "prost"] }
diff --git a/arrow-flight/gen/src/main.rs b/arrow-flight/gen/src/main.rs
new file mode 100644
index 000000000..a3541c63b
--- /dev/null
+++ b/arrow-flight/gen/src/main.rs
@@ -0,0 +1,86 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::{
+ fs::OpenOptions,
+ io::{Read, Write},
+ path::Path,
+};
+
+fn main() -> Result<(), Box<dyn std::error::Error>> {
+ let proto_dir = Path::new("../format");
+ let proto_path = Path::new("../format/Flight.proto");
+
+ tonic_build::configure()
+ // protoc in unbuntu builder needs this option
+ .protoc_arg("--experimental_allow_proto3_optional")
+ .out_dir("src")
+ .compile_with_config(prost_config(), &[proto_path], &[proto_dir])?;
+
+ // read file contents to string
+ let mut file = OpenOptions::new()
+ .read(true)
+ .open("src/arrow.flight.protocol.rs")?;
+ let mut buffer = String::new();
+ file.read_to_string(&mut buffer)?;
+ // append warning that file was auto-generate
+ let mut file = OpenOptions::new()
+ .write(true)
+ .truncate(true)
+ .open("src/arrow.flight.protocol.rs")?;
+ file.write_all("// This file was automatically generated through the
build.rs script, and should not be edited.\n\n".as_bytes())?;
+ file.write_all(buffer.as_bytes())?;
+
+ let proto_dir = Path::new("../format");
+ let proto_path = Path::new("../format/FlightSql.proto");
+
+ tonic_build::configure()
+ // protoc in ubuntu builder needs this option
+ .protoc_arg("--experimental_allow_proto3_optional")
+ .out_dir("src/sql")
+ .compile_with_config(prost_config(), &[proto_path], &[proto_dir])?;
+
+ // read file contents to string
+ let mut file = OpenOptions::new()
+ .read(true)
+ .open("src/sql/arrow.flight.protocol.sql.rs")?;
+ let mut buffer = String::new();
+ file.read_to_string(&mut buffer)?;
+ // append warning that file was auto-generate
+ let mut file = OpenOptions::new()
+ .write(true)
+ .truncate(true)
+ .open("src/sql/arrow.flight.protocol.sql.rs")?;
+ file.write_all("// This file was automatically generated through the
build.rs script, and should not be edited.\n\n".as_bytes())?;
+ file.write_all(buffer.as_bytes())?;
+
+ // Prost currently generates an empty file, this was fixed but then
reverted
+ // https://github.com/tokio-rs/prost/pull/639
+ let google_protobuf_rs = Path::new("src/sql/google.protobuf.rs");
+ if google_protobuf_rs.exists() &&
google_protobuf_rs.metadata().unwrap().len() == 0 {
+ std::fs::remove_file(google_protobuf_rs).unwrap();
+ }
+
+ // As the proto file is checked in, the build should not fail if the file
is not found
+ Ok(())
+}
+
+fn prost_config() -> prost_build::Config {
+ let mut config = prost_build::Config::new();
+ config.bytes([".arrow"]);
+ config
+}
diff --git a/arrow-flight/regen.sh b/arrow-flight/regen.sh
new file mode 100755
index 000000000..d83f9d580
--- /dev/null
+++ b/arrow-flight/regen.sh
@@ -0,0 +1,21 @@
+#!/usr/bin/env bash
+
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
+cd $SCRIPT_DIR && cargo run --manifest-path gen/Cargo.toml
