-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[R-package] Fallback to MinGW when VS build fails #664
Changes from 13 commits
70b1bc6
7570715
e2aead2
2c5ea8e
4098fc1
48f8f44
ed00f7b
2914c1f
52276e6
9557b61
6a4c23c
31bbe84
6bcc4b0
91bcdb3
9ccac03
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -42,19 +42,28 @@ if (!use_precompile) { | |
setwd(build_dir) | ||
|
||
# Prepare installation steps | ||
cmake_cmd <- "cmake " | ||
build_cmd <- "make _lightgbm -j4" | ||
cmake_base <- "cmake " | ||
build_cmd <- "make _lightgbm -j" | ||
lib_folder <- file.path(R_PACKAGE_SOURCE, "src", fsep = "/") | ||
|
||
# Check if Windows installation (for gcc vs Visual Studio) | ||
if (WINDOWS) { | ||
if (use_mingw) { | ||
cmake_cmd <- paste0(cmake_cmd, " -G \"MinGW Makefiles\" ") | ||
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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It that okay for double cmake for vs ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Double cmake for VS does not cause issues. This is the message printed when using twice cmake with VS: |
||
} else { | ||
cmake_cmd <- paste0(cmake_cmd, " -DCMAKE_GENERATOR_PLATFORM=x64 ") | ||
build_cmd <- "cmake --build . --target _lightgbm --config Release" | ||
lib_folder <- file.path(R_PACKAGE_SOURCE, "src/Release", fsep = "/") | ||
cmake_cmd <- paste0(cmake_base, " -DCMAKE_GENERATOR_PLATFORM=x64 ") | ||
tryVS <- system(paste0(cmake_cmd, " ..")) | ||
if (tryVS == 1) { | ||
unlink("./*", recursive = TRUE) # Clean up build directory | ||
cmake_cmd <- paste0(cmake_base, " -G \"MinGW Makefiles\" ") # Switch to MinGW on failure, try build once | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is better to remove all cmake caches if last camke is fail. you can delete all contents in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you know where is the build folder in all the folders? I can't seem to get rid of it. Objective would be:
|
||
system(paste0(cmake_cmd, " ..")) # Must build twice for Windows due sh.exe in Rtools | ||
build_cmd <- "mingw32-make.exe _lightgbm -j" | ||
} else { | ||
build_cmd <- "cmake --build . --target _lightgbm --config Release" | ||
lib_folder <- file.path(R_PACKAGE_SOURCE, "src/Release", fsep = "/") | ||
} | ||
} | ||
} | ||
|
||
|
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 think the description in line 19 and 21 should be updated. What do you think ?