Possible discrepancy between where drat and CRAN places binaries when .Platform$pkgType is "mac.binary.big-sur-x86_64"

[arnejohannesholmin]

Hi,

Thank you for a brilliant R package.

I have however an issue I would like to check on. I am a bit confused about the folder in which binaries are placed by drat on macOS. On my Intel Mac I get

.Platform$pkgType
[1] "mac.binary.big-sur-x86_64"

which is also reflected in

utils::contrib.url("https://cloud.r-project.org", type = "binary")
[1] "https://cloud.r-project.org/bin/macosx/big-sur-x86_64/contrib/4.3"

This is the path used by install.packages(), as far as I understand.

However, in my repo generated by drat the macOS binaries for R 4.3 are placed in

"bin/macosx/contrib/4.3"

I have not had the chance to test where drat places macOS binaries for the Apple silicon (M1/M2) Macs, but at least "mac.binary.big-sur-arm64" is listed as one of the DRAT_BINARY_TYPES in the file

archivePackages.R

Could it be that drat should place the Intel Mac binaries in a folder "bin/macosx/big-sur-x86_64/contrib/4.3" instead of "bin/macosx/contrib/4.3"?

[eddelbuettel]

That is very much possible, and as I would need your help in correcting / tuning this.

There is a moderate amount of code in the package trying to find these paths, dependending on both the system / OS / cpu / version / ... and it is possibly that we need an update for R 4.3.0 on macOS with big-sur. Can you take a poke at updating / correcting this ?

[arnejohannesholmin]

So, I cloned this repo and made a copy in our organization (got confused about forking), made the following two changes to the insertPackage.R file, and voila, I got the correct bin/macosx/big-sur-x86_64/contrib/4.3 folder:

1. On line 259, change

} else if(pkginfo["osxFolder"] %in% c("mavericks","el-capitan","big-sur-arm64")) {

to

} else if(pkginfo["osxFolder"] %in% c("mavericks","el-capitan","big-sur-arm64", "big-sur-x86_64")) {

2. Change lines 311 - 315

osxFolder <- switch(fields["OSflavour"],
                        "x86_64-apple-darwin13.4.0" = "mavericks",
                        "x86_64-apple-darwin15.6.0" = "el-capitan",
                        "aarch64-apple-darwin20"    = "big-sur-arm64",
                        "")

to lines 311 - 316

 osxFolder <- switch(fields["OSflavour"],
                        "x86_64-apple-darwin13.4.0" = "mavericks",
                        "x86_64-apple-darwin15.6.0" = "el-capitan",
                        "aarch64-apple-darwin20"    = "big-sur-arm64",
                        "x86_64-apple-darwin20"     = "big-sur-x86_64",
                        "")

[eddelbuettel]

So much awesome. I think I can carry that over as is (unless you want a PR to put you 'officially' on the record -- your call).

Also poke into the (very involved, that was contributed) tests in the package -- we have binary artifacts of small packages. Could you create one like those (basically: a handful of kilobytes for a trivial package foo or bar like those there, but for R 4.3.0 and you macOS flavour)? Then we would have tests too.

[arnejohannesholmin]

At first I did not see a need for a PR, but then I realised that there were more changes required that the two listed above (function argument defaults and documentation), and I also added a small package and a test as you suggested PR 139. It passed R CMD check locally, so hopefully I did not mess up anything.

[eddelbuettel]

Fixed in #139 -- thanks so much!

I'll look into a release to CRAN 'real soon'.

[eddelbuettel]

Hah. Just noticed I had this as an uncommitted note (that will go out now as I update ChangeLog and NEWS) in one of the test support files:

+
+## TODO Use
+##   > as.package_version(system("sw_vers -productVersion", intern = TRUE)) == 13.1
+##   [1] TRUE
+## if on Darwin and x86_64 (as in tiledb-r's configure.ac with
+##   uname=`uname`
+##   machine=`uname -m`
+##   if test x"${uname}" = x"Darwin" -a x"${machine}" = x"x86_64"; then
+## which we can rework as R system() calls.
+##
+## Use https://en.wikipedia.org/wiki/MacOS_version_history for mininum versions

[eddelbuettel]

Hey @arnejohannesholmin we are looking mostly really good. I merged, rolled the version, then forgot I actually check the version in DESCRIPTION against the one in NEWS, updated at force pushed.

At r-universe, which as a default runs a broader set than we do in CI, we do see an error with macOS and 'oldrel' ie 4.2.
Would have be able to get hold of R 4.2 on macOS to take a look? It may be as simple as a test (implicitly ?) assuming R 4.3.* in the big-sur test so maybe we should make sure that that test only runs when R is 4.3?

Screenshot of the state of the world:

URL of the run is https://github.com/r-universe/eddelbuettel/actions/runs/4973726533/jobs/8899774367

[arnejohannesholmin]

Hi,

I ran R CMD check on 4.2.3 and got the same error you showed from r-universe. However, I am not sure that it is related to the added test for mac.binary.big-sur-x86_64. When I ran R CMD check on the latest commit before I started poking around, it failed with the same error. It seems that when in R 4.2.3 there are 7 rows in repoinfo on line 123 of skeleton_git2r.R but only 6 rows (as expected by the test) on R 4.3.0.

When I tried disabling line 131 R CMD check ran without any errors in R 4.2.3.

I am not sure I understand why we get 7 and not 6 rows in R 4.2.3. The repoinfo table has an extra second row that seems identical to the following row except for that the type is "binary" and not "mac.binary". So it seems we get a copy for some reason.

It is tempting to add a condition for R 4.2 allowing 7 rows, which I tried locally and it worked.

[eddelbuettel]

Oh good point, good point -- that may have been older than your PR! Sorry for implying otherwise.

Those tests are quite comprehensive and also quite incomprehensible 😀 I didn't write them, and sweated debugging them too. I think I will glance at your PR, likely merge it and if r-universe (which gives me multi-release tests for free, as it does to everybody else ...) passes I think we can ship this version to CRAN.

Thanks for your diligence here. It helps a lot with the macOS side I cannot really cover.

[eddelbuettel]

Whoops. There was no PR. My bad (I had a PR on a work project ahead of it on the notification page).

Would you mind sending a 'if 4.2 then 7 rows' to make the tests fully 'green' ?

[arnejohannesholmin]

Yes, I made a new PR with the 7 rows for R 4.1 and 4.2. Hope it works.

[eddelbuettel]

I think at this point I just trust you and wait the hour or so it takes for r-universe to catch up. Fingers crossed!

[eddelbuettel]

Boo-hoo. Now windows-oldrel no longer likes us:

Per https://github.com/r-universe/eddelbuettel/actions/runs/4985115061/jobs/8924384187

[eddelbuettel]

So I punted in 79af2e9 and turned a stop() into a warning(). Should probably have done that earlier also with the one you fixed. Now all is good: