diff --git a/.github/workflows/revdep2.yaml b/.github/workflows/revdep2.yaml index 8226c8bf0e3..7fcdce41131 100644 --- a/.github/workflows/revdep2.yaml +++ b/.github/workflows/revdep2.yaml @@ -70,7 +70,7 @@ on: type: string default: "" packages: - description: "Packages to check (comma/space separated; default: all reverse dependencies)" + description: "Packages to check (comma/space separated), or 'broken' to take what the committed revdep/ report lists as broken or failed; default: all reverse dependencies" type: string default: "" which: @@ -149,9 +149,12 @@ env: REVDEP2_PREBUILT_MAX_RUNS: ${{ vars.REVDEP2_PREBUILT_MAX_RUNS || '5' }} REVDEP2_PREBUILT_MAX_AGE_DAYS: ${{ vars.REVDEP2_PREBUILT_MAX_AGE_DAYS || '14' }} # Per-check timeout: factor times the package's CRAN check time, but never - # below the floor -- CRAN's machines are not these runners. + # below the floor -- CRAN's machines are not these runners. The floor was 10 + # minutes and killed 19 of 770 packages in run 31048405399, every one of them + # compile-heavy (Stan models, mostly) and fast on CRAN's own numbers: 13 of + # them had the floor as their whole budget. 20 minutes covers all 19. REVDEP2_TIMEOUT_FACTOR: ${{ vars.REVDEP2_TIMEOUT_FACTOR || '1.5' }} - REVDEP2_TIMEOUT_MIN_MINUTES: ${{ vars.REVDEP2_TIMEOUT_MIN_MINUTES || '10' }} + REVDEP2_TIMEOUT_MIN_MINUTES: ${{ vars.REVDEP2_TIMEOUT_MIN_MINUTES || '20' }} REVDEP2_DEADLINE_MINUTES: ${{ vars.REVDEP2_DEADLINE_MINUTES || '300' }} jobs: @@ -177,11 +180,13 @@ jobs: actions: read steps: - - uses: actions/checkout@v6 + - name: Check out the ref under test + uses: actions/checkout@v6 with: ref: ${{ inputs.ref || github.ref }} - - uses: r-lib/actions/setup-r@v2 + - name: Set up R + uses: r-lib/actions/setup-r@v2 with: use-public-rspm: true @@ -190,8 +195,8 @@ jobs: install.packages("jsonlite") shell: Rscript {0} - - id: plan - name: Plan shards + - name: Plan shards + id: plan env: GH_TOKEN: ${{ github.token }} OUT: ${{ runner.temp }}/plan.json @@ -199,7 +204,8 @@ jobs: Rscript ./.github/workflows/revdep2/plan.R shell: bash - - uses: actions/upload-artifact@v6 + - name: Upload the shard plan + uses: actions/upload-artifact@v6 if: steps.plan.outputs.shards != '0' with: name: revdep2-plan @@ -221,11 +227,13 @@ jobs: contents: read steps: - - uses: actions/checkout@v6 + - name: Check out the ref under test + uses: actions/checkout@v6 with: ref: ${{ inputs.ref || github.ref }} - - uses: ./.github/workflows/install + - name: Install R and the package dependencies + uses: ./.github/workflows/install with: token: ${{ secrets.GITHUB_TOKEN }} cache-version: revdep2-1 @@ -238,7 +246,8 @@ jobs: Rscript ./.github/workflows/revdep2/build.R shell: bash - - uses: actions/upload-artifact@v6 + - name: Upload the package binary + uses: actions/upload-artifact@v6 with: name: revdep2-pkg path: ${{ runner.temp }}/pkg @@ -268,7 +277,8 @@ jobs: actions: read steps: - - uses: actions/checkout@v6 + - name: Check out the ref under test + uses: actions/checkout@v6 with: ref: ${{ inputs.ref || github.ref }} @@ -290,7 +300,8 @@ jobs: free -g shell: bash - - uses: r-lib/actions/setup-r@v2 + - name: Set up R + uses: r-lib/actions/setup-r@v2 with: use-public-rspm: true @@ -300,14 +311,16 @@ jobs: install.packages("jsonlite") shell: Rscript {0} - - uses: actions/download-artifact@v7 + - name: Download the shard plan + uses: actions/download-artifact@v7 with: name: revdep2-plan path: ${{ runner.temp }}/plan # The preflight downloads every dependency binary once; saving the pak # cache under the plan's hash hands the shards a warm start. - - uses: actions/cache@v5 + - name: Cache the pak package cache + uses: actions/cache@v5 with: path: ~/.cache/R/pkgcache key: revdep2-pak-${{ needs.plan.outputs.plan_hash }} @@ -337,7 +350,8 @@ jobs: du -sh ~/.cache/R/pkgcache "${RUNNER_TEMP}/lib" 2>/dev/null || true shell: bash - - uses: actions/upload-artifact@v6 + - name: Upload the preflight report + uses: actions/upload-artifact@v6 if: always() with: name: revdep2-preflight @@ -350,7 +364,8 @@ jobs: # dependency universe, gigabytes of it -- so it is kept exactly as long # as a plan would still reuse it, while the index that describes it is # cheap and outlives it. - - uses: actions/upload-artifact@v6 + - name: Upload the preflight library + uses: actions/upload-artifact@v6 if: always() with: name: revdep2-lib @@ -359,7 +374,8 @@ jobs: retention-days: ${{ env.REVDEP2_PREBUILT_MAX_AGE_DAYS }} overwrite: true - - uses: actions/upload-artifact@v6 + - name: Upload the library index + uses: actions/upload-artifact@v6 if: always() with: name: revdep2-lib-index @@ -419,17 +435,29 @@ jobs: _R_CHECK_FORCE_SUGGESTS_: false steps: - - uses: actions/checkout@v6 + - name: Check out the ref under test + uses: actions/checkout@v6 with: ref: ${{ inputs.ref || github.ref }} - - uses: r-lib/actions/setup-pandoc@v2 + - name: Set up pandoc + uses: r-lib/actions/setup-pandoc@v2 - - uses: r-lib/actions/setup-r@v2 + - name: Close the log group that setup-pandoc leaves open + # See `.github/workflows/install/action.yml` for why this is needed. + # Fixed upstream by https://github.com/r-lib/actions/pull/1103. + if: runner.os == 'Linux' + run: | + echo "::endgroup::" + shell: bash + + - name: Set up R + uses: r-lib/actions/setup-r@v2 with: use-public-rspm: true - - uses: r-lib/actions/setup-tinytex@v2 + - name: Set up TinyTeX + uses: r-lib/actions/setup-tinytex@v2 - name: Install system tools for checking run: | @@ -443,19 +471,22 @@ jobs: install.packages(c("jsonlite", "rcmdcheck")) shell: Rscript {0} - - uses: actions/cache/restore@v5 + - name: Restore the pak package cache + uses: actions/cache/restore@v5 with: path: ~/.cache/R/pkgcache key: revdep2-pak-${{ needs.plan.outputs.plan_hash }} restore-keys: | revdep2-pak- - - uses: actions/download-artifact@v7 + - name: Download the shard plan + uses: actions/download-artifact@v7 with: name: revdep2-plan path: ${{ runner.temp }}/plan - - uses: actions/download-artifact@v7 + - name: Download the package binary + uses: actions/download-artifact@v7 with: name: revdep2-pkg path: ${{ runner.temp }}/pkg @@ -464,7 +495,8 @@ jobs: # building the same packages the preflight built minutes ago. It is a # `needs`, so it is always there -- unless the preflight could not pack # one, which is a slower shard, not a broken one. - - uses: actions/download-artifact@v7 + - name: Download this run's preflight library + uses: actions/download-artifact@v7 continue-on-error: true with: name: revdep2-lib @@ -472,7 +504,8 @@ jobs: # The baseline lives on an earlier run; absence is not an error, the # shard just checks the CRAN version fresh. - - uses: actions/download-artifact@v7 + - name: Download the baseline results + uses: actions/download-artifact@v7 if: needs.plan.outputs.baseline_run != '0' continue-on-error: true with: @@ -502,7 +535,8 @@ jobs: # Named per attempt: a re-run of one shard must not overwrite the results # the other shards uploaded in the first attempt; the collector reads # every attempt and lets the later one win per package. - - uses: actions/upload-artifact@v6 + - name: Upload the shard results + uses: actions/upload-artifact@v6 if: always() with: name: revdep2-results-${{ matrix.shard }}-${{ github.run_attempt }} @@ -536,16 +570,19 @@ jobs: name: "Collect results and report" permissions: - contents: read + # To commit the report back to the checked branch. + contents: write # To download the retried run's report artifact, which lives on that run. actions: read steps: - - uses: actions/checkout@v6 + - name: Check out the ref under test + uses: actions/checkout@v6 with: ref: ${{ inputs.ref || github.ref }} - - uses: r-lib/actions/setup-r@v2 + - name: Set up R + uses: r-lib/actions/setup-r@v2 with: use-public-rspm: true @@ -565,7 +602,8 @@ jobs: pak::pkg_install("krlmlr/revdepcheck") shell: Rscript {0} - - uses: actions/download-artifact@v7 + - name: Download the shard plan + uses: actions/download-artifact@v7 with: name: revdep2-plan path: ${{ runner.temp }}/plan @@ -573,13 +611,15 @@ jobs: # Tolerated because a run in which no shard uploaded anything is still # worth a report: the collector reconciles against the plan and names # every package it never heard about, which is what `retry-run` needs. - - uses: actions/download-artifact@v7 + - name: Download the results of all shards + uses: actions/download-artifact@v7 continue-on-error: true with: pattern: revdep2-results-* path: ${{ runner.temp }}/results - - uses: actions/download-artifact@v7 + - name: Download the report of the run being retried + uses: actions/download-artifact@v7 if: env.REVDEP2_RETRY_RUN != '' continue-on-error: true with: @@ -603,7 +643,8 @@ jobs: Rscript ./.github/workflows/revdep2/collect.R shell: bash - - uses: actions/upload-artifact@v6 + - name: Upload the report + uses: actions/upload-artifact@v6 if: always() with: name: revdep2-report @@ -611,7 +652,73 @@ jobs: retention-days: 90 overwrite: true - - uses: actions/upload-artifact@v6 + # The report is also the repository's own record: `revdep/README.md` and + # friends have been committed here since `revdepcheck::cloud_check()` + # wrote them, and `recheck-report` reads them back. Only the four + # generated files and the manifest are staged -- the analysis, the + # examples and the notification scripts next to them are human-authored, + # and `pkgs/` is gigabytes of check output that belongs in the artifact + # only. + # + # A ref that is not a branch (a tag, a SHA, a fork's ref) cannot receive + # a commit; that is a fact about the dispatch, not a failure, so the step + # says so and stops. `continue-on-error` covers the rest: a protected + # branch, a read-only token on a fork, a race with another push. The + # report is in the artifact either way. + - name: Commit the report to the checked ref + if: always() && vars.REVDEP2_COMMIT_REPORT != 'false' + continue-on-error: true + env: + REF_INPUT: ${{ inputs.ref }} + run: | + set -eu + # Without a `ref` input the dispatch decides, and GitHub says outright + # what kind of ref it was; with one, only the remote can say whether + # it names a branch. Anything else -- a tag, a SHA, a branch that does + # not exist here -- has nowhere to put a commit. + if [ -z "${REF_INPUT}" ]; then + REF="${GITHUB_REF_NAME}" + if [ "${GITHUB_REF_TYPE}" != "branch" ]; then + echo "::notice::${REF} is a ${GITHUB_REF_TYPE}, not a branch; the report stays in the artifact only." + exit 0 + fi + else + REF="${REF_INPUT}" + if ! git ls-remote --exit-code --heads origin "${REF}" > /dev/null 2>&1; then + echo "::notice::${REF} is not a branch of this repository; the report stays in the artifact only." + exit 0 + fi + fi + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + # Only what this run generated, and only what it managed to write: + # a collector that fell back to the manifest-derived summary has a + # README.md and no problems.md, which is a report, not an error. + files="" + for f in README.md problems.md failures.md cran.md manifest.json; do + if [ -f "revdep/${f}" ]; then + files="${files} revdep/${f}" + fi + done + if [ -z "${files}" ]; then + echo "::notice::No report files were written; nothing to commit." + exit 0 + fi + git add -- ${files} + if git diff --cached --quiet; then + echo "::notice::The report is unchanged; nothing to commit." + exit 0 + fi + git commit -m "chore: Update revdep report from run ${GITHUB_RUN_ID}" \ + -m "https://github.com/${GITHUB_REPOSITORY}/actions/runs/${GITHUB_RUN_ID}" + git fetch origin "${REF}" + git rebase "origin/${REF}" + git push origin "HEAD:${REF}" + echo "::notice::Report committed to ${REF}." + shell: bash + + - name: Upload the new baseline + uses: actions/upload-artifact@v6 if: always() with: name: revdep2-baseline @@ -622,7 +729,8 @@ jobs: # What the run cost, per package and per shard. Small on purpose and # separate from the report: the next plan downloads it to calibrate its # cost model, and should not have to fetch a report to do so. - - uses: actions/upload-artifact@v6 + - name: Upload the measured timings + uses: actions/upload-artifact@v6 if: always() with: name: revdep2-timings diff --git a/.github/workflows/revdep2/README.md b/.github/workflows/revdep2/README.md index c5fd354d252..ff451b6cc35 100644 --- a/.github/workflows/revdep2/README.md +++ b/.github/workflows/revdep2/README.md @@ -100,6 +100,11 @@ The per-check timeout stays on CRAN's number and is not calibrated: A timeout is a safety net for a check that has gone wrong, so it should be generous where the estimate is merely typical — and against the local estimate that same factor would be a third as forgiving. +The floor matters more than the factor: +19 of 770 packages in run 31048405399 were killed by a 10-minute one, +all of them compile-heavy (Stan models, mostly) and cheap by CRAN's numbers, +13 with the floor as their entire budget. +It is 20 minutes now, which covers every one of them. ### The shard count is bounded by the parallel capacity @@ -507,11 +512,55 @@ gh workflow run revdep2.yaml -f retry-run= The retry's collector carries the donor run's untouched results over, so its report is complete again, not a fragment. +## The report is the repository's record + +`revdep/` in the checkout is where the results live between runs. +`revdepcheck::cloud_check()` wrote `README.md`, `problems.md`, +`failures.md` and `cran.md` there long before this workflow existed, +`revdep/run-broken.R` read them back to re-check what was broken, +and the analysis next to them — `problems-analysis.md`, `examples/`, +the notification scripts — is what a human adds on top. +So the collector writes the same four files, in the same format +(they come out of revdepcheck itself), plus `manifest.json`, +and commits them back to the ref that was checked. + +Only those five paths are staged. +The analysis and the examples beside them are human-authored, +and `pkgs/` — the raw check output, gigabytes of it — +belongs in the `revdep2-report` artifact and nowhere near a commit. + +A ref that cannot receive a commit simply does not get one: +a tag, a SHA, a fork's branch. +That is a fact about the dispatch rather than a failure, +so the step says so with a `::notice::` and stops; +the report is in the artifact either way. +The same applies to a protected branch, a read-only token, or a push +that races with someone else's — the step is `continue-on-error`, +because a report that cannot be committed is still a report. +Set the repository variable `REVDEP2_COMMIT_REPORT` to `false` +to turn the commit off entirely. + +### Re-checking what was broken + +`packages: broken` takes the packages to check from that committed report: +`manifest.json` when this workflow wrote it (every result that is not `ok`), +and otherwise revdepcheck's own markdown — +the `# ()` headings of `problems.md` and `failures.md`, +plus the "Failed to check" table in `README.md`. +That is `revdep/run-broken.R`'s loop, as a dispatch input. + +It is the cheap run: 39 packages rather than 771 for the report as it stands, +one wave, and every one of them a package that was wrong last time. +`retry-run: ` is the sibling for a run that did not finish — +the report is about *results*, a retry is about *coverage*. + ## Failure modes | Situation | Outcome | | --- | --- | | A revdep breaks under the dev version | `newly_broken` in manifest and report; the run stays green | +| The checked ref is a tag or a SHA | the report is not committed; a `::notice::` says so and the artifact still has it | +| The report cannot be pushed (protection, fork, race) | the step is `continue-on-error`; the run keeps its result | | A revdep fails under both versions | `ok` (no *new* problems), visible in the report's tables | | A check times out | rcmdcheck kills it at `max(floor, factor × its CRAN time)`; compared as `t-`, reported `failed` | | A revdep's strong dependencies cannot install | `depfail`, check not attempted, named in the shard summary | @@ -547,7 +596,9 @@ at the next `if`. | Knob | Input | Variable | Default | | --- | --- | --- | --- | | Ref to check (branch, tag, SHA) | `ref` | — | the dispatched ref | -| Packages to check | `packages` | — | all revdeps | +| Packages to check, or `broken` for the committed report's | `packages` | `REVDEP2_PACKAGES` | all revdeps | +| Where that report lives | — | `REVDEP2_REPORT_DIR` | `revdep` | +| Commit the report back to the checked branch | — | `REVDEP2_COMMIT_REPORT` | on | | Revdep set | `which` | — | `strong` | | Revdep depth (`1`, `2`, …, `all`) | `depth` | — | 1 | | Retry a run | `retry-run` | — | — | @@ -568,7 +619,7 @@ at the next `if`. | Fixed cost of one shard | — | `REVDEP2_SETUP_MINUTES` | measured, else 6 min | | Cost of one more dependency install | — | `REVDEP2_INSTALL_SECONDS` | measured, else 2.5 s | | Per-check timeout factor | — | `REVDEP2_TIMEOUT_FACTOR` | 1.5 × CRAN time | -| Per-check timeout floor | — | `REVDEP2_TIMEOUT_MIN_MINUTES` | 10 | +| Per-check timeout floor | — | `REVDEP2_TIMEOUT_MIN_MINUTES` | 20 | | Shard graceful deadline | — | `REVDEP2_DEADLINE_MINUTES` | 300 | ## Prior art diff --git a/.github/workflows/revdep2/plan.R b/.github/workflows/revdep2/plan.R index f57d5fa670c..90002458bec 100644 --- a/.github/workflows/revdep2/plan.R +++ b/.github/workflows/revdep2/plan.R @@ -30,7 +30,8 @@ # # Environment variables (inputs): # REVDEP2_PACKAGES - explicit packages to check (comma/space separated; -# default: all reverse dependencies) +# default: all reverse dependencies), or the word +# `broken` to take them from the committed report # REVDEP2_WHICH - "strong" (default) or "most" (adds Suggests/ # Enhances dependents) # REVDEP2_RETRY_RUN - run id of an earlier revdep2 run; check only the @@ -38,6 +39,9 @@ # REVDEP2_PART - "i/G": check one G-th of the batch, for a revdep # set too big for a single run (the plan refuses # such a batch and prints the G it needs) +# REVDEP2_RECHECK_REPORT - if truthy, check what the committed report lists +# as broken or failed (same as REVDEP2_PACKAGES=broken) +# REVDEP2_REPORT_DIR - where that report lives (default: revdep) # REVDEP2_SHARD_BUDGET_MINUTES - check-time target per shard (default: 45) # REVDEP2_SHARD_CAPACITY_MINUTES - check minutes one shard may be given at # most, which is what forces a second wave @@ -114,6 +118,8 @@ prebuilt_max_age <- env_num("REVDEP2_PREBUILT_MAX_AGE_DAYS", 14) history_runs <- env_num("REVDEP2_HISTORY_RUNS", 40) max_measured_runs <- env_num("REVDEP2_MEASURED_MAX_RUNS", 3) measured_max_age <- env_num("REVDEP2_MEASURED_MAX_AGE_DAYS", 60) +recheck_report <- env_flag("REVDEP2_RECHECK_REPORT") +report_dir <- env_chr("REVDEP2_REPORT_DIR", "revdep") overhead_minutes <- env_num("REVDEP2_PACKAGE_OVERHEAD_MINUTES", 0.5) retry_run <- env_chr("REVDEP2_RETRY_RUN") repo <- env_chr("GITHUB_REPOSITORY") @@ -351,9 +357,42 @@ packages_input <- trimws(strsplit( )[[1]]) packages_input <- packages_input[nzchar(packages_input)] +# `packages: broken` is a selector, not a package name: the dispatch form has +# room for few inputs, and "what was wrong last time" belongs with "what to +# check" rather than beside it. +if ( + length(packages_input) == 1 && + tolower(packages_input) %in% c("broken", "failed", "report") +) { + recheck_report <- TRUE + packages_input <- character() +} + if (length(packages_input) > 0) { selection <- "explicit" candidates <- unique(packages_input) +} else if (recheck_report) { + # The committed report is the durable record of what was wrong last time: + # every package it lists as a problem or a failure, re-checked. This is the + # `revdep/run-broken.R` loop that predates this workflow, as an input. + found <- report_packages(report_dir) + if (length(found$packages) == 0) { + plan_nothing(sprintf( + "%s lists no broken or failed packages (looked for manifest.json, problems.md, failures.md, README.md)", + report_dir + )) + } + selection <- sprintf("broken and failed in %s", report_dir) + candidates <- found$packages + inform( + "Re-checking ", + length(candidates), + " package(s) from ", + report_dir, + " (", + found$source, + ")" + ) } else if (nzchar(retry_run)) { selection <- sprintf("retry of run %s", retry_run) selection_md <- sprintf("retry of run %s", run_link(retry_run)) diff --git a/.github/workflows/revdep2/util.R b/.github/workflows/revdep2/util.R index 001d92a1849..95e36850e4e 100644 --- a/.github/workflows/revdep2/util.R +++ b/.github/workflows/revdep2/util.R @@ -516,6 +516,80 @@ restore_prebuilt <- function(plan, lib, wanted) { restored } +# ------------------------------------------------------- the last report ---- + +# The packages an earlier report says were not ok, read from the `revdep/` +# directory in the checkout rather than from a run's artifacts. +# +# That directory is the durable record: the collector commits it back to the +# checked branch, and before this workflow existed `revdepcheck::cloud_check()` +# wrote the same four files there. So both generations are read: `manifest.json` +# when this workflow wrote it (it says exactly which result each package got), +# and otherwise revdepcheck's own markdown -- one `# ()` +# heading per package in problems.md and failures.md, plus the "Failed to +# check" table in README.md, which is where a package that produced no +# comparison at all is named. +report_packages <- function(dir) { + none <- list(packages = character(), source = "") + if (!nzchar(dir %||% "") || !dir.exists(dir)) { + return(none) + } + manifest <- file.path(dir, "manifest.json") + if (file.exists(manifest)) { + entries <- tryCatch(read_json(manifest), error = function(e) NULL) + if (length(entries) > 0) { + names <- vapply(entries, function(e) e$package %||% "", character(1)) + results <- vapply(entries, function(e) e$result %||% "", character(1)) + take <- nzchar(names) & vapply(results, needs_recheck, logical(1)) + if (any(take)) { + return(list( + packages = sort(unique(names[take])), + source = "manifest.json" + )) + } + } + } + headings <- function(file) { + if (!file.exists(file)) { + return(character()) + } + lines <- grep("^# ", readLines(file, warn = FALSE), value = TRUE) + trimws(sub("^# ([^ (]+).*$", "\\1", lines)) + } + failed_table <- function(file) { + if (!file.exists(file)) { + return(character()) + } + lines <- readLines(file, warn = FALSE) + from <- grep("^#+ +Failed to check", lines) + if (length(from) == 0) { + return(character()) + } + after <- grep("^#+ ", lines) + after <- after[after > from[[1]]] + block <- lines[seq( + from[[1]], + if (length(after) > 0) after[[1]] - 1L else length(lines) + )] + cells <- trimws(sub( + "^\\|([^|]*)\\|.*$", + "\\1", + grep("^\\|", block, value = TRUE) + )) + # Drop the header and the alignment row; what is left is one package each. + cells[nzchar(cells) & cells != "package" & !grepl("^:?-+:?$", cells)] + } + packages <- unique(c( + headings(file.path(dir, "problems.md")), + headings(file.path(dir, "failures.md")), + failed_table(file.path(dir, "README.md")) + )) + list( + packages = sort(packages[nzchar(packages)]), + source = "problems.md, failures.md, README.md" + ) +} + # --------------------------------------------------------- measured timings -- # What a run measured about itself, so the next plan can stop guessing. diff --git a/revdep/.gitignore b/revdep/.gitignore index b06d4d82019..6ff7b4db622 100644 --- a/revdep/.gitignore +++ b/revdep/.gitignore @@ -2,3 +2,8 @@ /cloud /review/ /notifications/ +# Written by the revdep2 collector into this directory; the reports next to +# them are committed, these are not: `pkgs/` is the raw check output (the +# `revdep2-report` artifact has it), `run.json` is per-run metadata. +/pkgs/ +/run.json