[R-package] Remove CLI-only objects

[jameslamb]

I realized, looking at logs, that the R package includes some objects that are only necessary for the LightGBM CLI. This PR proposes removing them.

Benefits of this change

• faster installation of the R package (CRAN and CMake)
• smaller installed size of the package
• less code that could possibly cause issues with CRAN checks

Notes for Reviewers

I think that the Python package would benefit from a change like this too, but I didn't want this PR to get too large.

[StrikerRUS]

Is it not enough to build only _lightgbm target for R?

LightGBM/R-package/src/install.libs.R

Line 135 in 0b5e60d

build_args <- "_lightgbm"

Do we build executable file for R somewhere? Could you please clarify what is a benefit from adding those ifs in CMakeLists.txt? Sorry, but I'm a bit confused by these changes for CMake. I thought we already excluded excess files from R builds by building only _lightgbm target.

[jameslamb]

Is it not enough to build only _lightgbm target for R?

Not as we currently have it set up. If you look in the logs for any of the R CI jobs, you'll see building the R package is still compiling application/application.o, which is specific to the CLI

Scanning dependencies of target _lightgbm
[  2%] Building CXX object CMakeFiles/_lightgbm.dir/src/application/application.cpp.o

Maybe I could just move application.cpp down into this?

LightGBM/CMakeLists.txt

Line 333 in 0b5e60d

add_executable(lightgbm src/main.cpp ${SOURCES})

All the if checks added here are because I was trying to do this in a way that only changes the behavior for the R package, since I'm unsure about other components.

[StrikerRUS]

@jameslamb

you'll see building the R package is still compiling application/application.o, which is specific to the CLI

OK, I see. I guess it comes from here. And excluding this line for _lightgbm target will be enough.

LightGBM/CMakeLists.txt

Lines 320 to 321 in 0b5e60d

	file(GLOB SOURCES
	src/application/*.cpp

LightGBM/CMakeLists.txt

Line 345 in 0b5e60d

add_library(_lightgbm SHARED ${SOURCES})

[jameslamb]

@jameslamb

you'll see building the R package is still compiling application/application.o, which is specific to the CLI

OK, I see. I guess it comes from here. And excluding this line for _lightgbm target will be enough.

LightGBM/CMakeLists.txt

Lines 320 to 321 in 0b5e60d

	file(GLOB SOURCES
	src/application/*.cpp

LightGBM/CMakeLists.txt

Line 345 in 0b5e60d

add_library(_lightgbm SHARED ${SOURCES})

I think so, yeah. That is what I meant in #3566 (comment). I can remove application.cpp from the line you linked, and then the new definition for the lightgbm target will be

 add_executable(lightgbm src/main.cpp src/application.cpp ${SOURCES}) 

Maybe I was being overly-cautious trying to only make changes that impact the R package. I'll update this and see how it goes.

[jameslamb]

After that change, I got this error

-- Configuring done
CMake Error at CMakeLists.txt:332 (add_executable):
  Cannot find source file:

    src/main.cpp

  Tried extensions .c .C .c++ .cc .cpp .cxx .cu .m .M .mm .h .hh .h++ .hm
  .hpp .hxx .in .txx


CMake Error at CMakeLists.txt:332 (add_executable):
  No SOURCES given to target: lightgbm

I don't think I can just add an if(NOT __BUILD_FOR_R) around

LightGBM/CMakeLists.txt

Line 333 in 0b5e60d

add_executable(lightgbm src/main.cpp ${SOURCES})

, because other places will fail (taking us back down the path of all the gross if statements I originally added).

So to solve this and avoid making too many invasive changes in CMakeLists.txt, I decided to revert the changes in build_r.R. I think it's ok to just leave application.cpp and main.cpp in the package created with CMake, as long as they aren't actually compiled. I'd prefer that to adding a lot of if statements in CMakeLists.txt.

[jameslamb]

moving this define from application.cpp to this file (the first source file that needs global_timer) to try to fix these errors:

https://dev.azure.com/lightgbm-ci/lightgbm-ci/_build/results?buildId=8009&view=logs&j=5ea309bb-dea2-5bcb-538b-5c76ecf159eb&t=aa1d7e0f-3509-5697-4ef8-f247c54ed420

[StrikerRUS]

LGTM for CMake part!

ping @guolinke for global_timer.

[guolinke]

LGTM

[github-actions]

This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this.