Improve the profiler-cli publish script and document the whole deployment in a better way

docs-developer/deploying.md

@@ -2,7 +2,7 @@
 
 Our hosting service is [Netlify](https://www.netlify.com/). Deploying on a nginx instance is also possible, see below.
 
-## Deploy to production
+## Branches and automatic deploys
 
 The `production` branch is configured to be automatically deployed to
 <https://profiler.firefox.com>.
@@ -12,7 +12,25 @@ https://main--perf-html.netlify.app. Every pull request will be deployed as well
 separate domain, whose link will be added automatically to the PR:
 ![The link to the preview deployment is in the sections where checks are](images/netlify-link.png)
 
-## How to merge l10n into main
+## The release, step by step
+
+A release happens in this order. The first two steps land on `main` **before**
+the deploy, the last two happen **after** the web version is live:
+
+1. [Merge l10n into main](#1-merge-l10n-into-main), if there are localization changes.
+2. [Bump the profiler-cli version](#2-bump-the-profiler-cli-version).
+3. [Deploy main to production](#3-deploy-main-to-production).
+4. [Publish profiler-cli to npm](#4-publish-profiler-cli-to-npm).
+5. [Tag the CLI release and create the GitHub release page](#5-tag-the-cli-release-and-create-the-github-release-page).
+
+Steps 2, 4 and 5 are only about the command-line interface. You can skip all
+three when nothing in this deploy affects the CLI. Be careful though: the CLI
+includes source from the main `src/` directory, like `profile-logic`, so a
+change outside `profiler-cli/` can still affect it. Processed or Gecko profile
+format version bumps, for example, only reach CLI users through a new release.
+When in doubt, publish. We are not running out of version numbers.
+
+## 1. Merge l10n into main
 
 Our localization process happens inside [Pontoon](https://pontoon.mozilla.org/projects/firefox-profiler/).
 Changes in Pontoon are being pushed into the `l10n` branch. They should be merged
@@ -31,14 +49,23 @@ git fetch upstream && git diff --name-only upstream/main...upstream/l10n | awk -
 Be careful to always use the **create a merge commit** functionality, not
 _squash_ or _rebase_, to keep a better history.
 
-## How to deploy main to production
+## 2. Bump the profiler-cli version
+
+Like the localization changes, the version bump for the
+[`profiler-cli`](../profiler-cli/README.md) package lands on `main` before the
+deploy, so that the deployed `production` branch already contains the version
+that will be published to npm.
 
-Before the deploy, changes in the [`l10n`](https://github.com/firefox-devtools/profiler/tree/l10n)
-branch should be merged into `main` if there are any localization changes. See
-the [section related to this step](#how-to-merge-l10n-into-main) for more details.
+Edit the `version` field in [`profiler-cli/package.json`](../profiler-cli/package.json)
+and land it as its own pull request, titled
+`Bump profiler-cli version to <VERSION>`. Nothing gets published at this point:
+that happens in [step 4](#4-publish-profiler-cli-to-npm), once the web version is
+deployed.
 
-After merging the `l10n` branch, we can continue with the deployment.
-The easiest by far is to
+## 3. Deploy main to production
+
+Make sure [step 1](#1-merge-l10n-into-main) and [step 2](#2-bump-the-profiler-cli-version)
+are landed on `main` first. Then, the easiest by far is to
 [create a pull request on GitHub](https://github.com/firefox-devtools/profiler/compare/production...main?expand=1).
 It would be nice to write down the main changes in the PR description ([see below](#user-content-helpful-git-commands-to-write-the-main-changes)).
 
@@ -71,52 +98,24 @@ git fetch upstream && git log upstream/production..upstream/main --first-parent
 git log upstream/production..upstream/main --grep '^Pontoon' --format="%(trailers:key=Co-authored-by,valueonly)" | awk NF | sed -E 's/([^<]*).*\(([a-z-]+)\)/\2: \1/i' | sort -h | uniq
 ```
 
-## How to revert to a previous version
-
-The easiest way is to reset the production branch to a previous version, and
-force push it. You'll need to enable force-pushing for the branch production,
-using the [Branch Settings on GitHub](https://github.com/firefox-devtools/profiler/settings/branches).
-
-You can use the following script:
-
-```
-sh bin/revert-last-deployment.sh
-```
-
-When you're ready with a fix landed on `main`, you can push a new version to the
-`production` branch as described in the first part.
-
-## Mozilla internal contacts
-
-You can find the Mozilla contacts about our deployment in [this Mozilla-only
-document](https://docs.google.com/document/d/16YRafdIbk4aFgu4EZjMEjX4F6jIcUJQsazW9AORNvfY/edit).
-
-# Deploying on a nginx instance
-
-To deploy on nginx (without support for direct upload from the Firefox UI), run `yarn build-prod`
-and point nginx at the `dist` directory, which needs to be at the root of the webserver. Additionally,
-a `error_page 404 =200 /index.html;` directive needs to be added so that unknown URLs respond with index.html.
-For a more production-ready configuration, have a look at the netlify [`_headers`](/res/_headers) file.
-
-# Publishing profiler-cli to npm
+## 4. Publish profiler-cli to npm
 
 The [`@firefox-devtools/profiler-cli`](https://www.npmjs.com/package/@firefox-devtools/profiler-cli)
 package is published to npm from this repository. It provides a command-line
-interface for querying Firefox Profiler profiles — see
+interface for querying Firefox Profiler profiles, see
 [`profiler-cli/README.md`](../profiler-cli/README.md) for usage.
 
-## Prerequisites
+### Prerequisites
 
-- Be logged in to npm (`npm login`) with publish access to the `@firefox-devtools` scope.
-- Make sure the working tree is clean and you are on the commit you want to publish.
+- The [version bump](#2-bump-the-profiler-cli-version) is landed and the web
+  version is [already deployed](#3-deploy-main-to-production).
+- Have an npm account with publish access to the `@firefox-devtools` scope. The
+  publish script runs `npm login` for you if you are not logged in already.
+- Be on the commit you want to publish. The publish script refuses to run on a
+  dirty working copy, so commit or stash everything first.
 - Run `yarn test-all` (or at least `yarn test-cli`) to confirm the CLI still builds and passes tests.
 
-## Bump the version
-
-Edit the `version` field in [`profiler-cli/package.json`](../profiler-cli/package.json),
-then land the version bump on `main` before publishing.
-
-## Publish
+### Publish
 
 From the repository root:
 
@@ -126,13 +125,22 @@ yarn publish-cli
 
 [`scripts/publish-profiler-cli.mjs`](../scripts/publish-profiler-cli.mjs) will:
 
-1. Run `yarn build-cli` to produce `profiler-cli/dist/profiler-cli.js` (a
+1. Check that the working copy is clean with `git status --porcelain`, and refuse
+   to publish otherwise. `yarn build-cli` below bundles the working copy rather
+   than a commit, so uncommitted changes would end up on npm with no commit or
+   tag matching them.
+2. Run `npm login` if `npm whoami` says you are not logged in, so the
+   interactive part happens before the long test run rather than after it.
+3. Run `yarn test-all`.
+4. Run `yarn build-cli` to produce `profiler-cli/dist/profiler-cli.js` (a
    single self-contained bundle with no runtime dependencies).
-2. Run `npm publish profiler-cli/`, picking `--tag next` when the version
-   contains `-` (e.g. `0.1.0-next.1`) and `--tag latest` otherwise.
-3. Trigger the `prepublishOnly` hook in `profiler-cli/package.json`, which runs
+5. Run `npm publish profiler-cli/`, picking `--tag next` when the version
+   contains `-` (e.g. `0.1.0-next.1`) and `--tag latest` otherwise. npm asks for
+   a second browser authentication here, for the 2FA check on the upload
+   itself. Being logged in does not replace it.
+6. Trigger the `prepublishOnly` hook in `profiler-cli/package.json`, which runs
    [`scripts/verify-profiler-cli-build.mjs`](../scripts/verify-profiler-cli-build.mjs)
-   to confirm the bundle exists and embeds the current `package.json` version —
+   to confirm the bundle exists and embeds the current `package.json` version,
    this guards against publishing a stale build.
 
 Extra arguments are forwarded to `npm publish`. For example:
@@ -145,7 +153,7 @@ yarn publish-cli --dry-run
 yarn publish-cli --tag alpha
 ```
 
-## Verify the release
+### Verify the release
 
 After publishing, confirm the new version is listed on
 [npm](https://www.npmjs.com/package/@firefox-devtools/profiler-cli) and installs
@@ -155,3 +163,58 @@ cleanly:
 npm install -g @firefox-devtools/profiler-cli@latest
 profiler-cli --version
 ```
+
+## 5. Tag the CLI release and create the GitHub release page
+
+Once the package is on npm, tag the release and publish the release notes. Tags
+follow the `profiler-cli-v<VERSION>` naming scheme and point at the
+`Bump profiler-cli version to <VERSION>` commit from
+[step 2](#2-bump-the-profiler-cli-version):
+
+```
+git fetch upstream
+git tag profiler-cli-v0.8.0 <BUMP_COMMIT_SHA>
+git push upstream profiler-cli-v0.8.0
+```
+
+Then create the [release page](https://github.com/firefox-devtools/profiler/releases)
+for that tag. The GitHub CLI can generate the changelog from the commits since
+the previous tag:
+
+```
+gh release create profiler-cli-v0.8.0 --title profiler-cli-v0.8.0 --generate-notes --latest
+```
+
+The generated notes need some editing afterwards: add a **Highlights** section
+describing the notable new commands or behaviour changes, remove the noisy
+commits, and link to the deploy announcement for this deploy on
+[Discourse](https://discourse.mozilla.org/c/firefox-tooling-announcements/521).
+Have a look at a [previous release](https://github.com/firefox-devtools/profiler/releases/tag/profiler-cli-v0.8.0)
+for the expected shape.
+
+## How to revert to a previous version
+
+The easiest way is to reset the production branch to a previous version, and
+force push it. You'll need to enable force-pushing for the branch production,
+using the [Branch Settings on GitHub](https://github.com/firefox-devtools/profiler/settings/branches).
+
+You can use the following script:
+
+```
+sh bin/revert-last-deployment.sh
+```
+
+When you're ready with a fix landed on `main`, you can push a new version to the
+`production` branch as described in the first part.
+
+## Mozilla internal contacts
+
+You can find the Mozilla contacts about our deployment in [this Mozilla-only
+document](https://docs.google.com/document/d/16YRafdIbk4aFgu4EZjMEjX4F6jIcUJQsazW9AORNvfY/edit).
+
+# Deploying on a nginx instance
+
+To deploy on nginx (without support for direct upload from the Firefox UI), run `yarn build-prod`
+and point nginx at the `dist` directory, which needs to be at the root of the webserver. Additionally,
+a `error_page 404 =200 /index.html;` directive needs to be added so that unknown URLs respond with index.html.
+For a more production-ready configuration, have a look at the netlify [`_headers`](/res/_headers) file.

profiler-cli/CONTRIBUTING.md

@@ -29,7 +29,10 @@ This means:
 - Developers working on the CLI use the root package.json dependencies
 - The `package.json` in this directory is for npm publishing only, not for development
 
-To publish, see [`docs-developer/deploying.md`](../docs-developer/deploying.md#publishing-profiler-cli-to-npm).
+To publish, see [`docs-developer/deploying.md`](../docs-developer/deploying.md#4-publish-profiler-cli-to-npm).
+The CLI release is part of the regular profiler deploy: the version bump lands on
+`main` before the deploy, and the npm publish plus the GitHub release happen right
+after it.
 
 ## Development Workflow
 

profiler-cli/package.json

@@ -5,6 +5,9 @@
   "scripts": {
     "prepublishOnly": "node ../scripts/verify-profiler-cli-build.mjs"
   },
+  "publishConfig": {
+    "registry": "https://registry.npmjs.org/"
+  },
   "main": "./dist/profiler-cli.js",
   "bin": {
     "profiler-cli": "dist/profiler-cli.js",

scripts/publish-profiler-cli.mjs

@@ -7,7 +7,7 @@ import { fileURLToPath } from 'url';
 
 const repoRoot = fileURLToPath(new URL('..', import.meta.url));
 const pkgUrl = new URL('../profiler-cli/package.json', import.meta.url);
-const { version } = JSON.parse(readFileSync(pkgUrl, 'utf8'));
+const { name, version } = JSON.parse(readFileSync(pkgUrl, 'utf8'));
 
 const forwardedArgs = process.argv.slice(2);
 const userSpecifiedTag = forwardedArgs.some(
@@ -18,13 +18,98 @@ const tagArgs = userSpecifiedTag
   ? []
   : ['--tag', isPrerelease ? 'next' : 'latest'];
 
-function run(cmd, args) {
-  const result = spawnSync(cmd, args, { cwd: repoRoot, stdio: 'inherit' });
+// Yarn 1 injects its own `npm_config_*` variables into the environment of the
+// scripts it runs, most importantly `npm_config_registry` pointing at
+// registry.yarnpkg.com. npm inherits those, and since that mirror is read-only
+// and holds none of the credentials from ~/.npmrc, `npm publish` fails with
+// ENEEDAUTH. Give npm the environment it would see in a plain shell.
+function envWithoutNpmConfig() {
+  return Object.fromEntries(
+    Object.entries(process.env).filter(([key]) => !key.startsWith('npm_'))
+  );
+}
+
+function run(cmd, args, options = {}) {
+  const result = spawnSync(cmd, args, {
+    cwd: repoRoot,
+    stdio: 'inherit',
+    ...options,
+  });
+  if (result.error) {
+    console.error(`Failed to run '${cmd}': ${result.error.message}`);
+    process.exit(1);
+  }
   if (result.status !== 0) {
     process.exit(result.status ?? 1);
   }
 }
 
+function runNpm(args) {
+  run('npm', args, { env: envWithoutNpmConfig() });
+}
+
+// `yarn build-cli` bundles the working copy, not a commit, so uncommitted
+// changes would ship to npm with no commit or tag matching them.
+function getDirtyFiles() {
+  const result = spawnSync('git', ['status', '--porcelain'], {
+    cwd: repoRoot,
+    encoding: 'utf8',
+  });
+  if (result.error || result.status !== 0) {
+    return null;
+  }
+  return result.stdout.split('\n').filter((line) => line.trim() !== '');
+}
+
+function checkWorkingCopyClean() {
+  const dirtyFiles = getDirtyFiles();
+  if (dirtyFiles === null) {
+    console.error(
+      `Could not run 'git status' in ${repoRoot} to check the working copy.`
+    );
+    process.exit(1);
+  }
+
+  if (dirtyFiles.length === 0) {
+    return;
+  }
+
+  const shown = dirtyFiles.slice(0, 20);
+  const rest = dirtyFiles.length - shown.length;
+  const listing = shown.join('\n') + (rest > 0 ? `\n... and ${rest} more` : '');
+
+  console.error(
+    `Working copy is not clean. 'yarn build-cli' bundles the working copy, so\n` +
+      `these changes would be published without a matching commit or tag:\n\n` +
+      `${listing}\n\n` +
+      `Commit or stash them before publishing.`
+  );
+  process.exit(1);
+}
+
+// `npm publish` needs a token in ~/.npmrc to start at all, and fails with
+// ENEEDAUTH rather than offering to log you in. Get that out of the way before
+// the long test run instead of after it. The separate browser round trip npm
+// makes at publish time is the 2FA check for the upload itself, so being
+// logged in here does not replace it.
+function isLoggedIn() {
+  const result = spawnSync('npm', ['whoami'], {
+    cwd: repoRoot,
+    env: envWithoutNpmConfig(),
+    stdio: 'ignore',
+  });
+  return result.status === 0;
+}
+
+console.log(`Publishing ${name}@${version} ${tagArgs.join(' ')}`.trim());
+
+checkWorkingCopyClean();
+
+if (!isLoggedIn()) {
+  console.log('Not logged in to npm, running `npm login`.');
+  runNpm(['login']);
+}
+
 run('yarn', ['test-all']);
 run('yarn', ['build-cli']);
-run('npm', ['publish', 'profiler-cli/', ...tagArgs, ...forwardedArgs]);
+runNpm(['publish', 'profiler-cli/', ...tagArgs, ...forwardedArgs]);