search

krlmlr / r-appveyor

Tools for using R with AppVeyor (https://appveyor.com)
132 stars 60 forks source link

Change the gcc version in Rtools 3.3 #42

Closed wush978 closed 8 years ago

wush978 commented 8 years ago

Hi @krlmlr ,

According to the new CHANGES on Rtools, it is possible to use gcc 4.9.3 on windows:

Both the gcc 4.6.3 toolchain and a toolchain based on gcc 4.9.3 and mingw-w64 v3 are now included. For use with the newer toolchain a compiled copy of libicu55 is also included; it will be installed in Rtools/mingw_libs if the newer compilers are selected.

Could you give me any suggestion to switch the version of the compiler on windows? On my machine, modifying the PATH resolve the issue. (c:\Rtools\gcc-4.6.3\bin => c:\Rtools\mingw_64\bin) However, I am wondering if modifying PATH in appveyor.yml is a good idea or not.

krlmlr commented 8 years ago

The image from rtools-portable is used here. The path you mentioned is available in the image, so I guess this would also work in AppVeyor. My only concern is that r-appveyor uses a hard-coded path, and overriding PATH in appveyor.yml probably won't help.

Would you mind submitting a pull request that allows configuring the path to bin/ for both R and RStudio via environment variable, and uses the current setting as default if the environment variable is unset?

wush978 commented 8 years ago

I'll try and send a PR to you if I success.

wush978 commented 8 years ago

Well, 0c37273 is just a try. It uses the environment variable GCC_PATH to control the gcc version. I don't like it.

I think the correct way is to check whether the field SystemRequirements:contains C++11 in the DESCRIPTION file. But I do not know the powershell well, so could you give me some example of checking the content of the DESCRIPTION file?

krlmlr commented 8 years ago

I wouldn't add too much magic here. It's only one setting in appveyor.yml, after all. And why can't I use the new toolchain without requiring C++11?

For now, the R path is hard-coded, too: https://github.com/krlmlr/r-appveyor/commit/0c3727361766d2de0541bd9cc3f1374c5d58317e#diff-c10313419fcc074711765ae1ac77ae4cR84. It would be great if this also was configurable.

wush978 commented 8 years ago

For gcc version, now it works!

The https://github.com/krlmlr/r-appveyor/compare/master...wush978:master uses the following way to decide which gcc version will be used:

  1. Does the user provide a environment vairalbe GCC_PATH? Currently, the GCC_PATH should be one of gcc-4.6.3, mingw_32 and mingw_64 which represents the name of the folder in Rtools. (I guess mingw_64 requires a 64-bit R and we are using 32-bit R now)
  2. Does the DESCRIPTION file contains the c++11 in the field SystemRequirements? If yes, the mingw_32 will be used. According to https://cran.r-project.org/doc/manuals/r-release/R-exts.html#Using-C_002b_002b11-code, CRAN will use the compiler for c++11 when the package specifices SystemRequirements: C++11 in the DESCRIPTION. https://github.com/wush978/r-appveyor/blob/master/scripts/appveyor-tool.ps1#L98 uses R to check this field.
  3. Then, gcc-4.6.3 is used for compatibility.

With the modified script, my test is passed: https://ci.appveyor.com/project/wush978/rcereal/build/1.0.46

If this is OK, I'll send a PR.

For the PATH of R, I suggest to open another issue for it.

krlmlr commented 8 years ago

This looks good so far, I may comment on the code if you submit a PR, which you then could update by adding commits to that branch.