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

[docs] fix R documentation builds (fixes #3655) #3656

Merged
merged 2 commits into from
Dec 18, 2020
Merged

Conversation

jameslamb
Copy link
Collaborator

This PR attempts to fix the broken R documentation (see #3655). Building the docs requires installing the R package. Now that #3405 has been merged, LightGBM's submodules have to be initialized for the package to be built successfully, and I think they were not on readthedocs builds.

latest build: https://readthedocs.org/projects/lightgbm/builds/12567523/

* checking for LF line-endings in source and make files and shell scripts
* checking for empty or unneeded directories
WARNING: directory ‘lightgbm/src/external_libs/fast_double_parser’ is empty
WARNING: directory ‘lightgbm/src/external_libs/fmt’ is empty

and then later

* installing to library ‘/home/docs/.conda/envs/r_env/lib/R/library’
* installing *source* package ‘lightgbm’ ...
** using staged installation
** libs
installing via 'install.libs.R' to /home/docs/.conda/envs/r_env/lib/R/library/00LOCK-lightgbm/00new/lightgbm
Building lib_lightgbm
In file included from /tmp/Rtmp4mfEea/R.INSTALL30a33e035de/lightgbm/src/include/LightGBM/config.h:16,
                 from /tmp/Rtmp4mfEea/R.INSTALL30a33e035de/lightgbm/src/include/LightGBM/boosting.h:8,
                 from /tmp/Rtmp4mfEea/R.INSTALL30a33e035de/lightgbm/src/src/boosting/boosting.cpp:5:
/tmp/Rtmp4mfEea/R.INSTALL30a33e035de/lightgbm/src/include/LightGBM/utils/common.h:35:10: fatal error: ../../../external_libs/fmt/include/fmt/format.h: No such file or directory
   35 | #include "../../../external_libs/fmt/include/fmt/format.h"
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Notes for Reviewers

@StrikerRUS could you please enable readthedocs builds for this branch?

docs/conf.py Outdated
@@ -248,8 +248,9 @@ def generate_r_docs(app):
source /home/docs/.conda/bin/activate r_env
export TAR=/bin/tar
cd {0}
git submodule update --init --recursive
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix! I think it will be better to use config file for such things.
https://docs.readthedocs.io/en/stable/config-file/v2.html#submodules

Could you please try include the following lines

submodules:
  include: all
  recursive: true

into
https://github.com/microsoft/LightGBM/blob/master/.readthedocs.yml?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P.S. I've enabled RTD builds for the docs/jlamb branch and will keep it turned on as you'd asked.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, I can do that. And thanks for keeping the branch up!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And thanks for keeping the branch up!

image

This will auto-disable RTD builds! 🤣

docs/conf.py Outdated
export R_LIBS="$CONDA_PREFIX/lib/R/library"
Rscript build_r.R
Rscript build_r.R || exit 1
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use -1 for the consistency. It may help to simply a search query in the repo if we decide to change something in the future.

Shouldn't Rscript -e "pkgdown::build_site() cause builds to fail as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah you're right

@StrikerRUS
Copy link
Collaborator

You are right in the root cause of the failures! Builds are now passing:

image

Copy link
Collaborator

@StrikerRUS StrikerRUS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updating the config and forcing build to fail!

@jameslamb jameslamb merged commit 336bd6b into master Dec 18, 2020
@jameslamb jameslamb deleted the docs/jlamb branch December 18, 2020 19:38
@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants