use system2 instead of system
#12
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
if someone wants to see the compiler call, they can do that by specifying verbose = TRUE
The errfile/msg contains compiler warnings, which will often be many, long, and non-useful if the code uses Boost, Eigen, etc. But R truncates the message at 1000 characters by default starting from the begining, which shows the first few compiler warnings and not the actual error message at the end. So, it is better to show the last few lines than the first few lines.
since errmsg is a character vector of multiple lines
because there is already a on.exit(setwd(wd))
facilitate showing what make would do via --dry-run do not setwd() because it is already in an on.exit() account for the limits on printing the error message, which may be thousands of lines long due to compiler warnings from Eigen / Boost
eddelbuettel
requested changes
Sep 3, 2020
DESCRIPTION
Outdated
| Version: 0.3.15 | ||
| Date: 2018-05-18 | ||
| Version: 0.3.16 | ||
| Date: 2020-07-11 |
There was a problem hiding this comment.
Please revert that to (at the most) 0.3.15.1 and current date.
Setting releases is what Maintainer: does.
There was a problem hiding this comment.
Also rebasing/squashing may be a good idea though I can do that too when I merge.
This reverts commit 889c965.
|
Thanks, squashed. Having |
|
Thanks again -- now on CRAN. |
|
Great!
…On Sun, Sep 6, 2020 at 11:52 AM Dirk Eddelbuettel ***@***.***> wrote:
Thanks again -- now on CRAN.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#12 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAZ2XKVBHSQSIMASECSFCODSEOV4NANCNFSM4QU3IHVQ>
.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
084dbdd is the the commit that actually changes to using the more portable
system2rather than the oldersystem, which fixes #11. The rest of the commits were just me flailing around trying to get the error / verbosity reporting to be reasonable in more situations. You might have a different definition of reasonable, but I think the main two things were:makewould do by callingR CMD SHLIBwith--dry-runifverbose = TRUEor if there was an erroroption()for this, but by default it is 10000 lines I think. Unfortunately, it is pretty common to have more than 10000 lines of unhelpful compiler warnings if using headers from BH or especially RcppEigen. So, if you feed the whole error file intostop, it would only show the first 10000 compiler warnings rather than the text of the actual compiler error. Now, it is feedingtail(errmsg)tostopwhen it exceeds the line limit, so you at least see the compiler error rather than the compiler warnings.I ran
revdepcheck::revdep_checkunder r-devel on Debian. mkin, RxODE, and themetagenomics have errors with inline on CRAN that are still errors with this PR but nothing got worse. I also ran it on Windows and didn't encounter any additional problems other than a couple packages were too complicated to install from source. Someone else ran it on Mac and said there were no additional problems. Other people have been installing my branch from GitHub and using it with RStan, so I think it is solid.The
system2function has anenvargument that you might want to use to set the environmental variables rather than setting them incxxfunction, but I didn't touch that.