search

r-multiverse / help

Discussions, issues, and feedback for R-multiverse
https://r-multiverse.org
MIT License
2 stars 2 forks source link

New package idea: install "safe" packages from https://r-releases.r-universe.dev #6

Closed wlandau closed 1 month ago

wlandau commented 4 months ago

@gmbecker's comment from https://github.com/r-universe-org/help/issues/363#issuecomment-1972105292 sparked an idea that would add confidence to the quality of packages downloaded from https://r-releases.r-universe.dev. My rendition on the idea is to keep the current publishing model in r-releases, but implement new alternatives to install.packages() which take into account the package check results shown at https://r-releases.r-universe.dev/builds. These new functions can go in a new package in the https://github.com/r-releases GitHub org.

Implementation

I am proposing a new function install_safe() which scrapes the check results of each package before installation begins. For now, these checks only include R CMD check runs from the time the package was last pushed, but if https://github.com/r-universe-org/help/issues/362 and https://github.com/r-universe-org/help/issues/368 are implemented in R-universe, these check results will become quite powerful.

I also propose a new function is_safe() which only scrapes the check results of the supplied packages and reports to the user on the safety of each one, according to the checks currently supported by R-universe.

I am open to different names for these functions if anyone has better ideas.

I am not sure what the best way is to scrape the check results from R-universe, and I am not sure how this will change when revdep checks are implemented. @jeroen, what would you recommend?

Importance

As I explain at https://github.com/r-universe-org/help/issues/363#issuecomment-1972489789, this approach could allow production-grade package compatibility guarantees that exceed even the standard that CRAN provides today. Whereas CRAN keeps failing packages online for 2 weeks, install_safe() and is_safe() catch and correct for check issues immediately, ensuring the user never downloads a package that is currently broken.

We would have the best of both worlds: a painless publishing experience with the package maintainers in control, and user-side guarantees stronger than we have ever seen for R packages.

Help

I would love it if someone else could develop and own this new proposed package. I have sprinted just to get this far with https://github.com/r-releases, and I will not have capacity to take on this new project for quite some time. Please reach out if you are interested.

llrs commented 4 months ago

I might be able to help with this package. It doesn't seem too complicated and it seems something that we can take some lessons from BiocManager, where they check if the installation is a valid one or not, which I am more or less familiar with.

But to be fair, I think it would be better if this was implemented in install.packages, but first let's test it independently how it goes in a package without messing with base R code.

wlandau commented 4 months ago

I might be able to help with this package. It doesn't seem too complicated and it seems something that we can take some lessons from BiocManager, where they check if the installation is a valid one or not, which I am more or less familiar with.

Thank you so much!

But to be fair, I think it would be better if this was implemented in install.packages, but first let's test it independently how it goes in a package without messing with base R code.

Once the project is mature enough to engage R Core, I think this is something we could pursue, although it seems like that may be a long-term thing.

One compromise for proof-of-concept purposes could be a shim like the one renv uses, but I am not sure how reliable that would be.

wlandau commented 4 months ago

On reflection, we won't even need R-universe itself to provide an API for check results. You could just scrape https://github.com/r-universe/r-releases/actions with gh::gh(). r.releases.utils has examples of using gh::gh() to review package registration pull requests. https://github.com/r-universe/r-releases/actions is a likely place for revdep checks to show up in the future (although I can't speak for @jeroen).

wlandau commented 4 months ago

And since those check results are public and scraping them is a read-only affair, you wouldn't even need to use the r-releases GitHub app. (Learning about GitHub apps was the hardest part about getting the automated pull request review system to work.)

wlandau commented 4 months ago

Although I do wonder about API rate limits for people who do not have GitHub personal access tokens...

wlandau commented 4 months ago

I was originally resistant to @gmbecker's 2-repo solution that he proposed in https://github.com/r-universe-org/help/issues/363#issuecomment-1972105292, on reflection I think it would work really well with what we already have in https://github.com/r-releases. I opened a new issue for this in #10. I think #6 and #10 are two different independent angles to implement basically the same user-side guarantees.

It could even exist modularly as a separate project outside https://github.com/r-releases. (In fact, would have to in order to create the new universe.)

shikokuchuo commented 4 months ago

As a concrete and real example of what I mentioned in https://github.com/r-releases/help/issues/10#issuecomment-1973791272, {nanonext} has dependencies {mirai} and {ichimoku}. mirai uses all of the scalability protocols goodness, ichimoku just uses the http client. Now all of the SPs could be temporarily broken for some reason but the http part could still function correctly. If the community here is finance then why shouldn't they transparently get to use ichimoku (checks fine)?

