Skip to content

[EXPERIMENT — do not merge] test whether dependencies: "hard" is still needed - #37

Closed
eliotmcintire wants to merge 12 commits into
developmentfrom
ci/test-deps-all
Closed

eliotmcintire wants to merge 12 commits into
developmentfrom
ci/test-deps-all

Conversation

@eliotmcintire

Copy link
Copy Markdown
Contributor

Draft / experiment. Not for merge. Opened only to get CI to run — the workflow triggers on pull_request, so a branch alone gets nothing.

Question

Is the pak solver bug that forced dependencies: '"hard"' still live?

The old workflow documented it:

dependencies: "hard" sidesteps a pak metadata bug: the combined Depends+Imports+Suggests dep graph trips pak's solver over an inconsistent Matrix entry (R>=4.7 from the /src/contrib/4.7.0/Recommended path coexisting with R>=4.4 from the main /src/contrib path). The conflict only materialises when Suggests is traversed transitively.

That failure depends on CRAN's metadata layout at a moment in time, so it is plausibly transient.

What this branch changes

Branched from ci/migrate-to-org-workflow (#31) so the only difference is dependency resolution:

-      extra-packages: |
-        any::archive
-        ... 16 entries ...
-        any::testthat
-      dependencies: '"hard"'
+      dependencies: '"all"'

The 16 extra-packages entries matched DESCRIPTION's Suggests exactly (I synced them in #31), so "all" should install the same set directly from DESCRIPTION. any::rcmdcheck is added by the org workflow itself, and extra-repositories already defaults to the PredictiveEcology r-universe — which scfmutils needs, since it is not on CRAN.

How to read the result

Either way this branch gets closed, not merged.

Refs #31

🤖 Generated with Claude Code

https://claude.ai/code/session_01T3NTQTNStSLZ3mA3CsLcon

achubaty and others added 12 commits September 14, 2020 17:30
All changes have been used in FireSenseTesting for some time; time to merge
Firesense_LCC_flammability downloads from ftp.maps.canada.ca (LCC) and
cwfis.cfs.nrcan.gc.ca (fire polygons) at build time. R-CMD-check runners
routinely can't reach these, failing the vignette build with
"Failed to connect to ... port 443". Probe both servers up front and skip
the live download/analysis chunks when either is unreachable, degrading to
documentation-only (same as the existing macOS/archive path). Adds curl to
Suggests for the reachability probe.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replaces this repo's hand-rolled matrix with a thin caller, so third-party
action versions, system dependencies, caching and the check itself are
maintained in one place instead of 17. Bumping actions/checkout there now
updates every repo at once.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012DVjmY3im9Xak7tXLSMGCa
The org concurrency budget is shared across every PredictiveEcology repo and is
the binding constraint on getting work through. These legs added R versions
below the default matrix; testing older R is explicitly not worth the runner
contention it causes.

oldrel-N is relative, so the default matrix's floor still rises on its own with
each R release -- no standing commitment to any particular old version.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012DVjmY3im9Xak7tXLSMGCa
NLMR is not referenced anywhere in this package -- not in DESCRIPTION, R/,
vignettes/, tests/ or man/. Its only appearances were the two CI workflows,
which installed it from GitHub on every leg:

    NLMR=?ignore
    post-install: pak::pkg_install("ropensci/NLMR")

That is the same dead weight the PE CI standards call out from SpaDES, where
ropensci/NLMR and s-u/fastshp were installed on every pkgdown build while
being referenced nowhere -- the slowest step and the largest failure surface.
Removed from both R-CMD-check.yaml and test-coverage.yaml (this PR migrates
only the former, but the install was equally unused in the latter, and "remove
it entirely" means both).

Also sync extra-packages to the current Suggests:

- drop cowplot: promoted to Imports in 7b5afe5, so it now installs as a hard
  dependency and does not belong in extra-packages.
- add curl: a real Suggests, and the LCC vignette's reachability probe is
  gated on requireNamespace("curl") -- without it the probe silently degrades
  to "skip" rather than testing anything.

Both files parse as valid YAML.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T3NTQTNStSLZ3mA3CsLcon
…l live

Experiment, not for merge. Branched from ci/migrate-to-org-workflow so the
only difference is the dependency resolution mode.

The old workflow set `dependencies: '"hard"'` to work around a pak solver
bug: the combined Depends+Imports+Suggests graph tripped over an inconsistent
Matrix entry (R>=4.7 from /src/contrib/4.7.0/Recommended coexisting with
R>=4.4 from the main /src/contrib path), and the conflict only materialised
when Suggests was traversed transitively. The 16 hand-listed Suggests in
extra-packages exist solely to re-add what "hard" excludes.

That failure mode depends on CRAN's metadata layout at a point in time, so it
may have aged out. This run tests it: `dependencies: '"all"'` (the org
default) with the extra-packages block removed entirely -- the 16 entries
matched DESCRIPTION's Suggests exactly, so "all" should install the same set
straight from DESCRIPTION.

Green => "hard" and the 16 lines can be deleted from #31, and the
DESCRIPTION/workflow drift that required manual syncing goes away.
Red with a Matrix/solver error => the workaround is still load-bearing and
should be kept, with the comment restored.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T3NTQTNStSLZ3mA3CsLcon
eliotmcintire added a commit that referenced this pull request Sep 1, 2026
The `dependencies: '"hard"'` setting plus 16 hand-listed Suggests worked
around a pak solver bug: an inconsistent Matrix entry (R>=4.7 from
/src/contrib/4.7.0/Recommended coexisting with R>=4.4 from the main
/src/contrib path) that only surfaced when Suggests was traversed
transitively. Hard-deps-only resolved, and the Suggests were re-added by hand.

That failure depended on CRAN's metadata layout at a point in time. Retested
in #37 (branched from this one, so the only variable was resolution mode):
`dependencies: '"all"'` with no extra-packages list came back 12/12 green --
every platform, every R version including devel, and the nosuggests leg. The
metadata that caused it has moved on.

- R-CMD-check.yaml: '"hard"' -> '"all"', extra-packages list removed (16
  entries, which duplicated DESCRIPTION's Suggests exactly). any::rcmdcheck
  is supplied by the reusable workflow.
- test-coverage.yaml: same change. Its comment deferred to R-CMD-check.yaml
  for a rationale this migration would have deleted, and its mirrored-Suggests
  note no longer applied.

This removes the drift class that bit earlier today: the list had gone stale
against DESCRIPTION (cowplot promoted to Imports in 7b5afe5 and still listed;
curl a Suggests and missing, which silently degraded the LCC vignette's
reachability probe to "skip"). There is now nothing to keep in sync.

The workaround is documented in-place, so if a pak resolve error naming
Matrix ever returns, the fix is recorded rather than rediscovered.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T3NTQTNStSLZ3mA3CsLcon
@eliotmcintire

Copy link
Copy Markdown
Contributor Author

Answered: the workaround has aged out. 12/12 green — every platform, every R version including devel, plus the nosuggests leg — with dependencies: '"all"' and no extra-packages list.

The pak Matrix metadata conflict that forced '"hard"' no longer reproduces. Applied to #31 in d8a7f0d, for both R-CMD-check.yaml and test-coverage.yaml, with the old workaround documented in-place so it can be reapplied if a pak resolve error naming Matrix ever comes back.

Closing — this branch existed only to run the experiment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants