Insert binary packages to wrong repository on ARM M1 Mac

[dipterix]

https://cran.rstudio.com/bin/macosx/big-sur-arm64/contrib/

On M1 Mac, the packages are added to bin/macosx/contrib, but that is for x64 R. CRAN is using bin/macosx/big-sur-arm64/contrib instead.

> R.version
               _                                          
platform       aarch64-apple-darwin20                     
arch           aarch64                                    
os             darwin20                                   
system         aarch64, darwin20                          
status         Patched                                    
major          4                                          
minor          1.0                                        
year           2021                                       
month          06                                         
day            14                                         
svn rev        80505                                      
language       R                                          
version.string R version 4.1.0 Patched (2021-06-14 r80505)
nickname       Camp Pontanezen  

[eddelbuettel]

Good catch, and very likely.

If you have a M1 machine, can you write and test a PR at your end?

[dipterix]

Please check #126

[josherrickson]

Is there anyway around this for package maintainers until this gets pulled in? I can insert the .tgz into the correct place and modify the PACKAGES file; but what needs to be done for PACKAGES.gz and PACKAGES.rds to fix it?

Edit: I ran the following:

tools::write_PACKAGES("bin/macosx/big-sur-arm64/contrib/4.1/", type = "mac.binary", latestOnly = FALSE)

Should that be sufficient?

[eddelbuettel]

Looks like the issue petered out when we never got a unit test submitted. It would be really nice if someone could add one.

Until then if you built drat locally with the suggested PR and tested that it would give us a bit more insight too.

[josherrickson]

I tried that and when I ran insertpackages() pointing to the .tgz created from the mac-builder, it placed that into src/contrib instead of the proper directory.

[eddelbuettel]

Yes, that is essentially what the unit tests / should do here and what is missing.

See https://github.com/eddelbuettel/drat/tree/master/inst/extdata -- an entire directory tree of mock packages that in tests get moved around to ensure they end up in the right place. Maybe we just neeed an arm64/ subdirectory of one the binaries (in 4.1, I presume) and run a test on it. Do you want to give it a shot? I should be straightforward: do what @FelixErnst does in the tests he added to find such a file, then call one or two drat functions on it and ensure it puts them where it should.

[eddelbuettel]

Ok trying to help this and the corresponding PR #126 along, I did the following

• updated the empty test package by @FelixErnst to not 'warn' under R CMD check (old habits die hard)
• create a binary for it at the Arm M1 'macbuilder'
• moved the patch in Fix - big-sur ARM OSX : binary packages are not set properly #126 along by checking it out locally here and rebasing it (no strictly needed but cleaner)
• install from it so I should now be 'Arm M1' aware
• install the test package.

Sadly I see no difference. I use the littler wrapper calling drat::insertPackage via dratInsert.r -r /tmp/newDratwPR/ ~/git/drat/inst/extdata/4.1/arm64/foo_1.3.tgz (and with /tmp/newDrat for the unpatched current version). I get the same target directory :

edd@rob:/tmp/drat-dipterix/drat-1(master)$ tree /tmp/newDratwPR/
/tmp/newDratwPR/
└── docs
    └── bin
        └── macosx
            └── contrib
                └── 4.1
                    ├── foo_1.3.tgz
                    ├── PACKAGES
                    ├── PACKAGES.gz
                    └── PACKAGES.rds

5 directories, 4 files
edd@rob:/tmp/drat-dipterix/drat-1(master)$ tree /tmp/newDrat/
/tmp/newDrat/
└── docs
    └── bin
        └── macosx
            └── contrib
                └── 4.1
                    ├── foo_1.3.tgz
                    ├── PACKAGES
                    ├── PACKAGES.gz
                    └── PACKAGES.rds

5 directories, 4 files
edd@rob:/tmp/drat-dipterix/drat-1(master)$ 

Shouldn't the M1 chipset be reflected here? Or because it is a non-binary package they use a common path for all macOS platforms? Could one of you maxOS users chime in?

