diff --git a/.github/workflows/build-element-call.yaml b/.github/workflows/build-element-call.yaml index f4071975e..2eeb8b577 100644 --- a/.github/workflows/build-element-call.yaml +++ b/.github/workflows/build-element-call.yaml @@ -14,6 +14,11 @@ on: description: The build mode for vite. Must be either 'development' or 'production' required: false default: production + layered: + type: boolean + description: Build against the matching branch of matrix-js-sdk (if any) instead of the locked commit. + required: false + default: false secrets: SENTRY_ORG: required: true @@ -43,8 +48,15 @@ jobs: cache: "pnpm" node-version-file: ".node-version" - name: Install dependencies + if: ${{ !inputs.layered }} # ignore-pnpmfile should never be commited. Make CI crash if it happened (`pnpmfileChecksum` is present) run: "pnpm install --frozen-lockfile --ignore-pnpmfile" + - name: Install dependencies (layered) + if: ${{ inputs.layered }} + # ignore-pnpmfile should never be commited. Make CI crash if it happened (`pnpmfileChecksum` is present) + run: "scripts/layered.sh --ignore-pnpmfile" + env: + PR_HEAD_REPO_OWNER: ${{ github.event.pull_request.head.repo.owner.login }} - name: Build Element Call run: pnpm run build:"$PACKAGE":"$BUILD_MODE" env: diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 3fe10723d..aa14a3dff 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -15,6 +15,7 @@ jobs: package: full vite_app_version: ${{ github.event.release.tag_name || github.sha }} build_mode: ${{ github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'development build') && 'development' || 'production' }} + layered: ${{ github.event_name == 'pull_request' }} secrets: SENTRY_ORG: ${{ secrets.SENTRY_ORG }} SENTRY_PROJECT: ${{ secrets.SENTRY_PROJECT }} @@ -65,6 +66,7 @@ jobs: package: embedded vite_app_version: ${{ github.event.release.tag_name || github.sha }} build_mode: ${{ github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'development build') && 'development' || 'production' }} + layered: ${{ github.event_name == 'pull_request' }} secrets: SENTRY_ORG: ${{ secrets.SENTRY_ORG }} SENTRY_PROJECT: ${{ secrets.SENTRY_PROJECT }} @@ -79,6 +81,7 @@ jobs: package: sdk vite_app_version: ${{ github.event.release.tag_name || github.sha }} build_mode: ${{ github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'development build') && 'development' || 'production' }} + layered: ${{ github.event_name == 'pull_request' }} secrets: SENTRY_ORG: ${{ secrets.SENTRY_ORG }} SENTRY_PROJECT: ${{ secrets.SENTRY_PROJECT }} diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml index ad92e7d1a..89d0baf62 100644 --- a/.github/workflows/lint.yaml +++ b/.github/workflows/lint.yaml @@ -17,9 +17,11 @@ jobs: with: cache: "pnpm" node-version-file: ".node-version" - - name: Install dependencies + - name: Install dependencies (layered) # ignore-pnpmfile should never be commited. Make CI crash if it happened (`pnpmfileChecksum` is present) - run: "pnpm install --frozen-lockfile --ignore-pnpmfile" + run: "scripts/layered.sh --ignore-pnpmfile" + env: + PR_HEAD_REPO_OWNER: ${{ github.event.pull_request.head.repo.owner.login }} - name: Formatting run: "pnpm run format:check" - name: i18n diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 3f86f093d..582e8ab2f 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -20,8 +20,15 @@ jobs: cache: "pnpm" node-version-file: ".node-version" - name: Install dependencies + if: github.event_name != 'pull_request' # ignore-pnpmfile should never be commited. Make CI crash if it happened (`pnpmfileChecksum` is present) run: "pnpm install --frozen-lockfile --ignore-pnpmfile" + - name: Install dependencies (layered) + if: github.event_name == 'pull_request' + # ignore-pnpmfile should never be commited. Make CI crash if it happened (`pnpmfileChecksum` is present) + run: "scripts/layered.sh --ignore-pnpmfile" + env: + PR_HEAD_REPO_OWNER: ${{ github.event.pull_request.head.repo.owner.login }} - name: Get Playwright version run: echo "PLAYWRIGHT_VERSION=$(pnpm list @playwright/test --depth=0 --json | jq -r '.[0].devDependencies["@playwright/test"].version')" >> $GITHUB_ENV - name: Cache Playwright binaries @@ -58,8 +65,14 @@ jobs: cache: "pnpm" node-version-file: ".node-version" - name: Install dependencies + if: github.event_name != 'pull_request' # ignore-pnpmfile should never be commited. Make CI crash if it happened (`pnpmfileChecksum` is present) run: pnpm install --frozen-lockfile --ignore-pnpmfile + - name: Install dependencies (layered) + if: github.event_name == 'pull_request' + run: scripts/layered.sh --ignore-pnpmfile + env: + PR_HEAD_REPO_OWNER: ${{ github.event.pull_request.head.repo.owner.login }} - name: Get Playwright version run: echo "PLAYWRIGHT_VERSION=$(pnpm list @playwright/test --depth=0 --json | jq -r '.[0].devDependencies["@playwright/test"].version')" >> $GITHUB_ENV - name: Cache Playwright binaries diff --git a/.gitignore b/.gitignore index 14e96439a..4233ecbeb 100644 --- a/.gitignore +++ b/.gitignore @@ -41,3 +41,6 @@ yarn-error.log *storybook.log storybook-static + +# Nested checkout made by scripts/layered.sh +/matrix-js-sdk/ diff --git a/docs/linking.md b/docs/linking.md index 3a18844d4..3ee49a1a9 100644 --- a/docs/linking.md +++ b/docs/linking.md @@ -60,6 +60,21 @@ git config --local core.hooksPath .githooks This will add the hook path for this repository only to .gihooks. which is a tracked (by git) folder containing the pre-commit hook. +## Testing against a matrix-js-sdk branch in CI + +Linking covers local development. In CI, `scripts/layered.sh` plays the same +role for `matrix-js-sdk`: after the usual frozen install, `scripts/fetchdep.sh` +looks for a matrix-js-sdk branch with the same name as the pull request's head +branch (in the fork first, when the pull request comes from one). If there is +one, it is cloned, packed and installed over the locked version. If there is +none, the install stays as `pnpm-lock.yaml` pins it. + +This means a change that needs a not yet merged matrix-js-sdk change can be tested +by giving both branches the same name. This mirrors how element-web tests against +matrix-js-sdk. Once the matrix-js-sdk branch is merged and deleted, the pull +request falls back to the lockfile and needs a lockfile bump to stay green, +which is also what keeps `main` consistent after the merge. + ## Background Information, why this approach is used can be found in the [linking concept reasoning](./linking_concept_reasoning.md) document. diff --git a/scripts/fetchdep.sh b/scripts/fetchdep.sh new file mode 100755 index 000000000..705b3b80a --- /dev/null +++ b/scripts/fetchdep.sh @@ -0,0 +1,73 @@ +#!/usr/bin/env bash + +# Clones the branch of a dependency's repository that has the same name as +# the pull request branch under test, so a change spanning both repositories +# can be tested before either side is merged. Inspired by scripts/fetchdep.sh +# element-web. +# +# Usage: scripts/fetchdep.sh [ref] +# +# The clone lands in ./. When the pull request comes from a fork and +# PR_HEAD_REPO_OWNER was set, the fork's own copy of the repository is tried +# before 's. Exits 0 when a branch was cloned and 1 when there is none, +# so the caller can keep the locked version in that case. Without a ref, +# nothing is closed outside a pull request. + +set -eu + +org=$1 +repo=$2 +ref=${3:-} + +rm -rf "$repo" + +# If a ref was passed, try to clone it or die. +if [ -n "$ref" ]; then + echo "fetchdep.sh: Using $org/$repo at $ref" + git clone --depth 1 "https://github.com/$org/$repo.git" "$repo" + git -C "$repo" fetch --depth 1 origin "$ref" + git -C "$repo" -c advice.detachedHead=false checkout FETCH_HEAD + git -C "$repo" log -1 + exit 0 +fi + +# Otherwise ensure that we're on a pull request. +head_ref=${GITHUB_HEAD_REF:-} +if [ -z "$head_ref" ]; then + echo "fetchdep.sh: Not a pull request, nothing to match" + exit 1 +fi + +# Bots name their branches the same way in every repository, so a match on +# one of theirs would be a coincidence rather than a paired change. +case "$head_ref" in + renovate/* | dependabot/*) + echo "fetchdep.sh: Not matching bot branch $head_ref" + exit 1 + ;; +esac + +# Clones a branch if it exists and ends the script on success. +clone() { + local try_org=$1 + local branch=$2 + echo "fetchdep.sh: Trying to use $try_org/$repo#$branch" + # Disable auth prompts: https://serverfault.com/a/665959 + if GIT_TERMINAL_PROMPT=0 git clone "https://github.com/$try_org/$repo.git" "$repo" --branch "$branch" --depth 1; then + git -C "$repo" log -1 + # Surface the match on the workflow run summary. + echo "::notice title=Matched $repo branch::Using $try_org/$repo#$branch at $(git -C "$repo" rev-parse HEAD)" + exit 0 + fi +} + +# Try cloning from the fork, then from the org. +head_owner=${PR_HEAD_REPO_OWNER:-} +if [ -n "$head_owner" ] && [ "$head_owner" != "${GITHUB_REPOSITORY_OWNER:-}" ]; then + clone "$head_owner" "$head_ref" +fi +clone "$org" "$head_ref" + +# Nothing worked. Die. +echo "fetchdep.sh: No branch named $head_ref in $org/$repo" +exit 1 diff --git a/scripts/layered.sh b/scripts/layered.sh new file mode 100755 index 000000000..878d7dc84 --- /dev/null +++ b/scripts/layered.sh @@ -0,0 +1,59 @@ +#!/usr/bin/env bash + +# Installs dependencies, then swaps matrix-js-sdk for a branch of the same +# name as the pull request branch under test if one exists. This is what +# lets CI test an Element Call branch together with a matrix-js-sdk branch +# before either is merged. Without a matching branch the install is left as +# pnpm-lock.yaml pins it. Inspired by scripts/layered.sh in element-web. +# +# Usage: scripts/layered.sh [pnpm install flags] +# +# The matrix-js-sdk checkout is nested inside this directory because some CI +# systems do not allow moving above the primary checkout. It is git-ignored. +# +# Set JS_SDK_GITHUB_BASE_REF to a ref to use that exact ref of matrix-js-sdk +# instead of looking for a matching branch. +# +# This script is only meant to be used in CI. For local development use +# see docs/linking.md. + +set -ex + +# Paths below are relative to the repository root. +cd "$(dirname "$0")/.." + +pnpm install --frozen-lockfile "$@" + +js_sdk_dep=$(jq -r '.devDependencies["matrix-js-sdk"]' < package.json) + +# Only layer when package.json follows the branch head rather than pinning a +# release, so a release build keeps its pinned version. +if [ "$js_sdk_dep" != "github:matrix-org/matrix-js-sdk#develop" ]; then + echo "layered.sh: Skipping matrix-js-sdk fetch and install as package.json pins $js_sdk_dep" + exit 0 +fi + +if ! scripts/fetchdep.sh matrix-org matrix-js-sdk "${JS_SDK_GITHUB_BASE_REF:-}"; then + echo "layered.sh: No matching matrix-js-sdk branch, keeping the locked version" + exit 0 +fi + +# matrix-js-sdk pins its pnpm version in `devEngines` only. Under corepack, +# pnpm does not switch versions on its own and corepack, finding no +# `packageManager` field there, would walk up to this project's and use +# ours. Pinning it in the clone makes every pnpm invocation inside that +# directory, including the `prepare` script run by pack, use the right one. +js_sdk_pnpm="pnpm@$(jq -r '.devEngines.packageManager.version' < matrix-js-sdk/package.json)" +jq --arg pm "$js_sdk_pnpm" '.packageManager = $pm' matrix-js-sdk/package.json > matrix-js-sdk/package.json.tmp +mv matrix-js-sdk/package.json.tmp matrix-js-sdk/package.json +export COREPACK_ENABLE_DOWNLOAD_PROMPT=0 + +# Install matrix-js-sdk's build dependencies. `prepare` runs on pack below, +# so it is skipped here. +(cd matrix-js-sdk && pnpm install --frozen-lockfile --ignore-scripts) + +# Rather than `pnpm link`, build matrix-js-sdk into a tarball and install +# that. This better reflects a clean install: dependencies may be hoisted +# and shared, and there is no tsconfig.json to pick up sources from. +(cd matrix-js-sdk && pnpm pack --out matrix-js-sdk.tgz) +pnpm install "$(pwd)/matrix-js-sdk/matrix-js-sdk.tgz" "$@"