Skip to content

Commit

Permalink
build(protobuf): Use crate cmake (#1137)
Browse files Browse the repository at this point in the history
Let the crate `cmake` handle the interaction with CMake executable. This simplifies our codebase and improves compatibility with other platforms.

`CMAKE_BUILD_TYPE` is handle automatically based on `opt-level`.

Set `CMAKE_GENERATOR` environment variable in CI instead of hardcoded dependency on `ninja-build`.
  • Loading branch information
caspermeijn authored Aug 28, 2024
1 parent 21208ab commit 8424775
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 27 deletions.
12 changes: 9 additions & 3 deletions .github/actions/setup-ninja/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,21 @@ runs:
steps:
- name: install ninja
if: runner.os == 'macOS'
run: brew install ninja
run: |
brew install ninja
echo "CMAKE_GENERATOR=Ninja" >> "$GITHUB_ENV"
shell: bash

- name: install ninja
if: runner.os == 'Windows'
run: choco install ninja
run: |
choco install ninja
echo "CMAKE_GENERATOR=Ninja" >> "$GITHUB_ENV"
shell: bash

- name: install ninja
if: runner.os == 'Linux'
run: sudo apt-get install ninja-build
run: |
sudo apt-get install ninja-build
echo "CMAKE_GENERATOR=Ninja" >> "$GITHUB_ENV"
shell: bash
1 change: 1 addition & 0 deletions protobuf/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ prost-types = { path = "../prost-types" }
anyhow = "1.0.1"
prost-build = { path = "../prost-build" }
tempfile = "3"
cmake = "0.1.51"

[package.metadata.cargo-machete]
ignored = ["prost", "prost-types"]
32 changes: 8 additions & 24 deletions protobuf/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,35 +118,19 @@ fn install_conformance_test_runner(
prefix_dir: &Path,
) -> Result<()> {
// Build and install protoc, the protobuf libraries, and the conformance test runner.
let rc = Command::new("cmake")
.arg("-GNinja")
.arg(src_dir.join("cmake"))
.arg("-DCMAKE_BUILD_TYPE=DEBUG")
.arg(format!("-DCMAKE_INSTALL_PREFIX={}", prefix_dir.display()))
.arg("-Dprotobuf_BUILD_CONFORMANCE=ON")
.arg("-Dprotobuf_BUILD_TESTS=OFF")
.current_dir(build_dir)
.status()
.context("failed to execute CMake")?;
assert!(rc.success(), "protobuf CMake failed");

let num_jobs = env::var("NUM_JOBS").context("NUM_JOBS environment variable not set")?;

let rc = Command::new("ninja")
.arg("-j")
.arg(&num_jobs)
.arg("install")
.current_dir(build_dir)
.status()
.context("failed to execute ninja protobuf")?;
ensure!(rc.success(), "failed to make protobuf");
cmake::Config::new(src_dir.join("cmake"))
.define("CMAKE_INSTALL_PREFIX", prefix_dir)
.define("protobuf_BUILD_CONFORMANCE", "ON")
.define("protobuf_BUILD_TESTS", "OFF")
.out_dir(build_dir)
.build();

// Install the conformance-test-runner binary, since it isn't done automatically.
fs::copy(
build_dir.join("conformance_test_runner"),
build_dir.join("build").join("conformance_test_runner"),
prefix_dir.join("bin").join("conformance-test-runner"),
)
.context("failed to move conformance-test-runner")?;
.context("failed to copy conformance-test-runner")?;

Ok(())
}

0 comments on commit 8424775

Please sign in to comment.