Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork218
Readonly file io error#1346
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
eddelbuettel left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Two quick comments, one small request for change -- looks good generally.
Uh oh!
There was an error while loading.Please reload this page.
| // copy the source file to the build dir | ||
| Rcpp::Function filecopy =Rcpp::Environment::base_env()["file.copy"]; | ||
| filecopy(cppSourcePath_,generatedCppSourcePath(),true); | ||
| filecopy(cppSourcePath_,generatedCppSourcePath(),true, Rcpp::_["copy.mode"] =false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Nice. I'll look some more at that tomorrow over morning coffee -- this ensures we get 0644 irrespective of original permissionss?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Have by now caught up ontohelp(file.copy) -- this is perfect. Had worried that this code, because it dates back to before we had filesystem semantics from C++17 and all that, would have to do dance to do this portably. But farming out to a call to then underlying R instance we always have is golden, and that function has that option. Very nice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I generally have a mild dislike ofRcpp::_[...] over the preferredRcpp::Named("...") but they are equivalent, and the former is used in the file already. So may change that another time....
Bug in Rcpp blocked using readonly CPP source from Nix store,fixed in this PR, awaiting merge.RcppCore/Rcpp#1346
eddelbuettel left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Looks good, one micro nag notwithstanding, and addresses the issue.
Thanks for putting it up.
fdd7205 intoRcppCore:masterUh oh!
There was an error while loading.Please reload this page.
Enables running a defined version of KGD from the Nix store, which requireda recent version of Rcpp with this bugfix:RcppCore/Rcpp#1346For example usage see gbs_prism eri-dev branch.
Fixes#1345
Checklist
R CMD checkstill passes all tests