Should we catch interrupts during install so that we never end up with a half-installed package?

for packages that need compilation. Requires r-lib/pkgbuild#33.

Fails with the current CRAN version of bindrcpp, because we need to compile Rcpp attributes twice before a stable state is achieved. Either way, I don't think pkginstall should change the source of the installed packages.

curl::curl_download("https://cran.r-project.org/src/contrib/bindrcpp_0.2.tar.gz", "bindrcpp_0.2.tar.gz")
withr::with_temp_libpaths(pkginstall::install_source("bindrcpp_0.2.tar.gz", quiet = FALSE))
#> Running /usr/lib/R/bin/R CMD INSTALL \
#>   /tmp/RtmpvwiYVQ/file4e0366051677/bindrcpp --build \
#>   '--library=/tmp/RtmpvwiYVQ/temp_libpath4e034b46ddc0'
#> * installing *source* package ‘bindrcpp’ ...
#> files ‘R/RcppExports.R’, ‘inst/include/bindrcpp_RcppExports.h’, ‘src/RcppExports.cpp’ have the wrong MD5 checksums
#> ** libs
#> g++  -I/usr/share/R/include -DNDEBUG -I../inst/include -I. -I"/home/muelleki/R/x86_64-pc-linux-gnu-library/3.4/Rcpp/include" -I"/home/muelleki/R/x86_64-pc-linux-gnu-library/3.4/plogr/include"    -fpic  -Wall -pedantic -c plogr.cpp -o plogr.o
#> g++  -I/usr/share/R/include -DNDEBUG -I../inst/include -I. -I"/home/muelleki/R/x86_64-pc-linux-gnu-library/3.4/Rcpp/include" -I"/home/muelleki/R/x86_64-pc-linux-gnu-library/3.4/plogr/include"    -fpic  -Wall -pedantic -c create.cpp -o create.o
#> g++  -I/usr/share/R/include -DNDEBUG -I../inst/include -I. -I"/home/muelleki/R/x86_64-pc-linux-gnu-library/3.4/Rcpp/include" -I"/home/muelleki/R/x86_64-pc-linux-gnu-library/3.4/plogr/include"    -fpic  -Wall -pedantic -c RcppExports.cpp -o RcppExports.o
#> g++  -I/usr/share/R/include -DNDEBUG -I../inst/include -I. -I"/home/muelleki/R/x86_64-pc-linux-gnu-library/3.4/Rcpp/include" -I"/home/muelleki/R/x86_64-pc-linux-gnu-library/3.4/plogr/include"    -fpic  -Wall -pedantic -c test.cpp -o test.o
#> g++ -shared -L/usr/lib/R/lib -Wl,-Bsymbolic-functions -Wl,-z,relro -fuse-ld=gold -o bindrcpp.so RcppExports.o create.o plogr.o test.o -L/usr/lib/R/lib -lR
#> installing to /tmp/RtmpvwiYVQ/temp_libpath4e034b46ddc0/bindrcpp/libs
#> ** R
#> ** inst
#> ** preparing package for lazy loading
#> ** help
#> *** installing help indices
#> ** building package indices
#> ** testing if installed package can be loaded
#> Error: package or namespace load failed for ‘bindrcpp’ in dyn.load(file, DLLpath = DLLpath, ...):
#>  unable to load shared object '/tmp/RtmpvwiYVQ/temp_libpath4e034b46ddc0/bindrcpp/libs/bindrcpp.so':
#>   /tmp/RtmpvwiYVQ/temp_libpath4e034b46ddc0/bindrcpp/libs/bindrcpp.so: undefined symbol: bindrcpp_callback_string
#> Error: loading failed
#> Execution halted
#> ERROR: loading failed
#> * removing ‘/tmp/RtmpvwiYVQ/temp_libpath4e034b46ddc0/bindrcpp’
#> Error: loading failed
#> installing to /tmp/RtmpvwiYVQ/temp_libpath4e034b46ddc0/bindrcpp/libs

Created on 2018-02-21 by the reprex package (v0.2.0).

And we can't rely on the package version having changed in order to trigger a re-install.

This is bit tricky since presumably CRAN doesn't rebuild old versions of the packages, so you might need to install from source.

