diff --git a/.github/workflows/bioccheck.yml b/.github/workflows/bioccheck.yml index 83bdf67..ec65fca 100644 --- a/.github/workflows/bioccheck.yml +++ b/.github/workflows/bioccheck.yml @@ -29,15 +29,15 @@ on: required: false jobs: - set-matrix: + setup: runs-on: ubuntu-latest outputs: - docker_tag: ${{ steps.matrix.outputs.docker_tag }} - is_release_branch: ${{ steps.matrix.outputs.is_release_branch }} - checkout_ref: ${{ steps.matrix.outputs.checkout_ref }} + docker_tag: ${{ steps.resolve.outputs.docker_tag }} + is_release_branch: ${{ steps.resolve.outputs.is_release_branch }} + checkout_ref: ${{ steps.resolve.outputs.checkout_ref }} steps: - - name: Set Matrix Variables - id: matrix + - name: Resolve Container Tag and Checkout Ref + id: resolve run: | OVERRIDE_TAG="${{ inputs.bioc_version }}" @@ -77,29 +77,28 @@ jobs: fi { - echo "docker_tag=[\"${DOCKER_TAG}\"]" + echo "docker_tag=${DOCKER_TAG}" echo "is_release_branch=${IS_RELEASE}" echo "checkout_ref=${CHECKOUT_REF}" } >> "$GITHUB_OUTPUT" echo "Resolved DOCKER_TAG=${DOCKER_TAG}, IS_RELEASE=${IS_RELEASE}, CHECKOUT_REF=${CHECKOUT_REF}" check: - needs: set-matrix + needs: setup runs-on: ubuntu-latest - strategy: - matrix: - docker_tag: ${{ fromJson(needs.set-matrix.outputs.docker_tag) }} container: - image: bioconductor/bioconductor_docker:${{ matrix.docker_tag }} + image: bioconductor/bioconductor_docker:${{ needs.setup.outputs.docker_tag }} env: R_REMOTES_NO_ERRORS_FROM_WARNINGS: 'true' GITHUB_PAT: ${{ github.token }} CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} + # pkgdown is built only on pushes to release branches, so it is installed only then. + INSTALL_PKGDOWN: ${{ inputs.enable_pkgdown && github.event_name != 'pull_request' && needs.setup.outputs.is_release_branch == 'true' }} steps: - name: Checkout Repository uses: actions/checkout@v7 with: - ref: ${{ needs.set-matrix.outputs.checkout_ref }} + ref: ${{ needs.setup.outputs.checkout_ref }} - name: Query R and System Dependencies id: get-deps @@ -116,15 +115,23 @@ jobs: with: path: /usr/local/lib/R/site-library # The "-1-" segment is a manual cache version; bump it to discard all existing caches. - key: ${{ runner.os }}-${{ hashFiles('.github/R-version') }}-1-${{ hashFiles('.github/depends.Rds') }} + # INSTALL_PKGDOWN is part of the key because a cache saved by a run without pkgdown is never re-saved. + key: ${{ runner.os }}-${{ hashFiles('.github/R-version') }}-1-${{ hashFiles('.github/depends.Rds') }}-pkgdown-${{ env.INSTALL_PKGDOWN }} restore-keys: ${{ runner.os }}-${{ hashFiles('.github/R-version') }}-1- - name: Install Package Dependencies + env: + ENABLE_CYCLOCOMP: ${{ inputs.enable_cyclocomp }} shell: Rscript {0} run: | options(repos = c(CRAN = "https://packagemanager.posit.co/cran/__linux__/noble/latest", BiocManager::repositories())) remotes::install_local(".", dependencies = TRUE, Ncpus = parallel::detectCores()) - BiocManager::install(c("rcmdcheck", "BiocCheck", "covr", "pkgdown", "cyclocomp"), Ncpus = parallel::detectCores()) + # Only install the tools for features that are used in this run. + pkgs <- c("rcmdcheck", "BiocCheck") + if (nzchar(Sys.getenv("CODECOV_TOKEN"))) pkgs <- c(pkgs, "covr") + if (Sys.getenv("INSTALL_PKGDOWN") == "true") pkgs <- c(pkgs, "pkgdown") + if (Sys.getenv("ENABLE_CYCLOCOMP") == "true") pkgs <- c(pkgs, "cyclocomp") + BiocManager::install(pkgs, Ncpus = parallel::detectCores()) - name: Run R CMD check env: @@ -148,7 +155,7 @@ jobs: ) - name: Test Coverage - if: success() && needs.set-matrix.outputs.is_release_branch == 'false' && env.CODECOV_TOKEN != '' + if: success() && needs.setup.outputs.is_release_branch == 'false' && env.CODECOV_TOKEN != '' shell: Rscript {0} run: | cov <- covr::package_coverage() @@ -182,25 +189,13 @@ jobs: if-no-files-found: warn - name: Build pkgdown Site - if: >- - ${{ - success() - && inputs.enable_pkgdown - && github.event_name != 'pull_request' - && needs.set-matrix.outputs.is_release_branch == 'true' - }} + if: success() && env.INSTALL_PKGDOWN == 'true' shell: Rscript {0} run: | pkgdown::build_site() - name: Upload pkgdown artifact - if: >- - ${{ - success() - && inputs.enable_pkgdown - && github.event_name != 'pull_request' - && needs.set-matrix.outputs.is_release_branch == 'true' - }} + if: success() && env.INSTALL_PKGDOWN == 'true' uses: actions/upload-pages-artifact@v5 with: path: docs @@ -208,12 +203,12 @@ jobs: deploy: needs: - check - - set-matrix + - setup if: >- ${{ inputs.enable_pkgdown && github.event_name != 'pull_request' - && needs.set-matrix.outputs.is_release_branch == 'true' + && needs.setup.outputs.is_release_branch == 'true' }} # NOTE: The caller workflow must grant `pages: write` and `id-token: write` permissions when `enable_pkgdown` is enabled. environment: diff --git a/.github/workflows/canary.yml b/.github/workflows/canary.yml index 30d3b12..b27fe60 100644 --- a/.github/workflows/canary.yml +++ b/.github/workflows/canary.yml @@ -4,6 +4,7 @@ name: Canary Check # executes correctly end-to-end against the Bioconductor container tag derived from the branch (main => devel). # Uses error_on: "never" so expected BiocCheck notes from this stub package # (no vignettes, no NEWS, etc.) do not cause a false failure. +# Enables cyclocomp so the conditional install and artifact upload are exercised. on: schedule: @@ -16,3 +17,4 @@ jobs: uses: ./.github/workflows/bioccheck.yml with: error_on: 'never' + enable_cyclocomp: true diff --git a/docs/adr/0001-dynamic-container-tagging.md b/docs/adr/0001-dynamic-container-tagging.md index 34ace05..f5ccae3 100644 --- a/docs/adr/0001-dynamic-container-tagging.md +++ b/docs/adr/0001-dynamic-container-tagging.md @@ -1,4 +1,4 @@ -# 0001. Use of Dynamic Matrix Container Tagging +# 0001. Use of Dynamic Container Tagging - **Status:** Accepted - **Date:** 2026-07-24 @@ -12,7 +12,7 @@ How do we ensure that package tests run in the correct Bioconductor container ve ## Decision -We will dynamically resolve the container tag during a `set-matrix` setup job based on the Git ref name. +We will dynamically resolve the container tag during a `setup` job (originally named `set-matrix`) based on the Git ref name. Specifically: - If a pull request or push targets a branch prefixed with `RELEASE_`, the workflow will use `bioconductor/bioconductor_docker:RELEASE_X_Y`. @@ -29,4 +29,4 @@ Specifically: - **Easier backports**: Developers can confidently push to `RELEASE_X_Y` branches knowing the CI will test against the correct environment. - **Reduced false failures**: Package tests won't spuriously fail due to breaking changes in upstream `devel` dependencies when working on stable release branches. -- **Slightly more complex workflow**: The CI pipeline now requires a preliminary `set-matrix` job to compute the tag before the main `check` job can run. This adds a few seconds to the total CI runtime and makes the YAML slightly harder to read. +- **Slightly more complex workflow**: The CI pipeline now requires a preliminary `setup` job to compute the tag before the main `check` job can run. This adds a few seconds to the total CI runtime and makes the YAML slightly harder to read. diff --git a/docs/adr/0008-cyclocomp-complexity-analysis.md b/docs/adr/0008-cyclocomp-complexity-analysis.md index d1747ba..6d9df59 100644 --- a/docs/adr/0008-cyclocomp-complexity-analysis.md +++ b/docs/adr/0008-cyclocomp-complexity-analysis.md @@ -1,6 +1,6 @@ # 0008. Opt-in Cyclomatic Complexity Analysis (cyclocomp) -- **Status:** Accepted +- **Status:** Accepted; the "Unconditional Installation" decision, the rejected "Conditional Installation" alternative, and the "Simple Maintenance" consequence are superseded by [ADR 0010](0010-conditional-tool-installation.md) - **Date:** 2026-07-24 - **Deciders:** Levi Waldron diff --git a/docs/adr/0010-conditional-tool-installation.md b/docs/adr/0010-conditional-tool-installation.md new file mode 100644 index 0000000..df809e1 --- /dev/null +++ b/docs/adr/0010-conditional-tool-installation.md @@ -0,0 +1,33 @@ +# 0010. Install Optional Tools Only When Used + +- **Status:** Accepted +- **Date:** 2026-10-21 +- **Deciders:** Levi Waldron + +## Context and Problem Statement + +`bioccheck.yml` installed `covr`, `pkgdown` and `cyclocomp` on every run, whether or not the corresponding feature was enabled. ADR 0008 chose this deliberately for `cyclocomp` because it is small and conditional R logic looked fragile. `pkgdown` is not small: it pulls in a large dependency tree, and it is used only on pushes to `RELEASE_*` branches with `enable_pkgdown: true`, which is a minority of runs. The default path, which the README sells as rapid, was paying for it every time (issue #34). + +## Decision + +Install each optional tool only in runs that use it: + +- `covr` when `CODECOV_TOKEN` is set. This includes release branches, where coverage is skipped; `covr` is small enough that the extra condition is not worth carrying. +- `pkgdown` when the pkgdown build step would run (`enable_pkgdown`, not a pull request, release branch). Within the `check` job the condition is a job-level `INSTALL_PKGDOWN` env var shared by the install, build and upload steps; the `deploy` job repeats it because a job's env is not visible to other jobs. +- `cyclocomp` when `enable_cyclocomp` is true. + +`rcmdcheck` and `BiocCheck` are always installed. + +Because the R package cache is saved only on a primary-key miss, a cache populated by a run without `pkgdown` would never be re-saved after a later run installed it. `INSTALL_PKGDOWN` is therefore part of the cache key. The `covr` and `cyclocomp` conditions are not: they are constant for a given caller, and if a caller later enables one, the only cost is reinstalling that tool each run until the dependency hash changes. + +## Alternatives Considered + +- **Keep unconditional installation** (ADR 0008): rejected once `pkgdown` was added; its install cost is not negligible. +- **Separate reusable workflows per feature**: rejected in ADR 0005 because they would re-resolve dependencies in a new container. +- **Install in the step that uses the tool**: equivalent in effect, but spreads `BiocManager::install` calls across the file. A single install step with a short list of conditions is easier to read. + +## Consequences + +- Default runs no longer install `pkgdown`, `cyclocomp` or `covr`. +- The install conditions must stay at least as permissive as the `if:` on the step that uses each tool. The canary enables `cyclocomp` so that one conditional branch is exercised weekly. +- ADR 0008's "Unconditional Installation" point and its "Simple Maintenance" consequence no longer apply. diff --git a/docs/adr/README.md b/docs/adr/README.md index a71fc47..4452f45 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -4,7 +4,7 @@ This directory contains Architecture Decision Records (ADRs) for the `bioconduct ## Index -- [0001. Use of Dynamic Matrix Container Tagging](0001-dynamic-container-tagging.md) +- [0001. Use of Dynamic Container Tagging](0001-dynamic-container-tagging.md) - [0002. Reject Source-Based Execution of BiocCheck](0002-source-based-execution-of-bioccheck.md) - [0003. Configurable `rcmdcheck` Severity (`error_on`)](0003-configurable-rcmdcheck-severity.md) - [0004. In-place Package Installation for Dependencies](0004-in-place-package-installation.md) @@ -13,3 +13,4 @@ This directory contains Architecture Decision Records (ADRs) for the `bioconduct - [0007. Web App Workflow Generator](0007-web-app-workflow-generator.md) - [0008. Opt-in Cyclomatic Complexity Analysis (`cyclocomp`)](0008-cyclocomp-complexity-analysis.md) - [0009. Separation of Package Checking from Container Building](0009-separation-of-container-building-concerns.md) +- [0010. Install Optional Tools Only When Used](0010-conditional-tool-installation.md) (Proposed)