From 55380e7b2ac1b93b4f152950b1423c4f4ef55fb0 Mon Sep 17 00:00:00 2001 From: Sebastian Jeltsch Date: Tue, 20 May 2025 09:56:17 +0200 Subject: [PATCH] Change the JS build flow: no longer install JS deps during `cargo build` to avoid concurrency issues. Instead require a prior, workspace-wide install. Update CI and Docker builds. Also update documentation and improve error messages. --- .github/workflows/release.yml | 4 +++ .pre-commit-config.yaml | 4 +-- Dockerfile | 5 ++++ README.md | 20 +++++++++++-- client/trailbase-rs/src/lib.rs | 4 +-- trailbase-build/src/lib.rs | 52 +++++++++++++++++++--------------- 6 files changed, 59 insertions(+), 30 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 9f732916..2e59ac1e 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -49,6 +49,7 @@ jobs: default: true - name: Rust Build run: | + pnpm install && \ RUSTFLAGS="-C target-feature=+crt-static" \ CARGO_PROFILE_RELEASE_LTO=fat CARGO_PROFILE_RELEASE_CODEGEN_UNITS=1 \ cargo build --target x86_64-unknown-linux-gnu --release --bin trail && \ @@ -84,6 +85,7 @@ jobs: default: true - name: Rust Build run: | + pnpm install && \ CARGO_PROFILE_RELEASE_LTO=fat CARGO_PROFILE_RELEASE_CODEGEN_UNITS=1 \ cargo build --target aarch64-apple-darwin --release --bin trail && \ zip -r -j trailbase_${{ github.ref_name }}_arm64_apple_darwin.zip target/aarch64-apple-darwin/release/trail CHANGELOG.md LICENSE @@ -118,6 +120,7 @@ jobs: default: true - name: Rust Build run: | + pnpm install && \ CARGO_PROFILE_RELEASE_LTO=fat CARGO_PROFILE_RELEASE_CODEGEN_UNITS=1 \ cargo build --target x86_64-apple-darwin --release --bin trail && \ zip -r -j trailbase_${{ github.ref_name }}_x86_64_apple_darwin.zip target/x86_64-apple-darwin/release/trail CHANGELOG.md LICENSE @@ -147,6 +150,7 @@ jobs: - name: Rust Build shell: bash run: | + pnpm install && \ CARGO_PROFILE_RELEASE_LTO=fat CARGO_PROFILE_RELEASE_CODEGEN_UNITS=1 \ cargo build --target x86_64-pc-windows-msvc --release --bin trail && \ zip -r -j trailbase_${{ github.ref_name }}_x86_64_windows.zip target/x86_64-pc-windows-msvc/release/trail.exe CHANGELOG.md LICENSE diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index b573007f..405f8400 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -38,7 +38,7 @@ repos: name: Cargo Clippy # Be verbose to at least still see warnings scroll by. verbose: true - entry: cargo clippy --workspace --no-deps + entry: sh -c 'pnpm i && cargo clippy --workspace --no-deps' language: system types: [rust] exclude: '^vendor/' @@ -46,7 +46,7 @@ repos: - id: cargotest name: Cargo Test - entry: cargo test --workspace -- --show-output + entry: sh -c 'pnpm i && cargo test --workspace -- --show-output' language: system types: [rust] exclude: '^(vendor|bindings)/' diff --git a/Dockerfile b/Dockerfile index 2566765f..e6985640 100644 --- a/Dockerfile +++ b/Dockerfile @@ -27,6 +27,11 @@ RUN cargo chef cook --release --recipe-path recipe.json COPY . . +# First install all JS deps. This is to avoid collisions due to parallel +# installs later-on while building `trailbase-assets` (auth, admin, client) and +# `trailbase-js` (runtime). +RUN pnpm -r install --frozen-lockfile + ARG TARGETPLATFORM RUN case ${TARGETPLATFORM} in \ diff --git a/README.md b/README.md index 397af415..afe00e9b 100644 --- a/README.md +++ b/README.md @@ -122,12 +122,23 @@ If you have all the necessary dependencies (Rust, protobuf, node.js, pnpm) installed, you can build TrailBase by running: ```bash +# Windows only: make sure to enable symlinks (w/o `mklink` permissions for your +# user, git will fall back to copies). +$ git config core.symlinks true && git reset --hard + +# Download necessary git sub-modules. $ git submodule update --init --recursive -$ cargo build --release + +# Install Javascript dependencies first. Required for the next step. +$ pnpm install + +# Build the main executable. Adding `--release` will build a faster release +# binary but slow down the build significantly. +$ cargo build --bin trail ``` -To build a static binary you have to explicitly specify the target platform, -e.g. for Linux using glibc: +To build a static binary you'll need to explicitly specify the target platform, +e.g. Linux using the GNU glibc: ```bash $ RUSTFLAGS="-C target-feature=+crt-static" cargo build --target x86_64-unknown-linux-gnu --release @@ -137,7 +148,10 @@ Alternatively, if you want to build a Docker image or don't have to deal with build dependencies, you can simply run: ```bash +# Download necessary git sub-modules. $ git submodule update --init --recursive + +# Build the container as defined in `Dockerfile`. $ docker build . -t trailbase ``` diff --git a/client/trailbase-rs/src/lib.rs b/client/trailbase-rs/src/lib.rs index 4ba19ec9..5d1d24a0 100644 --- a/client/trailbase-rs/src/lib.rs +++ b/client/trailbase-rs/src/lib.rs @@ -272,7 +272,7 @@ pub enum CompareOp { } impl CompareOp { - fn to_str(&self) -> &'static str { + fn format(&self) -> &'static str { return match self { Self::Equal => "$eq", Self::NotEqual => "$ne", @@ -406,7 +406,7 @@ impl RecordApi { Cow::Owned(format!( "{path}[{col}][{op}]", col = filter.column, - op = op.to_str() + op = op.format() )), Cow::Owned(filter.value), )); diff --git a/trailbase-build/src/lib.rs b/trailbase-build/src/lib.rs index 96c67ef6..0f7d622b 100644 --- a/trailbase-build/src/lib.rs +++ b/trailbase-build/src/lib.rs @@ -74,22 +74,10 @@ pub fn pnpm_run(args: &[&str]) -> Result { write_output(std::io::stderr(), &output.stderr, &header)?; if !output.status.success() { - let msg = format!( + return Err(std::io::Error::other(format!( "Failed to run '{args:?}'\n\t{}", String::from_utf8_lossy(&output.stderr) - ); - - fn is_true(v: &str) -> bool { - return matches!(v.to_lowercase().as_str(), "true" | "1" | ""); - } - - // NOTE: We don't want to break backend-builds on frontend errors, at least for dev builds. - match env::var("SKIP_ERROR") { - Ok(v) if is_true(&v) => warn!("{}", msg), - _ => { - return Err(std::io::Error::other(msg)); - } - } + ))); } Ok(output) @@ -97,19 +85,37 @@ pub fn pnpm_run(args: &[&str]) -> Result { pub fn build_js(path: impl AsRef) -> Result<()> { let path = path.as_ref().to_string_lossy().to_string(); - // We deliberately choose not to use "--frozen-lockfile" here, since this is not a CI use-case. + // Note that `--frozen-lockfile` and `-ignore-workspace` are practically exclusive + // Because ignoring the workspace one, will require to create a new lockfile. let out_dir = std::env::var("OUT_DIR").unwrap(); - let _install_output = if out_dir.contains("target/package") { - pnpm_run(&["--dir", &path, "install", "--ignore-workspace"])? - } else if cfg!(windows) { - // We're having parallel installs into ./node_modules/.pnpm for trailbase-assets, - // trailbase-runtime, ... To avoid "ERR_PNPM_EBUSY" on windows avoid shared workspace installs. - pnpm_run(&["--dir", &path, "install", "--ignore-workspace"])? + let build_result = if out_dir.contains("target/package") { + // When we build cargo packages, we cannot rely on the workspace and prior installs. + pnpm_run(&["--dir", &path, "install", "--ignore-workspace"]) } else { - pnpm_run(&["--dir", &path, "install"])? + // `trailbase-asstes` and `trailbase-js` both build JS packages. We've seen issues with + // parallel installs in the past. We thus require all JS depdencies to be installed once + // upfront centrally into the workspace, i.e. `/node_modules`. + let args = ["--dir", &path, "install", "--frozen-lockfile", "--offline"]; + let build_result = pnpm_run(&args); + if build_result.is_err() { + error!( + "`pnpm {}` failed. Make sure to install all JS deps first using `pnpm install`", + args.join(" ") + ) + } + build_result }; - let _build_output = pnpm_run(&["--dir", &path, "build"])?; + let _ = build_result?; + + let build_output = pnpm_run(&["--dir", &path, "build"]); + if build_output.is_err() && cfg!(windows) { + error!( + "pnpm build failed on Windows. Make sure to enable symlinks: `git config core.symlinks true && git reset --hard`." + ); + } + + let _ = build_output?; return Ok(()); }