From fec6eb16c13752055f2d4cdfd4555dee78acaf68 Mon Sep 17 00:00:00 2001 From: Parker Timmerman Date: Wed, 29 Jan 2025 10:56:42 -0500 Subject: [PATCH] [build] Get the `npm` crate working with Bazel (#31220) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR fixes our flamegraph rendering with our builtin memory and CPU profiling. Previously the vendored JS and CSS artifacts were not getting properly included into our build because the Cargo Build script that is responsible for downloading them gets run in a different sandbox than the action that builds the library. Now the `npm::ensure(...)` call optionally takes an `OUT_DIR` param that it will copy the artifacts into and thus they will get included as part of the Bazel build. I manually built the `materialized` Docker image with Bazel and confirmed the flamegraphs worked! ### Motivation Fix our builtin memory and CPU profiling ### Tips for reviewer ### Checklist - [ ] This PR has adequate test coverage / QA involvement has been duly considered. ([trigger-ci for additional test/nightly runs](https://trigger-ci.dev.materialize.com/)) - [ ] This PR has an associated up-to-date [design doc](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/design/README.md), is a design doc ([template](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/design/00000000_template.md)), or is sufficiently small to not require a design. - [ ] If this PR evolves [an existing `$T ⇔ Proto$T` mapping](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/command-and-response-binary-encoding.md) (possibly in a backwards-incompatible way), then it is tagged with a `T-proto` label. - [ ] If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label ([example](https://github.com/MaterializeInc/cloud/pull/5021)). - [ ] If this PR includes major [user-facing behavior changes](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/guide-changes.md#what-changes-require-a-release-note), I have pinged the relevant PM to schedule a changelog post. --- src/environmentd/build.rs | 3 ++- src/environmentd/src/http/root.rs | 7 ++++--- src/http-util/src/lib.rs | 16 +++++++++++++--- src/npm/src/lib.rs | 21 +++++++++++++++++---- src/prof-http/build.rs | 3 ++- src/prof-http/src/lib.rs | 7 ++++--- 6 files changed, 42 insertions(+), 15 deletions(-) diff --git a/src/environmentd/build.rs b/src/environmentd/build.rs index cda91dbc28b2b..961fe1962c955 100644 --- a/src/environmentd/build.rs +++ b/src/environmentd/build.rs @@ -16,5 +16,6 @@ fn main() -> Result<(), anyhow::Error> { .file("src/environmentd/sys.c") .compile("environmentd_sys"); - mz_npm::ensure() + let out_dir = std::env::var("OUT_DIR").ok().map(std::path::PathBuf::from); + mz_npm::ensure(out_dir) } diff --git a/src/environmentd/src/http/root.rs b/src/environmentd/src/http/root.rs index 6b29fe3da7204..aa7530c75a12b 100644 --- a/src/environmentd/src/http/root.rs +++ b/src/environmentd/src/http/root.rs @@ -31,7 +31,8 @@ pub async fn handle_home(profiling: bool) -> impl IntoResponse { } mz_http_util::make_handle_static!( - include_dir::include_dir!("$CARGO_MANIFEST_DIR/src/http/static"), - "src/http/static", - "src/http/static-dev" + dir_1: ::include_dir::include_dir!("$CARGO_MANIFEST_DIR/src/http/static"), + dir_2: ::include_dir::include_dir!("$OUT_DIR/src/http/static"), + prod_base_path: "src/http/static", + dev_base_path: "src/http/static-dev", ); diff --git a/src/http-util/src/lib.rs b/src/http-util/src/lib.rs index 6aadc9e644674..18c27324fa5b5 100644 --- a/src/http-util/src/lib.rs +++ b/src/http-util/src/lib.rs @@ -38,17 +38,27 @@ where /// and two strings representing the (crate-local) paths to the production and development /// static files. macro_rules! make_handle_static { - ($static_dir:expr, $prod_base_path:expr, $dev_base_path:expr) => { + ( + dir_1: $dir_1:expr, + $(dir_2: $dir_2:expr,)? + prod_base_path: $prod_base_path:expr, + dev_base_path: $dev_base_path:expr$(,)? + ) => { #[allow(clippy::unused_async)] pub async fn handle_static( path: ::axum::extract::Path, ) -> impl ::axum::response::IntoResponse { #[cfg(not(feature = "dev-web"))] - const STATIC_DIR: ::include_dir::Dir = $static_dir; + const DIR_1: ::include_dir::Dir = $dir_1; + $( + #[cfg(not(feature = "dev-web"))] + const DIR_2: ::include_dir::Dir = $dir_2; + )? + #[cfg(not(feature = "dev-web"))] fn get_static_file(path: &str) -> Option<&'static [u8]> { - STATIC_DIR.get_file(path).map(|f| f.contents()) + DIR_1.get_file(path).or_else(|| DIR_2.get_file(path)).map(|f| f.contents()) } #[cfg(feature = "dev-web")] diff --git a/src/npm/src/lib.rs b/src/npm/src/lib.rs index c3f8eef64f8ed..3639ad8485e26 100644 --- a/src/npm/src/lib.rs +++ b/src/npm/src/lib.rs @@ -196,7 +196,7 @@ impl NpmPackage { } } -pub fn ensure() -> Result<(), anyhow::Error> { +pub fn ensure(out_dir: Option) -> Result<(), anyhow::Error> { println!("ensuring all npm packages are up-to-date..."); let client = reqwest::blocking::Client::new(); @@ -273,9 +273,22 @@ expected: {} for dir in &[CSS_VENDOR, JS_PROD_VENDOR, JS_DEV_VENDOR] { for entry in WalkDir::new(dir) { let entry = entry?; - if entry.file_type().is_file() && !known_paths.contains(entry.path()) { - println!("removing stray vendor file {}", entry.path().display()); - fs::remove_file(entry.path())?; + if entry.file_type().is_file() { + if !known_paths.contains(entry.path()) { + println!("removing stray vendor file {}", entry.path().display()); + fs::remove_file(entry.path())?; + } else if let Some(out_dir) = &out_dir { + let dst_path = out_dir.join(entry.path()); + println!( + "copying path to OUT_DIR, src {}, dst {}", + entry.path().display(), + dst_path.display(), + ); + if let Some(parent) = dst_path.parent() { + fs::create_dir_all(parent)?; + } + fs::copy(entry.path(), dst_path)?; + } } } } diff --git a/src/prof-http/build.rs b/src/prof-http/build.rs index 3d32d79f168f9..782a59286b493 100644 --- a/src/prof-http/build.rs +++ b/src/prof-http/build.rs @@ -8,5 +8,6 @@ // by the Apache License, Version 2.0. fn main() -> Result<(), anyhow::Error> { - mz_npm::ensure() + let out_dir = std::env::var("OUT_DIR").ok().map(std::path::PathBuf::from); + mz_npm::ensure(out_dir) } diff --git a/src/prof-http/src/lib.rs b/src/prof-http/src/lib.rs index 5736f41dc564c..5b389f63d2277 100644 --- a/src/prof-http/src/lib.rs +++ b/src/prof-http/src/lib.rs @@ -42,9 +42,10 @@ static EXECUTABLE: LazyLock = LazyLock::new(|| { }); mz_http_util::make_handle_static!( - include_dir::include_dir!("$CARGO_MANIFEST_DIR/src/http/static"), - "src/http/static", - "src/http/static-dev" + dir_1: ::include_dir::include_dir!("$CARGO_MANIFEST_DIR/src/http/static"), + dir_2: ::include_dir::include_dir!("$OUT_DIR/src/http/static"), + prod_base_path: "src/http/static", + dev_base_path: "src/http/static-dev", ); /// Creates a router that serves the profiling endpoints.