[eddelbuettel]

In other words do we need a 'big-sur-arm64' in there as in https://cloud.r-project.org/bin/macosx/big-sur-arm64/contrib/4.1/ ?

[josherrickson]

Yes, for M1 binary, the path needs to be bin/macosx/big-sur-arm64/contrib/4.1/ to work. I've confirmed this by manually placing the .tgz created from the mac builder in that correct folder, then running the tools::write_PACKAGES command I gave above. Following this, the addRepo(), install.packages(..., type = "mac.binary") code works as expected.

[eddelbuettel]

Thanks for confirming, So we need to adjust at least one of the path-creating helper functions.

[eddelbuettel]

I just committed two packages: foo_1.3.tar.gz which is R only and package created by the M1 macbuilder. This may be a package generally applicable as it contains only R code. And I added a second package bar_1.0.tar,gz along with binary also made by macbuild which has

Built: R 4.1.1; aarch64-apple-darwin20; 2022-04-06 00:14:41 UTC; unix
Archs: bar.so.dSYM

in DESCRIPTION. We would have to make sure that this gets picked up correctly -- and maybe it already does but I, not having an M1 mac, cannot really check. If one of you could pick up the loose ball here and run and few more meters down the field we'd be in better shape. I don't think there is much missing, but I would love to add a test for these two (i.e. inject them into drat and confirm they are being seen by available.packages(). For the binary the test could limit itself to Arm M1.

[josherrickson]

Are you looking for formal tests? If so, can you point me to existing such tests in the current codebase? Or are you just looking for someone on M1 to confirm it works as expected?

[eddelbuettel]

Formal tests would not hurt.

It all happens inside tests/skeleton_git2r,R which I altered yesterday just to accomodate the number of tar.gz files it plays with. You will notice two main functions: one, optional, to test with git2r via a repo in gh-pages branch (old school), one with docs. They inject some files and check that the right number of files come back.

What we want now, I think, is a new (smaller) function (or just if ... block) that if M1 is detected injects the source package foo 1.3 and the binary package bar 1.0 and verifies that it gets them back (following the other logic in the file, relying of base R functions to query a package repo).

Does that makes sense to you too?

[eddelbuettel]

And I should add that the current locations of the macbuilder / Arm M1 generated files foo 1.3 and bar 1.0 are "guesses" I made. If they need to be in a different directory (to better match their final locations under Arm M1) then let's adjust that. I don't have the proper hardware to test much more here.

[josherrickson]

I'm having trouble running and/or understanding the testing script. However, I think I know changes that need to be made. It's just a few tweaks to R/insertPackages.R:

diff --git a/R/insertPackage.R b/R/insertPackage.R
index ab523fd..c95acf8 100644
--- a/R/insertPackage.R
+++ b/R/insertPackage.R
@@ -246,7 +246,7 @@ identifyPackageType <- function(file, pkginfo = getPackageInfo(file)) {
                           "3.5" = paste0(ret,".el-capitan"),
                           "3.6" = paste0(ret,".el-capitan"),
                           ret)
-        } else if(pkginfo["osxFolder"] %in% c("mavericks","el-capitan")) {
+        } else if(pkginfo["osxFolder"] %in% c("mavericks","el-capitan","big-sur-arm64")) {
             ret <- paste0(ret,".",pkginfo["osxFolder"])
         } else {
             stop("mac.binary subtype couldn't be determined. This shouldn't ",
@@ -301,6 +301,7 @@ getPackageInfo <- function(file) {
     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",
                         "")

     fields <- c(fields, "Rmajor" = unname(rmajor), "osxFolder" = osxFolder)

After making these changes,

insertPackage(file = "inst/extdata/big-sur-arm64/bin/4.1/bar_1.0.tgz", repodir = "docs")

puts the file in the proper directory:

><> tree docs/docs
docs/docs
├── bin
│   └── macosx
│       └── big-sur-arm64
│           └── contrib
│               └── 4.1
│                   ├── PACKAGES
│                   ├── PACKAGES.gz
│                   ├── PACKAGES.rds
│                   └── bar_1.0.tgz
├── index.html
└── src
    └── contrib
        ├── PACKAGES
        ├── PACKAGES.gz
        ├── PACKAGES.rds
        ├── foo_1.1.tar.gz
        └── foo_1.2.tar.gz

Does this help? I can put it in a PR if you'd like but the changes are so minor I figured you could just add them. Unfortunately I've run out of time trying to figure out the testing script.

[eddelbuettel]

Yes that does help and seems to move very much in the intended direction. I could see that we needed new subdirs big-sur-arm64 I just could not check on what they would to be tagged off (though building the binary as I via the macbuilder lead to aarch64-apple-darwin20 as you use here) but it needs someone like you to assert that the packages are actually installable. (And I agree that Felix's test script is tricky. We could possibly also leave it and a a new (Arm-only) one in tests/ that does something similar at least with bar_1.0.tgz.)

As for a PR I would prefer if we could do either in the same PR (or in combination with) as the existing/open PR #126. Are you using that?

[josherrickson]

My understanding of PR's is that I cannot modify someone else's PR without being given maintainer privleges on this repo. I'd be happy to push a new PR later this afternoon though.

[eddelbuettel]

I concur, but we still want to fold both those PRs. So let me propose the following:

• I will (now: have) again rebase the existing PR so that it is current (i.e. its two commits ahead of what we have here)
• You can clone that repo and work off it and then your changes would be relative to it, and this repo

Don't mean to overcomplicate it but #126 did important first steps, and it was only because I got too busy that this fell to side. We should include it if we can (and its changes seem orthogonal to what we do / need).

And so I just did the rebase, if you look at https://github.com/dipterix/drat-1/commits/master it is its two commits ahead.

[eddelbuettel]

Thanks for putting up #131. You and I should still add a test that 'pushing a package' into an Arm 1 repo makes it accessible. You be easy is. Can you remind me how would test for Arm M1? I have code in a configure.ac but that uses uname. What does does Sys.info()[["machine"]] return, and do we need to test for something else?

[josherrickson]

On my M1 machine:

> Sys.info()[["machine"]]
[1] "arm64"

On my colleague's Intel Mac:

> Sys.info()[["machine"]]
[1] "x86_64"

[eddelbuettel]

Yep, thanks -- that should do and arm64 should be sufficient. I'll try to cook something simple up later.

[eddelbuettel]

Ok, I just committed a new test file. It has two parts: one that runs everywhere for a round-turn on a source package, and then another one for Arm binaries (only conditional on 4.1 as that is the binary we have). I just ran that at the macbuilder and it passed, and it would be great if you could test it too.

You can either just drop this file into the tests/ directory and do usual build-and-test cycle (or even just run the file via Rscript). Or, if you prefer, you could rebase your repo against this origin repo and test then.

[josherrickson]

I grabbed your run.sh script, and ran ./run.sh run_tests and I believe this file passed. It does flag a few other warnings, so I've included the entire output below.

I did try stepping through the test file manually and it did not work; I'm unsure if that's expected or not.

Building with: R CMD build --no-build-vignettes --no-manual
* checking for file ‘./DESCRIPTION’ ... OK
* preparing ‘drat’:
* checking DESCRIPTION meta-information ... OK
* checking for LF line-endings in source and make files and shell scripts
* checking for empty or unneeded directories
* building ‘drat_0.2.2.1.tar.gz’
Testing with: R CMD check "drat_0.2.2.1.tar.gz" --no-vignettes --no-manual --as-cran --install-args=--install-tests
(CRAN incoming checks are off)
* using log directory ‘/Users/josh/repositories/R/drat-1/drat.Rcheck’
* using R version 4.1.3 (2022-03-10)
* using platform: aarch64-apple-darwin21.3.0 (64-bit)
* using session charset: UTF-8
* using options ‘--no-manual --no-vignettes --as-cran’
* checking for file ‘drat/DESCRIPTION’ ... OK
* checking extension type ... Package
* this is package ‘drat’ version ‘0.2.2.1’
* package encoding: UTF-8
* checking package namespace information ... OK
* checking package dependencies ... OK
* checking if this is a source package ... OK
* checking if there is a namespace ... OK
* checking for executable files ... OK
* checking for hidden files and directories ... OK
* checking for portable file names ... OK
* checking for sufficient/correct file permissions ... OK
* checking whether package ‘drat’ can be installed ... OK
* checking installed package size ... OK
* checking package directory ... OK
* checking for future file timestamps ... OK
* checking DESCRIPTION meta-information ... OK
* checking top-level files ... NOTE
Non-standard files/directories found at top level:
  ‘run.sh’
* checking for left-over files ... OK
* checking index information ... OK
* checking package subdirectories ... OK
* checking R files for non-ASCII characters ... OK
* checking R files for syntax errors ... OK
* checking whether the package can be loaded ... OK
* checking whether the package can be loaded with stated dependencies ... OK
* checking whether the package can be unloaded cleanly ... OK
* checking whether the namespace can be loaded with stated dependencies ... OK
* checking whether the namespace can be unloaded cleanly ... OK
* checking loading without being on the library search path ... OK
* checking dependencies in R code ... OK
* checking S3 generic/method consistency ... OK
* checking replacement functions ... OK
* checking foreign function calls ... OK
* checking R code for possible problems ... OK
* checking Rd files ... OK
* checking Rd metadata ... OK
* checking Rd line widths ... OK
* checking Rd cross-references ... OK
* checking for missing documentation entries ... OK
* checking for code/documentation mismatches ... OK
* checking Rd \usage sections ... OK
* checking Rd contents ... OK
* checking for unstated dependencies in examples ... OK
* checking files in ‘vignettes’ ... WARNING
Files in the 'vignettes' directory but no files in 'inst/doc':
  ‘CombiningDratAndTravis.html’, ‘CombiningDratAndTravis.md’,
    ‘DratFAQ.html’, ‘DratFAQ.md’, ‘DratForPackageAuthors.html’,
    ‘DratForPackageAuthors.md’, ‘DratForPackageUsers.html’,
    ‘DratForPackageUsers.md’, ‘DratStepByStep.html’,
    ‘DratStepByStep.md’, ‘WhyDrat.html’, ‘WhyDrat.md’, ‘water.css’
* checking examples ... OK
* checking for unstated dependencies in ‘tests’ ... OK
* checking tests ...
  Running ‘simpleTests.R’
  Running ‘skeleton_git2r.R’
  Running ‘testArmBinary.R’
  Running ‘version.R’
 OK
* checking for unstated dependencies in vignettes ... OK
* checking package vignettes in ‘inst/doc’ ... WARNING
dir.exists(dir) is not TRUEdir.exists(dir) is not TRUEdir.exists(dir) is not TRUEdir.exists(dir) is not TRUEdir.exists(dir) is not TRUEdir.exists(dir) is not TRUE
Package vignettes without corresponding single PDF/HTML:
   ‘CombiningDratAndTravis.md’
   ‘DratFAQ.md’
   ‘DratForPackageAuthors.md’
   ‘DratForPackageUsers.md’
   ‘DratStepByStep.md’
   ‘WhyDrat.md’

* checking running R code from vignettes ... SKIPPED
* checking re-building of vignette outputs ... SKIPPED
* checking for non-standard things in the check directory ... OK
* checking for detritus in the temp directory ... OK
* DONE
Status: 2 WARNINGs, 1 NOTE

[eddelbuettel]

Yes you ran testArmBinary.sh (along with the other one) on aarch64-apple-darwin21.3.0 so I think we can call it a win!

[eddelbuettel]

Thanks everyone, this is now merged. Yay!