(This might not be the package which ends up handling this, but I figured we should record so we don't forget about it)

using pkgbuild::build(), only show compilation output if it fails.

i.e. C++11, stringi unicode data

IIRC I did some experiments a few years ago that suggested it was mostly to move a directory containing a locked DLL, even though you couldn't delete it.

If that fails, we'll need to check to see if the dll is loaded, and fail with an informative before trying to copy.

Would need a reasonable timeout, maybe configurable via an option?

i.e. https://github.com/r-lib/filelock

https://travis-ci.org/r-lib/pkginstall/jobs/353787723

  ── 1. Error: kill_all_processes (@test-install-parts.R#295)  ───────────────────
  callr_is_alive: No child processes
  1: kill_all_processes(state) at testthat/test-install-parts.R:295
  2: proc$is_alive()
  3: process_is_alive(self, private)
  
  ── 2. Error: kill_all_processes  ───────────────────────────────────────────────
  callr_kill: No child processes
  1: p3$kill()
  2: process_kill(self, private, grace)
  
  ══ DONE ════════════════════════════════════════════════════════════════════════
  Don't worry, you'll get it.
  Error: Test failures
  Execution halted

I.e. rstudio/rstudio#2387

E.g. tidyverse/tidyverse#81

modify the values in some way ala https://github.com/hadley/devtools/blob/a0c8d7333be185f979a60823dcfad66c9f3ea35e/R/install-remote.R#L75-L102

Also need to invalidate the MD5 if we do this
https://github.com/hadley/devtools/blob/a0c8d7333be185f979a60823dcfad66c9f3ea35e/R/install-remote.R#L106-L116

leads to failures in install_source(), because of file.rename() calls. Maybe create a hidden subdirectory in the library and move there?

Otherwise, no problems are expected.

E.g. if installing into lib1 and package pkg1 is a dependency, and it is not in lib1, but it is in lib2 (in .libPaths()), then should we install pkg1 in lib1 (as well)?

What if a new version of pkg1 is required? Leave the old one in lib2 and put the new one in lib1?

Should libraries just be independent completely? I.e. when installing into lib1 we would ignore lib2 completely.

Seems like the child processes are running very slowly, so probably a callr/processx issue.

On top of callr:rcmd_process, similarly to rcmdcheck::rmcdcheck_process and pkgbuild::rcmdbuild_process`.

E.g. the ones using withr::with_connection internally.

One solution is to avoid loading pkginstall and the package being installed at the same time, at any point of the installation.

Maybe this already fails, but we need a test case for it, still, to make sure that we fail gracefully.

This either belongs to pkginstall or pkgman.

@jimhester do you know why it fails? I can take a look.

For all output.

library() checks if an installed package was built for the current operating system. Maybe we should be checking this as well when we install the package?

https://github.com/wch/r-source/blob/aeb448374dec7076b87a9bb1b4063fa041e5c2a3/src/library/base/R/library.R#L74-L106

This needs pkgdepends currently. Then next time pkgman/pkgdepends is used to install the same package, it will pick up the built (binary) from the cache.

Who knows if CRAN will ever let archive in, we will have to use R's internal unzip / tar code, or maybe https://github.com/r-lib/zip

In addition to atomic individual installs. This is a bit harder, but I think it would be a cool feature.

It should show the process(es) that are running "currently", not just the last messages.

To make it easier to pass down parameters from pkgman, without cluttering the user-visible api.

• factor out the simple installation without a plan
• do not repeat default values in internal functions
• write assertions for arguments
• avoid generating output in child processes
• pass status from child process to parent explicitly, instead of just parsing the stdout/stderr.

These are also needed for cli integration.

After some changes these are left to do:

• output, including progress bars
• more extensive tests
• pretty-printing the result
• coloring compiler output (might move to pkgbuild)
• 100% code coverage later

Once this is in the install plan: r-lib/pkgdepends#82

I'm running into a similar problem as the one discussed in r-lib/pak#100 when using pak::pkg_install() on a cluster. I've tracked the location of the error to pkginstall::install_extracted_binary():

The temp directory on the cluster is (I believe) on a shared network drive and the error occurs when updating a package that already exists in my lib path at the point where file.rename() tries to move the existing package to my temp directory. Manually running the above lines (while trying to install tidyr) results in the following

pkginstall:::create_temp_dir()
## [1] "/scratch/<cluster_ip>/RtmpPknhh9/file3ac92810967e"
move_to <- file.path(pkginstall:::create_temp_dir(), "tidyr")
move_to
## [1] "/scratch/<cluster_ip>/RtmpPknhh9/file3ac9561aa77c/tidyr"
file.rename(file.path(.libPaths()[1], "tidyr"), move_to)
## [1] FALSE
## Warning message:
## In file.rename(file.path(.libPaths()[1], "tidyr"), move_to) :
##   cannot rename file '/home/<private_lib>/3.5.2/tidyr' to '/scratch/<cluster_ip>/RtmpPknhh9/file3ac9561aa77c/tidyr', reason 'Invalid cross-device link'

In my issue searching, I found a similar issue in vtest that suggests replacing file.remove() with file.copy() followed by unlink().

As a final note, if the goal is to remove the existing package, can the error message be modified to read "Failed to remove existing package at..."? The advantage is the hint provided that the user could resolve the error by manually removing the existing package. I found the current error message confusing as to which package was being moved and where.

Might be an RStudio bug, actually...