From b8487d3045a51b498760b9f43ceb4690eca7e838 Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Tue, 9 Jun 2026 10:57:04 -0700 Subject: [PATCH 1/2] add commit hash --- crates/pet/build.rs | 13 +++++++++++++ crates/pet/src/jsonrpc.rs | 11 +++++++++++ crates/pet/tests/jsonrpc_client.rs | 1 + crates/pet/tests/jsonrpc_server_test.rs | 6 ++++++ docs/JSONRPC.md | 7 +++++++ 5 files changed, 38 insertions(+) diff --git a/crates/pet/build.rs b/crates/pet/build.rs index bf8bd6a8..818f2c96 100644 --- a/crates/pet/build.rs +++ b/crates/pet/build.rs @@ -4,6 +4,9 @@ fn main() { println!("cargo:rerun-if-env-changed=PET_BUILD_ID"); println!("cargo:rerun-if-env-changed=BUILD_BUILDID"); + println!("cargo:rerun-if-env-changed=PET_COMMIT_SHA"); + println!("cargo:rerun-if-env-changed=BUILD_SOURCEVERSION"); + println!("cargo:rerun-if-env-changed=GITHUB_SHA"); if let Some(build_id) = std::env::var("PET_BUILD_ID") .ok() @@ -13,6 +16,16 @@ fn main() { println!("cargo:rustc-env=PET_BUILD_ID={build_id}"); } + // BUILD_SOURCEVERSION is set by Azure Pipelines; GITHUB_SHA by GitHub Actions. + if let Some(commit_sha) = std::env::var("PET_COMMIT_SHA") + .ok() + .or_else(|| std::env::var("BUILD_SOURCEVERSION").ok()) + .or_else(|| std::env::var("GITHUB_SHA").ok()) + .filter(|value| !value.is_empty()) + { + println!("cargo:rustc-env=PET_COMMIT_SHA={commit_sha}"); + } + #[cfg(target_os = "windows")] { if std::env::var("CARGO_BIN_NAME").is_err() { diff --git a/crates/pet/src/jsonrpc.rs b/crates/pet/src/jsonrpc.rs index 8ff6c2c2..81abbc97 100644 --- a/crates/pet/src/jsonrpc.rs +++ b/crates/pet/src/jsonrpc.rs @@ -523,6 +523,8 @@ pub struct InfoResponse { pub pet_version: String, #[serde(skip_serializing_if = "Option::is_none")] pub build_id: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub commit_sha: Option, } impl InfoResponse { @@ -532,6 +534,9 @@ impl InfoResponse { build_id: option_env!("PET_BUILD_ID") .filter(|value| !value.is_empty()) .map(ToString::to_string), + commit_sha: option_env!("PET_COMMIT_SHA") + .filter(|value| !value.is_empty()) + .map(ToString::to_string), } } } @@ -1541,10 +1546,16 @@ mod tests { let info = InfoResponse::current(); assert_eq!(info.pet_version, env!("CARGO_PKG_VERSION")); + // build_id / commit_sha are populated from env vars set at compile time by CI. + // For local dev builds they will be None; assert non-empty only when present. assert!(info .build_id .as_deref() .is_none_or(|build_id| !build_id.is_empty())); + assert!(info + .commit_sha + .as_deref() + .is_none_or(|commit_sha| !commit_sha.is_empty())); } #[test] diff --git a/crates/pet/tests/jsonrpc_client.rs b/crates/pet/tests/jsonrpc_client.rs index a10270b4..9612bce1 100644 --- a/crates/pet/tests/jsonrpc_client.rs +++ b/crates/pet/tests/jsonrpc_client.rs @@ -26,6 +26,7 @@ pub struct RefreshResult { pub struct PetInfoResponse { pub pet_version: String, pub build_id: Option, + pub commit_sha: Option, } #[derive(Debug, Clone, Deserialize, PartialEq, Eq)] diff --git a/crates/pet/tests/jsonrpc_server_test.rs b/crates/pet/tests/jsonrpc_server_test.rs index 4e16ed5c..99762dd6 100644 --- a/crates/pet/tests/jsonrpc_server_test.rs +++ b/crates/pet/tests/jsonrpc_server_test.rs @@ -117,10 +117,16 @@ fn info_reports_pet_version_and_optional_build_id() { let info = client.info().expect("info request failed"); assert_eq!(info.pet_version, env!("CARGO_PKG_VERSION")); + // build_id / commit_sha are populated from env vars set at compile time by CI. + // For local dev builds they will be None; assert non-empty only when present. assert!(info .build_id .as_deref() .is_none_or(|build_id| !build_id.is_empty())); + assert!(info + .commit_sha + .as_deref() + .is_none_or(|commit_sha| !commit_sha.is_empty())); } #[test] diff --git a/docs/JSONRPC.md b/docs/JSONRPC.md index a2388833..85292326 100644 --- a/docs/JSONRPC.md +++ b/docs/JSONRPC.md @@ -35,8 +35,15 @@ interface InfoResponse { petVersion: string; /** * Build identifier baked into the binary when built by CI. + * Sourced from `PET_BUILD_ID` or Azure Pipelines `BUILD_BUILDID`. */ buildId?: string; + /** + * Source git commit SHA baked into the binary when built by CI. + * Sourced from `PET_COMMIT_SHA`, Azure Pipelines `BUILD_SOURCEVERSION`, + * or GitHub Actions `GITHUB_SHA`. Absent for local dev builds. + */ + commitSha?: string; } ``` From 356f4af54bcccdffa11f34603bcab31f8ff7522b Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Tue, 9 Jun 2026 13:25:54 -0700 Subject: [PATCH 2/2] address PR feedback: filter empty env vars per-candidate, add JSON wire-format test, rename tests - build.rs: push empty-string filter into each candidate of build_id/commit_sha chains so an explicitly-set-but-empty primary var (e.g. PET_COMMIT_SHA from an unset GitHub Actions context) doesn't short-circuit fallbacks. - jsonrpc.rs: add test_info_response_serializes_camel_case_and_omits_none covering camelCase rename + skip_serializing_if without requiring CI env vars at compile time. - rename existing tests to ..._optional_build_metadata for accurate scope. --- crates/pet/build.rs | 20 +++++++++++++++--- crates/pet/src/jsonrpc.rs | 27 ++++++++++++++++++++++++- crates/pet/tests/jsonrpc_server_test.rs | 2 +- 3 files changed, 44 insertions(+), 5 deletions(-) diff --git a/crates/pet/build.rs b/crates/pet/build.rs index 818f2c96..94877f2c 100644 --- a/crates/pet/build.rs +++ b/crates/pet/build.rs @@ -8,10 +8,16 @@ fn main() { println!("cargo:rerun-if-env-changed=BUILD_SOURCEVERSION"); println!("cargo:rerun-if-env-changed=GITHUB_SHA"); + // Filter empties per-candidate so an explicitly-set-but-empty primary var + // doesn't short-circuit the fallback chain (e.g., `PET_BUILD_ID=""`). if let Some(build_id) = std::env::var("PET_BUILD_ID") .ok() - .or_else(|| std::env::var("BUILD_BUILDID").ok()) .filter(|value| !value.is_empty()) + .or_else(|| { + std::env::var("BUILD_BUILDID") + .ok() + .filter(|value| !value.is_empty()) + }) { println!("cargo:rustc-env=PET_BUILD_ID={build_id}"); } @@ -19,9 +25,17 @@ fn main() { // BUILD_SOURCEVERSION is set by Azure Pipelines; GITHUB_SHA by GitHub Actions. if let Some(commit_sha) = std::env::var("PET_COMMIT_SHA") .ok() - .or_else(|| std::env::var("BUILD_SOURCEVERSION").ok()) - .or_else(|| std::env::var("GITHUB_SHA").ok()) .filter(|value| !value.is_empty()) + .or_else(|| { + std::env::var("BUILD_SOURCEVERSION") + .ok() + .filter(|value| !value.is_empty()) + }) + .or_else(|| { + std::env::var("GITHUB_SHA") + .ok() + .filter(|value| !value.is_empty()) + }) { println!("cargo:rustc-env=PET_COMMIT_SHA={commit_sha}"); } diff --git a/crates/pet/src/jsonrpc.rs b/crates/pet/src/jsonrpc.rs index 81abbc97..778ffaf1 100644 --- a/crates/pet/src/jsonrpc.rs +++ b/crates/pet/src/jsonrpc.rs @@ -1542,7 +1542,7 @@ mod tests { } #[test] - fn test_info_response_uses_package_version_and_optional_build_id() { + fn test_info_response_uses_package_version_and_optional_build_metadata() { let info = InfoResponse::current(); assert_eq!(info.pet_version, env!("CARGO_PKG_VERSION")); @@ -1558,6 +1558,31 @@ mod tests { .is_none_or(|commit_sha| !commit_sha.is_empty())); } + #[test] + fn test_info_response_serializes_camel_case_and_omits_none() { + // Guards the JSON wire format (camelCase rename + skip_serializing_if) + // independently of whether CI env vars were set at compile time. + let json = serde_json::to_value(InfoResponse { + pet_version: "1.2.3".to_string(), + build_id: Some("42".to_string()), + commit_sha: Some("abc123".to_string()), + }) + .unwrap(); + assert_eq!(json["petVersion"], "1.2.3"); + assert_eq!(json["buildId"], "42"); + assert_eq!(json["commitSha"], "abc123"); + + let json = serde_json::to_value(InfoResponse { + pet_version: "1.2.3".to_string(), + build_id: None, + commit_sha: None, + }) + .unwrap(); + assert_eq!(json["petVersion"], "1.2.3"); + assert!(json.get("buildId").is_none()); + assert!(json.get("commitSha").is_none()); + } + #[test] fn test_parse_refresh_options_rejects_non_empty_array() { assert!(parse_refresh_options(json!([{"searchKind": "Conda"}])).is_err()); diff --git a/crates/pet/tests/jsonrpc_server_test.rs b/crates/pet/tests/jsonrpc_server_test.rs index 99762dd6..bf59b06f 100644 --- a/crates/pet/tests/jsonrpc_server_test.rs +++ b/crates/pet/tests/jsonrpc_server_test.rs @@ -111,7 +111,7 @@ fn assert_single_environment( } #[test] -fn info_reports_pet_version_and_optional_build_id() { +fn info_reports_pet_version_and_optional_build_metadata() { let client = PetJsonRpcClient::spawn().expect("failed to spawn PET server"); let info = client.info().expect("info request failed");