diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 5a9f4487..55e885b1 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -138,8 +138,31 @@ jobs: ORG_URL="https://dev.azure.com/questdb/" PROJECT="questdb-enterprise" PIPELINE_NAME="build-and-test-e2e-javascript-client" - PIPELINES=$(curl -fsS -u ":${ENT_DISPATCH_PAT}" \ - "${ORG_URL}${PROJECT}/_apis/pipelines?api-version=7.0") + # Azure explains a rejection in the response body. --fail-with-body + # keeps it, but on stdout, which $(...) captures; under set -e the + # step would exit before anything printed it. Each call therefore + # reports the captured body itself when curl fails: Azure's JSON + # `message` when it is a non-empty string, otherwise the start of the + # body. The filter outputs nothing for anything else, so jq -e fails + # without first printing `null` or another non-string value. + # An empty body means curl failed before Azure answered (DNS, TLS, + # connection) or Azure sent no explanation; curl's own -sS line + # above then names the cause, so don't blame Azure for it. + azure_failure() { + if [ -z "$2" ]; then + echo "The Azure DevOps $1 failed without a response body; see curl's error above." >&2 + else + echo "Azure DevOps rejected the $1: $( + jq -er '.message | strings | select(length > 0)' <<<"$2" 2>/dev/null || + head -c 500 <<<"$2" + )" >&2 + fi + } + if ! PIPELINES=$(curl --fail-with-body -sS -u ":${ENT_DISPATCH_PAT}" \ + "${ORG_URL}${PROJECT}/_apis/pipelines?api-version=7.0"); then + azure_failure "pipeline lookup" "$PIPELINES" + exit 1 + fi PIPELINE_ID=$(echo "$PIPELINES" | jq -r --arg name "$PIPELINE_NAME" \ '.value[] | select(.name == $name) | .id' | head -1) if [ -z "$PIPELINE_ID" ] || [ "$PIPELINE_ID" = "null" ]; then @@ -152,15 +175,25 @@ jobs: --arg pr "$CLIENT_PR_NUMBER" \ --arg branch "$CLIENT_BRANCH" \ '{ - templateParameters: { - javascriptClientCommit: $commit, - javascriptClientPrNumber: $pr, - clientBranch: $branch - } + templateParameters: ( + { + javascriptClientCommit: $commit, + clientBranch: $branch + } + # Azure rejects an empty string for a string parameter + # ("is not a valid String", HTTP 400), which broke every + # push, schedule and workflow_dispatch run. Omitting it + # applies the pipeline default, which skips the GitHub + # status post. + + (if $pr == "" then {} else {javascriptClientPrNumber: $pr} end) + ) }') - RESPONSE=$(curl -fsS -u ":${ENT_DISPATCH_PAT}" \ + if ! RESPONSE=$(curl --fail-with-body -sS -u ":${ENT_DISPATCH_PAT}" \ -H "Content-Type: application/json" \ -X POST \ -d "$BODY" \ - "${ORG_URL}${PROJECT}/_apis/pipelines/${PIPELINE_ID}/runs?api-version=7.0") + "${ORG_URL}${PROJECT}/_apis/pipelines/${PIPELINE_ID}/runs?api-version=7.0"); then + azure_failure "dispatch" "$RESPONSE" + exit 1 + fi echo "Enterprise E2E queued: $(echo "$RESPONSE" | jq -r '._links.web.href')" diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 8f730ed4..c701e851 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -2,6 +2,17 @@ name: publish on: workflow_dispatch: + inputs: + packages: + description: >- + Packages to publish. Both manifests must carry the same version; + a package left out can be published later under that version. + type: choice + options: + - both + - nodejs + - browser + default: both jobs: test: @@ -61,12 +72,18 @@ jobs: - name: Check for build artifacts run: node scripts/check-build-artifacts.mjs - # Both packages are published from one commit, and npm-publish silently - # skips a version that already exists. Fail here instead of half-releasing. + # The selected packages are published from one commit, and npm-publish + # silently skips a version that already exists. Fail here instead of + # half-releasing or publishing nothing. + # Every gate above still covers both packages: they share client-core, so + # a broken browser build is a broken build even on a Node-only release. - name: Check release versions + env: + RELEASE_PACKAGES: ${{ inputs.packages }} run: node scripts/check-release-versions.mjs - name: Publish @questdb/nodejs-client + if: inputs.packages == 'both' || inputs.packages == 'nodejs' uses: JS-DevTools/npm-publish@v3 with: token: ${{ secrets.CI_TOKEN }} @@ -75,6 +92,7 @@ jobs: package: packages/nodejs-client/package.json - name: Publish @questdb/browser-client + if: inputs.packages == 'both' || inputs.packages == 'browser' uses: JS-DevTools/npm-publish@v3 with: token: ${{ secrets.CI_TOKEN }} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3d0572ab..d2db6c03 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -131,13 +131,16 @@ earlier, and a strict diff would be unsatisfiable rather than merely noisy. ## Releasing -`.github/workflows/publish.yml` publishes both packages from one commit, so -bump `packages/nodejs-client/package.json` and +`.github/workflows/publish.yml` publishes the selected packages (both by +default, see below) from one commit, so bump `packages/nodejs-client/package.json` and `packages/browser-client/package.json` to the same new version before dispatching it. `node scripts/check-release-versions.mjs` runs first and -refuses a dispatch whose version **every** package has already published, -because the publish action skips an existing version silently and such a -dispatch would publish nothing at all. +refuses a dispatch whose version **every** selected package has already +published, because the publish action skips an existing version silently and +such a dispatch would publish nothing at all. For a single-package dispatch +that means re-dispatching a package already on npm is refused even while the +other package is still missing; the error names the missing package and the +`packages` value that publishes it. If a dispatch fails between the two publish steps, leaving one package on npm and the other not, **re-dispatch the same commit**. That is the supported @@ -146,9 +149,28 @@ commit being dispatched, reports `resuming a partial release`, and passes. The published package no-ops inside the publish action, and the missing one lands. A package with the same version but a different or missing `gitHead` is an older unrelated artifact, not a partial release; the gate rejects it and the version -must be bumped. Do not bump merely to escape a genuine same-commit partial +must be bumped, unless that package was deliberately released on its own (see +below). Do not bump merely to escape a genuine same-commit partial release -- that strands the missing half at the old version permanently. +### Releasing one package on its own + +The workflow's `packages` input (`both`, `nodejs`, or `browser`; default +`both`) publishes a subset, for example when a package depends on server +support that has not shipped yet. The manifests must still carry the same +version, but the gate checks only the selected packages against npm, so the +other package can follow later under that same version from a later commit: +dispatch it with its own `packages` value. Do not dispatch `both` for a +follow-up from a later commit; the gate sees the already-published package's +`gitHead` differ from the new commit and rejects the release as an unrelated +artifact. A follow-up from the same commit may dispatch either the missing +package or `both`; with `both`, the gate reports it as a resumed partial +release and the already-published package is skipped. + +If the first package needs a fix before the second ships, bump both manifests +and dispatch only the first again. The second then ships under the newer +version. + `check-release-versions.mjs` decides *whether* to bump, never by how much. Pick the level from the changes the release carries: a change that alters the behaviour of a published API, its emitted wire bytes, or the errors it throws diff --git a/scripts/check-release-versions.mjs b/scripts/check-release-versions.mjs index 77215ef5..1d1fa504 100644 --- a/scripts/check-release-versions.mjs +++ b/scripts/check-release-versions.mjs @@ -1,4 +1,4 @@ -// Guards a two-package release dispatched from one commit. +// Guards a lockstep-versioned release of one or both packages. // // JS-DevTools/npm-publish skips a version that is already on npm, which was // harmless while this repo published one package: a forgotten bump made the @@ -9,19 +9,47 @@ // // The distinction that matters is *which* half is missing. A version no package // has published is a normal release. A version every package has published is a -// forgotten bump, and refusing it is the point of this script. A version only -// some packages have published is resumable only when npm says the published -// artifact came from this exact release commit. Then it is the wreckage of a -// dispatch whose first publish step succeeded and whose second failed, and the -// publish action's skip-if-present behaviour makes re-dispatching it exactly -// the right repair. A different or missing gitHead is an older unrelated -// artifact that must never authorize publishing new code under the same -// version. +// forgotten bump, and refusing it is the point of this script. On a `both` +// dispatch, a version only some packages have published is resumable only +// when npm says the published artifact came from this exact release commit. +// Then it is either the wreckage of a dispatch whose first publish step +// succeeded and whose second failed, or a package deliberately released on its +// own from this commit (see RELEASE_PACKAGES below). Either way the publish +// action's skip-if-present behaviour makes publishing the rest from this commit +// exactly right. A different or missing gitHead is an older unrelated artifact +// that must never authorize publishing new code under the same version. +// +// RELEASE_PACKAGES (the publish workflow's `packages` input) narrows the +// dispatch to one package: `nodejs`, `browser`, or `both` (the default). The +// manifests must still carry the same version, so the repository never drifts +// into per-package versioning, but only the selected packages are checked +// against npm. That lets one package ship first and the other follow later +// under the same version, from a later commit, with its own dispatch. import { execFileSync } from "node:child_process"; import { readFileSync } from "node:fs"; import { join } from "node:path"; -const PACKAGES = ["packages/nodejs-client", "packages/browser-client"]; +const PACKAGES = { + nodejs: "packages/nodejs-client", + browser: "packages/browser-client", +}; +const SELECTIONS = { + both: ["nodejs", "browser"], + nodejs: ["nodejs"], + browser: ["browser"], +}; + +const selectionName = (process.env.RELEASE_PACKAGES ?? "").trim() || "both"; +const selection = Object.hasOwn(SELECTIONS, selectionName) + ? SELECTIONS[selectionName] + : undefined; +if (!selection) { + console.error( + `refusing to publish:\n unknown RELEASE_PACKAGES value ${JSON.stringify(selectionName)}; ` + + `expected one of ${Object.keys(SELECTIONS).join(", ")}.`, + ); + process.exit(1); +} function manifest(packageDirectory) { return JSON.parse( @@ -59,34 +87,58 @@ function publishedGitHead(name, version) { } const problems = []; -const releases = PACKAGES.map(manifest).map(({ name, version }) => ({ - name, - version, -})); +const manifests = Object.entries(PACKAGES).map(([key, directory]) => { + const { name, version } = manifest(directory); + return { key, name, version }; +}); -const versions = new Set(releases.map((release) => release.version)); +const versions = new Set(manifests.map((release) => release.version)); if (versions.size > 1) { problems.push( - `the published packages are on different versions: ${releases + `the published packages are on different versions: ${manifests .map((release) => `${release.name}@${release.version}`) - .join(", ")}. Release them in lockstep.`, + .join(", ")}. Keep the manifests in lockstep.`, ); } +const releases = manifests.filter((release) => selection.includes(release.key)); +const label = ({ name, version }) => `${name}@${version}`; + +function isOnRegistry({ name, version }) { + const onRegistry = publishedVersions(name); + const list = Array.isArray(onRegistry) ? onRegistry : [onRegistry]; + return list.includes(version); +} + const published = []; const pending = []; for (const release of releases) { - const onRegistry = publishedVersions(release.name); - const list = Array.isArray(onRegistry) ? onRegistry : [onRegistry]; - (list.includes(release.version) ? published : pending).push(release); + (isOnRegistry(release) ? published : pending).push(release); } -const label = ({ name, version }) => `${name}@${version}`; if (pending.length === 0) { - problems.push( - `every package has already published this version (${published.map(label).join(", ")}), ` + - `so this dispatch would publish nothing. Bump the version first.`, - ); + if (releases.length === manifests.length) { + problems.push( + `every package has already published this version (${published.map(label).join(", ")}), ` + + `so this dispatch would publish nothing. Bump the version first.`, + ); + } else { + // A narrowed dispatch of an already-published package is most likely the + // follow-up of a single-package release with the wrong package selected. + // Point at the unselected package still missing from npm, if any. + const missing = manifests.filter( + (release) => !selection.includes(release.key) && !isOnRegistry(release), + ); + problems.push( + `the selected package has already published this version (${published.map(label).join(", ")}), ` + + `so this dispatch would publish nothing. ` + + (missing.length > 0 + ? `${missing.map(label).join(", ")} is not on npm yet; dispatch with ` + + `packages=${missing.map((release) => release.key).join(",")} to publish it ` + + `under this version, or bump the version first.` + : `Bump the version first.`), + ); + } } if (published.length > 0 && pending.length > 0) { @@ -100,7 +152,9 @@ if (published.length > 0 && pending.length > 0) { problems.push( `${label(release)} was published from gitHead ${gitHead ?? ""}, ` + `not this release commit ${releaseCommit}; this is an older unrelated artifact, ` + - `not a partial release that can be resumed. Bump the version first.`, + `not a partial release that can be resumed. Bump the version first, or, if ` + + `${release.name} was deliberately released on its own, dispatch with ` + + `packages=${pending.map((p) => p.key).join(",")} to publish only the rest.`, ); } } @@ -112,16 +166,19 @@ if (problems.length > 0) { } if (published.length > 0) { - // Resuming a partial release. Say so loudly: the run is legitimate, but an - // earlier dispatch failed midway and that is worth seeing in the log. + // Resuming a partial release. Say so loudly: the run is legitimate, but + // either an earlier dispatch failed midway or a package was released on its + // own from this commit, and which one is worth checking in the log. console.warn( - `resuming a partial release: ${published.map(label).join(", ")} already on npm, ` + - `publishing ${pending.map(label).join(", ")}. The published package(s) will be skipped.`, + `resuming a partial release: ${published.map(label).join(", ")} already on npm ` + + `from this commit, publishing ${pending.map(label).join(", ")}. Either an earlier ` + + `dispatch failed midway or the published package(s) were released on their own; ` + + `they will be skipped.`, ); } console.log( - `release check passed: ${releases + `release check passed (packages=${selectionName}): ${releases .map((release) => `${release.name}@${release.version}`) .join(", ")}`, ); diff --git a/test/release-version.test.ts b/test/release-version.test.ts index b9ea24c9..1e7d851f 100644 --- a/test/release-version.test.ts +++ b/test/release-version.test.ts @@ -1,17 +1,28 @@ -import { chmod, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { chmod, mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { delimiter, join } from "node:path"; import { spawnSync } from "node:child_process"; -import { describe, expect, it } from "vitest"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +// The only version the fake registry knows. The tests run the script against +// fixture manifests in a temporary root rather than the repository's own, so +// an ordinary version bump cannot change what they observe. +const PUBLISHED = "5.0.0"; + +const NODEJS = "@questdb/nodejs-client"; +const BROWSER = "@questdb/browser-client"; + +// FAKE_NPM_PUBLISHED lists, comma-separated, the packages that have published +// PUBLISHED; every one of them reports FAKE_NPM_GIT_HEAD as its gitHead. const fakeNpm = `#!/usr/bin/env node const [, , command, specifier, field] = process.argv; if (command !== "view") process.exit(2); +const published = (process.env.FAKE_NPM_PUBLISHED ?? "").split(","); if (field === "versions") { - console.log(specifier === "@questdb/nodejs-client" ? '["5.0.0"]' : '[]'); + console.log(published.includes(specifier) ? '["${PUBLISHED}"]' : '[]'); } else if ( - specifier === "@questdb/nodejs-client@5.0.0" && - field === "gitHead" + field === "gitHead" && + published.some((name) => specifier === name + "@${PUBLISHED}") ) { console.log(JSON.stringify(process.env.FAKE_NPM_GIT_HEAD)); } else { @@ -19,40 +30,174 @@ if (field === "versions") { } `; -describe("release version gate", () => { - it.skipIf(process.platform === "win32")( - "resumes only a same-commit partial release", - async () => { - const bin = await mkdtemp(join(tmpdir(), "qwp-release-npm-")); - const npm = join(bin, "npm"); - await writeFile(npm, fakeNpm, "utf8"); - await chmod(npm, 0o755); - - const run = (gitHead: string) => - spawnSync(process.execPath, ["scripts/check-release-versions.mjs"], { - cwd: process.cwd(), - encoding: "utf8", - env: { - ...process.env, - PATH: `${bin}${delimiter}${process.env.PATH ?? ""}`, - GITHUB_SHA: "release-commit", - FAKE_NPM_GIT_HEAD: gitHead, - }, - }); - - try { - const unrelated = run("older-commit"); - expect(unrelated.status).toBe(1); - expect(unrelated.stderr).toContain("older unrelated artifact"); - expect(unrelated.stderr).toContain("Bump the version first"); - - const resumable = run("release-commit"); - expect(resumable.status).toBe(0); - expect(resumable.stderr).toContain("resuming a partial release"); - expect(resumable.stdout).toContain("release check passed"); - } finally { - await rm(bin, { recursive: true, force: true }); - } - }, - ); +const script = join(process.cwd(), "scripts", "check-release-versions.mjs"); + +describe.skipIf(process.platform === "win32")("release version gate", () => { + let scratch: string; + let bin: string; + let root: string; + + /** A repository root holding only the two published manifests. */ + async function manifestRoot( + name: string, + versions: { nodejs: string; browser: string }, + ) { + const directory = join(scratch, name); + const manifests = { + "nodejs-client": { + name: "@questdb/nodejs-client", + version: versions.nodejs, + }, + "browser-client": { + name: "@questdb/browser-client", + version: versions.browser, + }, + }; + for (const [pkg, manifest] of Object.entries(manifests)) { + await mkdir(join(directory, "packages", pkg), { recursive: true }); + await writeFile( + join(directory, "packages", pkg, "package.json"), + JSON.stringify(manifest), + "utf8", + ); + } + return directory; + } + + beforeAll(async () => { + scratch = await mkdtemp(join(tmpdir(), "qwp-release-")); + bin = join(scratch, "bin"); + await mkdir(bin); + const npm = join(bin, "npm"); + await writeFile(npm, fakeNpm, "utf8"); + await chmod(npm, 0o755); + root = await manifestRoot("release", { + nodejs: PUBLISHED, + browser: PUBLISHED, + }); + }); + + afterAll(async () => { + await rm(scratch, { recursive: true, force: true }); + }); + + const run = ( + gitHead: string, + packages?: string, + { cwd = root, published = [NODEJS] } = {}, + ) => + spawnSync(process.execPath, [script], { + cwd, + encoding: "utf8", + env: { + ...process.env, + PATH: `${bin}${delimiter}${process.env.PATH ?? ""}`, + GITHUB_SHA: "release-commit", + FAKE_NPM_GIT_HEAD: gitHead, + FAKE_NPM_PUBLISHED: published.join(","), + RELEASE_PACKAGES: packages ?? "", + }, + }); + + it("resumes only a same-commit partial release", () => { + const unrelated = run("older-commit"); + expect(unrelated.status).toBe(1); + expect(unrelated.stderr).toContain("older unrelated artifact"); + expect(unrelated.stderr).toContain("Bump the version first"); + expect(unrelated.stderr).toContain("packages=browser"); + + const resumable = run("release-commit"); + expect(resumable.status).toBe(0); + expect(resumable.stderr).toContain("resuming a partial release"); + expect(resumable.stdout).toContain("release check passed"); + }); + + it("publishes a selected package on its own", () => { + // The Node package was released on its own from an earlier commit; + // the browser package follows under the same version. + const browserOnly = run("older-commit", "browser"); + expect(browserOnly.status).toBe(0); + expect(browserOnly.stderr).not.toContain("resuming"); + expect(browserOnly.stdout).toContain( + "release check passed (packages=browser): @questdb/browser-client@", + ); + expect(browserOnly.stdout).not.toContain("@questdb/nodejs-client"); + + // The Node package ships first, before either package is on npm. + const nodeOnly = run("release-commit", "nodejs", { published: [] }); + expect(nodeOnly.status).toBe(0); + expect(nodeOnly.stderr).not.toContain("resuming"); + expect(nodeOnly.stdout).toContain( + "release check passed (packages=nodejs): @questdb/nodejs-client@", + ); + expect(nodeOnly.stdout).not.toContain("@questdb/browser-client"); + }); + + it("refuses a selected package that is already published", () => { + // Everything selected is already on npm: a no-op dispatch. The package + // still missing from npm is the one the follow-up should have selected. + const nodeOnly = run("release-commit", "nodejs"); + expect(nodeOnly.status).toBe(1); + expect(nodeOnly.stderr).toContain( + "the selected package has already published", + ); + expect(nodeOnly.stderr).not.toContain("every package"); + expect(nodeOnly.stderr).toContain( + `${BROWSER}@${PUBLISHED} is not on npm yet`, + ); + expect(nodeOnly.stderr).toContain("packages=browser"); + + const browserOnly = run("release-commit", "browser", { + published: [BROWSER], + }); + expect(browserOnly.status).toBe(1); + expect(browserOnly.stderr).toContain( + `${NODEJS}@${PUBLISHED} is not on npm yet`, + ); + expect(browserOnly.stderr).toContain("packages=nodejs"); + }); + + it("refuses a version every package has already published", () => { + // A forgotten bump. A narrowed dispatch has no other package to point + // at, so it must not suggest one. + const published = { published: [NODEJS, BROWSER] }; + + const both = run("release-commit", "both", published); + expect(both.status).toBe(1); + expect(both.stderr).toContain( + "every package has already published this version", + ); + expect(both.stderr).toContain("Bump the version first"); + + for (const packages of ["nodejs", "browser"]) { + const narrowed = run("release-commit", packages, published); + expect(narrowed.status, packages).toBe(1); + expect(narrowed.stderr, packages).toContain( + "the selected package has already published", + ); + expect(narrowed.stderr, packages).toContain("Bump the version first"); + expect(narrowed.stderr, packages).not.toContain("not on npm yet"); + expect(narrowed.stderr, packages).not.toContain("dispatch with"); + } + }); + + it("rejects an unknown package selection", () => { + const unknown = run("release-commit", "node"); + expect(unknown.status).toBe(1); + expect(unknown.stderr).toContain("unknown RELEASE_PACKAGES value"); + }); + + it("keeps the manifests in lockstep whatever the selection", async () => { + const drifted = await manifestRoot("drifted", { + nodejs: "5.1.0", + browser: PUBLISHED, + }); + for (const packages of ["both", "nodejs", "browser"]) { + const result = run("release-commit", packages, { cwd: drifted }); + expect(result.status, packages).toBe(1); + expect(result.stderr, packages).toContain( + "the published packages are on different versions", + ); + } + }); });