Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Install should fail if there's an error copying DLL on Windows #113

Closed
wch opened this issue Sep 22, 2017 · 8 comments

Comments

Projects
None yet
3 participants
@wch
Copy link
Member

commented Sep 22, 2017

If a package is currently loaded on Windows and you try to install over it, some of the files will be copied over, but the DLL will not.

Real-world example here: r-lib/scales#101

To reproduce:

> library(scales)
> remotes::install_github("hadley/scales")
Downloading GitHub repo hadley/scales@master
Installing package into ‘C:/Users/IEUser/Documents/R/win-library/3.4’
(as ‘lib’ is unspecified)
* installing *source* package 'scales' ...
** libs

*** arch - i386
c:/Rtools/mingw_32/bin/g++  -I"C:/PROGRA~1/R/R-34~1.0/include" -DNDEBUG  -I"C:/Users/IEUser/Documents/R/win-library/3.4/Rcpp/include"   -I"d:/Compiler/gcc-4.9.3/local330/include"     -O2 -Wall  -mtune=core2 -c RcppExports.cpp -o RcppExports.o
c:/Rtools/mingw_32/bin/g++  -I"C:/PROGRA~1/R/R-34~1.0/include" -DNDEBUG  -I"C:/Users/IEUser/Documents/R/win-library/3.4/Rcpp/include"   -I"d:/Compiler/gcc-4.9.3/local330/include"     -O2 -Wall  -mtune=core2 -c colors.cpp -o colors.o
c:/Rtools/mingw_32/bin/g++ -shared -s -static-libgcc -o scales.dll tmp.def RcppExports.o colors.o -Ld:/Compiler/gcc-4.9.3/local330/lib/i386 -Ld:/Compiler/gcc-4.9.3/local330/lib -LC:/PROGRA~1/R/R-34~1.0/bin/i386 -lR
installing to C:/Users/IEUser/Documents/R/win-library/3.4/scales/libs/i386

*** arch - x64
c:/Rtools/mingw_64/bin/g++  -I"C:/PROGRA~1/R/R-34~1.0/include" -DNDEBUG  -I"C:/Users/IEUser/Documents/R/win-library/3.4/Rcpp/include"   -I"d:/Compiler/gcc-4.9.3/local330/include"     -O2 -Wall  -mtune=core2 -c RcppExports.cpp -o RcppExports.o
c:/Rtools/mingw_64/bin/g++  -I"C:/PROGRA~1/R/R-34~1.0/include" -DNDEBUG  -I"C:/Users/IEUser/Documents/R/win-library/3.4/Rcpp/include"   -I"d:/Compiler/gcc-4.9.3/local330/include"     -O2 -Wall  -mtune=core2 -c colors.cpp -o colors.o
c:/Rtools/mingw_64/bin/g++ -shared -s -static-libgcc -o scales.dll tmp.def RcppExports.o colors.o -Ld:/Compiler/gcc-4.9.3/local330/lib/x64 -Ld:/Compiler/gcc-4.9.3/local330/lib -LC:/PROGRA~1/R/R-34~1.0/bin/x64 -lR
installing to C:/Users/IEUser/Documents/R/win-library/3.4/scales/libs/x64
Warning in file.copy(files, dest, overwrite = TRUE) :
  problem copying .\scales.dll to C:\Users\IEUser\Documents\R\win-library\3.4\scales\libs\x64\scales.dll: Permission denied
** R
** preparing package for lazy loading
** help
Loading required package: scales
*** installing help indices
** building package indices
** testing if installed package can be loaded
*** arch - i386
*** arch - x64
* DONE (scales)
@gaborcsardi

This comment has been minimized.

Copy link
Member

commented Sep 22, 2017

I think this is coming from install.packages, so it is a bug in base R. It should fail instead of the warning.

@wch

This comment has been minimized.

Copy link
Member Author

commented Sep 22, 2017

When I try to use install.packages, it just doesn't allow it (R 3.4.0, Windows):

> install.packages('scales')
Installing package into ‘C:/Users/IEUser/Documents/R/win-library/3.4’
(as ‘lib’ is unspecified)
trying URL 'https://cloud.r-project.org/bin/windows/contrib/3.4/scales_0.5.0.zip'
Content type 'application/zip' length 695672 bytes (679 KB)
downloaded 679 KB

package ‘scales’ successfully unpacked and MD5 sums checked
Warning: cannot remove prior installation of package ‘scales’

The downloaded binary packages are in
        C:\Users\IEUser\AppData\Local\Temp\Rtmp6PvtwD\downloaded_packages

And when I try to install it using R CMD INSTALL (while scales is loaded in an R session), it says:

C:\Users\IEUser\Downloads>R CMD INSTALL scales_0.5.0.zip
* installing to library 'C:/Users/IEUser/Documents/R/win-library/3.4'
package 'scales' successfully unpacked and MD5 sums checked
Warning: cannot remove prior installation of package 'scales'
@gaborcsardi

This comment has been minimized.

Copy link
Member

commented Sep 22, 2017

Looks like the behavior is different for source and binary packages then. Still, this is coming from install.packages.

@wch

This comment has been minimized.

Copy link
Member Author

commented Sep 22, 2017

Oops, you're right - when I use type="source", it installs the files except for the DLL.

@gaborcsardi

This comment has been minimized.

Copy link
Member

commented Sep 22, 2017

We'll handle this better in pkginstall.

@jimhester

This comment has been minimized.

Copy link
Member

commented Aug 17, 2018

I looked into this again and unfortunately R CMD INSTALL unconditionally sets options(warn = 1), so it is not possible to override this in a temporary .RProfile to change the behavior. I opened an issue suggesting this line be removed https://bugs.r-project.org/bugzilla/show_bug.cgi?id=17453.

We could do something like

library(glue)
system2("Rterm.exe", c(
  "-e",
  shQuote("withCallingHandlers(tools:::.install_packages(commandArgs(TRUE)), warning = function(e) stop(e))"),
  "--args",
  "glue_1.3.0.tar.gz"
  )
)

Which does convert the warnings to errors, but it seems somewhat fragile.

Other approaches; such as checking the packages for integrity by md5 hashing seem out of scope for remotes right now.

@gaborcsardi

This comment has been minimized.

Copy link
Member

commented Aug 17, 2018

Other approaches; such as checking the packages for integrity by md5 hashing seem out of scope for remotes right now.

AFAIR, on Windows, where this matters, R hashes all files, so we would only need to check the hashes.

@jimhester

This comment has been minimized.

Copy link
Member

commented Aug 20, 2018

So Martin Maechler changed tools:::.install_packges() (which is what R CMD INSTALL calls) in R-devel so that it respects options(warn = 2) wch/r-source@828a04f, so we could now do this on later versions of R.

jimhester added a commit that referenced this issue Aug 31, 2018

Convert warnings to errors when installing (#141)
`install.packages()` and `R CMD INSTALL` can fail to install a package containing a dll on Windows, usually because the DLL is already opened, either by the package already being loaded in the current R session or another R session.

When this happens the call to `file.copy()` moving the newly built DLL into the install location fails, but only with a warning.

After this occurs you have new package code trying to use an old DLL, which typically means installed package is completely broken. The situation is worsened by the fact that the install seems to have succeeded, the warning message is easily overlooked.

This tries to fix the problem by converting all warnings to errors, by using a temporary R_PROFILE_USER file and setting `options(warn=2)`. Then at least it will be clear something has gone wrong during the installation.

Unfortunately this will only work in R versions > 3.5.1, as versions prior to that unconditionally set `options(warn = 1)` during `R CMD INSTALL`, so there is no way to change the setting.

Fixes #113
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.