Skip to content

unload namespace before reinstalling, #4403#4404

Merged
mattdowle merged 4 commits intomasterfrom
upg-dev-pkg
Jun 2, 2020
Merged

unload namespace before reinstalling, #4403#4404
mattdowle merged 4 commits intomasterfrom
upg-dev-pkg

Conversation

@jangorecki
Copy link
Copy Markdown
Member

closes #4403

#R version 4.0.0 (2020-04-24) -- "Arbor Day"
#Copyright (C) 2020 The R Foundation for Statistical Computing
#Platform: x86_64-w64-mingw32/x64 (64-bit)
#...
data.table::update.dev.pkg()
#trying URL 'https://Rdatatable.gitlab.io/data.table/bin/windows/contrib/4.0/data.table_1.12.9.zip'
#Content type 'application/zip' length 2675878 bytes (2.6 MB)
#downloaded 2.6 MB
#
#package 'data.table' successfully unpacked and MD5 sums checked
#
#The downloaded binary packages are in
#        C:\Users\jan\AppData\Local\Temp\Rtmps5UOqq\downloaded_packages
#R data.table package has been updated to dd7609e83132e19c9be80d71c73e8a8f95e19b27 (1.12.9)

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 27, 2020

Codecov Report

Merging #4404 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4404   +/-   ##
=======================================
  Coverage   99.60%   99.60%           
=======================================
  Files          72       72           
  Lines       13970    13970           
=======================================
  Hits        13915    13915           
  Misses         55       55           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8a57236...8346e79. Read the comment docs.

@jangorecki
Copy link
Copy Markdown
Member Author

codecov fail is false positive

@jangorecki jangorecki added this to the 1.12.9 milestone Apr 27, 2020
@jangorecki
Copy link
Copy Markdown
Member Author

jangorecki commented Apr 27, 2020

We might not even need on.exit() but just unloadNamespace alone (see code chunk below), but better to leave it.
If PR would not always fix the issue, then we could add Sys.sleep(2) after unloadNamespace and before install.packages. More testing on windows R 4.0.0 would be appreciated.

R -q
unload = function() {unloadNamespace("pkg"); !"pkg"%in%loadedNamespaces()}
utils::package.skeleton("pkg", list="unload")
q("no")
rm pkg/man/unload.Rd
R CMD build pkg
R CMD INSTALL pkg_1.0.tar.gz 
R -q
library(pkg)
unload()
#[1] TRUE

Comment thread R/devel.R Outdated
Comment thread R/devel.R
on.exit({
if (upg) {
unloadNamespace(pkg)
utils::install.packages(pkg, repos=repo, type=type, lib=lib, ...)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe wrap this with a tryCatch()? Just so that a more helpful error message can be produced instead; e.g. "Please close all R sessions to release the dll lock on Windows, and try again. If the dll lock is still open, try a reboot which should release the lock on Windows. Also, if you have a .Rpofile which loads data.table on startup, turn that off temporarily in order to upgrade the dll on Windows."

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.Rprofile should not be an issue, because DT is already loaded at this point.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would lean towards keeping on.exit chunk as simple as possible

@jangorecki jangorecki modified the milestones: 1.12.11, 1.12.9 May 26, 2020
@mattdowle mattdowle added this to the 1.12.9 milestone Jun 2, 2020
@mattdowle mattdowle merged commit 09fb6c3 into master Jun 2, 2020
@mattdowle mattdowle deleted the upg-dev-pkg branch June 2, 2020 01:47
mattdowle added a commit that referenced this pull request Jun 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

data.table::update.dev.pkg() warning or error

2 participants