From e6ed77d42c8078657f13c9ae3350bfff726ce089 Mon Sep 17 00:00:00 2001 From: Brent Gardner Date: Mon, 24 Oct 2022 16:07:12 -0600 Subject: [PATCH 1/5] Don't make users install protoc --- .github/workflows/rust.yml | 45 ------------------------------------- datafusion/proto/Cargo.toml | 3 +++ datafusion/proto/build.rs | 30 ++++++++++++++++++++++--- 3 files changed, 30 insertions(+), 48 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index f0fa56089a81..a44c0251c196 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -81,16 +81,6 @@ jobs: - uses: actions/checkout@v3 with: submodules: true - - name: Install protobuf compiler - shell: bash - run: | - mkdir -p $HOME/d/protoc - cd $HOME/d/protoc - export PROTO_ZIP="protoc-21.4-linux-x86_64.zip" - curl -LO https://github.com/protocolbuffers/protobuf/releases/download/v21.4/$PROTO_ZIP - unzip $PROTO_ZIP - export PATH=$PATH:$HOME/d/protoc/bin - protoc --version - name: Cache Cargo uses: actions/cache@v3 with: @@ -103,11 +93,9 @@ jobs: rust-version: stable - name: Run tests run: | - export PATH=$PATH:$HOME/d/protoc/bin cargo test --features avro,jit,scheduler,json - name: Run examples run: | - export PATH=$PATH:$HOME/d/protoc/bin # test datafusion-sql examples cargo run --example sql # test datafusion-examples @@ -190,16 +178,6 @@ jobs: - uses: actions/checkout@v3 with: submodules: true - - name: Install protobuf compiler - shell: bash - run: | - mkdir -p $HOME/d/protoc - cd $HOME/d/protoc - export PROTO_ZIP="protoc-21.4-win64.zip" - curl -LO https://github.com/protocolbuffers/protobuf/releases/download/v21.4/$PROTO_ZIP - unzip $PROTO_ZIP - export PATH=$PATH:$HOME/d/protoc/bin - protoc.exe --version # TODO: this won't cache anything, which is expensive. Setup this action # with a OS-dependent path. - name: Setup Rust toolchain @@ -210,7 +188,6 @@ jobs: - name: Run tests shell: bash run: | - export PATH=$PATH:$HOME/d/protoc/bin cargo test env: # do not produce debug symbols to keep memory usage down @@ -223,17 +200,6 @@ jobs: - uses: actions/checkout@v3 with: submodules: true - - name: Install protobuf compiler - shell: bash - run: | - mkdir -p $HOME/d/protoc - cd $HOME/d/protoc - export PROTO_ZIP="protoc-21.4-osx-x86_64.zip" - curl -LO https://github.com/protocolbuffers/protobuf/releases/download/v21.4/$PROTO_ZIP - unzip $PROTO_ZIP - echo "$HOME/d/protoc/bin" >> $GITHUB_PATH - export PATH=$PATH:$HOME/d/protoc/bin - protoc --version # TODO: this won't cache anything, which is expensive. Setup this action # with a OS-dependent path. - name: Setup Rust toolchain @@ -312,16 +278,6 @@ jobs: # - uses: actions/checkout@v3 # with: # submodules: true - # - name: Install protobuf compiler - # shell: bash - # run: | - # mkdir -p $HOME/d/protoc - # cd $HOME/d/protoc - # export PROTO_ZIP="protoc-21.4-linux-x86_64.zip" - # curl -LO https://github.com/protocolbuffers/protobuf/releases/download/v21.4/$PROTO_ZIP - # unzip $PROTO_ZIP - # export PATH=$PATH:$HOME/d/protoc/bin - # protoc --version # - name: Setup Rust toolchain # run: | # rustup toolchain install stable @@ -335,7 +291,6 @@ jobs: # key: cargo-coverage-cache3- # - name: Run coverage # run: | - # export PATH=$PATH:$HOME/d/protoc/bin # rustup toolchain install stable # rustup default stable # cargo install --version 0.20.1 cargo-tarpaulin diff --git a/datafusion/proto/Cargo.toml b/datafusion/proto/Cargo.toml index ef5d8211248a..80c0f59f269c 100644 --- a/datafusion/proto/Cargo.toml +++ b/datafusion/proto/Cargo.toml @@ -58,3 +58,6 @@ tokio = "1.18" [build-dependencies] pbjson-build = { version = "0.5", optional = true } prost-build = { version = "0.11.1" } +reqwest = "0.11.12" +tokio = "1.18" +zip-extract = "0.1.1" diff --git a/datafusion/proto/build.rs b/datafusion/proto/build.rs index 3efd71350300..5d2a7d7451e1 100644 --- a/datafusion/proto/build.rs +++ b/datafusion/proto/build.rs @@ -18,17 +18,18 @@ type Error = Box; type Result = std::result::Result; -fn main() -> Result<(), String> { +#[tokio::main] +async fn main() -> Result<(), String> { // for use in docker build where file changes can be wonky println!("cargo:rerun-if-env-changed=FORCE_REBUILD"); println!("cargo:rerun-if-changed=proto/datafusion.proto"); - build()?; + build().await?; Ok(()) } -fn build() -> Result<(), String> { +async fn build() -> Result<(), String> { use std::io::Write; let out = std::path::PathBuf::from( @@ -36,6 +37,29 @@ fn build() -> Result<(), String> { ); let descriptor_path = out.join("proto_descriptor.bin"); + // compute protoc distribution URL + let host = std::env::var("HOST").expect("HOST not specifed!"); + let proto_platform = match host.as_str() { + "x86_64-unknown-linux-gnu" => "linux-x86_64", + _ => panic!("No protobuf found for OS type: {}", host), + }; + let proto_base = "https://github.com/protocolbuffers/protobuf/releases/download"; + let proto_ver = "21.8"; + let proto_url = + format!("{proto_base}/v{proto_ver}/protoc-{proto_ver}-{proto_platform}.zip"); + + // Download protoc binary + let target_dir = out.join("protoc"); + if !target_dir.exists() { + let archive = reqwest::get(proto_url) + .await + .expect("Can't download protoc"); + let archive = archive.bytes().await.expect("Can't download protoc"); + let archive = std::io::Cursor::new(archive); + zip_extract::extract(archive, &target_dir, true).expect("Can't extract protoc"); + } + std::env::set_var("PROTOC", out.join("protoc/bin/protoc")); + prost_build::Config::new() .file_descriptor_set_path(&descriptor_path) .compile_well_known_types() From eb1a1914b4e77b84db558835201c82ce1f4a8c56 Mon Sep 17 00:00:00 2001 From: Brent Gardner Date: Mon, 24 Oct 2022 16:25:51 -0600 Subject: [PATCH 2/5] Other platforms than mine --- datafusion/proto/build.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/datafusion/proto/build.rs b/datafusion/proto/build.rs index 5d2a7d7451e1..defaedc3dd0a 100644 --- a/datafusion/proto/build.rs +++ b/datafusion/proto/build.rs @@ -38,9 +38,12 @@ async fn build() -> Result<(), String> { let descriptor_path = out.join("proto_descriptor.bin"); // compute protoc distribution URL - let host = std::env::var("HOST").expect("HOST not specifed!"); + let host = std::env::var("HOST").expect("HOST not specified!"); let proto_platform = match host.as_str() { + "todo" => "linux-aarch_64", // TODO: arm "x86_64-unknown-linux-gnu" => "linux-x86_64", + "x86_64-pc-windows-msvc" => "win64", + "x86_64-apple-darwin" => "osx-x86_64", _ => panic!("No protobuf found for OS type: {}", host), }; let proto_base = "https://github.com/protocolbuffers/protobuf/releases/download"; From d287968b74443d72a1f24b2406a5eab3591202f8 Mon Sep 17 00:00:00 2001 From: Brent Gardner Date: Mon, 24 Oct 2022 16:37:28 -0600 Subject: [PATCH 3/5] Deal with windows --- datafusion/proto/build.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/datafusion/proto/build.rs b/datafusion/proto/build.rs index defaedc3dd0a..006e0d5a8003 100644 --- a/datafusion/proto/build.rs +++ b/datafusion/proto/build.rs @@ -39,11 +39,11 @@ async fn build() -> Result<(), String> { // compute protoc distribution URL let host = std::env::var("HOST").expect("HOST not specified!"); - let proto_platform = match host.as_str() { - "todo" => "linux-aarch_64", // TODO: arm - "x86_64-unknown-linux-gnu" => "linux-x86_64", - "x86_64-pc-windows-msvc" => "win64", - "x86_64-apple-darwin" => "osx-x86_64", + let (proto_platform, suffix) = match host.as_str() { + "todo" => ("linux-aarch_64", ""), // TODO: arm + "x86_64-unknown-linux-gnu" => ("linux-x86_64", ""), + "x86_64-pc-windows-msvc" => ("win64", ".exe"), + "x86_64-apple-darwin" => ("osx-x86_64", ""), _ => panic!("No protobuf found for OS type: {}", host), }; let proto_base = "https://github.com/protocolbuffers/protobuf/releases/download"; @@ -61,7 +61,7 @@ async fn build() -> Result<(), String> { let archive = std::io::Cursor::new(archive); zip_extract::extract(archive, &target_dir, true).expect("Can't extract protoc"); } - std::env::set_var("PROTOC", out.join("protoc/bin/protoc")); + std::env::set_var("PROTOC", out.join(format!("protoc/bin/protoc{suffix}"))); prost_build::Config::new() .file_descriptor_set_path(&descriptor_path) From fad89e00499d314fe946fe2232389eced442b8dd Mon Sep 17 00:00:00 2001 From: Brent Gardner Date: Mon, 24 Oct 2022 17:39:55 -0600 Subject: [PATCH 4/5] Don't override user choice --- datafusion/proto/build.rs | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/datafusion/proto/build.rs b/datafusion/proto/build.rs index 006e0d5a8003..03bff06bfb85 100644 --- a/datafusion/proto/build.rs +++ b/datafusion/proto/build.rs @@ -52,16 +52,18 @@ async fn build() -> Result<(), String> { format!("{proto_base}/v{proto_ver}/protoc-{proto_ver}-{proto_platform}.zip"); // Download protoc binary - let target_dir = out.join("protoc"); - if !target_dir.exists() { - let archive = reqwest::get(proto_url) - .await - .expect("Can't download protoc"); - let archive = archive.bytes().await.expect("Can't download protoc"); - let archive = std::io::Cursor::new(archive); - zip_extract::extract(archive, &target_dir, true).expect("Can't extract protoc"); + if std::env::var("PROTOC").is_err() { + let target_dir = out.join("protoc"); + if !target_dir.exists() { + let archive = reqwest::get(proto_url) + .await + .expect("Can't download protoc"); + let archive = archive.bytes().await.expect("Can't download protoc"); + let archive = std::io::Cursor::new(archive); + zip_extract::extract(archive, &target_dir, true).expect("Can't extract protoc"); + } + std::env::set_var("PROTOC", out.join(format!("protoc/bin/protoc{suffix}"))); } - std::env::set_var("PROTOC", out.join(format!("protoc/bin/protoc{suffix}"))); prost_build::Config::new() .file_descriptor_set_path(&descriptor_path) From 83b4f0645dbe7d994d5ad0f8ca2f5aeac6bb7699 Mon Sep 17 00:00:00 2001 From: Brent Gardner Date: Mon, 24 Oct 2022 17:42:06 -0600 Subject: [PATCH 5/5] fmt --- datafusion/proto/build.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/datafusion/proto/build.rs b/datafusion/proto/build.rs index 03bff06bfb85..29b72d82914c 100644 --- a/datafusion/proto/build.rs +++ b/datafusion/proto/build.rs @@ -60,7 +60,8 @@ async fn build() -> Result<(), String> { .expect("Can't download protoc"); let archive = archive.bytes().await.expect("Can't download protoc"); let archive = std::io::Cursor::new(archive); - zip_extract::extract(archive, &target_dir, true).expect("Can't extract protoc"); + zip_extract::extract(archive, &target_dir, true) + .expect("Can't extract protoc"); } std::env::set_var("PROTOC", out.join(format!("protoc/bin/protoc{suffix}"))); }