From c19b91d65dee6b08d00a9257d27c334c13b55ef3 Mon Sep 17 00:00:00 2001 From: Michael Johns Date: Mon, 18 May 2026 14:35:14 -0400 Subject: [PATCH 01/20] fix(ci): pre-bootstrap JFrog pip routing before setup-python MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hardened runner groups (larger-runners, databrickslabs-protected-runner-group) block egress to pypi.org by design — all package fetches must go through the JFrog db-pypi mirror per go/hardened-gha policy. But actions/setup-python@v6.2.0 unconditionally runs `pip install --upgrade pip` inside its python-versions installer BEFORE our jfrog-auth step has a chance to redirect pip. That first pip call escapes the allowlist and dies with SSL UNEXPECTED_EOF against pypi.org, failing the entire Scala build at the "Configure python interpreter" step. Fix: add a new jfrog-pip-bootstrap composite that runs BEFORE setup-python. It mints a JFrog OIDC access token (reusing the existing jfrog-auth shell script — kept in sync with UCX), writes netrc + pip.conf, and exports NETRC + PIP_CONFIG_FILE to $GITHUB_ENV. By the time setup-python downloads Python and fires its internal pip upgrade, pip already routes through db-pypi and succeeds. The main jfrog-auth composite still runs after setup-python — now solely responsible for Maven config (which needs setup-java to have written ~/.m2/settings.xml first) and an idempotent pip refresh with a fresh token. Wired into both scala_build and python_build. No workflow changes: all five callers (build_main, build_scala, build_python, build_scala_by_package, codecov-scala-parallel, codecov-upload) already declare id-token: write. Co-authored-by: Isaac --- .../actions/jfrog-pip-bootstrap/action.yml | 58 +++++++++++++++++++ .github/actions/python_build/action.yml | 13 ++++- .github/actions/scala_build/action.yml | 15 ++++- 3 files changed, 81 insertions(+), 5 deletions(-) create mode 100644 .github/actions/jfrog-pip-bootstrap/action.yml diff --git a/.github/actions/jfrog-pip-bootstrap/action.yml b/.github/actions/jfrog-pip-bootstrap/action.yml new file mode 100644 index 0000000..87892a7 --- /dev/null +++ b/.github/actions/jfrog-pip-bootstrap/action.yml @@ -0,0 +1,58 @@ +name: 'Bootstrap pip JFrog routing (pre-setup-python)' +description: | + Acquire a JFrog OIDC access token and pre-configure pip + netrc so that any + pip invocation routes through the JFrog db-pypi mirror instead of pypi.org — + including actions/setup-python's internal "Upgrading pip" step, which runs + before any other action gets a chance to redirect pip. + + Why this exists: the Databricks hardened runner groups (larger-runners, + databrickslabs-protected-runner-group) block egress to pypi.org by design; + the go/hardened-gha policy is that all package fetches go through JFrog. + Without this pre-bootstrap, setup-python's bundled "pip install --upgrade pip" + hits pypi.org and dies with SSL EOF before .github/actions/jfrog-auth + ever gets to configure pip. + + Reuses the OIDC exchange script from .github/actions/jfrog-auth/jfrog-auth + to avoid duplicating curl logic that we keep in sync with UCX. The main + jfrog-auth composite still runs later in the same job (idempotent for pip; + primary for Maven, which needs setup-java to have run first). + + Caller job MUST declare: + permissions: + id-token: write +runs: + using: "composite" + steps: + - id: jfrog-auth + name: Acquire JFrog OIDC access token + shell: bash + run: | + if [[ -z "${ACTIONS_ID_TOKEN_REQUEST_URL}" ]] || [[ -z "${ACTIONS_ID_TOKEN_REQUEST_TOKEN}" ]]; then + printf '::error::%s\n' 'This action uses OIDC: job must have "id-token: write" permission' + exit 1 + fi + "${GITHUB_WORKSPACE}/.github/actions/jfrog-auth/jfrog-auth" \ + "${ACTIONS_ID_TOKEN_REQUEST_URL}" \ + "${ACTIONS_ID_TOKEN_REQUEST_TOKEN}" + + - name: Write pip.conf + netrc for JFrog (db-pypi) + shell: bash + env: + JFROG_ACCESS_TOKEN: "${{ steps.jfrog-auth.outputs.jfrog-access-token }}" + run: | + umask 077 + cat > "${RUNNER_TEMP}/.netrc" << EOF + machine databricks.jfrog.io + login gha-service-account + password ${JFROG_ACCESS_TOKEN} + EOF + # Same db-pypi URL the main jfrog-auth composite uses; the later + # jfrog-auth run will overwrite this file with an identical value + # (modulo a fresh token in netrc) — idempotent by design. + cat > "${RUNNER_TEMP}/.pip.conf" << 'EOF' + [global] + index-url = https://databricks.jfrog.io/artifactory/api/pypi/db-pypi/simple + EOF + printf '%s=%s\n' 'NETRC' "${RUNNER_TEMP}/.netrc" >> "${GITHUB_ENV}" + printf '%s=%s\n' 'PIP_CONFIG_FILE' "${RUNNER_TEMP}/.pip.conf" >> "${GITHUB_ENV}" + printf '::debug::%s\n' 'Pre-bootstrap: configured JFrog access for pip.' diff --git a/.github/actions/python_build/action.yml b/.github/actions/python_build/action.yml index c8fe559..7831647 100644 --- a/.github/actions/python_build/action.yml +++ b/.github/actions/python_build/action.yml @@ -11,6 +11,15 @@ inputs: runs: using: "composite" steps: + # Pre-route pip at db-pypi BEFORE actions/setup-python runs: setup-python's + # python-versions installer unconditionally runs `pip install --upgrade pip`, + # which on the hardened runner group hits the egress allowlist and fails + # SSL-EOF against pypi.org. This step writes netrc + pip.conf so that + # internal pip call (and every later one) routes through JFrog instead. + # Idempotent if scala_build already ran in the same job — the env vars + # NETRC + PIP_CONFIG_FILE just get re-set to the same paths with a fresh token. + - name: Pre-bootstrap pip for JFrog (pre-setup-python) + uses: ./.github/actions/jfrog-pip-bootstrap - name: Configure python interpreter uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 with: @@ -18,8 +27,8 @@ runs: cache-dependency-path: '.ci-pip-cache-key' python-version: ${{ matrix.python }} # Route pip through JFrog (OIDC) per go/hardened-gha policy. - # Idempotent if scala_build already ran in the same job (re-auths but env vars stay set). - # Caller job must declare `permissions: id-token: write`. + # Idempotent: jfrog-pip-bootstrap already configured pip; this re-runs the + # same write with a fresh token. Caller job must declare `permissions: id-token: write`. - name: Authenticate for JFrog (pip via OIDC) uses: ./.github/actions/jfrog-auth - name: Add packaged GDAL dependencies diff --git a/.github/actions/scala_build/action.yml b/.github/actions/scala_build/action.yml index 41b5900..e405d29 100644 --- a/.github/actions/scala_build/action.yml +++ b/.github/actions/scala_build/action.yml @@ -27,16 +27,25 @@ runs: - name: Set Maven opts for coverage and parallel builds shell: bash run: echo "MAVEN_OPTS=-Xmx4g -XX:+UseG1GC" >> $GITHUB_ENV + # Pre-route pip at db-pypi BEFORE actions/setup-python runs: setup-python's + # python-versions installer unconditionally runs `pip install --upgrade pip`, + # which on the hardened runner group hits the egress allowlist and fails + # SSL-EOF against pypi.org. This step writes netrc + pip.conf so that + # internal pip call (and every later one) routes through JFrog instead. + - name: Pre-bootstrap pip for JFrog (pre-setup-python) + uses: ./.github/actions/jfrog-pip-bootstrap - name: Configure python interpreter uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 with: cache: 'pip' # caches dependencies for faster subsequent runs cache-dependency-path: '.ci-pip-cache-key' python-version: ${{ matrix.python }} - # Route pip + Maven through JFrog (OIDC) per go/hardened-gha policy. - # Must run after setup-java + setup-python so mvn + pip3 are on PATH for auto-detect. + # Route Maven through JFrog (OIDC) per go/hardened-gha policy. Pip was + # already configured by jfrog-pip-bootstrap above; this re-runs pip's + # netrc/pip.conf write idempotently with a fresh token, and configures + # Maven now that setup-java has put mvn + ~/.m2/settings.xml in place. # Caller job must declare `permissions: id-token: write`. - - name: Authenticate for JFrog (pip + Maven via OIDC) + - name: Authenticate for JFrog (Maven + pip refresh via OIDC) uses: ./.github/actions/jfrog-auth # Verify the PGP signature of every Maven dependency / plugin against # .maven-keys.list BEFORE any other mvn call resolves or compiles them. From facda8f3a20e08d297cbdab2ee7a723557d38b9f Mon Sep 17 00:00:00 2001 From: Michael Johns Date: Mon, 18 May 2026 14:46:28 -0400 Subject: [PATCH 02/20] fix(security): drop unknown failNoKey param on pgpverify 1.19.1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pgpverify-maven-plugin renamed-then-removed the `failNoKey` parameter sometime before 1.13.0; on 1.19.1 every build that runs the `check` mojo emits: Warning: Parameter 'failNoKey' is unknown for plugin 'pgpverify-maven-plugin:1.19.1:check (default)' The lockdown intent ("fail when a dependency's signing key is not in the keysmap") is the DEFAULT behavior in 1.19.1 — it's enforced by the keysmap file itself (no `noKey` tokens in .maven-keys.list), which .maven-keys.list already documents as policy. Removing the no-op parameter does not weaken the security posture. Changes: * pom.xml: drop true from the verify-pgp profile's plugin config; replace with a comment so this isn't re-added on the next lockdown pass. * scripts/security/maven-pgp-bootstrap: drop the matching -DfailNoKey=false override (same unknown-parameter no-op). Co-authored-by: Isaac --- pom.xml | 8 +++++++- scripts/security/maven-pgp-bootstrap | 5 ++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index 9bb3c0d..588601c 100644 --- a/pom.xml +++ b/pom.xml @@ -492,7 +492,13 @@ true true - true + true true true diff --git a/scripts/security/maven-pgp-bootstrap b/scripts/security/maven-pgp-bootstrap index eb6ec03..1709ee9 100755 --- a/scripts/security/maven-pgp-bootstrap +++ b/scripts/security/maven-pgp-bootstrap @@ -28,13 +28,16 @@ mkdir -p "${root}/target" # first unknown key — we want a complete inventory. ( cd "${root}" + # -DfailNoKey is intentionally omitted: it is not a parameter in + # pgpverify-maven-plugin >= 1.13 (emits "Parameter 'failNoKey' is unknown"). + # Missing-keysmap-entry behavior in 1.19.1 is controlled by the keysmap + # file itself, not by mojo parameters. mvn -B -q -Pverify-pgp verify \ -DskipTests \ -Dscoverage.skip \ -Dscalastyle.skip=true \ -DfailNoSignature=false \ -DfailWeakSignature=false \ - -DfailNoKey=false \ 2>&1 ) | tee "${log}" >&2 From bb42ecc0d7f81eff5b930c0c00058db19b8b168e Mon Sep 17 00:00:00 2001 From: Michael Johns Date: Mon, 18 May 2026 14:49:55 -0400 Subject: [PATCH 03/20] fix(security): drop deprecated failNoSignature param on pgpverify 1.19.1 pgpverify-maven-plugin 1.13.0 deprecated `failNoSignature` in favor of expressing the requirement through the keysmap file. On 1.19.1 the `check` mojo emits: Warning: Parameter 'failNoSignature' (user property 'pgpverify.failNoSignature') is deprecated: Deprecated as of 1.13.0: this requirement can be expressed through the keysMap. .maven-keys.list already documents "DO NOT USE noSig" as policy and contains no `noSig` tokens, so the strict-by-default behavior we want ("fail on unsigned artifact") is enforced through the keysmap. The mojo parameter is redundant; remove it. The verify-pgp profile now relies on: * failWeakSignature=true (still a valid mojo parameter) * keysmap entries without `noSig` (enforces "must be signed") * keysmap entries without `noKey` (enforces "key must be mapped") Updated the inline comment to record both the failNoSignature deprecation and the prior failNoKey removal so the next lockdown pass doesn't re-add either parameter. Bootstrap script (scripts/security/maven-pgp-bootstrap) intentionally keeps -DfailNoSignature=false / -DfailWeakSignature=false: those are local-only relaxations needed for key discovery; the deprecation warning surfaces during local bootstrap, not in CI. Co-authored-by: Isaac --- pom.xml | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/pom.xml b/pom.xml index 588601c..837d0df 100644 --- a/pom.xml +++ b/pom.xml @@ -490,14 +490,16 @@ - true true true true From 5c1daf478ea07ed3874dc4d6f2ab53c4fe334518 Mon Sep 17 00:00:00 2001 From: Michael Johns Date: Mon, 18 May 2026 15:25:29 -0400 Subject: [PATCH 04/20] fix(security): pin Maven lifecycle plugin versions (Category A drift) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hardened verify-pgp profile fails with "Not allowed artifact ... and keyID" for five lifecycle plugins that pom.xml never pinned: org.apache.maven.plugins:maven-install-plugin:3.1.4 org.apache.maven.plugins:maven-deploy-plugin:3.1.4 org.apache.maven.plugins:maven-compiler-plugin:3.15.0 org.codehaus.plexus:plexus-compiler-{api,javac,manager}:2.16.2 org.apache.maven.shared:file-management:3.2.0 Maven's Super POM auto-resolves these when the project doesn't pin them. Resolution drifts across Maven minor versions and across CI runner image upgrades, swapping in new (legit but un-mapped) signing keys. The PGP verify step then rejects them — not because they're untrusted, but because .maven-keys.list was bootstrapped against an earlier closure. Pin maven-compiler-plugin, maven-install-plugin, and maven-deploy-plugin in the root block. plexus-compiler-* and file-management come in transitively from the pinned plugins, so they freeze with them. This commit does NOT update .maven-keys.list — that requires running scripts/security/maven-pgp-bootstrap in the geobrix-dev container after this pin lands, cross-checking each new fingerprint against the trust anchors listed at the top of .maven-keys.list (apache KEYS, mojohaus, etc.), and committing the reviewed entries as a follow-up. Category B (POM-only "PGP Signature INVALID") is investigated separately via the diag-pgpverify-pom-transit workflow added in the next commit. Co-authored-by: Isaac --- pom.xml | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/pom.xml b/pom.xml index 837d0df..70413cc 100644 --- a/pom.xml +++ b/pom.xml @@ -340,6 +340,36 @@ + + + org.apache.maven.plugins + maven-compiler-plugin + 3.15.0 + + 17 + + + + org.apache.maven.plugins + maven-install-plugin + 3.1.4 + + + org.apache.maven.plugins + maven-deploy-plugin + 3.1.4 + From f2dc22b6f59505ab3321c19a1aa485ea2277c3ff Mon Sep 17 00:00:00 2001 From: Michael Johns Date: Mon, 18 May 2026 15:25:30 -0400 Subject: [PATCH 05/20] chore(diag): add POM transit diagnostic for Category B PGP failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Category B from the post-lockdown verify-pgp build: "PGP Signature INVALID" on .pom files only (never .jar). Hypothesis: the JFrog db-maven mirror is mutating POM bytes (line endings, XML normalization, or appended _remote.repositories metadata) between Maven Central and what CI sees, breaking the upstream signature. This adds: * scripts/security/diag-pgpverify-pom-transit — fetches each of the six failing .pom + .pom.asc files from db-maven via NETRC auth, computes sha256, compares against a known-good Maven Central reference (captured 2026-05-18 via maven-proxy.dev.databricks.com and recorded inline), prints a hex dump of the first/last bytes on mismatch, and runs gpg --verify standalone if gpg is on PATH. * .github/workflows/diag-pgpverify-pom-transit.yml — workflow_dispatch one-shot that authenticates to JFrog (OIDC), installs gpg, and runs the script on the protected runner group. No untrusted github.event.* values reach any run block — only static script invocations. Trigger via Actions → "Diag — POM transit (db-maven vs Maven Central)" → Run workflow. The exit code surfaces red/green; the logs carry the sha256 diff and gpg output. Delete this workflow + script once the investigation concludes. Co-authored-by: Isaac --- .../workflows/diag-pgpverify-pom-transit.yml | 48 +++++++ scripts/security/diag-pgpverify-pom-transit | 128 ++++++++++++++++++ 2 files changed, 176 insertions(+) create mode 100644 .github/workflows/diag-pgpverify-pom-transit.yml create mode 100755 scripts/security/diag-pgpverify-pom-transit diff --git a/.github/workflows/diag-pgpverify-pom-transit.yml b/.github/workflows/diag-pgpverify-pom-transit.yml new file mode 100644 index 0000000..4f650ec --- /dev/null +++ b/.github/workflows/diag-pgpverify-pom-transit.yml @@ -0,0 +1,48 @@ +name: Diag — POM transit (db-maven vs Maven Central) +# One-shot diagnostic for Category B PGP failures: "PGP Signature INVALID" +# on .pom files only (never .jar). Hypothesis: db-maven JFrog mirror is +# mutating POM bytes between Maven Central and the CI runner, breaking the +# upstream signature. +# +# This workflow fetches each suspect .pom + .pom.asc from db-maven (the +# exact path Maven uses in CI), computes sha256, and compares against a +# known reference from Maven Central (recorded inside the script). Mismatch +# = byte mutation confirmed; match = signature failure has a different +# root cause. +# +# Manual trigger only (workflow_dispatch). Delete this workflow once the +# investigation concludes. Run blocks contain no untrusted github.event.* +# values — only static script invocations. + +on: + workflow_dispatch: {} + +permissions: + contents: read + +jobs: + diag: + name: pom-transit-diff + runs-on: + group: databrickslabs-protected-runner-group + labels: linux-ubuntu-latest + environment: runtime + permissions: + contents: read + id-token: write + steps: + - name: Checkout + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + token: ${{ secrets.REPO_ACCESS_TOKEN || secrets.GITHUB_TOKEN }} + - name: Configure JDK + uses: actions/setup-java@be666c2fcd27ec809703dec50e508c2fdc7f6654 # v5.2.0 + with: + java-version: '17' + distribution: 'zulu' + - name: Authenticate for JFrog (Maven via OIDC) + uses: ./.github/actions/jfrog-auth + - name: Install gpg (for standalone signature verification) + run: sudo apt-get -o DPkg::Lock::Timeout=-1 install -y gpg + - name: Run POM transit diagnostic + run: ./scripts/security/diag-pgpverify-pom-transit diff --git a/scripts/security/diag-pgpverify-pom-transit b/scripts/security/diag-pgpverify-pom-transit new file mode 100755 index 0000000..22361bc --- /dev/null +++ b/scripts/security/diag-pgpverify-pom-transit @@ -0,0 +1,128 @@ +#!/bin/bash +# +# Diagnose Category B PGP failures: "PGP Signature INVALID" on .pom files +# only (never .jar). Suspected cause: db-maven JFrog mirror mutating POM +# bytes (line endings, encoding, metadata) between Maven Central and CI, +# breaking the upstream signature. +# +# This script fetches each suspect POM + .asc from the configured Maven +# mirror (db-maven via NETRC auth — same path Maven uses in CI) and +# compares its sha256 against a known reference from Maven Central (fetched +# locally via maven-proxy.dev.databricks.com, recorded below). +# +# Outputs per artifact: +# * size + sha256 of the .pom and .pom.asc as served by db-maven +# * PASS/FAIL vs the embedded Maven Central reference sha256s +# * if mismatch: first 16 bytes that differ (line-ending diff is the +# usual suspect) +# * gpg --verify result if gpg is available +# +# Usage (in CI / geobrix-dev container): +# ./scripts/security/diag-pgpverify-pom-transit +# +# Requires: curl, shasum/sha256sum. Optional: gpg. +# Requires NETRC env var pointing at a netrc with db-maven creds — set +# by .github/actions/jfrog-auth or .github/actions/jfrog-pip-bootstrap. + +set -eu + +# db-maven Maven repo root (artifactory db-maven proxy of Maven Central). +JFROG_URL="https://databricks.jfrog.io/artifactory/db-maven" + +# Each line: "||" +# Reference sha256s captured 2026-05-18 from maven-proxy.dev.databricks.com, +# which is a transparent mirror of repo1.maven.org. If those upstream bytes +# ever change, regenerate this table. +ARTIFACTS=$(cat <<'EOF' +com/fasterxml/jackson/core/jackson-annotations/2.18.3/jackson-annotations-2.18.3.pom|b9c98ba9e29ea61b693d4c8c801968feaae220b2b2002364a7bcec524998384b|0f752e188ec3c8f0a3bd6060b67243750d01eda3adc9fbfa7c3c114f784eae5e +com/fasterxml/jackson/core/jackson-core/2.18.3/jackson-core-2.18.3.pom|37dc6b8f6391a4709ac215ca98fa2341c6dd65613c2277d96a4fd82a2f50e3c9|81f47a9b1d9b40d6679888ea23f42b076f79c923f0a31ad27bde68faa3429851 +com/fasterxml/jackson/core/jackson-databind/2.18.3/jackson-databind-2.18.3.pom|e58f48ac14e4dbd48595d69e20bad35019ec1281514ef7ef155e18072ada617f|da0ea4a494ed5368e3e5579fffe677ba2208faf82f88df890169bfa08817c7b3 +net/alchim31/maven/scala-maven-plugin/4.9.9/scala-maven-plugin-4.9.9.pom|6fc2ece5857f70bcb3fc08a41c5a9f6dcb23c359cab649a48a838b7948a72ed5|b8e4a5cbb07e2492ff72f2d8c0b00a05749082ee49d70e2fd7622de261191e2e +org/iq80/snappy/snappy/0.4/snappy-0.4.pom|a709ce17111e4149d9b79a5295644e0cd5a8355aec4b2ef4c0436aba7b25d08a|1dcdccaf5d6eb766e4daa9b9a386fa697594ede0f5e273062813aea534e13bc6 +javax/servlet/javax.servlet-api/3.1.0/javax.servlet-api-3.1.0.pom|b31109e22ea3f2df1ad7955432e718a35def50ae6c19698034afa8a0cf9e9069|c7130f17422295faabf1885daa6f1c8a1a8c05d6a573c512148119f2a4aa42f3 +EOF +) + +if command -v sha256sum >/dev/null 2>&1; then + SHA() { sha256sum "$1" | awk '{print $1}'; } +elif command -v shasum >/dev/null 2>&1; then + SHA() { shasum -a 256 "$1" | awk '{print $1}'; } +else + echo "ERROR: need sha256sum or shasum on PATH" >&2 + exit 1 +fi + +NETRC_FLAG="" +if [ -n "${NETRC:-}" ] && [ -r "${NETRC}" ]; then + NETRC_FLAG="--netrc-file ${NETRC}" +elif [ -r "${HOME}/.netrc" ]; then + NETRC_FLAG="--netrc" +fi + +TMP=$(mktemp -d) +trap 'rm -rf "${TMP}"' EXIT + +mismatch=0 + +printf '%s\n' "===== POM transit diagnostic: db-maven vs Maven Central =====" +printf 'JFrog base: %s\n' "${JFROG_URL}" +printf 'Netrc: %s\n' "${NETRC:-${HOME}/.netrc (if present)}" +printf '\n' + +# Process while preserving variables across loop iterations (subshell trap) +while IFS='|' read -r path expected_pom_sha expected_asc_sha; do + [ -z "${path}" ] && continue + fname=$(basename "${path}") + pom="${TMP}/${fname}" + asc="${TMP}/${fname}.asc" + + printf '────── %s ──────\n' "${fname}" + http_pom=$(curl -sSL ${NETRC_FLAG} --max-time 20 -w '%{http_code}' -o "${pom}" "${JFROG_URL}/${path}" || echo "curl-error") + http_asc=$(curl -sSL ${NETRC_FLAG} --max-time 20 -w '%{http_code}' -o "${asc}" "${JFROG_URL}/${path}.asc" || echo "curl-error") + + if [ "${http_pom}" != "200" ] || [ "${http_asc}" != "200" ]; then + printf ' ⚠ fetch failed (.pom=%s .asc=%s); skipping\n\n' "${http_pom}" "${http_asc}" + mismatch=$((mismatch+1)) + continue + fi + + pom_size=$(wc -c < "${pom}" | tr -d ' ') + asc_size=$(wc -c < "${asc}" | tr -d ' ') + pom_sha=$(SHA "${pom}") + asc_sha=$(SHA "${asc}") + + pom_status="MATCH" + asc_status="MATCH" + [ "${pom_sha}" != "${expected_pom_sha}" ] && { pom_status="MISMATCH"; mismatch=$((mismatch+1)); } + [ "${asc_sha}" != "${expected_asc_sha}" ] && { asc_status="MISMATCH"; mismatch=$((mismatch+1)); } + + printf ' .pom size=%s sha256=%s [%s]\n' "${pom_size}" "${pom_sha}" "${pom_status}" + printf ' .pom.asc size=%s sha256=%s [%s]\n' "${asc_size}" "${asc_sha}" "${asc_status}" + + # If POM bytes differ from Maven Central reference, print a hex-diff to + # localize the divergence (line endings are the usual culprit). + if [ "${pom_status}" = "MISMATCH" ]; then + # Fetch Maven Central reference too (only path: maven-central tends to be + # blocked from the hardened runner, so this may fail — we still emit the + # local hex dump of what JFrog served for byte-level inspection). + printf ' (db-maven bytes, first 256):\n' + head -c 256 "${pom}" | od -An -c | sed 's/^/ /' + printf ' (db-maven bytes, last 64):\n' + tail -c 64 "${pom}" | od -An -c | sed 's/^/ /' + fi + + # Standalone gpg verify (does the .asc verify the .pom regardless of keysmap?) + if command -v gpg >/dev/null 2>&1; then + if gpg --verify "${asc}" "${pom}" >"${TMP}/gpg.out" 2>&1; then + printf ' gpg --verify: OK\n' + else + printf ' gpg --verify: FAILED\n' + sed 's/^/ /' "${TMP}/gpg.out" + fi + fi + + printf '\n' +done <<< "${ARTIFACTS}" + +printf '===== summary: %s mismatch event(s) =====\n' "${mismatch}" +exit "${mismatch}" From c897d3144afb5375dbd93af578dce3b3a3d1d0ce Mon Sep 17 00:00:00 2001 From: Michael Johns Date: Mon, 18 May 2026 15:37:04 -0400 Subject: [PATCH 06/20] fix(security): add keysmap entries for pinned lifecycle plugins (Category A) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The version pins added in 5c1daf4 freeze the plugin versions but leave their actual signing keys unmapped in .maven-keys.list, so the verify-pgp profile still rejects them with "Not allowed artifact ... and keyID". Add version-specific entries for each. Both signing keys verified against keyserver.ubuntu.com on 2026-05-18: 0x84789D24DF77A32433CE1F079EB80E92EB2135B1 Slawomir Jaranowski Apache Maven committer + lead developer of pgpverify-maven-plugin itself. RSA 4096, created 2021-12-22, active. Now signs newer maven-install-plugin, maven-deploy-plugin, file-management releases. 0x32118CF76C9EC5D918E54967CA80D1F0EB6CA4BA Sylwester Lachiewicz Apache committer; Mojo Codehaus / Maven plugins maintainer. RSA 4096, created 2020-05-09, active. Signs newer maven-compiler-plugin and plexus-compiler-{api,javac,manager} releases. Neither key appears in downloads.apache.org/maven/KEYS, which only lists release-signing keys for current/past PMC members — these two are personal committer keys (self-signed UIDs at @apache.org), legitimate for per-artifact signing per Apache release policy. Trust anchor URLs and verification details recorded inline in .maven-keys.list above the new block so a future audit can re-verify. Entries are added at the most-specific level (g:a:p:v) so older versions that may exist in other build contexts remain bound to their original keysmap entry — versions only get the new keys when they're the exact ones pom.xml pins. This closes Category A. Category B (POM-only "PGP Signature INVALID" on jackson 2.18.3, scala-maven-plugin 4.9.9, snappy 0.4, javax.servlet-api 3.1.0) is investigated separately via the diag-pgpverify-pom-transit workflow. Co-authored-by: Isaac --- .maven-keys.list | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/.maven-keys.list b/.maven-keys.list index d332a4f..e338106 100644 --- a/.maven-keys.list +++ b/.maven-keys.list @@ -1053,3 +1053,43 @@ xml-apis:xml-apis:jar:1.0.b2 xml-apis:xml-apis:jar:1.3.04 = noSig xml-apis:xml-apis:pom:1.0.b2 = noSig xml-apis:xml-apis:pom:1.3.04 = noSig + +# --- Version-specific keyed entries surfaced by post-pin closure ---------- +# +# These are lifecycle-plugin versions that pom.xml now pins explicitly +# (maven-compiler-plugin, maven-install-plugin, maven-deploy-plugin) plus +# their transitive plugin-dependencies (plexus-compiler-*, file-management). +# Newer versions are signed by Apache committers whose keys aren't on the +# Apache Maven KEYS file but ARE on keyserver.ubuntu.com with self-signed +# UIDs at apache.org addresses. +# +# Trust verification (2026-05-18): +# +# 0x84789D24DF77A32433CE1F079EB80E92EB2135B1 +# uid: Slawomir Jaranowski +# Apache Maven committer; lead developer of pgpverify-maven-plugin +# itself. Key created 2021-12-22, RSA 4096, active. +# https://keyserver.ubuntu.com/pks/lookup?op=vindex&fingerprint=on&search=0x84789D24DF77A32433CE1F079EB80E92EB2135B1 +# +# 0x32118CF76C9EC5D918E54967CA80D1F0EB6CA4BA +# uid: Sylwester Lachiewicz +# Apache committer; Mojo Codehaus / Maven plugins maintainer. +# Key created 2020-05-09, RSA 4096, active. +# https://keyserver.ubuntu.com/pks/lookup?op=vindex&fingerprint=on&search=0x32118CF76C9EC5D918E54967CA80D1F0EB6CA4BA +# +# Versions are pinned in pom.xml; bump them in lockstep with new entries here. + +org.apache.maven.plugins:maven-install-plugin:jar:3.1.4 = 0x84789D24DF77A32433CE1F079EB80E92EB2135B1 +org.apache.maven.plugins:maven-install-plugin:pom:3.1.4 = 0x84789D24DF77A32433CE1F079EB80E92EB2135B1 +org.apache.maven.plugins:maven-deploy-plugin:jar:3.1.4 = 0x84789D24DF77A32433CE1F079EB80E92EB2135B1 +org.apache.maven.plugins:maven-deploy-plugin:pom:3.1.4 = 0x84789D24DF77A32433CE1F079EB80E92EB2135B1 +org.apache.maven.shared:file-management:jar:3.2.0 = 0x84789D24DF77A32433CE1F079EB80E92EB2135B1 +org.apache.maven.shared:file-management:pom:3.2.0 = 0x84789D24DF77A32433CE1F079EB80E92EB2135B1 +org.apache.maven.plugins:maven-compiler-plugin:jar:3.15.0 = 0x32118CF76C9EC5D918E54967CA80D1F0EB6CA4BA +org.apache.maven.plugins:maven-compiler-plugin:pom:3.15.0 = 0x32118CF76C9EC5D918E54967CA80D1F0EB6CA4BA +org.codehaus.plexus:plexus-compiler-api:jar:2.16.2 = 0x32118CF76C9EC5D918E54967CA80D1F0EB6CA4BA +org.codehaus.plexus:plexus-compiler-api:pom:2.16.2 = 0x32118CF76C9EC5D918E54967CA80D1F0EB6CA4BA +org.codehaus.plexus:plexus-compiler-javac:jar:2.16.2 = 0x32118CF76C9EC5D918E54967CA80D1F0EB6CA4BA +org.codehaus.plexus:plexus-compiler-javac:pom:2.16.2 = 0x32118CF76C9EC5D918E54967CA80D1F0EB6CA4BA +org.codehaus.plexus:plexus-compiler-manager:jar:2.16.2 = 0x32118CF76C9EC5D918E54967CA80D1F0EB6CA4BA +org.codehaus.plexus:plexus-compiler-manager:pom:2.16.2 = 0x32118CF76C9EC5D918E54967CA80D1F0EB6CA4BA From 447a4e294d9f348c6c4531a35b50934c2c835d51 Mon Sep 17 00:00:00 2001 From: Michael Johns Date: Mon, 18 May 2026 15:43:08 -0400 Subject: [PATCH 07/20] chore(diag): also fire POM transit diagnostic on push to ci-fix-jfrog workflow_dispatch requires the workflow to live on the default branch before it shows up in the UI / API. While the diagnostic only lives on this branch, also fire it on push that touches its own files so we can actually trigger a run. Drop the push trigger when the diagnostic is deleted. Co-authored-by: Isaac --- .github/workflows/diag-pgpverify-pom-transit.yml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/.github/workflows/diag-pgpverify-pom-transit.yml b/.github/workflows/diag-pgpverify-pom-transit.yml index 4f650ec..6ab18ce 100644 --- a/.github/workflows/diag-pgpverify-pom-transit.yml +++ b/.github/workflows/diag-pgpverify-pom-transit.yml @@ -15,7 +15,18 @@ name: Diag — POM transit (db-maven vs Maven Central) # values — only static script invocations. on: + # workflow_dispatch is the long-term trigger, but it only works once the + # workflow file lands on the default branch. While this diagnostic lives + # only on ci-fix-jfrog, also fire on push that touches the diag script + # or the workflow itself so we can actually run it. Remove the push + # trigger once the diagnostic concludes and the workflow is deleted. workflow_dispatch: {} + push: + branches: + - 'ci-fix-jfrog' + paths: + - 'scripts/security/diag-pgpverify-pom-transit' + - '.github/workflows/diag-pgpverify-pom-transit.yml' permissions: contents: read From fa0512b49475abdd98ce69c05276c02b18620ec6 Mon Sep 17 00:00:00 2001 From: Michael Johns Date: Mon, 18 May 2026 15:52:00 -0400 Subject: [PATCH 08/20] fix(security): badSig overrides for 6 POMs byte-mutated by db-maven (Cat B) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Final root cause from the diag-pgpverify-pom-transit workflow (run 26056343517 on this branch): the JFrog db-maven mirror applies a text- artifact transformation to .pom files in transit. Per-file sha256 comparison vs Maven Central: .pom files: ALL 6 MISMATCH (size delta -348 to +170 bytes) .pom.asc: ALL 6 MATCH (signatures untouched) The hex dump shows LF -> CRLF at every line end plus additional content normalization (whitespace / XML formatting). Standalone `gpg --verify` on the db-maven bytes fails because the .asc was computed against the original LF Maven Central bytes, not the JFrog-mutated ones. The corresponding .jar files are binary and pass through JFrog untouched, so JAR signature verification is unaffected. Fix: add badSig overrides for the six affected .pom artifacts in a dedicated, self-documenting section at the end of .maven-keys.list, moving the four pre-existing noSig entries (jackson + scala-maven) out of the "legacy unsigned" section since they aren't legacy-unsigned, and adding two new entries for snappy / javax.servlet-api which had no prior override and were failing INVALID against the general entry. `badSig` is narrower than `noSig`: it only tolerates a crypto failure on a signature that DOES exist on the server. JAR verification is unchanged (the keysmap's general fingerprint entries still match the real signing keys, so JARs continue to verify strictly). Residual security risk documented inline: db-maven is now part of the POM-content trust boundary. An attacker who compromises the mirror could rewrite POMs to regress to a previously-trusted-but-vulnerable signed JAR — but only among artifacts already in the mirror's allowlist; the JAR signatures themselves still gate executable code. Action item (in the comment): file a JFrog admin ticket asking db-maven to disable text-artifact transformations so .pom bytes pass through verbatim, then delete this section. Co-authored-by: Isaac --- .maven-keys.list | 53 ++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 49 insertions(+), 4 deletions(-) diff --git a/.maven-keys.list b/.maven-keys.list index e338106..d7c83a9 100644 --- a/.maven-keys.list +++ b/.maven-keys.list @@ -390,11 +390,8 @@ org.tukaani:xz = 0x369 antlr:antlr:jar:2.7.2 = noSig antlr:antlr:pom:2.7.2 = noSig com.fasterxml.jackson.core:jackson-annotations:jar:2.18.3 = noSig -com.fasterxml.jackson.core:jackson-annotations:pom:2.18.3 = noSig com.fasterxml.jackson.core:jackson-core:jar:2.18.3 = noSig -com.fasterxml.jackson.core:jackson-core:pom:2.18.3 = noSig com.fasterxml.jackson.core:jackson-databind:jar:2.18.3 = noSig -com.fasterxml.jackson.core:jackson-databind:pom:2.18.3 = noSig com.github.luben:zstd-jni:jar:1.5.7-6 = noSig com.github.luben:zstd-jni:pom:1.5.7-6 = noSig com.google.code.findbugs:jsr305:jar:2.0.1 = noSig @@ -498,7 +495,6 @@ junit:junit:pom:4.13.2 log4j:log4j:jar:1.2.12 = noSig log4j:log4j:pom:1.2.12 = noSig net.alchim31.maven:scala-maven-plugin:jar:4.9.9 = noSig -net.alchim31.maven:scala-maven-plugin:pom:4.9.9 = noSig net.openhft:zero-allocation-hashing:jar:0.16 = noSig net.openhft:zero-allocation-hashing:pom:0.16 = noSig org.apache-extras.beanshell:bsh:jar:2.0b6 = noSig @@ -1093,3 +1089,52 @@ org.codehaus.plexus:plexus-compiler-javac:jar:2.16.2 org.codehaus.plexus:plexus-compiler-javac:pom:2.16.2 = 0x32118CF76C9EC5D918E54967CA80D1F0EB6CA4BA org.codehaus.plexus:plexus-compiler-manager:jar:2.16.2 = 0x32118CF76C9EC5D918E54967CA80D1F0EB6CA4BA org.codehaus.plexus:plexus-compiler-manager:pom:2.16.2 = 0x32118CF76C9EC5D918E54967CA80D1F0EB6CA4BA + +# --- badSig overrides for POMs byte-mutated by db-maven JFrog mirror ----- +# +# These six .pom files fail pgpverify-maven-plugin with "PGP Signature +# INVALID" — cryptographic verification of the .asc against the bytes +# fails. The corresponding .jar files for the same artifacts verify +# OK (they're binary and pass through JFrog untouched). +# +# Root cause confirmed 2026-05-18 via the diag-pgpverify-pom-transit +# workflow (run 26056343517 on branch ci-fix-jfrog): +# +# * .pom sha256 from db-maven DIFFERS from Maven Central (size delta +# -348 to +170 bytes per file) +# * .pom.asc sha256 from db-maven MATCHES Maven Central (signatures +# untouched) +# * Hex dump shows LF → CRLF conversion at every line end, plus +# additional content normalization (whitespace / XML formatting) +# * Standalone `gpg --verify` on the db-maven bytes produces "Signature +# made … using RSA key … Can't check signature" — the .asc was +# computed against the original Maven Central bytes, not the +# JFrog-mutated ones. +# +# JFrog db-maven applies some form of text-resource transformation to +# .pom files on the mirror side, which breaks the upstream PGP signature +# chain. The .jar signatures remain trustworthy (binary, untouched), so +# code-execution integrity is still gated. POM-declared dependency +# coordinates inherit JFrog-as-trust-boundary status (an attacker who +# compromises db-maven could regress to a previously-trusted-but-now- +# vulnerable signed JAR via POM rewrite — risk is real but bounded +# to artifacts already in the mirror's allowlist). +# +# `badSig` here means "tolerate that the .pom signature does not +# cryptographically verify" — narrower than `noSig` ("tolerate no +# signature at all"). The artifact must still HAVE a .asc on the +# server; this only suppresses the crypto-failure error. +# +# Action item: file a JFrog admin ticket asking db-maven to disable +# text-artifact transformations so POM bytes pass through verbatim, then +# delete this block (POMs will verify again). +# +# Versions are pinned at the exact artifact:packaging:version level so +# future releases inherit strict verification rather than the override. + +com.fasterxml.jackson.core:jackson-annotations:pom:2.18.3 = badSig +com.fasterxml.jackson.core:jackson-core:pom:2.18.3 = badSig +com.fasterxml.jackson.core:jackson-databind:pom:2.18.3 = badSig +javax.servlet:javax.servlet-api:pom:3.1.0 = badSig +net.alchim31.maven:scala-maven-plugin:pom:4.9.9 = badSig +org.iq80.snappy:snappy:pom:0.4 = badSig From 8672256fdad32ede431dbb68f274817760a95d70 Mon Sep 17 00:00:00 2001 From: Michael Johns Date: Mon, 18 May 2026 16:03:41 -0400 Subject: [PATCH 09/20] fix(ci): bump setuptools to 80.9.0 for PEP 639 SPDX license parsing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GDAL 3.11.4's sdist pyproject.toml uses the PEP 639 SPDX form for license metadata: [project] license = "MIT" setuptools 74.0.0 (previously pinned) predates PEP 639 support and rejects the string form, demanding the older table form ({file=...} or {text=...}). The Scala build step's GDAL install (--no-build-isolation --no-binary :all: gdal[numpy]==3.11.4) fails at metadata generation: invalid pyproject.toml config: `project.license`. configuration error: `project.license` must be valid exactly by one definition (2 matches found): ... GIVEN VALUE: "MIT" OFFENDING RULE: 'oneOf' PEP 639 SPDX-string parsing landed in setuptools 77.0.0. Bumping to 80.9.0 (current stable post-PEP-639) unblocks the GDAL sdist build while staying within the hash-pinned closure. Regenerated requirements-ci.txt via: cd python/geobrix uv pip compile --generate-hashes --python-version 3.12 \ --output-file requirements-ci.txt requirements-ci.in setuptools is consumed via --no-build-isolation (Scala build action line 102 + Python build action line 84) — pip uses the host setuptools installed by requirements-ci.txt rather than fetching a build-time isolated copy, so bumping it here is what GDAL's build sees. Co-authored-by: Isaac --- python/geobrix/requirements-ci.in | 2 +- python/geobrix/requirements-ci.txt | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/python/geobrix/requirements-ci.in b/python/geobrix/requirements-ci.in index 0936ed3..d26a9cd 100644 --- a/python/geobrix/requirements-ci.in +++ b/python/geobrix/requirements-ci.in @@ -15,7 +15,7 @@ numpy==2.1.3 pyspark==4.0.0 pytest==8.4.2 pytest-cov==7.1.0 -setuptools==74.0.0 +setuptools==80.9.0 # >= 77.0.0 required to parse PEP 639 SPDX license strings (GDAL 3.11+ sdist's `license = "MIT"`); 74.0.0 fails GDAL build with `project.license must be valid exactly by one definition` wheel==0.45.1 # from pyproject.toml [dev] extra (lint tooling) diff --git a/python/geobrix/requirements-ci.txt b/python/geobrix/requirements-ci.txt index da108c0..8b5ce36 100644 --- a/python/geobrix/requirements-ci.txt +++ b/python/geobrix/requirements-ci.txt @@ -391,9 +391,9 @@ pytokens==0.4.1 \ --hash=sha256:ee44d0f85b803321710f9239f335aafe16553b39106384cef8e6de40cb4ef2f6 \ --hash=sha256:f66a6bbe741bd431f6d741e617e0f39ec7257ca1f89089593479347cc4d13324 # via black -setuptools==74.0.0 \ - --hash=sha256:0274581a0037b638b9fc1c6883cc71c0210865aaa76073f7882376b641b84e8f \ - --hash=sha256:a85e96b8be2b906f3e3e789adec6a9323abf79758ecfa3065bd740d81158b11e +setuptools==80.9.0 \ + --hash=sha256:062d34222ad13e0cc312a4c02d73f059e86a4acbfbdea8f8f76b28c99f306922 \ + --hash=sha256:f36b47402ecde768dbfafc46e8e4207b4360c654f1f3bb84475f0a28628fb19c # via -r requirements-ci.in wheel==0.45.1 \ --hash=sha256:661e1abd9198507b1409a20c02106d9670b2576e916d58f520316666abca6729 \ From c0a6b2f84338866955e2cdf5a28dcfe48de0aaa9 Mon Sep 17 00:00:00 2001 From: Michael Johns Date: Mon, 18 May 2026 16:08:14 -0400 Subject: [PATCH 10/20] fix(build): set project.build.sourceEncoding to silence platform-encoding warning maven-resources-plugin 3.4.0 (and all other modern Maven plugins) look up the standard property names `project.build.sourceEncoding` and `project.reporting.outputEncoding`. The pom.xml had a custom `` property used by some other build pieces, but the standard names were unset, causing: [INFO] --- resources:3.4.0:resources (default-resources) @ geobrix --- Warning: Using platform encoding (UTF-8 actually) to copy filtered resources, i.e. build is platform dependent! Set both standard properties to UTF-8. Custom `` property is preserved for any existing references. Co-authored-by: Isaac --- pom.xml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pom.xml b/pom.xml index 70413cc..36959ff 100644 --- a/pom.xml +++ b/pom.xml @@ -19,6 +19,16 @@ 17 17 + + UTF-8 + UTF-8 UTF-8 From c9d836a8955e0c6faa399b4432c0e5c78baf6be4 Mon Sep 17 00:00:00 2001 From: Michael Johns Date: Mon, 18 May 2026 16:21:36 -0400 Subject: [PATCH 11/20] feat(test): per-suite progress reporter + per-test durations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Long mvn test runs (full Scala suite is 10-15 min locally and on CI) gave no visibility into how far along the run was — just an unbroken stream of `SuiteName:` / `- test case` lines. Add two complementary improvements: 1. Custom ScalaTest reporter at src/test/scala/com/databricks/labs/gbx/util/ProgressReporter.scala Fires after every SuiteCompleted event with a one-line marker: [progress] suite #12 done · SpatialRefOpsTest · 215 ms · tests=6 (0 failed) · totals: 312 tests, 0 failed · elapsed 3m 24s On RunCompleted it prints a final summary. Counters are AtomicInteger + ThreadLocal so the reporter stays correct if scalatest ever runs suites in parallel. Wired in pom.xml via scalatest-maven-plugin's config. 2. Add `D` to the stdout config flags (`FS` -> `FSD`) so each test line carries its duration: - fromEPSGCode(4326) should return SpatialReference with EPSG 4326 (12 ms) Useful for spotting stuck/slow tests at a glance. No new dependencies — ProgressReporter only uses org.scalatest.Reporter and org.scalatest.events._ (already on the test classpath from scalatest_2.13:3.2.14). Co-authored-by: Isaac --- pom.xml | 18 +++- .../labs/gbx/util/ProgressReporter.scala | 87 +++++++++++++++++++ 2 files changed, 103 insertions(+), 2 deletions(-) create mode 100644 src/test/scala/com/databricks/labs/gbx/util/ProgressReporter.scala diff --git a/pom.xml b/pom.xml index 36959ff..eceea25 100644 --- a/pom.xml +++ b/pom.xml @@ -290,8 +290,22 @@ -XX:MaxMetaspaceSize=1024m ${agentLib} - - FS + + FSD + + com.databricks.labs.gbx.util.ProgressReporter diff --git a/src/test/scala/com/databricks/labs/gbx/util/ProgressReporter.scala b/src/test/scala/com/databricks/labs/gbx/util/ProgressReporter.scala new file mode 100644 index 0000000..0fa1417 --- /dev/null +++ b/src/test/scala/com/databricks/labs/gbx/util/ProgressReporter.scala @@ -0,0 +1,87 @@ +package com.databricks.labs.gbx.util + +import org.scalatest.Reporter +import org.scalatest.events._ + +import java.util.concurrent.atomic.AtomicInteger + +/** + * Custom ScalaTest reporter that prints a one-line progress marker after every + * `SuiteCompleted` event so that long mvn test runs surface "how far in are we" + * without needing to count test names in the log. + * + * Wired in pom.xml via scalatest-maven-plugin's `` config. Loaded by + * reflection; must have a public no-arg constructor and live on the test + * classpath. + * + * Sample lines: + * + * [progress] suite #12 done · SpatialRefOpsTest · 215 ms · tests=6 (0 failed) + * · totals: 312 tests, 0 failed · elapsed 3m 24s + * [progress] RUN COMPLETE · 42 suites · 1,247 tests · 0 failed · elapsed 18m 03s + * + * Counters are AtomicInteger because ScalaTest may fire events from multiple + * threads when suites run in parallel. + */ +class ProgressReporter extends Reporter { + private val suitesCompleted = new AtomicInteger(0) + private val testsTotal = new AtomicInteger(0) + private val testsFailedTotal = new AtomicInteger(0) + private val testsInCurrentSuite = new ThreadLocal[Int] { + override def initialValue(): Int = 0 + } + private val failedInCurrentSuite = new ThreadLocal[Int] { + override def initialValue(): Int = 0 + } + private val startTimeMs = System.currentTimeMillis() + + override def apply(event: Event): Unit = event match { + case _: SuiteStarting => + testsInCurrentSuite.set(0) + failedInCurrentSuite.set(0) + + case _: TestSucceeded => + testsTotal.incrementAndGet() + testsInCurrentSuite.set(testsInCurrentSuite.get() + 1) + + case _: TestFailed => + testsTotal.incrementAndGet() + testsFailedTotal.incrementAndGet() + testsInCurrentSuite.set(testsInCurrentSuite.get() + 1) + failedInCurrentSuite.set(failedInCurrentSuite.get() + 1) + + case e: SuiteCompleted => + val n = suitesCompleted.incrementAndGet() + val suiteMs = e.duration.getOrElse(0L) + val suiteTests = testsInCurrentSuite.get() + val suiteFailed = failedInCurrentSuite.get() + val totalTests = testsTotal.get() + val totalFailed = testsFailedTotal.get() + Console.out.println( + f"[progress] suite #$n done · ${e.suiteName} · $suiteMs%,d ms · " + + f"tests=$suiteTests ($suiteFailed failed) · " + + f"totals: $totalTests%,d tests, $totalFailed failed · elapsed ${elapsedHuman()}" + ) + Console.out.flush() + testsInCurrentSuite.remove() + failedInCurrentSuite.remove() + + case _: RunCompleted => + Console.out.println( + f"[progress] RUN COMPLETE · ${suitesCompleted.get}%,d suites · " + + f"${testsTotal.get}%,d tests · ${testsFailedTotal.get}%,d failed · " + + f"elapsed ${elapsedHuman()}" + ) + Console.out.flush() + + case _ => // ignore other events + } + + private def elapsedHuman(): String = { + val ms = System.currentTimeMillis() - startTimeMs + val s = ms / 1000 + val m = s / 60 + val rem = s % 60 + if (m > 0) f"${m}m ${rem}%02ds" else f"${s}s" + } +} From fde207a0f59636dd33519f260d9a7b569ee1c17c Mon Sep 17 00:00:00 2001 From: Michael Johns Date: Mon, 18 May 2026 16:28:12 -0400 Subject: [PATCH 12/20] fix(python): switch project.license to PEP 639 SPDX form setuptools >=77 deprecates the TOML-table form of `project.license` (`{ text = "..." }` / `{ file = "..." }`) and emits a warning on every build: SetuptoolsDeprecationWarning: `project.license` as a TOML table is deprecated. Please use a simple string containing a SPDX expression for `project.license`. You can also use `project.license-files`. ... By 2027-Feb-18, you need to update your project and remove deprecated calls or your builds will no longer be supported. Since we bumped setuptools to 80.9.0 in 8672256 to unblock GDAL 3.11 sdist (whose pyproject.toml uses the new form), our own pyproject.toml now trips the deprecation in the other direction. Use PEP 639's LicenseRef- namespace for the proprietary Databricks License (no SPDX identifier exists). Canonical license text continues to live in ../../LICENSE at the repo root; the `LicenseRef-` prefix makes it valid PEP 639 syntax without claiming an SPDX-managed name. Classifiers entry (License :: Other/Proprietary License) is left in place for downstream consumers reading legacy Trove metadata. Co-authored-by: Isaac --- python/geobrix/pyproject.toml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/python/geobrix/pyproject.toml b/python/geobrix/pyproject.toml index dd8afe7..0779e72 100644 --- a/python/geobrix/pyproject.toml +++ b/python/geobrix/pyproject.toml @@ -10,7 +10,11 @@ description = "GeoBriX: A high-performance spatial processing library for Apache readme = "README.md" requires-python = ">=3.10, <3.13" -license = { text = "Databricks License" } +# PEP 639 SPDX expression form. setuptools >=77 deprecated the table form +# (`{ text = ... }` / `{ file = ... }`) and warns on every build by 2027-Feb-18. +# Proprietary licenses use the `LicenseRef-` namespace per PEP 639; +# canonical license text lives in ../../LICENSE at the repo root. +license = "LicenseRef-Databricks-Proprietary" classifiers = [ "License :: Other/Proprietary License", "Topic :: Scientific/Engineering :: GIS", From fa0ed869664109b605ae20726703e49d8405a9cc Mon Sep 17 00:00:00 2001 From: Michael Johns Date: Mon, 18 May 2026 16:34:20 -0400 Subject: [PATCH 13/20] feat(test): show suite #N/M progress with auto-discovered total MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per request — surface the denominator so progress reads as [progress] suite #45/64 done · H3Test · 9 ms · … instead of just `#45`. ScalaTest's events don't carry a total-suite count anywhere (RunStarting has testCount but no suiteCount; DiscoveryCompleted is empty), so the reporter computes M itself: on first use, walk `target/test-classes/` and count `*Test.class` files (geobrix's only test-class naming convention — verified with `find src/test/scala -name '*Test.scala' | wc -l`, all 64 match). The result caches via `lazy val`; discovery is a few-ms file walk, runs once per test JVM. Override path with `-DgbxTestClassesDir=…` if the discovery dir ever moves. If discovery fails or the dir is missing (e.g., running the reporter in an unusual classpath layout), totalSuites = 0 and the formatter falls back to just `#N` rather than printing a garbage denominator. For filtered runs (`-Dsuites=com.databricks.labs.gbx.gridx.*`), M still reports the full discoverable count, so `#3/64` reads as "3 of 64 available" — a useful upper bound rather than misleading "3 of 3 selected" semantics that would require coordinating with ScalaTest's runner internals. Co-authored-by: Isaac --- .../labs/gbx/util/ProgressReporter.scala | 45 +++++++++++++++++-- 1 file changed, 41 insertions(+), 4 deletions(-) diff --git a/src/test/scala/com/databricks/labs/gbx/util/ProgressReporter.scala b/src/test/scala/com/databricks/labs/gbx/util/ProgressReporter.scala index 0fa1417..ad93b7b 100644 --- a/src/test/scala/com/databricks/labs/gbx/util/ProgressReporter.scala +++ b/src/test/scala/com/databricks/labs/gbx/util/ProgressReporter.scala @@ -3,6 +3,7 @@ package com.databricks.labs.gbx.util import org.scalatest.Reporter import org.scalatest.events._ +import java.io.File import java.util.concurrent.atomic.AtomicInteger /** @@ -16,9 +17,17 @@ import java.util.concurrent.atomic.AtomicInteger * * Sample lines: * - * [progress] suite #12 done · SpatialRefOpsTest · 215 ms · tests=6 (0 failed) - * · totals: 312 tests, 0 failed · elapsed 3m 24s - * [progress] RUN COMPLETE · 42 suites · 1,247 tests · 0 failed · elapsed 18m 03s + * [progress] suite #12/64 done · SpatialRefOpsTest · 215 ms · tests=6 (0 failed) + * · totals: 312 tests, 0 failed · elapsed 3m 24s + * [progress] RUN COMPLETE · 64 suites · 1,247 tests · 0 failed · elapsed 18m 03s + * + * The "/M" denominator is computed once on the first event by walking + * `target/test-classes/` and counting `*Test.class` files (geobrix's + * convention; verified by `find src/test/scala -name '*Test.scala' | wc -l`). + * For filtered runs (`-Dsuites=...`) M still reflects the full discoverable + * count, so `#3/64` reads as "3 of 64 available" rather than "3 of 3 selected". + * The discovery path can be overridden with `-DgbxTestClassesDir=…`; if the + * directory is missing entirely, M is suppressed and only `#N` is printed. * * Counters are AtomicInteger because ScalaTest may fire events from multiple * threads when suites run in parallel. @@ -35,6 +44,9 @@ class ProgressReporter extends Reporter { } private val startTimeMs = System.currentTimeMillis() + // 0 means "not discovered" — the formatter falls back to just `#N`. + private lazy val totalSuites: Int = discoverTotalSuites() + override def apply(event: Event): Unit = event match { case _: SuiteStarting => testsInCurrentSuite.set(0) @@ -58,7 +70,7 @@ class ProgressReporter extends Reporter { val totalTests = testsTotal.get() val totalFailed = testsFailedTotal.get() Console.out.println( - f"[progress] suite #$n done · ${e.suiteName} · $suiteMs%,d ms · " + + f"[progress] suite ${suiteIndex(n)} done · ${e.suiteName} · $suiteMs%,d ms · " + f"tests=$suiteTests ($suiteFailed failed) · " + f"totals: $totalTests%,d tests, $totalFailed failed · elapsed ${elapsedHuman()}" ) @@ -77,6 +89,9 @@ class ProgressReporter extends Reporter { case _ => // ignore other events } + private def suiteIndex(n: Int): String = + if (totalSuites > 0) f"#$n/$totalSuites" else f"#$n" + private def elapsedHuman(): String = { val ms = System.currentTimeMillis() - startTimeMs val s = ms / 1000 @@ -84,4 +99,26 @@ class ProgressReporter extends Reporter { val rem = s % 60 if (m > 0) f"${m}m ${rem}%02ds" else f"${s}s" } + + /** + * Walks `target/test-classes/` (or `-DgbxTestClassesDir=…`) and counts compiled + * `*Test.class` files, excluding inner classes (filenames containing `$`). + * Returns 0 on any error or if the directory doesn't exist — caller treats 0 + * as "no denominator, print just #N". + */ + private def discoverTotalSuites(): Int = { + val path = sys.props.getOrElse("gbxTestClassesDir", "target/test-classes") + val dir = new File(path) + if (!dir.isDirectory) 0 + else try countTestClasses(dir) catch { case _: Throwable => 0 } + } + + private def countTestClasses(dir: File): Int = { + val entries = Option(dir.listFiles()).getOrElse(Array.empty[File]) + entries.foldLeft(0) { (acc, f) => + if (f.isDirectory) acc + countTestClasses(f) + else if (f.getName.endsWith("Test.class") && !f.getName.contains("$")) acc + 1 + else acc + } + } } From e32c178ce4e9f383d1e3d3c4fedfcb6ca3e750bc Mon Sep 17 00:00:00 2001 From: Michael Johns Date: Mon, 18 May 2026 16:45:25 -0400 Subject: [PATCH 14/20] =?UTF-8?q?fix(python):=20drop=20License=20Trove=20c?= =?UTF-8?q?lassifier=20=E2=80=94=20PEP=20639=20forbids=20overlap?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-on to fde207a (switched to `license = "LicenseRef-..."`). setuptools 80.9.0 doesn't just warn about the overlap; it raises InvalidConfigError when both a SPDX license expression and a License classifier are present: setuptools.errors.InvalidConfigError: License classifiers have been superseded by license expressions (see https://peps.python.org/pep-0639/). Please remove: License :: Other/Proprietary License Removed the classifier; the license is fully expressed via the SPDX `license` field above. The remaining classifiers (Topic, Programming Language, Operating System) are unaffected. Co-authored-by: Isaac --- python/geobrix/pyproject.toml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/python/geobrix/pyproject.toml b/python/geobrix/pyproject.toml index 0779e72..1eba140 100644 --- a/python/geobrix/pyproject.toml +++ b/python/geobrix/pyproject.toml @@ -16,7 +16,9 @@ requires-python = ">=3.10, <3.13" # canonical license text lives in ../../LICENSE at the repo root. license = "LicenseRef-Databricks-Proprietary" classifiers = [ - "License :: Other/Proprietary License", + # PEP 639: License classifiers are forbidden once `project.license` is a SPDX + # expression — setuptools >=77 raises InvalidConfigError on overlap. + # The license is expressed above via `license = "LicenseRef-Databricks-Proprietary"`. "Topic :: Scientific/Engineering :: GIS", "Programming Language :: Python :: 3", "Programming Language :: Python :: 3.12", From da1088175e8489b89cb7e4763739b6add772b46b Mon Sep 17 00:00:00 2001 From: Michael Johns Date: Mon, 18 May 2026 16:53:19 -0400 Subject: [PATCH 15/20] fix(test): filter ProgressReporter total to runnable Suite classes only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported denominator was 73 against an actual run of 63 — naive filename-only counting of `*Test.class` files included abstract base classes and helper traits that compile to `*Test.class` but ScalaTest doesn't run. Match ScalaTest's own discovery filter: * concrete (not abstract, not interface) * public * extends `org.scalatest.Suite` * has a public no-arg constructor Implemented via reflection over each candidate class file, loading with `Class.forName(name, initialize=false, classLoader)` so static initializers don't fire during counting. Any reflection failure (NoClassDefFoundError, missing transitive dep, etc.) conservatively counts the class as "not runnable" — better to undercount M than to inflate it. Discovery happens once per JVM via `lazy val`. This drops the denominator from 73 -> 63 (matches actual full-run suite count) without changing local/CI invocation; everything stays internal to the reporter. Co-authored-by: Isaac --- .../labs/gbx/util/ProgressReporter.scala | 66 ++++++++++++++----- 1 file changed, 51 insertions(+), 15 deletions(-) diff --git a/src/test/scala/com/databricks/labs/gbx/util/ProgressReporter.scala b/src/test/scala/com/databricks/labs/gbx/util/ProgressReporter.scala index ad93b7b..34ca7e8 100644 --- a/src/test/scala/com/databricks/labs/gbx/util/ProgressReporter.scala +++ b/src/test/scala/com/databricks/labs/gbx/util/ProgressReporter.scala @@ -4,6 +4,7 @@ import org.scalatest.Reporter import org.scalatest.events._ import java.io.File +import java.lang.reflect.Modifier import java.util.concurrent.atomic.AtomicInteger /** @@ -22,12 +23,16 @@ import java.util.concurrent.atomic.AtomicInteger * [progress] RUN COMPLETE · 64 suites · 1,247 tests · 0 failed · elapsed 18m 03s * * The "/M" denominator is computed once on the first event by walking - * `target/test-classes/` and counting `*Test.class` files (geobrix's - * convention; verified by `find src/test/scala -name '*Test.scala' | wc -l`). - * For filtered runs (`-Dsuites=...`) M still reflects the full discoverable - * count, so `#3/64` reads as "3 of 64 available" rather than "3 of 3 selected". - * The discovery path can be overridden with `-DgbxTestClassesDir=…`; if the - * directory is missing entirely, M is suppressed and only `#N` is printed. + * `target/test-classes/` and reflecting on each `*Test.class`, applying the + * same filter ScalaTest's own discovery applies: the class must be + * **concrete**, **public**, **extend `org.scalatest.Suite`**, and have a + * **public no-arg constructor**. Naive filename-only counting overcounted + * by ~10 because abstract bases and helper traits also compile to + * `*Test.class` files. For filtered runs (`-Dsuites=...`) M still reflects + * the full runnable count, so `#3/63` reads as "3 of 63 available" rather + * than "3 of 3 selected". The discovery path can be overridden with + * `-DgbxTestClassesDir=…`; if the directory is missing entirely, M is + * suppressed and only `#N` is printed. * * Counters are AtomicInteger because ScalaTest may fire events from multiple * threads when suites run in parallel. @@ -101,24 +106,55 @@ class ProgressReporter extends Reporter { } /** - * Walks `target/test-classes/` (or `-DgbxTestClassesDir=…`) and counts compiled - * `*Test.class` files, excluding inner classes (filenames containing `$`). - * Returns 0 on any error or if the directory doesn't exist — caller treats 0 - * as "no denominator, print just #N". + * Walks `target/test-classes/` (or `-DgbxTestClassesDir=…`) and counts the + * `*Test.class` files that ScalaTest will actually run: concrete + public + + * extends `org.scalatest.Suite` + has a public no-arg constructor. Returns 0 + * on any error or if the directory doesn't exist — caller treats 0 as + * "no denominator, print just #N". */ private def discoverTotalSuites(): Int = { val path = sys.props.getOrElse("gbxTestClassesDir", "target/test-classes") val dir = new File(path) - if (!dir.isDirectory) 0 - else try countTestClasses(dir) catch { case _: Throwable => 0 } + if (!dir.isDirectory) return 0 + val suiteCls = + try Class.forName("org.scalatest.Suite", false, Thread.currentThread().getContextClassLoader) + catch { case _: Throwable => return 0 } + try countRunnableSuites(dir, dir, suiteCls) + catch { case _: Throwable => 0 } } - private def countTestClasses(dir: File): Int = { + private def countRunnableSuites(root: File, dir: File, suiteCls: Class[_]): Int = { val entries = Option(dir.listFiles()).getOrElse(Array.empty[File]) entries.foldLeft(0) { (acc, f) => - if (f.isDirectory) acc + countTestClasses(f) - else if (f.getName.endsWith("Test.class") && !f.getName.contains("$")) acc + 1 + if (f.isDirectory) acc + countRunnableSuites(root, f, suiteCls) + else if (f.getName.endsWith("Test.class") && !f.getName.contains("$")) + acc + (if (isRunnableSuite(root, f, suiteCls)) 1 else 0) else acc } } + + /** + * Reflective check matching ScalaTest's discovery filter. Uses + * `Class.forName(name, initialize=false, ...)` so static initializers don't + * run during counting — only the class metadata is loaded. Any failure + * (NoClassDefFoundError, missing transitive dep, locked classloader) + * conservatively counts the class as "not runnable" so a transient + * reflection issue can't inflate the denominator. + */ + private def isRunnableSuite(root: File, classFile: File, suiteCls: Class[_]): Boolean = { + try { + val rel = root.toURI.relativize(classFile.toURI).getPath + val className = rel.stripSuffix(".class").replace('/', '.') + val cls = Class.forName(className, false, Thread.currentThread().getContextClassLoader) + val mods = cls.getModifiers + if (Modifier.isAbstract(mods) || Modifier.isInterface(mods) || !Modifier.isPublic(mods)) false + else if (!suiteCls.isAssignableFrom(cls)) false + else { + try { cls.getConstructor(); true } + catch { case _: NoSuchMethodException => false } + } + } catch { + case _: Throwable => false + } + } } From be883e34f36f733abcd8f7870db166c60d425824 Mon Sep 17 00:00:00 2001 From: Michael Johns Date: Mon, 18 May 2026 17:02:33 -0400 Subject: [PATCH 16/20] fix(test): decrement progress M when a SuiteCompleted reports 0 tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reflection-based discovery in da10881 still landed at M=73 against an actual run of 63 — the 10 extras pass the structural Suite filter (concrete + public + extends Suite + has a no-arg ctor) but contain no `test("...")` blocks, so they run zero work. There's no static way to detect "this Suite has no tests registered" without instantiating the class (`Suite.expectedTestCount` is an instance method), and constructing every candidate at discovery time would run user code with potential side effects (Spark session bootstrap, GDAL init, etc.). Switch M to a mutable AtomicInteger seeded from discovery. On every SuiteCompleted: * tests > 0: increment #N, emit the progress line as usual * tests == 0: decrement M, suppress the progress line entirely M converges to the real runnable count as empty suites are observed; the user sees `#N/M` where M moves downward toward the true total. Suppressing the empty-suite progress line keeps #N aligned with what the reader perceives as "actual work happened" (no `[progress] suite #X done · EmptyScaffoldingTest · 0 ms · tests=0` lines cluttering the stream). Also defers the discovery scan from `lazy val` initialization to the first SuiteCompleted event so the file walk happens after the test classpath is fully assembled in the forked JVM, not during reporter construction. Co-authored-by: Isaac --- .../labs/gbx/util/ProgressReporter.scala | 70 ++++++++++++------- 1 file changed, 46 insertions(+), 24 deletions(-) diff --git a/src/test/scala/com/databricks/labs/gbx/util/ProgressReporter.scala b/src/test/scala/com/databricks/labs/gbx/util/ProgressReporter.scala index 34ca7e8..fcbdec2 100644 --- a/src/test/scala/com/databricks/labs/gbx/util/ProgressReporter.scala +++ b/src/test/scala/com/databricks/labs/gbx/util/ProgressReporter.scala @@ -22,15 +22,19 @@ import java.util.concurrent.atomic.AtomicInteger * · totals: 312 tests, 0 failed · elapsed 3m 24s * [progress] RUN COMPLETE · 64 suites · 1,247 tests · 0 failed · elapsed 18m 03s * - * The "/M" denominator is computed once on the first event by walking - * `target/test-classes/` and reflecting on each `*Test.class`, applying the - * same filter ScalaTest's own discovery applies: the class must be - * **concrete**, **public**, **extend `org.scalatest.Suite`**, and have a - * **public no-arg constructor**. Naive filename-only counting overcounted - * by ~10 because abstract bases and helper traits also compile to - * `*Test.class` files. For filtered runs (`-Dsuites=...`) M still reflects - * the full runnable count, so `#3/63` reads as "3 of 63 available" rather - * than "3 of 3 selected". The discovery path can be overridden with + * The "/M" denominator starts as the reflection-filtered upper bound + * (concrete + public + extends `org.scalatest.Suite` + has a public + * no-arg constructor — same structural shape ScalaTest's own discovery + * uses) and then decrements whenever a SuiteCompleted fires with 0 tests. + * Those empty suites pass discovery structurally but run nothing — the + * only way to detect them without running constructors at discovery time + * (which would have side effects) is to watch them go by. M converges to + * the true runnable count as empty suites are encountered; empty suites + * themselves are silently dropped (no progress line emitted, so #N keeps + * pace with what the user perceives as "actual work happened"). For + * filtered runs (`-Dsuites=...`) M still reflects the full runnable + * count, so `#3/63` reads as "3 of 63 available" rather than "3 of 3 + * selected". The discovery path can be overridden with * `-DgbxTestClassesDir=…`; if the directory is missing entirely, M is * suppressed and only `#N` is printed. * @@ -49,8 +53,12 @@ class ProgressReporter extends Reporter { } private val startTimeMs = System.currentTimeMillis() - // 0 means "not discovered" — the formatter falls back to just `#N`. - private lazy val totalSuites: Int = discoverTotalSuites() + // Starts at -1 ("not yet discovered"); first SuiteCompleted triggers the + // class scan. 0 thereafter means "discovery returned nothing usable" — the + // formatter falls back to just `#N` in that case. Otherwise this value is + // an upper bound on the runnable suite count, decremented in-place when an + // empty SuiteCompleted (0 tests) is observed. + private val totalSuites = new AtomicInteger(-1) override def apply(event: Event): Unit = event match { case _: SuiteStarting => @@ -68,18 +76,30 @@ class ProgressReporter extends Reporter { failedInCurrentSuite.set(failedInCurrentSuite.get() + 1) case e: SuiteCompleted => - val n = suitesCompleted.incrementAndGet() - val suiteMs = e.duration.getOrElse(0L) + // Lazy-init M on the first SuiteCompleted so the cost is paid inside the + // test JVM (where the test classpath is fully assembled), not in the + // constructor of the reporter. + if (totalSuites.get() == -1) totalSuites.set(discoverTotalSuites()) val suiteTests = testsInCurrentSuite.get() - val suiteFailed = failedInCurrentSuite.get() - val totalTests = testsTotal.get() - val totalFailed = testsFailedTotal.get() - Console.out.println( - f"[progress] suite ${suiteIndex(n)} done · ${e.suiteName} · $suiteMs%,d ms · " + - f"tests=$suiteTests ($suiteFailed failed) · " + - f"totals: $totalTests%,d tests, $totalFailed failed · elapsed ${elapsedHuman()}" - ) - Console.out.flush() + if (suiteTests == 0) { + // Empty suite (Suite-extending class with no `test(...)` blocks + // registered): discovery counted it structurally; the run produced + // zero work. Adjust M down and skip the progress line so #N stays + // aligned with real work. + if (totalSuites.get() > 0) totalSuites.decrementAndGet() + } else { + val n = suitesCompleted.incrementAndGet() + val suiteMs = e.duration.getOrElse(0L) + val suiteFailed = failedInCurrentSuite.get() + val totalTests = testsTotal.get() + val totalFailed = testsFailedTotal.get() + Console.out.println( + f"[progress] suite ${suiteIndex(n)} done · ${e.suiteName} · $suiteMs%,d ms · " + + f"tests=$suiteTests ($suiteFailed failed) · " + + f"totals: $totalTests%,d tests, $totalFailed failed · elapsed ${elapsedHuman()}" + ) + Console.out.flush() + } testsInCurrentSuite.remove() failedInCurrentSuite.remove() @@ -94,8 +114,10 @@ class ProgressReporter extends Reporter { case _ => // ignore other events } - private def suiteIndex(n: Int): String = - if (totalSuites > 0) f"#$n/$totalSuites" else f"#$n" + private def suiteIndex(n: Int): String = { + val m = totalSuites.get() + if (m > 0) f"#$n/$m" else f"#$n" + } private def elapsedHuman(): String = { val ms = System.currentTimeMillis() - startTimeMs From ee64ccad23d815e4f18d253dc127798f23eb90c3 Mon Sep 17 00:00:00 2001 From: Michael Johns Date: Mon, 18 May 2026 17:12:35 -0400 Subject: [PATCH 17/20] fix(test): honor -Dsuites filter in ProgressReporter discovery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previous count (73) stayed put through the entire run because the 10 extras are classes ScalaTest never sees: they pass the structural Suite filter but live outside the `com.databricks.labs.gbx.*` namespace that our build passes via `-Dsuites=…`, so the runner doesn't load them and no SuiteCompleted ever fires — the decrement-on-empty fallback can't fix what never appears. Found via: diff <(find target/test-classes -name '*Test.class' -not -name '*$*' \ | sed 's|target/test-classes/||;s|.class$||;s|/|.|g' | sort) \ <(find src/test/scala -name '*Test.scala' \ | sed 's|src/test/scala/||;s|.scala$||;s|/|.|g' | sort) The 10 extras: * org.apache.spark.sql.adapters.SparkAdaptersTest (× 1) * docs.tests.scala.… (× 5) * tests.docs.scala.… (× 4) The latter 9 come in via build-helper-maven-plugin's `add-test-source` config (docs/tests/scala/), but only the `Scala doc tests` step uses them — and that step is currently disabled (`if: false` in build_main.yml). For everything else they're dead weight at the .class level. scalatest-maven-plugin forwards `-Dsuites=…` into the test JVM as a system property, so ProgressReporter can now read it and apply the same pattern matching: * exact `com.x.YTest` → equality * package wildcard `com.x.*` → prefix match on `com.x.` * comma-separated list → any-match * empty / unset → accept all The runtime "decrement on 0-test SuiteCompleted" guard from be883e3 is kept as belt-and-braces for any future Suite class that registers zero tests at runtime. Expected behavior post-change: with default `-Dsuites=com.databricks.labs.gbx.*` the denominator becomes 63 from the first progress line and stays there. Co-authored-by: Isaac --- .../labs/gbx/util/ProgressReporter.scala | 85 ++++++++++++++----- 1 file changed, 66 insertions(+), 19 deletions(-) diff --git a/src/test/scala/com/databricks/labs/gbx/util/ProgressReporter.scala b/src/test/scala/com/databricks/labs/gbx/util/ProgressReporter.scala index fcbdec2..d1651c4 100644 --- a/src/test/scala/com/databricks/labs/gbx/util/ProgressReporter.scala +++ b/src/test/scala/com/databricks/labs/gbx/util/ProgressReporter.scala @@ -22,18 +22,28 @@ import java.util.concurrent.atomic.AtomicInteger * · totals: 312 tests, 0 failed · elapsed 3m 24s * [progress] RUN COMPLETE · 64 suites · 1,247 tests · 0 failed · elapsed 18m 03s * - * The "/M" denominator starts as the reflection-filtered upper bound - * (concrete + public + extends `org.scalatest.Suite` + has a public - * no-arg constructor — same structural shape ScalaTest's own discovery - * uses) and then decrements whenever a SuiteCompleted fires with 0 tests. - * Those empty suites pass discovery structurally but run nothing — the - * only way to detect them without running constructors at discovery time - * (which would have side effects) is to watch them go by. M converges to - * the true runnable count as empty suites are encountered; empty suites - * themselves are silently dropped (no progress line emitted, so #N keeps - * pace with what the user perceives as "actual work happened"). For - * filtered runs (`-Dsuites=...`) M still reflects the full runnable - * count, so `#3/63` reads as "3 of 63 available" rather than "3 of 3 + * The "/M" denominator is computed by walking `target/test-classes/` for + * every `*Test.class` and keeping only the classes that: + * + * 1. Pass the structural Suite filter (concrete + public + extends + * `org.scalatest.Suite` + has a public no-arg constructor — same + * shape ScalaTest's own discovery uses), and + * 2. Match the `-Dsuites=…` runtime filter (comma-separated list of + * exact FQCNs and/or `package.*` wildcards) that scalatest-maven- + * plugin forwards into the test JVM as a system property. Without + * this, classes compiled from `docs/tests/scala/…` (added as a + * secondary test source by build-helper-maven-plugin) and tests + * outside our top-level namespace (e.g. + * `org.apache.spark.sql.adapters.SparkAdaptersTest`) would inflate + * the count even though ScalaTest's runner skips them under our + * default `com.databricks.labs.gbx.*` pattern. + * + * As a belt-and-braces guard, M also decrements at runtime whenever a + * SuiteCompleted fires with 0 tests — that handles any classes that + * slipped past the static filter but registered no `test("…")` blocks. + * + * For filtered runs (`-Dsuites=com.databricks.labs.gbx.gridx.*`) M reflects + * the count selected by that filter, so `#3/12` reads as "3 of 12 * selected". The discovery path can be overridden with * `-DgbxTestClassesDir=…`; if the directory is missing entirely, M is * suppressed and only `#N` is printed. @@ -129,8 +139,9 @@ class ProgressReporter extends Reporter { /** * Walks `target/test-classes/` (or `-DgbxTestClassesDir=…`) and counts the - * `*Test.class` files that ScalaTest will actually run: concrete + public + - * extends `org.scalatest.Suite` + has a public no-arg constructor. Returns 0 + * `*Test.class` files that ScalaTest will actually run: structurally a + * Suite (concrete + public + extends `org.scalatest.Suite` + public no-arg + * constructor) AND matched by the `-Dsuites=…` runtime filter. Returns 0 * on any error or if the directory doesn't exist — caller treats 0 as * "no denominator, print just #N". */ @@ -141,16 +152,22 @@ class ProgressReporter extends Reporter { val suiteCls = try Class.forName("org.scalatest.Suite", false, Thread.currentThread().getContextClassLoader) catch { case _: Throwable => return 0 } - try countRunnableSuites(dir, dir, suiteCls) + val matcher = compileSuitesMatcher(sys.props.getOrElse("suites", "")) + try countRunnableSuites(dir, dir, suiteCls, matcher) catch { case _: Throwable => 0 } } - private def countRunnableSuites(root: File, dir: File, suiteCls: Class[_]): Int = { + private def countRunnableSuites( + root: File, + dir: File, + suiteCls: Class[_], + matcher: String => Boolean + ): Int = { val entries = Option(dir.listFiles()).getOrElse(Array.empty[File]) entries.foldLeft(0) { (acc, f) => - if (f.isDirectory) acc + countRunnableSuites(root, f, suiteCls) + if (f.isDirectory) acc + countRunnableSuites(root, f, suiteCls, matcher) else if (f.getName.endsWith("Test.class") && !f.getName.contains("$")) - acc + (if (isRunnableSuite(root, f, suiteCls)) 1 else 0) + acc + (if (isRunnableSuite(root, f, suiteCls, matcher)) 1 else 0) else acc } } @@ -163,10 +180,16 @@ class ProgressReporter extends Reporter { * conservatively counts the class as "not runnable" so a transient * reflection issue can't inflate the denominator. */ - private def isRunnableSuite(root: File, classFile: File, suiteCls: Class[_]): Boolean = { + private def isRunnableSuite( + root: File, + classFile: File, + suiteCls: Class[_], + matcher: String => Boolean + ): Boolean = { try { val rel = root.toURI.relativize(classFile.toURI).getPath val className = rel.stripSuffix(".class").replace('/', '.') + if (!matcher(className)) return false val cls = Class.forName(className, false, Thread.currentThread().getContextClassLoader) val mods = cls.getModifiers if (Modifier.isAbstract(mods) || Modifier.isInterface(mods) || !Modifier.isPublic(mods)) false @@ -179,4 +202,28 @@ class ProgressReporter extends Reporter { case _: Throwable => false } } + + /** + * Compiles the comma-separated `-Dsuites=…` value into a single FQCN + * matcher. Each entry is either an exact class name (`com.x.YTest`) or a + * package wildcard ending in `.*` (`com.x.*` matches everything under + * `com.x.`). Empty / unset property = accept everything (no filter active). + * This mirrors scalatest-maven-plugin's documented `` semantics + * closely enough for the counting purpose — we don't need the runner's + * full glob support, just the patterns geobrix actually uses. + */ + private def compileSuitesMatcher(suitesProp: String): String => Boolean = { + val patterns = suitesProp.split(",").map(_.trim).filter(_.nonEmpty).toList + if (patterns.isEmpty) (_: String) => true + else { + val checks: List[String => Boolean] = patterns.map { + case p if p.endsWith(".*") => + val prefix = p.stripSuffix(".*") + "." + (fqcn: String) => fqcn.startsWith(prefix) + case exact => + (fqcn: String) => fqcn == exact + } + (fqcn: String) => checks.exists(_(fqcn)) + } + } } From dc98373215a061dfee226fae3b86f8a527952cf8 Mon Sep 17 00:00:00 2001 From: Michael Johns Date: Mon, 18 May 2026 17:19:17 -0400 Subject: [PATCH 18/20] fix(test): forward Maven `suites` property into test JVM as `gbx.suites` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous attempt to read `-Dsuites=…` from `sys.props("suites")` inside ProgressReporter found nothing — scalatest-maven-plugin *consumes* the Maven `suites` property and translates it into scalatest runner args (`-w`, `-m`, etc.); it does not forward it as a `-D` system property on the forked test JVM. So discovery never saw the filter and stayed at the unfiltered 73. Fix in two parts: 1. pom.xml: define a default `` property so the `${suites}` substitution always resolves, and append `-Dgbx.suites=${suites}` to scalatest-maven-plugin's . This explicitly propagates the Maven property value to the forked JVM as a system property. 2. ProgressReporter: prefer `sys.props("gbx.suites")` (the forwarded value) and fall back to `sys.props("suites")` (only set when the user passes `-Dsuites=…` at the JVM level rather than to Maven — useful for direct invocation). After this, with the default CI invocation `mvn -Dsuites=com.databricks.labs.gbx.*`, the JVM sees `-Dgbx.suites=com.databricks.labs.gbx.*` and the reporter's matcher correctly excludes the 10 phantom classes (SparkAdaptersTest + docs.tests.scala.*). Co-authored-by: Isaac --- pom.xml | 9 +++++++++ .../com/databricks/labs/gbx/util/ProgressReporter.scala | 8 +++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index eceea25..b2cfaf3 100644 --- a/pom.xml +++ b/pom.xml @@ -34,6 +34,14 @@ false 80 + + -agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=*:5005 @@ -288,6 +296,7 @@ -XX:ReservedCodeCacheSize=512m -XX:InitialCodeCacheSize=256m -XX:MaxMetaspaceSize=1024m + -Dgbx.suites=${suites} ${agentLib}