Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Wasm R package binary assets downloading when exporting a Shinylive app #131

Merged
merged 7 commits into from
Oct 15, 2024

Conversation

georgestagg
Copy link
Collaborator

This PR contains three changes to improve the bundling of static assets for loading R package binaries into webR.

  1. Switch to using .tgz formatted R package binaries, support for which is introduced in webR 0.4.2 (Add support for mounting .tgz files with filesystem metadata appended r-wasm/webr#477). This additionally fixes bundling R-Universe packages as static assets, as it now also serves packages in this new format.
  2. When downloading package libraries served via GitHub Assets, allow for compressed libraries with filename library.data.gz.
  3. For R packages hosted by the Bioconductor repository, grab the equivalent package from R-Universe's mirror. Due to 1, this fixes exporting Shinylive apps that use bioconductor packages.

This relies on using webR v0.4.2, and so requires a new version of the Shinylive assets. So, I will open a linked PR on posit-dev/shinylive and convert this from a Draft PR once new assets are ready.

@georgestagg
Copy link
Collaborator Author

georgestagg commented Sep 12, 2024

Note: this will break compatibility with older shinylive assets. Below I note what the failure looks like:

This PR's r-shinylive, older shinylive assets (<=0.7.0)

App exports successfully, R package binaries are bundled with the app assets in the new format. At runtime, download of R package binaries fails, because they are in the new format. The JS console log reports the failure:

Screenshot 2024-09-12 at 15 20 23

At this point, a fallback kicks in and the packages are downloaded from the webR public package repository instead, over the internet. If all packages are available on CRAN, and versions match, this works and the app starts. The app works, but the bundled R packages have been ignored.

EDIT: In the latest commit wasm_packages is ignored if assets_version() <= "0.7.0". That will help push adoption toward the latest without fulling breaking the use of older assets. It also means we don't need to maintain legacy package bundling code.


Older r-shinylive (0.2.0 on CRAN), latest shinylive assets (from this PR).

App exports successfully, R package binaries are bundled with the app assets but using the older .data/.js.metadata format. At runtime the new webR assets can still read this older format, so things works OK.

Instead, show a warning asking the user to upgrade their Shinylive
assets to a compatible version.
@georgestagg
Copy link
Collaborator Author

Related py-shinylive PR: posit-dev/py-shinylive#42. Onced merged, the integration test here should work again.

This should be less of an issue in the future once posit-dev/py-shinylive#33 is implemented, because we can just override using SHINYLIVE_ASSETS_VERSION env var.

@georgestagg
Copy link
Collaborator Author

georgestagg commented Oct 7, 2024

posit-dev/py-shinylive#42 has been merged, so latest py-shinylive will use 0.8.0 assets.

@schloerke / @gadenbuie How do we feel about switching out this line in the integration test to pull from posit-dev/py-shinylive@main rather than the release version?

pip install shinylive

Otherwise I don't think the test will pass until the next py-shinylive version has been released proper?

@gadenbuie
Copy link
Contributor

How do we feel about switching out this line in the integration test to pull from posit-dev/py-shinylive@main rather than the release version?

I don't mind. I'd really like for py-shinylive to be flexible and able to use other asset versions rather than a hard-coded value. I think it's reasonable to want all ✅ in CI here and it's not great to keep ignoring the ❌ ... (although without the friction I'm worried we'll forget about py-shinylive)

@schloerke
Copy link
Collaborator

Lets disable it for now. Thank you

filename = glue::glue("{pkg}_{ver}.js.metadata"),
url = glue::glue("{contrib}/{pkg}_{ver}.js.metadata")
filename = glue::glue("{pkg}_{ver}.tgz"),
url = glue::glue("{contrib}/{pkg}_{ver}.tgz")
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: This change switches from downloading the older webR VFS format to the newer (single .tgz file) format. Both repo.r-wasm.org and R-Universe are already publishing packages in this new format.

R/packages.R Outdated Show resolved Hide resolved
@georgestagg
Copy link
Collaborator Author

I've disabled the test. We can re-enable it once py-shinylive has posit-dev/py-shinylive#33.

Thanks, I think this is now all ready for review for merging.

@georgestagg georgestagg requested a review from schloerke October 8, 2024 08:14
Co-authored-by: Garrick Aden-Buie <garrick@adenbuie.com>
@georgestagg georgestagg merged commit cf739ae into main Oct 15, 2024
12 checks passed
@georgestagg georgestagg deleted the tgz-packages branch October 15, 2024 12:54
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.

3 participants