Skip to content
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] move R source files into R-package, reduce duplication in build_r.R #3087

Merged
merged 3 commits into from
May 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .ci/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ if [[ $TRAVIS == "true" ]] && [[ $TASK == "lint" ]]; then
echo "Linting R code"
Rscript ${BUILD_DIRECTORY}/.ci/lint_r_code.R ${BUILD_DIRECTORY} || exit -1
echo "Linting C++ code"
cpplint --filter=-build/c++11,-build/include_subdir,-build/header_guard,-whitespace/line_length --recursive ./src ./include || exit -1
cpplint --filter=-build/c++11,-build/include_subdir,-build/header_guard,-whitespace/line_length --recursive ./src ./include ./R-package || exit -1
exit 0
fi

Expand Down
3 changes: 0 additions & 3 deletions .github/CODEOWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@ src/ @guolinke @chivee @btrotta
CMakeLists.txt @guolinke @chivee @Laurae2 @jameslamb @wxchan @henry0312 @StrikerRUS @huanzhang12 @btrotta

# R code
include/LightGBM/lightgbm_R.h @Laurae2 @jameslamb
include/LightGBM/R_object_helper.h @Laurae2 @jameslamb
src/lightgbm_R.cpp @Laurae2 @jameslamb
R-package/ @Laurae2 @jameslamb
*.R @Laurae2 @jameslamb

Expand Down
2 changes: 1 addition & 1 deletion R-package/.Rbuildignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
^build_package.R$
^build_r.R$
\.gitkeep$
^docs$
^pkgdown$
Expand Down
File renamed without changes.
3 changes: 2 additions & 1 deletion src/lightgbm_R.cpp → R-package/src/lightgbm_R.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
* Copyright (c) 2017 Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See LICENSE file in the project root for license information.
*/
#include <LightGBM/lightgbm_R.h>

#include "lightgbm_R.h"

#include <LightGBM/utils/common.h>
#include <LightGBM/utils/log.h>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
#define LIGHTGBM_R_H_

#include <LightGBM/c_api.h>
#include <LightGBM/R_object_helper.h>
#include "R_object_helper.h"

/*!
* \brief get string message of the last error
Expand Down
34 changes: 25 additions & 9 deletions build_r.R
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

args <- commandArgs(trailingOnly = TRUE)
INSTALL_AFTER_BUILD <- !("--skip-install" %in% args)
TEMP_R_DIR <- file.path(getwd(), "lightgbm_r")
TEMP_SOURCE_DIR <- file.path(TEMP_R_DIR, "src")

# R returns FALSE (not a non-zero exit code) if a file copy operation
# breaks. Let's fix that
Expand Down Expand Up @@ -56,61 +58,75 @@ INSTALL_AFTER_BUILD <- !("--skip-install" %in% args)
}

# Make a new temporary folder to work in
unlink(x = "lightgbm_r", recursive = TRUE)
dir.create("lightgbm_r")
unlink(x = TEMP_R_DIR, recursive = TRUE)
dir.create(TEMP_R_DIR)

# copy in the relevant files
result <- file.copy(
from = "R-package/./"
, to = "lightgbm_r/"
, to = sprintf("%s/", TEMP_R_DIR)
, recursive = TRUE
, overwrite = TRUE
)
.handle_result(result)

result <- file.copy(
from = "include/"
, to = file.path("lightgbm_r", "src/")
, to = sprintf("%s/", TEMP_SOURCE_DIR)
, recursive = TRUE
, overwrite = TRUE
)
.handle_result(result)

result <- file.copy(
from = "src/"
, to = file.path("lightgbm_r", "src/")
, to = sprintf("%s/", TEMP_SOURCE_DIR)
, recursive = TRUE
, overwrite = TRUE
)
.handle_result(result)

result <- file.copy(
from = "compute/"
, to = file.path("lightgbm_r", "src/")
, to = sprintf("%s/", TEMP_SOURCE_DIR)
, recursive = TRUE
, overwrite = TRUE
)
.handle_result(result)

result <- file.copy(
from = "CMakeLists.txt"
, to = file.path("lightgbm_r", "inst", "bin/")
, to = file.path(TEMP_R_DIR, "inst", "bin/")
, overwrite = TRUE
)
.handle_result(result)

# copy files into the place CMake expects
for (src_file in c("lightgbm_R.cpp", "lightgbm_R.h", "R_object_helper.h")) {
result <- file.copy(
from = file.path(TEMP_SOURCE_DIR, src_file)
, to = file.path(TEMP_SOURCE_DIR, "src", src_file)
, overwrite = TRUE
)
.handle_result(result)
result <- file.remove(
file.path(TEMP_SOURCE_DIR, src_file)
)
.handle_result(result)
}

# 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
.run_shell_command("R", c("CMD", "build", "lightgbm_r", "--keep-empty-dirs"))
.run_shell_command("R", c("CMD", "build", TEMP_R_DIR, "--keep-empty-dirs"))

# Install the package
version <- gsub(
"Version: ",
"",
grep(
"Version: "
, readLines(con = file.path("lightgbm_r", "DESCRIPTION"))
, readLines(con = file.path(TEMP_R_DIR, "DESCRIPTION"))
, value = TRUE
)
)
Expand Down