Include the SVN revision in the r-devel package cache key

[schloerke]

When testing on r-devel, breaking internal core R code changes can happen within the same day. It would be good to try to allow for rapid GHA development with a consistent cache key while busting the cache if the internal core R code changes. To bust this cache, the SVN revision number has been included (in addition to make formatting similar with regular r-version values).

Existing output of non-devel r-version value:

[1] "R version 4.1.3 (2022-03-10)"

Existing output of an r-devel r-version value:

[1] "4.3.0"

Proposed output of an r-devel r-version value:

[1] "R version 4.3.0 (r83776)"

---

Recent motivation:

• Example action that failed due to core change: https://github.com/rstudio/shiny/actions/runs/4166018903/jobs/7209818945#step:4:318. Error: Graphics API version mismatch
• Fixed action by manually busting the cache: https://github.com/rstudio/shiny/actions/runs/4167567186/jobs/7213321903

cc @jcheng5

[gaborcsardi]

Thanks! Including the SVN revision there is not great, because the graphics API only changes ~ once a year, and the SVN revision changes practically every day.

So maybe we could query the graphics API version instead, plus maybe .Internal(internalsID())?

[schloerke]

I like the idea of using the internalsID(), but https://github.com/r-devel/r-svn/blame/bbf4b924e66dbe097b0570ce549f9b77698dba7c/src/include/Defn.h#L95 hasn't changed in two years. However the install error happened recently.

I'm not familiar enough on the internals to suggest a better value 😢 . (I did not know about internalsID()). And using a collection of internal version values (like the multiple graphics version values) seems incomplete and error prone.

the SVN revision changes practically every day

Fair. There were ~15 commits just today. This would bust the cache quite often.

---

I'll defer to you if we can enhance the cache key. But finding the error message while testing was quite confusing and I'd rather err on longer compute times than confusing errors.

[gaborcsardi]

hasn't changed in two years. However the install error happened recently.

I meant the graphics api version + the internal ID. There is only one graphics api version number, an integer number. Those are just symbolic names for the various (recent) versions.

Fair. There were ~15 commits just today. This would bust the cache quite often.

But we only have at most one R-devel build per day, so it is once a day at most. But that's not ideal, either.

Anyway, I think the graphics api version + the internal ID would work.

[jcheng5]

Anyway, I think the graphics api version + the internal ID would work.

That sounds reasonable to me. There's value in these workflows completing quickly.

[schloerke]

Graphics API Version + Internal ID sounds good then!

@gaborcsardi Do you have another incantation to expose this value? I could not find a way without making/compiling/calling a C function. Thank you!

[gaborcsardi]

Yeah, one is .Internal(internalsID()), and the other one needs C code. It goes roughly like this, I haven't tested it much:

lines <- c(
  "#include <Rinternals.h>",
  "#include <R_ext/GraphicsEngine.h>",
  "SEXP ghapi_version() {",
  "return ScalarInteger(R_GE_getVersion());",
  "}"
)

dir.create(tmp <- tempfile())
src <- file.path(tmp, "ghapi.c")
writeLines(lines, src)
so <- file.path(tmp, paste0("ghapi", .Platform$dynlib.ext))

rbin <- file.path(R.home("bin"), "R")
tools:::.shlib_internal(c("-o", so, src))
dl <- dyn.load(so)

ghapi_version <- getNativeSymbolInfo("ghapi_version", dl)
cat(.Call(ghapi_version))

and you need to look at the last line of the output only. IDK if there is a way to discard the rest of the output.

[jcheng5]

Looks like you can get it by calling dev.new(); recordPlot() and then it’s one of the attributes on the returned object.

[schloerke]

Running the devel code path on R 4.1.3:

[1] "R version 4.1.3 (ge14:2fdf6c18-697a-4ba7-b8ef-11c0d92f1327)"

[codecov-commenter]

Codecov Report

Merging #699 (730d281) into v2-branch (9611ec5) will not change coverage.
The diff coverage is n/a.

❗ Current head 730d281 differs from pull request most recent head fc529f1. Consider uploading reports for the commit fc529f1 to get more accurate results

@@             Coverage Diff             @@
##           v2-branch      #699   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
===========================================
  Files              2         2           
  Lines             10        10           
===========================================
  Hits              10        10           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[gaborcsardi]

Thanks!

[github-actions]

This pull request has been automatically locked. If you believe you have found a related problem, please file a new issue and include a link to this pull request.