diff --git a/.github/workflows/pacquet-ci.yml b/.github/workflows/pacquet-ci.yml index 8b99e93132..bbe135b319 100644 --- a/.github/workflows/pacquet-ci.yml +++ b/.github/workflows/pacquet-ci.yml @@ -682,13 +682,7 @@ jobs: with: install: false - - name: Bundle the runtime loader the Rust crates embed - run: pnpm install --frozen-lockfile --filter @pnpm/esm-loader - - name: Run dylint - # Not a `pnpm pipeline`: pnpm runs scripts with the workspace toolchain - # first on `PATH`, and cargo-dylint needs the rustup proxies there to - # build the lint library with the nightly the library pins. # `-D warnings` must come in via `RUSTFLAGS`, not as a trailing # `-- -D warnings` after `cargo dylint`'s `--`: `cargo dylint` # forwards args after `--` to `cargo check`, which would treat @@ -696,7 +690,7 @@ jobs: # how KSXGitHub/parallel-disk-usage invokes the same lint. env: RUSTFLAGS: '-D warnings' - run: cargo dylint --all -- --all-targets --workspace + run: pnpm pipeline rust-dylint --include-workspace-root --full --no-cache # Single aggregate gate — the only Rust CI context branch protection # needs to require. Listing the individual jobs as required checks does diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d0a8b0bce8..fb1b8bc837 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -27,7 +27,7 @@ The repository holds two implementations of the same package manager: the TypeSc ### JavaScript and TypeScript CLI 1. Install pnpm using one of the [official installation methods](https://pnpm.io/installation). **Do not use Corepack.** The scripts in this repository invoke pnpm through the `pn` and `pnx` aliases, which the official installation methods create. Corepack only provides the `pnpm` and `pnpx` commands, so with a Corepack-managed pnpm the build fails with errors like `pn: Permission denied` ([pnpm/pnpm#12448](https://github.com/pnpm/pnpm/issues/12448)). -1. Set up the Rust toolchain first, as described under [Rust toolchain and git hooks](#rust-toolchain-and-git-hooks). `pnpm install` also installs the Rust dependencies and runs `cargo`, so it fails when `cargo` is not on `PATH`. +1. Set up the Rust tools first, as described under [Rust toolchain and git hooks](#rust-toolchain-and-git-hooks). 1. Run `pnpm install` in the root of the repository to install all dependencies. 1. Run `pnpm add ./pnpm11/pnpm/dev -g` to make pnpm from the repository available in the command line via the `pd` command. 1. Run `pnpm run compile` to create an initial build of pnpm from the source in the repository. @@ -45,7 +45,7 @@ sudo dnf install make automake gcc gcc-c++ kernel-devel Rust is now the primary language in this repository, so most contributions need a working Rust toolchain and the Rust developer tools. The Rust workspace (`Cargo.toml`, `rust-toolchain.toml`, `justfile`) lives at the repository root; run `cargo` and `just` from there. -1. Install [`rustup`](https://rustup.rs). You do not need to select a toolchain by hand. `rust-toolchain.toml` pins the version the project builds with, and `rustup` installs it, together with `clippy` from the pinned `default` profile, the first time you run `cargo` inside the repository. +1. Install [`rustup`](https://rustup.rs). `pnpm install` installs the toolchain `rust-toolchain.toml` pins, and the root `package.json` scripts run it. rustup provides the `cargo` you run outside those scripts, such as through `just`, and the dated nightlies that the [pinned formatter](#rust-formatting) and the dylint check build with. 2. Install [`just`](https://just.systems) (the task runner) and [`cargo-binstall`](https://github.com/cargo-bins/cargo-binstall), then install the task tools from the repository root: diff --git a/package.json b/package.json index e7f037e3cc..c03c39b864 100644 --- a/package.json +++ b/package.json @@ -55,6 +55,7 @@ "ci:rust-deny": "cargo deny check", "ci:rust-fmt": "node pnpm/scripts/rustfmt.mjs --all -- --check", "ci:toml-fmt": "taplo format --check", + "ci:rust-dylint": "node pnpm/scripts/cargo-dylint.mjs --all -- --all-targets --workspace", "ci:build-pnpr": "cargo build --locked --release -p pnpr --bin pnpr -p pnpr-fixtures --bin pnpr-prepare" }, "devDependencies": { diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index c547ba2f36..9cb217a02d 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -83,6 +83,7 @@ pipelines: rust-doc: [ci:rust-doc] rust-deny: [ci:rust-deny] rust-format: [ci:rust-fmt, ci:toml-fmt] + rust-dylint: [ci:rust-dylint] build-pnpr: [ci:build-pnpr] # Every entry names a script the root manifest alone defines. A name a @@ -142,6 +143,8 @@ tasks: concurrencyGroup: cargo ci:rust-doc: concurrencyGroup: cargo + ci:rust-dylint: + concurrencyGroup: cargo ci:rust-test: concurrencyGroup: cargo diff --git a/pnpm/scripts/cargo-dylint.mjs b/pnpm/scripts/cargo-dylint.mjs new file mode 100644 index 0000000000..58e285dd4a --- /dev/null +++ b/pnpm/scripts/cargo-dylint.mjs @@ -0,0 +1,28 @@ +// Run `cargo dylint` with rustup's proxies first on `PATH`. +// +// cargo-dylint runs `rustup` itself and builds its lint library with the +// nightly the library pins, which only rustup's `cargo` selects. pnpm runs +// scripts with the workspace toolchain first on `PATH`, ahead of rustup. +import { spawnSync } from 'node:child_process' +import fs from 'node:fs' +import path from 'node:path' +import process from 'node:process' + +const exe = process.platform === 'win32' ? '.exe' : '' + +const pathKey = Object.keys(process.env).find((key) => key.toUpperCase() === 'PATH') ?? 'PATH' +const searchPath = process.env[pathKey] ?? '' +const proxies = searchPath + .split(path.delimiter) + .find((dir) => path.isAbsolute(dir) && ['rustup', 'cargo'].every((name) => fs.existsSync(path.join(dir, name + exe)))) +if (proxies == null) { + process.stderr.write('cargo-dylint needs rustup, and no directory on PATH holds both rustup and cargo.\n') + process.exit(1) +} + +const result = spawnSync(path.join(proxies, 'cargo' + exe), ['dylint', ...process.argv.slice(2)], { + stdio: 'inherit', + env: { ...process.env, [pathKey]: [proxies, searchPath].join(path.delimiter) }, +}) +if (result.error != null) throw result.error +process.exit(result.status ?? 1) diff --git a/pnpm/scripts/cargo-dylint.test.mjs b/pnpm/scripts/cargo-dylint.test.mjs new file mode 100644 index 0000000000..67991e207c --- /dev/null +++ b/pnpm/scripts/cargo-dylint.test.mjs @@ -0,0 +1,65 @@ +import assert from 'node:assert/strict' +import { spawnSync } from 'node:child_process' +import fs from 'node:fs' +import os from 'node:os' +import path from 'node:path' +import process from 'node:process' +import { test } from 'node:test' +import { fileURLToPath } from 'node:url' + +const SCRIPT = path.join(path.dirname(fileURLToPath(import.meta.url)), 'cargo-dylint.mjs') + +function stubDirs (context, layout) { + const root = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'pnpm-cargo-dylint-'))) + context.after(() => fs.rmSync(root, { recursive: true, force: true })) + const record = path.join(root, 'invocations') + const dirs = {} + for (const [dir, names] of Object.entries(layout)) { + dirs[dir] = path.join(root, dir) + fs.mkdirSync(dirs[dir]) + for (const name of names) { + fs.writeFileSync( + path.join(dirs[dir], name), + `#!/bin/sh\necho "${dir}/${name} $* PATH=$PATH" >> '${record}'\n`, + { mode: 0o755 } + ) + } + } + return { dirs, record } +} + +function runHelper (searchPath) { + return spawnSync(process.execPath, [SCRIPT, '--all'], { + encoding: 'utf8', + env: { PATH: [...searchPath, '/usr/bin', '/bin'].join(path.delimiter) }, + }) +} + +test('runs the cargo beside rustup, ahead of a toolchain cargo earlier on PATH', { skip: process.platform === 'win32' }, (context) => { + const { dirs, record } = stubDirs(context, { + toolchain: ['cargo'], + rustupOnly: ['rustup'], + proxies: ['rustup', 'cargo'], + }) + + const result = runHelper([dirs.toolchain, dirs.rustupOnly, dirs.proxies]) + + assert.equal(result.status, 0, result.stderr) + const [invocation, ...rest] = fs.readFileSync(record, 'utf8').trim().split('\n') + assert.deepEqual(rest, []) + assert.ok(invocation.startsWith('proxies/cargo dylint --all PATH='), invocation) + assert.equal(invocation.split('PATH=')[1].split(path.delimiter)[0], dirs.proxies) +}) + +test('fails without a directory that holds both rustup and cargo', { skip: process.platform === 'win32' }, (context) => { + const { dirs, record } = stubDirs(context, { + toolchain: ['cargo'], + rustupOnly: ['rustup'], + }) + + const result = runHelper([dirs.toolchain, dirs.rustupOnly]) + + assert.equal(result.status, 1) + assert.match(result.stderr, /needs rustup/) + assert.equal(fs.existsSync(record), false) +}) diff --git a/pnpm/scripts/pre-push-rust.sh b/pnpm/scripts/pre-push-rust.sh index 8013664ecc..2a4c1f9268 100755 --- a/pnpm/scripts/pre-push-rust.sh +++ b/pnpm/scripts/pre-push-rust.sh @@ -36,24 +36,20 @@ if command -v cargo >/dev/null 2>&1; then failed=1 fi - if command -v cargo-dylint >/dev/null 2>&1; then + if command -v cargo-dylint >/dev/null 2>&1 && command -v rustup >/dev/null 2>&1; then yellow '▸ RUSTFLAGS="-D warnings" cargo dylint --all -- --all-targets --workspace' # Git exports its repository-local variables (GIT_DIR, GIT_INDEX_FILE, # ...) to hooks. When dylint builds a driver for a new toolchain it # runs `git checkout` in its own clone of rust-clippy, and an inherited # GIT_DIR makes that checkout read this repository instead and fail # (https://github.com/trailofbits/dylint/issues/2105). - # pnpm runs this hook with the workspace toolchain first on PATH. - # cargo-dylint builds its lint library with the nightly the library - # pins, which only the rustup proxies installed beside `rustup` select. - rustup_proxies=$(dirname "$(command -v rustup || command -v cargo)") # shellcheck disable=SC2046 - if ! (unset $(git rev-parse --local-env-vars) && PATH="$rustup_proxies:$PATH" RUSTFLAGS='-D warnings' cargo dylint --all -- --all-targets --workspace); then + if ! (unset $(git rev-parse --local-env-vars) && RUSTFLAGS='-D warnings' node pnpm/scripts/cargo-dylint.mjs --all -- --all-targets --workspace); then red '✗ cargo dylint reported lints — `just dylint-fix` applies the ones it can, then commit.' failed=1 fi else - yellow '! cargo-dylint not found on PATH — skipping dylint check (install from source with `cargo install cargo-dylint dylint-link`).' + yellow '! cargo-dylint or rustup not found on PATH — skipping dylint check (install rustup, then cargo-dylint from source with `cargo install cargo-dylint dylint-link`).' fi else yellow '! cargo not found on PATH — skipping Rust format, doc, and dylint checks.' diff --git a/pnpm/scripts/pre-push-rust.test.mjs b/pnpm/scripts/pre-push-rust.test.mjs index 17437f2674..c074c69b9b 100644 --- a/pnpm/scripts/pre-push-rust.test.mjs +++ b/pnpm/scripts/pre-push-rust.test.mjs @@ -10,7 +10,7 @@ import { temporaryRepo } from './git-fixture.mjs' const SCRIPT = path.join(path.dirname(fileURLToPath(import.meta.url)), 'pre-push-rust.sh') -function stubbedCheckout (context) { +function stubbedCheckout (context, { rustup = true } = {}) { const dir = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'pnpm-pre-push-'))) context.after(() => fs.rmSync(dir, { recursive: true, force: true })) const bin = path.join(dir, 'bin') @@ -19,10 +19,12 @@ function stubbedCheckout (context) { const stub = (name, body) => fs.writeFileSync(path.join(bin, name), `#!/bin/sh\n${body}\nexit 0\n`, { mode: 0o755 }) stub('cargo', `if [ "$1" = dylint ]; then env | grep '^GIT_' > '${record}'; fi`) stub('cargo-dylint', '') + if (rustup) stub('rustup', '') stub('typos', '') stub('taplo', '') fs.mkdirSync(path.join(dir, 'pnpm/scripts'), { recursive: true }) fs.writeFileSync(path.join(dir, 'pnpm/scripts/rustfmt.mjs'), '') + fs.copyFileSync(path.join(path.dirname(SCRIPT), 'cargo-dylint.mjs'), path.join(dir, 'pnpm/scripts/cargo-dylint.mjs')) return { dir, bin, record } } @@ -44,3 +46,17 @@ test('runs cargo dylint without the git variables a hook inherits', { skip: proc assert.equal(result.status, 0, result.stderr) assert.equal(fs.readFileSync(record, 'utf8'), '') }) + +test('skips cargo dylint when rustup is not on PATH', { skip: process.platform === 'win32' }, (context) => { + const { dir, bin, record } = stubbedCheckout(context, { rustup: false }) + + const result = spawnSync('bash', [SCRIPT], { + cwd: dir, + encoding: 'utf8', + env: { PATH: [bin, path.dirname(process.execPath), '/usr/bin', '/bin'].join(path.delimiter) }, + }) + + assert.equal(result.status, 0, result.stderr) + assert.match(result.stderr, /skipping dylint check/) + assert.equal(fs.existsSync(record), false) +})