From da9bb5fd7292e993dfff036f6e579495085add18 Mon Sep 17 00:00:00 2001
From: James Lamb <jaylamb20@gmail.com>
Date: Wed, 1 May 2024 12:55:33 -0500
Subject: [PATCH] [R-package] always name the shared library 'lightgbm', not
 'lib_lightgbm' (#6432)

---
 CMakeLists.txt               | 13 ++++++++++++-
 R-package/NAMESPACE          |  2 +-
 R-package/R/lightgbm.R       |  2 +-
 R-package/src/install.libs.R |  6 +++---
 build-cran-package.sh        | 20 +-------------------
 build_r.R                    | 36 ------------------------------------
 6 files changed, 18 insertions(+), 61 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 4136575e4..ad7e44067 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -461,11 +461,22 @@ if(BUILD_STATIC_LIB)
 else()
   add_library(_lightgbm SHARED)
 endif()
+
+# R expects libraries of the form <project>.{dll,dylib,so}, not lib_<project>.{dll,dylib,so}
+if(__BUILD_FOR_R)
+  set_target_properties(
+    _lightgbm
+    PROPERTIES
+      PREFIX ""
+      OUTPUT_NAME "lightgbm"
+  )
+endif()
+
 # LightGBM headers include openmp, cuda, R etc. headers,
 # thus PUBLIC is required for building _lightgbm_swig target.
 target_link_libraries(_lightgbm PUBLIC lightgbm_capi_objs lightgbm_objs)
 
-if(MSVC)
+if(MSVC AND NOT __BUILD_FOR_R)
   set_target_properties(_lightgbm PROPERTIES OUTPUT_NAME "lib_lightgbm")
 endif()
 
diff --git a/R-package/NAMESPACE b/R-package/NAMESPACE
index 1f6ed2488..49ef2b5cb 100644
--- a/R-package/NAMESPACE
+++ b/R-package/NAMESPACE
@@ -62,4 +62,4 @@ importFrom(parallel,detectCores)
 importFrom(stats,quantile)
 importFrom(utils,modifyList)
 importFrom(utils,read.delim)
-useDynLib(lib_lightgbm , .registration = TRUE)
+useDynLib(lightgbm , .registration = TRUE)
diff --git a/R-package/R/lightgbm.R b/R-package/R/lightgbm.R
index e5df7a93f..f1a0090f9 100644
--- a/R-package/R/lightgbm.R
+++ b/R-package/R/lightgbm.R
@@ -325,7 +325,7 @@ NULL
 #' @import methods
 #' @importFrom Matrix Matrix
 #' @importFrom R6 R6Class
-#' @useDynLib lib_lightgbm , .registration = TRUE
+#' @useDynLib lightgbm , .registration = TRUE
 NULL
 
 # Suppress false positive warnings from R CMD CHECK about
diff --git a/R-package/src/install.libs.R b/R-package/src/install.libs.R
index cda512e08..f9d423304 100644
--- a/R-package/src/install.libs.R
+++ b/R-package/src/install.libs.R
@@ -227,9 +227,9 @@ if (!makefiles_already_generated) {
 }
 
 # build the library
-message("Building lib_lightgbm")
+message(paste0("Building lightgbm", SHLIB_EXT))
 .run_shell_command(build_cmd, build_args)
-src <- file.path(lib_folder, paste0("lib_lightgbm", SHLIB_EXT), fsep = "/")
+src <- file.path(lib_folder, paste0("lightgbm", SHLIB_EXT), fsep = "/")
 
 # Packages with install.libs.R need to copy some artifacts into the
 # expected places in the package structure.
@@ -247,7 +247,7 @@ if (file.exists(src)) {
   }
 
 } else {
-  stop(paste0("Cannot find lib_lightgbm", SHLIB_EXT))
+  stop(paste0("Cannot find lightgbm", SHLIB_EXT))
 }
 
 # clean up the "build" directory
diff --git a/build-cran-package.sh b/build-cran-package.sh
index 767c128d9..89c660680 100755
--- a/build-cran-package.sh
+++ b/build-cran-package.sh
@@ -165,23 +165,6 @@ cd "${TEMP_R_DIR}"
         -e 's/\.\..*fast_double_parser\.h/LightGBM\/fast_double_parser\.h/' \
         src/include/LightGBM/utils/common.h
 
-    # When building an R package with 'configure', it seems
-    # you're guaranteed to get a shared library called
-    #  <packagename>.so/dll/dylib. The package source code expects
-    # 'lib_lightgbm.so', not 'lightgbm.so', to comply with the way
-    # this project has historically handled installation
-    echo "Changing lib_lightgbm to lightgbm"
-    for file in R/*.R; do
-        sed \
-            -i.bak \
-            -e 's/lib_lightgbm/lightgbm/' \
-            "${file}"
-    done
-    sed \
-        -i.bak \
-        -e 's/lib_lightgbm/lightgbm/' \
-        NAMESPACE
-
     # 'processx' is listed as a 'Suggests' dependency in DESCRIPTION
     # because it is used in install.libs.R, a file that is not
     # included in the CRAN distribution of the package
@@ -191,8 +174,7 @@ cd "${TEMP_R_DIR}"
         DESCRIPTION
 
     echo "Cleaning sed backup files"
-    rm R/*.R.bak
-    rm NAMESPACE.bak
+    rm *.bak
 
 cd "${ORIG_WD}"
 
diff --git a/build_r.R b/build_r.R
index 50d61e550..c2703778a 100644
--- a/build_r.R
+++ b/build_r.R
@@ -398,42 +398,6 @@ description_contents <- gsub(
 )
 writeLines(description_contents, DESCRIPTION_FILE)
 
-# CMake-based builds can't currently use R's builtin routine registration,
-# so have to update NAMESPACE manually, with a statement like this:
-#
-# useDynLib(lib_lightgbm, LGBM_DatasetCreateFromFile_R, ...)
-#
-# See https://cran.r-project.org/doc/manuals/r-release/R-exts.html#useDynLib for
-# documentation of this approach, where the NAMESPACE file uses a statement like
-# useDynLib(foo, myRoutine, myOtherRoutine)
-NAMESPACE_FILE <- file.path(TEMP_R_DIR, "NAMESPACE")
-namespace_contents <- readLines(NAMESPACE_FILE)
-dynlib_line <- grep(
-  pattern = "^useDynLib"
-  , x = namespace_contents
-)
-
-c_api_contents <- readLines(file.path(TEMP_SOURCE_DIR, "src", "lightgbm_R.h"))
-c_api_contents <- c_api_contents[startsWith(c_api_contents, "LIGHTGBM_C_EXPORT")]
-c_api_contents <- gsub(
-  pattern = "LIGHTGBM_C_EXPORT SEXP "
-  , replacement = ""
-  , x = c_api_contents
-  , fixed = TRUE
-)
-c_api_symbols <- gsub(
-  pattern = "\\(.*"
-  , replacement = ""
-  , x = c_api_contents
-)
-dynlib_statement <- paste0(
-  "useDynLib(lib_lightgbm, "
-  , toString(c_api_symbols)
-  , ")"
-)
-namespace_contents[dynlib_line] <- dynlib_statement
-writeLines(namespace_contents, NAMESPACE_FILE)
-
 # NOTE: --keep-empty-dirs is necessary to keep the deep paths expected
 #       by CMake while also meeting the CRAN req to create object files
 #       on demand