Repository navigation
Build R packages with bundled libuv and verify them - #129
Open
benjaminguttmann-avtq wants to merge 2 commits into
Open
benjaminguttmann-avtq wants to merge 2 commits into
benjaminguttmann-avtq wants to merge 2 commits into
Conversation
fs >= 2.0.0 requires libuv headers that the stacks do not ship, so shiny and its dependencies silently failed to install (R exits 0 on dependency failures) and were missing from the r 4.6.2 artifacts. - Set USE_BUNDLED_LIBUV=1 for all R package installs. - Fail the build if fs, Rserve, forecast, shiny or plumber are not installed.
There was a problem hiding this comment.
🔵 Needs a closer look
FakeRunner-only coverage leaves bundled-libuv compilation, namespace loading, and exit-status propagation unverified in a real R/parity build on the target stacks.
No open issues 🎉
What changed in this PR
This PR updates the R recipe to use bundled libuv during package installation and fail the build when required packages cannot load, preventing incomplete R artifacts.
Changes:
- Sets
USE_BUNDLED_LIBUV=1for all R package installs. - Shares the explicit dependency list between installation and verification, which runs before removing
remotes. - Adds tests for the install environment, verification ordering and package coverage, and verification failure handling.
| File | Description |
|---|---|
internal/recipe/recipe_compiled_test.go |
Tests bundled-libuv settings and package verification behavior. |
internal/recipe/r.go |
Enables bundled libuv and verifies required packages before packaging. |
internal/recipe/export_test.go |
Exposes the verification command to external-package tests. |
Review effort: Balanced
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
fs >= 2.0.0(pulled in byshinyviasass/bslib) needs libuv headers, which the cflinuxfs stacks do not ship. Thefsinstall failed, and because R exits 0 wheninstall.packagesfails for a dependency, the build still succeeded. As a resultshinyand its dependencies were missing from the r 4.6.2 artifacts.Changes
internal/recipe/r.goremotes, explicit deps, sub-deps) withUSE_BUNDLED_LIBUV=1viaRunWithEnv, sofsbuilds its vendored libuv.remove.packages("remotes"). It fails the build unlessfs, the explicit deps (cli,curl,lifecycle,R6,rlang,withr,processx,callr,stringr) and the sub-deps (Rserve,forecast,shiny,plumber) can be loaded withrequireNamespace. This catches both missing packages and broken native builds.rExplicitDepsso the install and verification steps share one package list.recipe_compiled_test.go, newexport_test.go)USE_BUNDLED_LIBUV=1.remotesis removed and coversfs, the explicit deps and the sub-deps.verifying installed R packageswhen verification fails.Testing
go test -race ./internal/recipe/ -run 'TestRRecipe|TestRserve'passes.rartifact. Please confirm via CI ormake parity-test DEP=r.TestLibunwindRecipeBuildfails locally on macOS (trailing slash inTMPDIR). It fails the same way onmainand is unrelated to this change.Notes for reviewers
requireNamespace, which loads each package. It is slightly slower thaninstalled.packages()but checks the failure this PR targets.Run, because it needs no extra environment.Should fix the failing tests in cloudfoundry/r-buildpack#400