[R] Remove extra gcc-specific options from Makevars.win #3627
Conversation
|
@kalibera is the man for all things Windows toolchain for R. I usually just nod politely, try to thank sincerely and adopt. In the past there was exactly and only one build system for R on Windows. gcc. In one version. Possibly for 32 and 64 bit. That changed but it remained one (fixed) gcc. This is now changing as we get to Windows on different cpus too and that (IIRC) opens the door for clang and hence some 'variability' in the toolchain. |
coatless
left a comment
coatless
left a comment
There was a problem hiding this comment.
@rcurtin this should be fine. If not, I'll work on it to bring it back into compliance.
As @eddelbuettel said, there has been quite a few changes to build machines at CRAN since mlpack for R launched back in the day. Plus, if @kalibera is urging or requesting the change, it's likely a good idea to adopt in a proactive manner vs. if a CRAN maintainer says hello 😉
coatless
changed the title
|
Yes, it would be nice if you could release to CRAN asap, so that the number of packages with pending changes doesn't grow too large (there are over 20K of packages, with complicated dependencies, etc). If this reveals problems with memory usage during compilation on Winbuilder, we can move from there (and of course feel free to remind in that case that I asked for this change): either conditionally use these options, or restructure the code, or use pre-compiled headers, etc. |
5 participants
CC: @coatless @eddelbuettel @kalibera
This is an experimental PR to try and remove the GCC-specific options from
Makevars.win. @kalibera pointed out via email that the R package build fails on Windows when using LLVM, because thetrack-macro-expansion=0compiler option isn't available in LLVM. If we wanted, we could make the option conditional, but @kalibera noted that the build was actually faster under gcc without those options. So, let's see what happens here.Those options were originally added to decrease RAM usage so that the CRAN Windows workers would build successfully (if I remember right), but perhaps they aren't needed anymore.
First we can try this PR here, and if this succeeds, I can port these tiny changes to the existing mlpack CRAN package and see if CRAN still builds them successfully.
The usual disclaimer is that I am not an R expert and will defer to others' input and advice here! 😄