Skip to content

Commit

Permalink
[build] Get the npm crate working with Bazel (#31220)
Browse files Browse the repository at this point in the history
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

<!--
Leave some tips for your reviewer, like:

    * The diff is much smaller if viewed with whitespace hidden.
    * [Some function/module/file] deserves extra attention.
* [Some function/module/file] is pure code movement and only needs a
skim.

Delete this section if no tips.
-->

### 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.
  <!-- Reference the design in the description. -->
- [ ] 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](MaterializeInc/cloud#5021)).
<!-- Ask in #team-cloud on Slack if you need help preparing the cloud
PR. -->
- [ ] 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.
  • Loading branch information
ParkMyCar authored Jan 29, 2025
1 parent d798e1f commit fec6eb1
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 15 deletions.
3 changes: 2 additions & 1 deletion src/environmentd/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
7 changes: 4 additions & 3 deletions src/environmentd/src/http/root.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
);
16 changes: 13 additions & 3 deletions src/http-util/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>,
) -> 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")]
Expand Down
21 changes: 17 additions & 4 deletions src/npm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ impl NpmPackage {
}
}

pub fn ensure() -> Result<(), anyhow::Error> {
pub fn ensure(out_dir: Option<PathBuf>) -> Result<(), anyhow::Error> {
println!("ensuring all npm packages are up-to-date...");

let client = reqwest::blocking::Client::new();
Expand Down Expand Up @@ -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)?;
}
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/prof-http/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
7 changes: 4 additions & 3 deletions src/prof-http/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,10 @@ static EXECUTABLE: LazyLock<String> = 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.
Expand Down

0 comments on commit fec6eb1

Please sign in to comment.