[R-package] Fallback to MinGW when VS build fails #664

Laurae2 · 2017-06-30T06:48:02Z

This PR is to help the release of CRAN package of LightGBM.

It removes Visual Studio requirement on Windows. User may still install LightGBM with Visual Studio from GitHub using Laurae2/lgbdl.

MinGW should be the default for the following reasons in Windows:

R expects MinGW compilation
Rtools, mandatory to start compiling packages, uses MinGW / gcc
Rtools is most likely installed than Visual Studio
CRAN does not use Visual Studio
Precompiled DLLs for CRAN are last resort only

Updates:

MinGW instead of Visual Studio by default
R README.md documentation to reflect changes

See issue #629.

guolinke · 2017-07-06T05:14:59Z

@Laurae2 Did you test the build with MinGW ?
Does it use the mingw in Rtools, or need to install a standalone MinGW ?

Laurae2 · 2017-07-06T07:03:42Z

@guolinke It seems to work now.

It does not need a separate MinGW, by default it should use Rtools.

Laurae2 · 2017-07-06T17:28:33Z

@guolinke In Windows, cmake with Rtools requires to run twice because Rtools has sh.exe which cmake does not like (must be run twice to bypass the error). If running twice and using another MinGW without Rtools, it won't cause an error because cmake will tell a warning that it has already configured LightGBM for MinGW.

guolinke · 2017-07-09T04:04:56Z

@Laurae2
I am thinking can we add a parameter named build_cran to https://github.com/Microsoft/LightGBM/blob/master/R-package/build_package.R ?
if build_cran is true, we change the default compiler to mingw in windows.

Laurae2 · 2017-07-13T06:53:00Z

@guolinke We already have use_mingw in install.libs.R, we may rename it to build_cran to specify building with minimum requirements (cmake / Rtools).

But in any case, Rtools must be the default choice and VS an optional choice (not the other way around, it makes it unpractical especially for automated installations and future CRAN release).

guolinke · 2017-07-13T07:29:44Z

@Laurae2 Thanks. I think we can list the installed compilers by using cmake --help. And using visual studio if it is installed, otherwise using the rtools.

BTW, does these cran test machine have the cmake ?

guolinke · 2017-07-13T15:39:59Z

R-package/src/install.libs.R

