From 75821962c32a7b91fd0c9f3a6092b2014c0728c0 Mon Sep 17 00:00:00 2001 From: Tom Jakubowski Date: Thu, 4 Dec 2025 14:59:43 -0800 Subject: [PATCH 1/6] Add DCO check to prepush Signed-off-by: Tom Jakubowski --- .husky/pre-push | 1 - package.json | 2 +- tools/scripts/lint.mjs | 1 + tools/scripts/prepush.mjs | 70 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 72 insertions(+), 2 deletions(-) create mode 100644 tools/scripts/prepush.mjs diff --git a/.husky/pre-push b/.husky/pre-push index bf440db5fa..0a4359365c 100755 --- a/.husky/pre-push +++ b/.husky/pre-push @@ -1,4 +1,3 @@ #!/bin/sh -. "$(dirname "$0")/_/husky.sh" pnpm run prepush diff --git a/package.json b/package.json index f1b0c34c3c..eaea302546 100644 --- a/package.json +++ b/package.json @@ -90,7 +90,7 @@ "test_python": "node tools/scripts/test_python.mjs", "clean": "node tools/scripts/clean.mjs", "start": "npm run start --workspace", - "prepush": "npm run lint", + "prepush": "node tools/scripts/prepush.mjs && npm run lint", "prepare": "husky install", "lint": "node tools/scripts/lint.mjs", "fix": "node tools/scripts/fix.mjs", diff --git a/tools/scripts/lint.mjs b/tools/scripts/lint.mjs index 09469da89e..f0723b3345 100644 --- a/tools/scripts/lint.mjs +++ b/tools/scripts/lint.mjs @@ -53,6 +53,7 @@ if (import.meta.url.startsWith("file:")) { // if (process.env.PSP_PROJECT === "python") { // await import("./lint_python.mjs"); // } else { + await lint_js(); lint_python(); // } diff --git a/tools/scripts/prepush.mjs b/tools/scripts/prepush.mjs new file mode 100644 index 0000000000..e8d52b6cce --- /dev/null +++ b/tools/scripts/prepush.mjs @@ -0,0 +1,70 @@ +// ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ +// ┃ ██████ ██████ ██████ █ █ █ █ █ █▄ ▀███ █ ┃ +// ┃ ▄▄▄▄▄█ █▄▄▄▄▄ ▄▄▄▄▄█ ▀▀▀▀▀█▀▀▀▀▀ █ ▀▀▀▀▀█ ████████▌▐███ ███▄ ▀█ █ ▀▀▀▀▀ ┃ +// ┃ █▀▀▀▀▀ █▀▀▀▀▀ █▀██▀▀ ▄▄▄▄▄ █ ▄▄▄▄▄█ ▄▄▄▄▄█ ████████▌▐███ █████▄ █ ▄▄▄▄▄ ┃ +// ┃ █ ██████ █ ▀█▄ █ ██████ █ ███▌▐███ ███████▄ █ ┃ +// ┣━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫ +// ┃ Copyright (c) 2017, the Perspective Authors. ┃ +// ┃ ╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌ ┃ +// ┃ This file is part of the Perspective library, distributed under the terms ┃ +// ┃ of the [Apache License 2.0](https://www.apache.org/licenses/LICENSE-2.0). ┃ +// ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ + +import "zx/globals"; + +function lint_git(sha) { + if (!sha || typeof sha !== "string") { + throw new Error(`invalid sha: ${sha}`); + } + const result = $.sync`git log -1 ${sha} | grep -F "Signed-off-by: "`; + if (result.exitCode !== 0) { + console.error( + "`git log -1 " + + sha + + "` is missing a Signed-off-by: and DCO check will surely fail.", + ); + console.error("To sign off, run:\ngit commit --amend --edit --sign"); + process.exit(1); + } +} + +async function readPrePushInput() { + // Git supplies information about the push to the hook on stdin. + // https://git-scm.com/docs/githooks#_pre_push + const chunks = []; + + if (process.stdin.isTTY) { + // Makes developing the pre-push script more convenient. In particular + // when you run `pnpm run prepush` from a shell terminal you don't have + // to send EOF on stdin. + return []; + } + for await (const chunk of process.stdin) { + chunks.push(chunk); + } + + const input = Buffer.concat(chunks).toString(); + const lines = input.split("\n").filter((l) => l.length > 0); + return lines.map((line) => { + const parts = line.trim().split(" "); + + return { + local_ref: parts[0], + local_object_name: parts[1], + remote_ref: parts[2], + remote_object_name: parts[3], + }; + }); +} + +if (import.meta.main) { + // Does not actually run all pre-push hook checks (it does not run the repo + // lint script). These are lints which run only in pre-push. The + // `prepush` script defined in package.json is responsible for running the + // repo lint script. + const pushes = await readPrePushInput(); + for (const push of pushes) { + const { local_object_name } = push; + lint_git(local_object_name); + } +} From ed2941011ab346a2f4612b66293d8c352d101522 Mon Sep 17 00:00:00 2001 From: Tom Jakubowski Date: Wed, 3 Dec 2025 12:55:53 -0800 Subject: [PATCH 2/6] Start on building aarch64 linux wheels - Use fork of actions-setup-cmake install until arm support is upstreamed - Add arm runner to matrix + use manylinux container - Upload built artifacts Signed-off-by: Tom Jakubowski --- .github/actions/install-deps/action.yaml | 3 ++- .github/workflows/build.yaml | 29 ++++++++++++++++-------- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/.github/actions/install-deps/action.yaml b/.github/actions/install-deps/action.yaml index 12d587bab7..f855ef584e 100644 --- a/.github/actions/install-deps/action.yaml +++ b/.github/actions/install-deps/action.yaml @@ -56,7 +56,8 @@ runs: # https://github.com/open-telemetry/opentelemetry-cpp/issues/2998 - name: Setup cmake - uses: jwlawson/actions-setup-cmake@v2 + # uses: jwlawson/actions-setup-cmake@v2 + uses: tomjakubowski/actions-setup-cmake@v0-aarch64-test4 with: cmake-version: "3.29.6" diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index d088828a23..096c4d0aff 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -97,9 +97,6 @@ jobs: - name: Lint run: pnpm run lint --nightly - # - name: Docs Build - # run: pnpm run docs - - uses: actions/upload-artifact@v4 with: name: perspective-metadata @@ -188,6 +185,7 @@ jobs: matrix: os: - ubuntu-22.04 + - ubuntu-22.04-arm - macos-14 - windows-2022 arch: @@ -201,19 +199,25 @@ jobs: arch: x86_64 python-version: 3.9 container: pagmo2/manylinux228_x86_64_with_deps + - os: ubuntu-22.04-arm + arch: aarch64 + python-version: 3.9 + container: pagmo2/manylinux228_aarch64_with_deps is-release: - ${{ startsWith(github.ref, 'refs/tags/v') || github.ref_name == 'master' }} exclude: - os: windows-2022 arch: aarch64 - - os: ubuntu-22.04 - arch: aarch64 - os: macos-14 is-release: false - os: macos-14 arch: x86_64 - os: windows-2022 is-release: false + - os: ubuntu-22.04 + arch: aarch64 + - os: ubuntu-22.04-arm + arch: x86_64 steps: - name: Free up disk space @@ -244,7 +248,7 @@ jobs: - name: Python Build run: pnpm run build - if: ${{ !contains(matrix.os, 'windows') }} + if: ${{ runner.os != 'Windows' }} env: PACKAGE: "python" PSP_ARCH: ${{ matrix.arch }} @@ -255,7 +259,7 @@ jobs: run: | New-Item -ItemType Directory -Path $env:CARGO_TARGET_DIR -Force pnpm run build - if: ${{ contains(matrix.os, 'windows') }} + if: ${{ runner.os == 'Windows' }} env: CARGO_TARGET_DIR: D:\psp-rust PSP_CPP_BUILD_DIR: D:\psp-build @@ -658,7 +662,7 @@ jobs: matrix: os: - ubuntu-22.04 - - macos-14 + - ubuntu-22.04-arm - macos-14 - windows-2022 arch: @@ -685,6 +689,8 @@ jobs: arch: aarch64 - os: ubuntu-22.04 arch: aarch64 + - os: ubuntu-22.04-arm + arch: x86_64 steps: - name: Checkout uses: actions/checkout@v4 @@ -702,8 +708,7 @@ jobs: - uses: actions/download-artifact@v4 with: - # the macos-14 runner tests artifacts built on macos-14 - name: perspective-python-dist-${{ matrix.arch }}-${{ matrix.os == 'macos-14' && 'macos-14' || matrix.os }}-${{ matrix.python-version }} + name: perspective-python-dist-${{ matrix.arch }}-${{ matrix.os }}-${{ matrix.python-version }} - uses: ./.github/actions/install-wheel @@ -955,6 +960,10 @@ jobs: with: name: perspective-python-dist-aarch64-macos-14-3.9 + - uses: actions/download-artifact@v4 + with: + name: perspective-python-dist-aarch64-ubuntu-22.04-arm-3.9 + # - uses: actions/download-artifact@v4 # with: # name: perspective-python-dist-x86_64-macos-14-3.9 From b7a87b7756129e56d4f3f3bcd56c4064f65fff7b Mon Sep 17 00:00:00 2001 From: Konstantinos Samaras-Tsakiris Date: Wed, 3 Dec 2025 15:29:45 +0100 Subject: [PATCH 3/6] Add arm64 builds to CI - Ensure rustup gets the version appropriate for the architecture - Add a manylinux container for arm - Fix target for native arm compilation - Fix cmake install in pagmo2/arm64 - Fix protoc arch in cmake Signed-off-by: Konstantinos Samaras-Tsakiris Signed-off-by: Tom Jakubowski --- .github/actions/install-deps/action.yaml | 15 +++++++++++++++ rust/perspective-server/build.rs | 3 +++ .../cmake/modules/FindProtoc.cmake | 17 +++++++++++++---- 3 files changed, 31 insertions(+), 4 deletions(-) diff --git a/.github/actions/install-deps/action.yaml b/.github/actions/install-deps/action.yaml index f855ef584e..06edadf9eb 100644 --- a/.github/actions/install-deps/action.yaml +++ b/.github/actions/install-deps/action.yaml @@ -55,6 +55,7 @@ runs: remove-codeql: "true" # https://github.com/open-telemetry/opentelemetry-cpp/issues/2998 + # Skip on arm64 Linux - use system cmake instead - name: Setup cmake # uses: jwlawson/actions-setup-cmake@v2 uses: tomjakubowski/actions-setup-cmake@v0-aarch64-test4 @@ -153,6 +154,20 @@ runs: targets: aarch64-apple-darwin components: rustfmt, clippy, rust-src + # Remove any pre-installed x86_64 rustup on arm64 runners + - name: Clean up incompatible rustup (aarch64 Linux) + if: ${{ inputs.rust == 'true' && inputs.arch == 'aarch64' && runner.os == 'Linux' }} + shell: bash + run: | + if command -v rustup &>/dev/null; then + if ! rustup --version &>/dev/null; then + echo "Removing incompatible rustup binary" + rm -f "$(which rustup)" + rm -rf ~/.cargo/bin + rm -rf ~/.rustup + fi + fi + - name: Install rust (aarch64 Linux) uses: dtolnay/rust-toolchain@nightly if: ${{ inputs.rust == 'true' && inputs.arch == 'aarch64' && runner.os == 'Linux' }} diff --git a/rust/perspective-server/build.rs b/rust/perspective-server/build.rs index 4aae144129..c27323b4ef 100644 --- a/rust/perspective-server/build.rs +++ b/rust/perspective-server/build.rs @@ -46,6 +46,9 @@ fn cmake_build() -> Result, std::io::Error> { dst.define("ARROW_BUILD_EXAMPLES", "OFF"); dst.define("RAPIDJSON_BUILD_EXAMPLES", "OFF"); dst.define("ARROW_CXX_FLAGS_DEBUG", "-Wno-error"); + // Only use protobuf-src's protoc if we're not on Linux arm64 + // On Linux arm64, let cmake download the correct protoc binary + #[cfg(not(all(target_os = "linux", target_arch = "aarch64")))] dst.define("PSP_PROTOC_PATH", protoc()); dst.define("CMAKE_COLOR_DIAGNOSTICS", "ON"); dst.define( diff --git a/rust/perspective-server/cmake/modules/FindProtoc.cmake b/rust/perspective-server/cmake/modules/FindProtoc.cmake index ae7b65f940..f9b91b99b2 100644 --- a/rust/perspective-server/cmake/modules/FindProtoc.cmake +++ b/rust/perspective-server/cmake/modules/FindProtoc.cmake @@ -9,7 +9,17 @@ function(download_protoc VERSION DESTINATION) elseif(CMAKE_HOST_SYSTEM_NAME STREQUAL "Darwin") set(PROTOC_ZIP "protoc-${VERSION}-osx-x86_64.zip") elseif(CMAKE_HOST_SYSTEM_NAME STREQUAL "Linux") - set(PROTOC_ZIP "protoc-${VERSION}-linux-x86_64.zip") + # Detect host architecture for Linux + execute_process( + COMMAND uname -m + OUTPUT_VARIABLE HOST_ARCH + OUTPUT_STRIP_TRAILING_WHITESPACE + ) + if(HOST_ARCH STREQUAL "aarch64") + set(PROTOC_ZIP "protoc-${VERSION}-linux-aarch_64.zip") + else() + set(PROTOC_ZIP "protoc-${VERSION}-linux-x86_64.zip") + endif() else() message(FATAL_ERROR "Unsupported host system: ${CMAKE_HOST_SYSTEM_NAME}") endif() @@ -56,7 +66,7 @@ else() OUTPUT_VARIABLE PROTOC_VERSION_OUTPUT OUTPUT_STRIP_TRAILING_WHITESPACE ) - + if(NOT PROTOC_VERSION_OUTPUT MATCHES "^libprotoc ([0-9]+)\\.([0-9]+)(:?\\.([0-9]+))?") message(WARNING "Unable to determine protoc version") return() @@ -71,7 +81,7 @@ else() else() set(FOUND_VERSION "${PROTOC_VERSION_MAJOR}.${PROTOC_VERSION_MINOR}.${PROTOC_VERSION_PATCH}") endif() - + # Force the external project to use the same version as our installed protoc CLI set(LIBPROTOBUF_VERSION "v${FOUND_VERSION}" PARENT_SCOPE) @@ -120,4 +130,3 @@ function(protobuf_generate_cpp SRCS HDRS) set(${SRCS} ${_PROTOBUF_GENERATE_CPP_SRCS} PARENT_SCOPE) set(${HDRS} ${_PROTOBUF_GENERATE_CPP_HDRS} PARENT_SCOPE) endfunction() - From 434b81b6b2ccebe383b34704b0fc5bf0f98f3c72 Mon Sep 17 00:00:00 2001 From: Konstantinos Samaras-Tsakiris Date: Sat, 6 Dec 2025 17:09:56 +0100 Subject: [PATCH 4/6] Review comments - Simplify arch detection in cmake - Try not reinstalling rustup - Use the existing method of installing protoc correctly Signed-off-by: Konstantinos Samaras-Tsakiris Signed-off-by: Tom Jakubowski --- .github/actions/install-deps/action.yaml | 14 -------------- rust/perspective-server/build.rs | 5 +---- .../cmake/modules/FindProtoc.cmake | 8 +------- 3 files changed, 2 insertions(+), 25 deletions(-) diff --git a/.github/actions/install-deps/action.yaml b/.github/actions/install-deps/action.yaml index 06edadf9eb..4622e788f2 100644 --- a/.github/actions/install-deps/action.yaml +++ b/.github/actions/install-deps/action.yaml @@ -154,20 +154,6 @@ runs: targets: aarch64-apple-darwin components: rustfmt, clippy, rust-src - # Remove any pre-installed x86_64 rustup on arm64 runners - - name: Clean up incompatible rustup (aarch64 Linux) - if: ${{ inputs.rust == 'true' && inputs.arch == 'aarch64' && runner.os == 'Linux' }} - shell: bash - run: | - if command -v rustup &>/dev/null; then - if ! rustup --version &>/dev/null; then - echo "Removing incompatible rustup binary" - rm -f "$(which rustup)" - rm -rf ~/.cargo/bin - rm -rf ~/.rustup - fi - fi - - name: Install rust (aarch64 Linux) uses: dtolnay/rust-toolchain@nightly if: ${{ inputs.rust == 'true' && inputs.arch == 'aarch64' && runner.os == 'Linux' }} diff --git a/rust/perspective-server/build.rs b/rust/perspective-server/build.rs index c27323b4ef..8c34d5620c 100644 --- a/rust/perspective-server/build.rs +++ b/rust/perspective-server/build.rs @@ -46,10 +46,7 @@ fn cmake_build() -> Result, std::io::Error> { dst.define("ARROW_BUILD_EXAMPLES", "OFF"); dst.define("RAPIDJSON_BUILD_EXAMPLES", "OFF"); dst.define("ARROW_CXX_FLAGS_DEBUG", "-Wno-error"); - // Only use protobuf-src's protoc if we're not on Linux arm64 - // On Linux arm64, let cmake download the correct protoc binary - #[cfg(not(all(target_os = "linux", target_arch = "aarch64")))] - dst.define("PSP_PROTOC_PATH", protoc()); + dst.define("PSP_PROTOC_PATH", protoc().parent()); dst.define("CMAKE_COLOR_DIAGNOSTICS", "ON"); dst.define( "PSP_PROTO_PATH", diff --git a/rust/perspective-server/cmake/modules/FindProtoc.cmake b/rust/perspective-server/cmake/modules/FindProtoc.cmake index f9b91b99b2..872704dc83 100644 --- a/rust/perspective-server/cmake/modules/FindProtoc.cmake +++ b/rust/perspective-server/cmake/modules/FindProtoc.cmake @@ -9,13 +9,7 @@ function(download_protoc VERSION DESTINATION) elseif(CMAKE_HOST_SYSTEM_NAME STREQUAL "Darwin") set(PROTOC_ZIP "protoc-${VERSION}-osx-x86_64.zip") elseif(CMAKE_HOST_SYSTEM_NAME STREQUAL "Linux") - # Detect host architecture for Linux - execute_process( - COMMAND uname -m - OUTPUT_VARIABLE HOST_ARCH - OUTPUT_STRIP_TRAILING_WHITESPACE - ) - if(HOST_ARCH STREQUAL "aarch64") + if(CMAKE_HOST_SYSTEM_PROCESSOR STREQUAL "aarch64") set(PROTOC_ZIP "protoc-${VERSION}-linux-aarch_64.zip") else() set(PROTOC_ZIP "protoc-${VERSION}-linux-x86_64.zip") From 7e0734f4fefd12ade4b99403b2deac981bbba0fb Mon Sep 17 00:00:00 2001 From: Tom Jakubowski Date: Tue, 9 Dec 2025 12:14:37 -0800 Subject: [PATCH 5/6] Fix protoc().parent() call `Path::parent()` returns `None` for root and empty paths, so handle it by panicking in this case. In this case it should never happen under non-contrived circumstances. Signed-off-by: Tom Jakubowski --- .github/actions/install-deps/action.yaml | 2 +- rust/perspective-server/build.rs | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/.github/actions/install-deps/action.yaml b/.github/actions/install-deps/action.yaml index 4622e788f2..5582515e5d 100644 --- a/.github/actions/install-deps/action.yaml +++ b/.github/actions/install-deps/action.yaml @@ -54,8 +54,8 @@ runs: remove-haskell: "true" remove-codeql: "true" + # Sticking to 3.29 because of: # https://github.com/open-telemetry/opentelemetry-cpp/issues/2998 - # Skip on arm64 Linux - use system cmake instead - name: Setup cmake # uses: jwlawson/actions-setup-cmake@v2 uses: tomjakubowski/actions-setup-cmake@v0-aarch64-test4 diff --git a/rust/perspective-server/build.rs b/rust/perspective-server/build.rs index 8c34d5620c..11efda5aee 100644 --- a/rust/perspective-server/build.rs +++ b/rust/perspective-server/build.rs @@ -46,7 +46,12 @@ fn cmake_build() -> Result, std::io::Error> { dst.define("ARROW_BUILD_EXAMPLES", "OFF"); dst.define("RAPIDJSON_BUILD_EXAMPLES", "OFF"); dst.define("ARROW_CXX_FLAGS_DEBUG", "-Wno-error"); - dst.define("PSP_PROTOC_PATH", protoc().parent()); + dst.define( + "PSP_PROTOC_PATH", + protoc() + .parent() + .expect("protoc() returned root path or empty string"), + ); dst.define("CMAKE_COLOR_DIAGNOSTICS", "ON"); dst.define( "PSP_PROTO_PATH", From cbfe4ecb46168e96b7c3eccc3945495d62669c2f Mon Sep 17 00:00:00 2001 From: Tom Jakubowski Date: Tue, 9 Dec 2025 13:39:36 -0800 Subject: [PATCH 6/6] Add disk free space reports to Python build These builds are on a knife's edge currently with respect to disk space. I think it is worth logging `df` output so we can keep an eye on it. Signed-off-by: Tom Jakubowski --- .github/workflows/build.yaml | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 096c4d0aff..435721208d 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -220,11 +220,19 @@ jobs: arch: x86_64 steps: + - name: Run df -h + if: ${{ runner.os == 'Linux' }} + run: df -h + - name: Free up disk space if: ${{ runner.os == 'Linux' }} run: | rm -rf /__t/* + - name: Run df -h + if: ${{ runner.os == 'Linux' }} + run: df -h + - name: Checkout uses: actions/checkout@v4 @@ -237,6 +245,10 @@ jobs: name: perspective-metadata path: rust/ + - name: Run df -h + if: ${{ runner.os == 'Linux' }} + run: df -h + - name: Initialize Build id: init-step uses: ./.github/actions/install-deps @@ -246,6 +258,10 @@ jobs: manylinux: ${{ matrix.container && 'true' || 'false' }} skip_cache: ${{ steps.config-step.outputs.SKIP_CACHE }} + - name: Run df -h + if: ${{ runner.os == 'Linux' }} + run: df -h + - name: Python Build run: pnpm run build if: ${{ runner.os != 'Windows' }} @@ -268,6 +284,10 @@ jobs: PSP_ARCH: ${{ matrix.arch }} PSP_BUILD_WHEEL: 1 + - name: Run df -h + if: ${{ runner.os == 'Linux' }} + run: df -h + # Windows sucks lol - uses: actions/upload-artifact@v4 if: ${{ runner.os == 'Windows' }}