From aecff0e966bf3ac2b3d85f17ae8140fc8faf994f Mon Sep 17 00:00:00 2001 From: Wendell Smith Date: Wed, 8 Jan 2025 03:00:12 -0500 Subject: [PATCH] build(deps,ci): add `protoc` feature to build `protoc` from source (#356) * build(deps): add protoc feature that depends on protobuf-src, and use it in docs.rs * build(deps): also add protoc feature to py, c, and tests crates * build(ci): update Rust workflow to use protoc feature, using vendored protoc Also test with and without vendored protoc * build: update version diff template --- .github/workflows/rust.yml | 40 ++++++++++++-------------------------- Cargo.lock | 21 ++++++++++++++++++++ c/Cargo.toml | 4 ++++ ci/version-diff-template | 24 +++++++++++------------ py/Cargo.toml | 7 +++++++ py/build.rs | 14 ++++++++++++- rs/Cargo.toml | 11 +++++++++++ rs/build.rs | 4 ++++ tests/Cargo.toml | 6 ++++++ tests/src/find_protoc.rs | 16 ++++++++++++++- 10 files changed, 105 insertions(+), 42 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 74ed1cdc..af0af547 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -14,11 +14,6 @@ jobs: - uses: actions/checkout@v4 with: submodules: recursive - - uses: arduino/setup-protoc@v3 - with: - version: "28.3" - # avoid rate-limiting - repo-token: ${{ secrets.GITHUB_TOKEN }} - uses: dtolnay/rust-toolchain@stable - uses: Swatinem/rust-cache@v2 - name: Check @@ -35,19 +30,23 @@ jobs: - uses: actions/checkout@v4 with: submodules: recursive - - uses: arduino/setup-protoc@v3 - with: - version: "28.3" - # avoid rate-limiting - repo-token: ${{ secrets.GITHUB_TOKEN }} - uses: dtolnay/rust-toolchain@stable - uses: actions/setup-python@v5 with: python-version: "3.x" if: ${{ matrix.os == 'ubuntu-latest' }} - uses: Swatinem/rust-cache@v2 - - name: Run unit tests - run: cargo test --all-features + - name: Run unit tests with vendored protoc + run: cargo test --all-features # Uses vendored protoc; not compatible with windows + if: ${{ matrix.os != 'windows-latest' }} + - uses: arduino/setup-protoc@v3 # Download and 'install' protoc + with: + version: "28.3" + # avoid rate-limiting + repo-token: ${{ secrets.GITHUB_TOKEN }} + - name: Run unit tests with system protoc + run: cargo test + if: ${{ matrix.os != 'windows-latest' }} - name: Install test runner dependencies run: python3 -m pip install --upgrade pip protobuf pyyaml click if: ${{ matrix.os == 'ubuntu-latest' }} @@ -77,11 +76,6 @@ jobs: - uses: actions/checkout@v4 with: submodules: recursive - - uses: arduino/setup-protoc@v3 - with: - version: "28.3" - # avoid rate-limiting - repo-token: ${{ secrets.GITHUB_TOKEN }} - uses: dtolnay/rust-toolchain@stable with: components: clippy @@ -96,11 +90,6 @@ jobs: - uses: actions/checkout@v4 with: submodules: recursive - - uses: arduino/setup-protoc@v3 - with: - version: "28.3" - # avoid rate-limiting - repo-token: ${{ secrets.GITHUB_TOKEN }} - uses: dtolnay/rust-toolchain@stable - uses: Swatinem/rust-cache@v2 - name: Doc @@ -116,11 +105,6 @@ jobs: - uses: actions/checkout@v4 with: submodules: recursive - - uses: arduino/setup-protoc@v3 - with: - version: "28.3" - # avoid rate-limiting - repo-token: ${{ secrets.GITHUB_TOKEN }} - uses: dtolnay/rust-toolchain@stable - name: Fetch Substrait submodule tags working-directory: substrait @@ -139,4 +123,4 @@ jobs: working-directory: rs env: CARGO_REGISTRY_TOKEN: ${{ secrets.CARGO_REGISTRY_TOKEN }} - run: cargo publish --allow-dirty + run: cargo publish --allow-dirty --features protoc diff --git a/Cargo.lock b/Cargo.lock index 71cb50db..37161a67 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -256,6 +256,15 @@ version = "0.7.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1462739cb27611015575c0c11df5df7601141071f07518d56fcc1be504cbec97" +[[package]] +name = "cmake" +version = "0.1.52" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c682c223677e0e5b6b7f63a64b9351844c3f1b1678a68b7ee617e30fb082620e" +dependencies = [ + "cc", +] + [[package]] name = "colorchoice" version = "1.0.3" @@ -1096,6 +1105,15 @@ dependencies = [ "prost", ] +[[package]] +name = "protobuf-src" +version = "2.1.0+27.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a7edafa3bcc668fa93efafcbdf58d7821bbda0f4b458ac7fae3d57ec0fec8167" +dependencies = [ + "cmake", +] + [[package]] name = "pyo3" version = "0.22.5" @@ -1427,6 +1445,7 @@ dependencies = [ "prost", "prost-build", "prost-types", + "protobuf-src", "regex", "semver", "serde_json", @@ -1466,6 +1485,7 @@ version = "0.1.1" dependencies = [ "dunce", "prost-build", + "protobuf-src", "pyo3", "pyo3-build-config 0.23.3", "substrait-validator", @@ -1519,6 +1539,7 @@ version = "0.1.1" dependencies = [ "glob", "prost-build", + "protobuf-src", "rayon", "serde", "serde_json", diff --git a/c/Cargo.toml b/c/Cargo.toml index f857f523..c326ac4d 100644 --- a/c/Cargo.toml +++ b/c/Cargo.toml @@ -8,6 +8,10 @@ license = "Apache-2.0" crate-type = ["cdylib", "staticlib"] doc = false +[features] +# Build and vendor protoc from source. See also rs/Cargo.toml. +protoc = ["substrait-validator/protoc"] + [build-dependencies] cbindgen = "0.27.0" diff --git a/ci/version-diff-template b/ci/version-diff-template index 6bbe4926..c66fb3c0 100644 --- a/ci/version-diff-template +++ b/ci/version-diff-template @@ -1,5 +1,5 @@ diff --git a/c/Cargo.toml b/c/Cargo.toml -index d8ad49b..4bd02ef 100644 +index 114b0cc..fc9ce3e 100644 --- a/c/Cargo.toml +++ b/c/Cargo.toml @@ -1,6 +1,6 @@ @@ -10,7 +10,7 @@ index d8ad49b..4bd02ef 100644 edition = "2021" license = "Apache-2.0" -@@ -12,7 +12,7 @@ doc = false +@@ -16,7 +16,7 @@ protoc = ["substrait-validator/protoc"] cbindgen = "0.27.0" [dependencies] @@ -20,7 +20,7 @@ index d8ad49b..4bd02ef 100644 thiserror = "2.0" once_cell = "1.19" diff --git a/derive/Cargo.toml b/derive/Cargo.toml -index 1cf1f7e..482c127 100644 +index b4ba0fc..7c24520 100644 --- a/derive/Cargo.toml +++ b/derive/Cargo.toml @@ -4,7 +4,7 @@ description = "Procedural macros for substrait-validator" @@ -33,7 +33,7 @@ index 1cf1f7e..482c127 100644 license = "Apache-2.0" diff --git a/py/Cargo.toml b/py/Cargo.toml -index d28af20..28abfb1 100644 +index 1ea9db1..027b71d 100644 --- a/py/Cargo.toml +++ b/py/Cargo.toml @@ -1,6 +1,6 @@ @@ -44,7 +44,7 @@ index d28af20..28abfb1 100644 edition = "2018" license = "Apache-2.0" include = [ -@@ -29,7 +29,7 @@ name = "substrait_validator" +@@ -33,7 +33,7 @@ name = "substrait_validator" doc = false [dependencies] @@ -54,7 +54,7 @@ index d28af20..28abfb1 100644 [build-dependencies] diff --git a/py/pyproject.toml b/py/pyproject.toml -index 6e602e6..dd144de 100644 +index b3b3464..dd144de 100644 --- a/py/pyproject.toml +++ b/py/pyproject.toml @@ -5,7 +5,7 @@ backend-path = ["."] @@ -67,7 +67,7 @@ index 6e602e6..dd144de 100644 readme = "README.md" license = {{ file = "LICENSE" }} diff --git a/rs/Cargo.toml b/rs/Cargo.toml -index 4144f94..791db63 100644 +index 1f9f762..84f67fa 100644 --- a/rs/Cargo.toml +++ b/rs/Cargo.toml @@ -4,7 +4,7 @@ description = "Substrait validator" @@ -79,7 +79,7 @@ index 4144f94..791db63 100644 edition = "2021" license = "Apache-2.0" include = ["src", "build.rs", "README.md"] -@@ -24,7 +24,7 @@ prost-types = "0.13.3" +@@ -29,7 +29,7 @@ prost-types = "0.13.4" # Prost doesn't generate any introspection stuff, so we hack that stuff in with # our own procedural macros. @@ -89,7 +89,7 @@ index 4144f94..791db63 100644 # Google/protobuf has a funny idea about case conventions (it converts them all # over the place) and prost remaps to Rust's conventions to boot. So, to diff --git a/rs/README.md b/rs/README.md -index 14f8216..5d908fe 100644 +index afead27..5d908fe 100644 --- a/rs/README.md +++ b/rs/README.md @@ -6,7 +6,7 @@ plans. @@ -111,7 +111,7 @@ index 14f8216..5d908fe 100644 This adds the `substrait_validator::Config::add_curl_yaml_uri_resolver()` diff --git a/tests/Cargo.toml b/tests/Cargo.toml -index cf1ba03..4f6c8d6 100644 +index 62d2f87..79cb4d5 100644 --- a/tests/Cargo.toml +++ b/tests/Cargo.toml @@ -1,6 +1,6 @@ @@ -122,8 +122,8 @@ index cf1ba03..4f6c8d6 100644 edition = "2018" license = "Apache-2.0" default-run = "runner" -@@ -14,7 +14,7 @@ name = "find_protoc" - path = "src/find_protoc.rs" +@@ -18,7 +18,7 @@ path = "src/find_protoc.rs" + protoc = ["dep:protobuf-src", "substrait-validator/protoc"] [dependencies] -substrait-validator = {{ path = "../rs", version = "{frm}" }} diff --git a/py/Cargo.toml b/py/Cargo.toml index d1c99eda..1ea9db14 100644 --- a/py/Cargo.toml +++ b/py/Cargo.toml @@ -16,6 +16,10 @@ include = [ "/tests", ] +[features] +# Build and vendor protoc from source. See also rs/Cargo.toml. +protoc = ["dep:protobuf-src", "substrait-validator/protoc"] + [lib] crate-type = ["cdylib"] @@ -37,3 +41,6 @@ prost-build = "0.13.4" pyo3-build-config = "0.23.3" walkdir = "2" dunce = "1" + +# Used in the 'protoc' feature. +protobuf-src = { version = "2.1", optional = true } \ No newline at end of file diff --git a/py/build.rs b/py/build.rs index 7a360b35..2a2522f7 100644 --- a/py/build.rs +++ b/py/build.rs @@ -78,7 +78,19 @@ fn main() { fs::create_dir_all(&intermediate_path).expect("failed to create protoc output directory"); // Run protoc. - let mut cmd = Command::new(prost_build::protoc_from_env()); + let protoc_path: PathBuf; + #[cfg(feature = "protoc")] + { + // Use vendored protobuf compiler if requested. + protoc_path = protobuf_src::protoc(); + println!("cargo:warning=Using vendored protoc: {:?}", protoc_path); + } + #[cfg(not(feature = "protoc"))] + { + protoc_path = prost_build::protoc_from_env(); + } + + let mut cmd = Command::new(protoc_path); for input_path in input_paths.iter() { let mut proto_path_arg = OsString::new(); proto_path_arg.push("--proto_path="); diff --git a/rs/Cargo.toml b/rs/Cargo.toml index 46a91127..1f9f7629 100644 --- a/rs/Cargo.toml +++ b/rs/Cargo.toml @@ -16,6 +16,11 @@ include = ["src", "build.rs", "README.md"] # functionally change anything. private_docs = [] +# Build and vendor protoc from source. This is useful to have a consistent +# version of protoc across platforms/build systems. In particular, docs.rs uses +# it. +protoc = ["dep:protobuf-src"] + [dependencies] # Prost is used to deal with protobuf serialization and deserialization. @@ -93,5 +98,11 @@ semver = "1.0" # Used for generating Rust structs from the protobuf definitions. prost-build = "0.13.4" +# Used in the 'protoc' feature. +protobuf-src = { version = "2.1", optional = true } + # Used to automatically find all protobuf files. walkdir = "2" + +[package.metadata.docs.rs] +features = ["protoc"] diff --git a/rs/build.rs b/rs/build.rs index 72647245..49b05f11 100644 --- a/rs/build.rs +++ b/rs/build.rs @@ -129,6 +129,10 @@ fn main() -> Result<()> { } } + #[cfg(feature = "protoc")] + // Use vendored protobuf compiler if requested. + std::env::set_var("PROTOC", protobuf_src::protoc()); + // Find all protobuf files in our resource directory. We just synchronized // these files if we're building from git. let proto_path = PathBuf::from(&resource_dir).join("proto"); diff --git a/tests/Cargo.toml b/tests/Cargo.toml index bfdd0353..62d2f874 100644 --- a/tests/Cargo.toml +++ b/tests/Cargo.toml @@ -13,6 +13,10 @@ path = "src/runner.rs" name = "find_protoc" path = "src/find_protoc.rs" +[features] +# Build and vendor protoc from source. See also rs/Cargo.toml. +protoc = ["dep:protobuf-src", "substrait-validator/protoc"] + [dependencies] substrait-validator = { path = "../rs", version = "0.1.1" } serde = { version = "1.0", features = ["derive"] } @@ -21,3 +25,5 @@ walkdir = "2" glob = "0.3" prost-build = "0.13.4" rayon = "1.10" +# Used in the 'protoc' feature. +protobuf-src = { version = "2.1", optional = true } \ No newline at end of file diff --git a/tests/src/find_protoc.rs b/tests/src/find_protoc.rs index 3ce72ac7..a5e0ec1d 100644 --- a/tests/src/find_protoc.rs +++ b/tests/src/find_protoc.rs @@ -3,6 +3,20 @@ //! Prost has some magic for finding the path to protoc, so let's use that in //! the Python code as well... +use std::path::PathBuf; + fn main() { - println!("{}", prost_build::protoc_from_env().display()); + let protoc_path: PathBuf; + #[cfg(feature = "protoc")] + { + // Use vendored protobuf compiler if requested. + protoc_path = protobuf_src::protoc(); + println!("cargo:warning=Using vendored protoc: {:?}", protoc_path); + } + #[cfg(not(feature = "protoc"))] + { + protoc_path = prost_build::protoc_from_env(); + } + + println!("{}", protoc_path.display()); }