+      cmake_cmd <- paste0(cmake_base, " -DCMAKE_GENERATOR_PLATFORM=x64 ")
+      tryVS <- system(paste0(cmake_cmd, " .."))
+      if (tryVS == 1) {
+        cmake_cmd <- paste0(cmake_base, " -G \"MinGW Makefiles\" ") # Switch to MinGW on failure, try build once


It is better to remove all cmake caches if last camke is fail. you can delete all contents in build folder.

Do you know where is the build folder in all the folders? I can't seem to get rid of it.

Objective would be:

try VS build

if VS build fails, try MinGW build but delete build folder first (I don't know where is the build folder in all the folders)

guolinke · 2017-07-13T15:40:24Z

R-package/src/install.libs.R

+      if (tryVS == 1) {
+        cmake_cmd <- paste0(cmake_base, " -G \"MinGW Makefiles\" ") # Switch to MinGW on failure, try build once
+        system(paste0(cmake_cmd, " ..")) # Must build twice for Windows due sh.exe in Rtools
+      }
      build_cmd <- "cmake --build . --target _lightgbm  --config Release"
      lib_folder <- file.path(R_PACKAGE_SOURCE, "src/Release", fsep = "/")


lib_path should change if vs build failed

guolinke · 2017-07-13T15:41:11Z

R-package/src/install.libs.R

-      build_cmd <- "mingw32-make.exe _lightgbm -j4"
+      cmake_cmd <- paste0(cmake_base, " -G \"MinGW Makefiles\" ")
+      build_cmd <- "mingw32-make.exe _lightgbm -j"
+      system(paste0(cmake_cmd, " ..")) # Must build twice for Windows due sh.exe in Rtools


It that okay for double cmake for vs ?

Double cmake for VS does not cause issues.

This is the message printed when using twice cmake with VS: CMake does not need to re-run because C:/tmp/RtmpygCFH6/devtools19ad439764e21/Laurae2-LightGBM-ed00f7b/R-package/src/build/CMakeFiles/generate.stamp is up-to-date.

guolinke · 2017-07-13T15:43:53Z

R-package/src/install.libs.R

@@ -56,6 +56,7 @@ if (!use_precompile) {
      cmake_cmd <- paste0(cmake_base, " -DCMAKE_GENERATOR_PLATFORM=x64 ")
      tryVS <- system(paste0(cmake_cmd, " .."))
      if (tryVS == 1) {
+        system("rm -rf .")


not sure the rm -rf can work in windows or not. you can use unlink("./*", recursive = TRUE)

It works with your method.

rm rf works, but I was denied access to remove . or .. apparently.

Laurae2 · 2017-07-13T15:44:38Z

@guolinke CRAN test machines should have cmake because there are packages requiring cmake.

For instance, this CRAN package requires cmake: https://cran.r-project.org/web/packages/zstdr/index.html

Laurae2 · 2017-07-13T15:46:44Z

@guolinke Build passing on my VS+MinGW Windows machine (VS selection) and MinGW Windows machine (VS failed, auto fallback to MinGW) now.

Laurae2 · 2017-07-13T15:48:15Z

but getting this error now on my MinGW-only machine:

Error in eval(ei, envir) : Cannot find lib_lightgbm.dll

guolinke · 2017-07-13T15:49:15Z

lib_path should change if vs build failed

guolinke · 2017-07-13T15:53:22Z

@Laurae2 Thanks, can you also update the documents ?

Laurae2 · 2017-07-13T15:56:54Z

Success on both machines, using R 3.4.0:

VS + MinGW machine:

> devtools::install_github("Laurae2/LightGBM@patch-2", subdir = "R-package")
(...)
** building package indices
** testing if installed package can be loaded
Warning: package 'R6' was built under R version 3.4.1
* DONE (lightgbm)
Warning message:
GitHub repo contains submodules, may not function as expected!

MinGW-only machine:

> devtools::install_github("Laurae2/LightGBM@patch-2", subdir = "R-package")
(...)
** building package indices
** testing if installed package can be loaded
Warning: package 'R6' was built under R version 3.4.1
* DONE (lightgbm)
Warning message:
GitHub repo contains submodules, may not function as expected!

Laurae2 · 2017-07-13T15:59:52Z

@guolinke I'll update docs now to change the default to VS and auto fallback to MinGW for Windows.

guolinke · 2017-07-13T16:01:11Z

R-package/README.md

-The default compiler is Visual Studio (or [MS Build](https://www.visualstudio.com/downloads/#build-tools-for-visual-studio-2017)) in Windows. You also can use [MinGW64](https://sourceforge.net/projects/mingw-w64/files/Toolchains%20targetting%20Win64/Personal%20Builds/mingw-builds/) (x86_64-posix-seh) to compile by setting `use_mingw` to `TRUE` in `R-package/src/install.libs.R`. For MinGW users who wants to install online, please check the end of this document for installation using a helper package ([Laurae2/lgbdl](https://github.com/Laurae2/lgbdl/)).
+The default compiler is Rtools or any [MinGW64](https://sourceforge.net/projects/mingw-w64/files/Toolchains%20targetting%20Win64/Personal%20Builds/mingw-builds/) (x86_64-posix-seh) to compile by setting `use_mingw` to `TRUE` in `R-package/src/install.libs.R`. You also can use Visual Studio (or [MS Build](https://www.visualstudio.com/downloads/#build-tools-for-visual-studio-2017)) in Windows.
+
+For Visual Studio users who wants to install online, please check the end of this document for installation using a helper package ([Laurae2/lgbdl](https://github.com/Laurae2/lgbdl/)).


I think the description in line 19 and 21 should be updated. What do you think ?

guolinke · 2017-07-13T16:07:10Z

Thanks very much. I think now I can merge it.

* Change VS -> MinGW for CRAN * Documentation to switch to MinGW by default * Force cmake to run twice * Try again dual build for Rtools * Switch to cmake for building twice * Try with Visual Studio as default, MinGW as fallback * Try to remove VS appropriately when failing * Attempt to get rid of build folder first * Switch to unlink from rm rf * Change lib_folder correctly when VS fails * Add README updates from 1c8355c * Update default compiler doc and add GPU online install doc * Better GPU doc

Laurae2 added 2 commits June 30, 2017 08:36

Change VS -> MinGW for CRAN

70b1bc6

Documentation to switch to MinGW by default

7570715

msftclas added the cla-already-signed label Jun 30, 2017

Laurae2 added 4 commits July 6, 2017 08:51

Merge branch 'master' into patch-2

e2aead2

Force cmake to run twice

2c5ea8e

Try again dual build for Rtools

4098fc1

Switch to cmake for building twice

48f8f44

Laurae2 added 3 commits July 13, 2017 17:27

Try with Visual Studio as default, MinGW as fallback

ed00f7b

Try to remove VS appropriately when failing

2914c1f

Attempt to get rid of build folder first

52276e6

guolinke reviewed Jul 13, 2017

View reviewed changes

Switch to unlink from rm rf

9557b61

Change lib_folder correctly when VS fails

6a4c23c

Add README updates from 1c8355c

31bbe84

Merge branch 'master' into patch-2

6bcc4b0

guolinke reviewed Jul 13, 2017

View reviewed changes

Update default compiler doc and add GPU online install doc

91bcdb3

guolinke changed the title ~~Change VS to MinGW for CRAN~~ [R] fallback to MinGW if VS build fails Jul 13, 2017

Better GPU doc

9ccac03

Laurae2 changed the title ~~[R] fallback to MinGW if VS build fails~~ [R-package] Fallback to MinGW when VS build fails Jul 13, 2017

guolinke merged commit 3421bc6 into microsoft:master Jul 13, 2017

Laurae2 deleted the patch-2 branch July 14, 2017 08:41

lock bot locked as resolved and limited conversation to collaborators Mar 12, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[R-package] Fallback to MinGW when VS build fails #664

[R-package] Fallback to MinGW when VS build fails #664

Laurae2 commented Jun 30, 2017 •

edited

Loading

guolinke commented Jul 6, 2017

Laurae2 commented Jul 6, 2017

Laurae2 commented Jul 6, 2017

guolinke commented Jul 9, 2017

Laurae2 commented Jul 13, 2017

guolinke commented Jul 13, 2017 •

edited

Loading

guolinke Jul 13, 2017

Laurae2 Jul 13, 2017

guolinke Jul 13, 2017

guolinke Jul 13, 2017

Laurae2 Jul 13, 2017

guolinke Jul 13, 2017

Laurae2 Jul 13, 2017

Laurae2 commented Jul 13, 2017

Laurae2 commented Jul 13, 2017

Laurae2 commented Jul 13, 2017

guolinke commented Jul 13, 2017

guolinke commented Jul 13, 2017

Laurae2 commented Jul 13, 2017

Laurae2 commented Jul 13, 2017

guolinke Jul 13, 2017

guolinke commented Jul 13, 2017

[R-package] Fallback to MinGW when VS build fails #664

[R-package] Fallback to MinGW when VS build fails #664

Conversation

Laurae2 commented Jun 30, 2017 • edited Loading

guolinke commented Jul 6, 2017

Laurae2 commented Jul 6, 2017

Laurae2 commented Jul 6, 2017

guolinke commented Jul 9, 2017

Laurae2 commented Jul 13, 2017

guolinke commented Jul 13, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Laurae2 commented Jul 13, 2017

Laurae2 commented Jul 13, 2017

Laurae2 commented Jul 13, 2017

guolinke commented Jul 13, 2017

guolinke commented Jul 13, 2017

Laurae2 commented Jul 13, 2017

Laurae2 commented Jul 13, 2017

Choose a reason for hiding this comment

guolinke commented Jul 13, 2017

Laurae2 commented Jun 30, 2017 •

edited

Loading

guolinke commented Jul 13, 2017 •

edited

Loading