wlandau commented 4 months ago

You are so right!

On reflection, the whole concept of a reverse dependency check does not fit our situation. In a revdep check, you have an incumbent release, you have a release candidate, and you want to find out if the candidate is worse. But in our case, there is no candidate because the release already happened.

What we actually need is a dependency check search: in the case of #10, only install the package if its check succeed and those of its dependency tree. So to implement #10, all we need to do is find the most efficient way to get a pass/fail verdict the package and its dependency tree from R-universe or GitHub.

wlandau commented 4 months ago

Sorry, maybe I still missed your original point. Let me try again: on second (third?) thought, I agree: as long as the current package's test pass, we shouldn't need to worry about checking anything upstream. That simplifies things a lot!

The only thing we need to ensure is that the checks on the current package were run with the latest versions of all its dependencies. From https://github.com/r-universe-org/help/issues/368#issuecomment-1972767243, R-universe reruns checks when there is a package update and once a month. We would get solid assurance if R-universe triggered a check whenever a dependency updates. I will file that issue.

wlandau commented 4 months ago

In summary:

  1. After https://github.com/r-universe-org/help/issues/370, implementation can begin.
  2. After https://github.com/r-universe-org/help/issues/369, user-side package correctness/compatibility guarantees will exceed those of CRAN.
shikokuchuo commented 4 months ago

Yes, I think we are now on the same page.

To also clarify my point about revdepchecks. In my mind this would be in a dashboard of sorts. This doesn't need to be in the package, or it could be something fancy that gets added at a much later point. But it would help pinpoint why/where a failure occurs.

I'm thinking primarily the situation where you have package 'x' installed (i.e. check was fine at that point). Then some of its dependencies are updated. You then run a check to see if it's still ok with the latest package versions and it shows a failure. You could then go through its dependencies and see the revdeps of each. If one particular dependency has a lot of revdep failures then that makes it a likely/possible cause. Not foolproof of course, but it might help. It's what I had in mind in any case.

shikokuchuo commented 4 months ago

Perhaps it's not 100% essential, but I might as well put it out there as an idea. It is not something that currently exists of course due to the practice of archival, which destroys such information.

shikokuchuo commented 4 months ago

Actually I never seem to have mentioned revdepchecks, at least not in relation to this topic...

In summary:

  1. After Efficiently get the check results of a small list of packages r-universe-org/help#370, implementation can begin.

  2. After Rerun a package's checks whenever a strong dependency updates r-universe-org/help#369, user-side package correctness/compatibility guarantees will exceed those of CRAN.

In full agreement with the above summary. Just the checks for each package should be sufficient.

The point I wanted to make, and got crossed with revdepchecks, is to always make the checks available in a summary table (can be like the current CRAN checks page, or a simplified summary) so it can help pinpoint failures. So this is an additional point about non-archival so that this information remains available.

wlandau commented 4 months ago

The point I wanted to make, and got crossed with revdepchecks, is to always make the checks available in a summary table (can be like the current CRAN checks page, or a simplified summary) so it can help pinpoint failures. So this is an additional point about non-archival so that this information remains available.

Yes, I agree that summarizing and displaying checks is the second major goal of this proposed package idea, equally as important as install_safe(). We could get those checks results using the API that @jeroen proposed at #370, and we could return anything from a data frame with a single package's check results to a Shiny dashboard with results from multiple packages.

gmbecker commented 4 months ago

As a concrete and real example of what I mentioned in #10 (comment), {nanonext} has dependencies {mirai} and {ichimoku}. mirai uses all of the scalability protocols goodness, ichimoku just uses the http client. Now all of the SPs could be temporarily broken for some reason but the http part could still function correctly. If the community here is finance then why shouldn't they transparently get to use ichimoku (checks fine)?

I'm sorry but I don't understand your point here at all. If a reverse dependency passes its checks, everything is fine. The question is what to do about {mirai} which in your hypothetical is broken if a new version of nanoext breaks the SPs. The possibilities are nothing (a terrible idea imo, because then install.packages("mirai", repos = ...) gives the user garbage, delay the release of the new nanoext until mirai passes check, or archive mirai (possibly moving it to "unsafe" sister repo if its still installable).

shikokuchuo commented 4 months ago

As a concrete and real example of what I mentioned in #10 (comment), {nanonext} has dependencies {mirai} and {ichimoku}. mirai uses all of the scalability protocols goodness, ichimoku just uses the http client. Now all of the SPs could be temporarily broken for some reason but the http part could still function correctly. If the community here is finance then why shouldn't they transparently get to use ichimoku (checks fine)?

I'm sorry but I don't understand your point here at all. If a reverse dependency passes its checks, everything is fine. The question is what to do about {mirai} which in your hypothetical is broken if a new version of nanoext breaks the SPs. The possibilities are nothing (a terrible idea imo, because then install.packages("mirai", repos = ...) gives the user garbage, delay the release of the new nanoext until mirai passes check, or archive mirai (possibly moving it to "unsafe" sister repo if its still installable).

Hi @gmbecker this is why I moved this discussion to this thread from #10 . Sorry for the confusion. The new install_safe() function would find that the checks fail for 'mirai' and simply not install the package.

llrs commented 4 months ago

Coming here back from #21, should the install_safe be in r.releases.utils package?

wlandau commented 4 months ago

Great question. I was hoping they could go in different packages, mainly because:

  1. They have different goals. r.releases.utils is for admins like me to run the CI/CD workflows. I would prefer not to put implicit pressure on users to understand review_pull_requests().
  2. Every new deployment of r.releases.utils is sensitive and difficult to test because of that CI/CD. I would strongly prefer to avoid modifying it more than absolutely necessary.
  3. r.releases.utils needs to stay light in terms of dependencies so those workflows run as fast as possible. E.g. I am deliberately avoiding testthat for r.releases.utils, but for the package in #6, it would be really nice to have.
  4. "r.releases.utils" is a cumbersome name from a public relations standpoint, and it would be nice to come up with something nicer and more catchy for this new package.
llrs commented 4 months ago

Ok, I'll start working on it this week on my own account and request to be transferred here. Depending on how lightweight we want the pacakge, we could even omit gh, or httr(2) dependencies and directly use xml2/jsonlite/yyjsonr.

wlandau commented 4 months ago

Wow, I didn’t know you would be able to start so soon!

Before you get too far ahead, could we discuss implementation details in the repo you create? Just want to be sure we are aligned. It would be good to brainstorm the package name ({releases}?), serious function names (install_releases() and check_releases()?), function signatures, and behaviors. It could take time before everything we need from R-universe is available, so we have time to plan together.

llrs commented 4 months ago

I thought about a name/title more in line with what you said in the meeting (and in light of the r-devel discussion): R has now a platform to build and test that it is between developer and production quality. For this reason I was thinking about {releaser} or {rqa} as a name.

About implementation details we can coordinate asynchronous or we could have a call just a few of us to see what we think about it. In my opinion, it makes sense to use a very light approach on dependencies as it will be used by many users and machines (perhaps not so lightweight as it could include testthat and maybe httr2 and httptest2). But it should be flexible to support the two repositories approach being discussed in #10 (or something like CRANhaven). At the same time it could have som functionality to create the PR to add a package to the r-release universe (might need to use {gh} or {usethis} in Suggests).

I didn't realize this Sunday closes the useR!2024 proposal of communications, and I am involved in at least one proposals, so I'm might not be so fast on work on this :)

wlandau commented 3 months ago

I thought about a name/title more in line with what you said in the meeting (and in light of the r-devel discussion): R has now a platform to build and test that it is between developer and production quality. For this reason I was thinking about {releaser} or {rqa} as a name.

Yeah, something along those lines for a name would be nice. However, the package will do more than just create releases ("releaser"), and it will go beyond QA to provide tools that production can use ("rqa"). What about an inclusive name like {releasetools}? I would be happy to rename the current {r.releases.utils} to the more specific {r.releases.internals} so users don't get confused.

In my opinion, it makes sense to use a very light approach on dependencies as it will be used by many users and machines (perhaps not so lightweight as it could include testthat and maybe httr2 and httptest2).

https://github.com/r-releases/r.releases.utils currently uses jsonlite and gh, which are both very light and based on reliable sets of packages, and I think they strike a great balance. And for the development and user experience, I do think it's worth using testthat and usethis. Perhaps not the lightest stack, but definitely reliable and easy to install.

But it should be flexible to support the two repositories approach being discussed in https://github.com/r-releases/help/issues/10 (or something like CRANhaven).

Yeah, this is one of the major use cases for the health checks we have been talking about.

At the same time it could have som functionality to create the PR to add a package to the r-release universe (might need to use {gh} or {usethis} in Suggests).

Great idea! This had not even occurred to me before.

I didn't realize this Sunday closes the useR!2024 proposal of communications, and I am involved in at least one proposals, so I'm might not be so fast on work on this :)

On this point: when I created this thread, I had been sprinting to make enough progress before the working group meeting, and I was really tired. But now I have recovered, and since this layer is vitally important to the whole effort, I do have a strong desire to actively contribute to the development.

Would it be okay if I create a stub of a repo under R-releases with the three of us as authors and schedule a call? It would be great to have all three of us developing this using a fork/pull model.

llrs commented 3 months ago

releasetools is a nice name, I don't like that it makes difficult to distinguish the two words but using capital letters in the name or underscore will be contentious...

I was a bit surprised by the hurry of activity, specially given your post on October. But I am glad you have more energy and time. Yes, we can use a new repo for this or just use email to schedule a call.

wlandau commented 3 months ago

I don't like that it makes difficult to distinguish the two words but using capital letters in the name or underscore will be contentious...

I see what you mean. We do have devtools, usethis, and testthat, but those are easier to read because they are shorter.

I have seen dots in package names more often, e.g. future.callr and future.batchtool. I have adopted that convention myself for crew.aws.batch and crew.cluster. Maybe release.tools? I will keep thinking this over.

shikokuchuo commented 3 months ago

The dot convention is usually to denote a backend or extension. So there would be a main package that consists of the first word alone. This wouldn't be the case here and I would find it confusing. We should keep thinking. One possibility is just 'releases'.

wlandau commented 3 months ago

Yeah, "releases" is my favorite candidate name at the moment.

wlandau commented 3 months ago

And for functions, so far I am thinking:

shikokuchuo commented 3 months ago

I think check_versions() doesn't need to be user-facing. It could be an internal function that caches its result per session. That would simplify things with just a single check_releases() or just releases::check().

Similarly we could have install_releases() or releases::install() - the alternative being to shim install.packages() if the package is not destined for CRAN.

Also really like the package contribution suggestion. Suggest that can remain contribute_packages() or just releases::contribute(). It just has a nicer 'community' feel about it than 'register' which I think is over-used (I'm guilty of this too)!

shikokuchuo commented 3 months ago

Note: Rstudio already shims utils::install.packages() so we'd have to take that into account, if we wanted to go down that route.

wlandau commented 3 months ago

Nice. This leaves us with just installation, checking, and contribution.

Do we think users will always use namespaced calls, e.g. releases::check(), like renv is used? If so, I think install(), check(), and contribute() are good names for functions. Otherwise, we might have conflicts with devtools::check() etc., in which case the longer names install_releases(), check_releases(), and contribute_releases() might make more sense.

shikokuchuo commented 3 months ago

You bring up a good point. No I don't believe the calls will be namespaced, so they should be the long versions. We want the package to be loaded in .Rprofile or Rprofile.site so our install function is always just available by default.

To that extent, I think it is potentially confusing to users to have a separate check function. I think we can roll it into install_releases() and either have it perform the check if the package is already installed (display the result, option to upgrade if there is one - so more like pak than base R), or have a dedicated dryrun (need to think of a user-friendly term) argument that would output verbose diagnostics rather than actually performing the install.

This would leave just install_releases() and contribute_releases().

wlandau commented 3 months ago

To that extent, I think it is potentially confusing to users to have a separate check function. I think we can roll it into install_releases() and either have it perform the check if the package is already installed (display the result, option to upgrade if there is one - so more like pak than base R), or have a dedicated dryrun (need to think of a user-friendly term) argument that would output verbose diagnostics rather than actually performing the install.

I see your point about the kind of functionality to emphasize, but I think there will be cases where a health check is desired even when there is no intent to install packages. If I have a package library I am happy with and I just want to know what is going on at https://r-releases.r-universe.dev, I would prefer a function like check_releases() because then I wouldn't risk accidentally setting the wrong argument and installing things. Also, if there are Shiny dashboards downstream, or for #10, there are good reasons to check packages even if you have no intention of installing them.

wlandau commented 3 months ago

I just created a stub of the package internally, and I hope to open-source it within the next couple days. I wrote a few lines of code for each function to get us started, but I did not get too far ahead. Hopefully everything will be readable at a glance.

wlandau commented 3 months ago

I got contribute_releases() fully working using only packages gert gh, httr2. (Example test: https://github.com/r-releases/r-releases/pull/126)

wlandau commented 3 months ago

The new releases package is now live: https://r-releases.github.io/releases/. It will soon be part of https://r-releases.r-universe.dev.

Charlie and I have decided to focus our efforts on #10. Maybe we will return to install_safe() if the dual-repo option does not work out.

wlandau commented 1 month ago

I think we have decided on #10. Happy to reopen if we end up revisiting this.