[R-package] [docs] remove devtools dependency for docs builds (fixes #3036)

[jameslamb]

I'd like to propose removing the dependency on devtools in our readthedocs builds. We are only using it for devtools::document(), which is just a thin wrapper around roxygen2::roxygenize().

devtools is a very expensive dependency (it depends recursively on successfully building 101 other R packages) compared to roxygen2 (41 recursive dependencies), and we're already installing roxygen2 anyway.

I'm also proposing bumping up our version of roxygen2 to 7.1.0, the most recent version used in DESCRIPTION

LightGBM/R-package/DESCRIPTION

Line 44 in 05d89a1

RoxygenNote: 7.1.0

How I got dependency counts

I figured out those recursive dependency numbers with the pkgnet package, a project I work on with some friends from a previous job. In an R environment, you can run pkgnet::CreatePackageReport('devtools') (for example), to see a summary of the dependencies.

[jameslamb]

@StrikerRUS I created this as a LightGBM branch, not on my fork. Could you enable it on readthedocs so we can test?

[StrikerRUS]

@jameslamb

Could you enable it on readthedocs so we can test?

Sure, done!

[jameslamb]

@StrikerRUS hey it looks like this is working!

• build: https://readthedocs.org/projects/lightgbm/builds/11019657/
• docs: https://lightgbm.readthedocs.io/en/fix-remove-devtools/

Some notes:

• I had to replace devtools::document() with roxygen2::roxygenize(load='installed') (not just roxygen2::roxygenize()) because of Error: must pass a package name, “DLLInfo” or “DllInfoReference” object r-lib/roxygen2#822 (comment)
• given ^, it's important that conda's R knows to look for lightgbm in the same place it was installed by build_r.R. That's why I added export R_LIBS...
• the first few builds were very tough to debug on https://readthedocs.org/projects/lightgbm/builds/ because the logs from build_r.R were disabled by the sed call. Can we please remove it so the logs can be used for debugging? They aren't THAT long, and I think they're valuable

By "it's working", I mean updating pkgdown and roxygen2 in the conda environment. I still need too add the use of pkgdown url option for #3036. Trying that next.

[jameslamb]

The changes for #3036 do not seem to be working :/

• build: https://readthedocs.org/projects/lightgbm/builds/11019779/
• docs: https://lightgbm.readthedocs.io/en/fix-remove-devtools/R/reference/

The GitHub logo was updated correctly, but links like Source: R/lightgbm.R point to the wrong place.

For example, Source: R/lightgbm.R on https://lightgbm.readthedocs.io/en/fix-remove-devtools/R/reference/agaricus.train.html points to https://github.com/Microsoft/LightGBM/blob/master/R/lightgbm.R

I'm investigating.

UPDATE 1: tried updating the YAML to repo: url.

• commit: fa3803f
• build: https://readthedocs.org/projects/lightgbm/builds/11019842/
• Did not work: https://lightgbm.readthedocs.io/en/fix-remove-devtools/R/reference/agaricus.train.html

UPDATE 2: Tried updating URL in R-package/DESCRIPTION. pkgdown docs make it seem like repo: url: in _pkgdown.yml will only be used if BugReports and URL are not given in DESCRIPTION

• commit: 27e7abb
• build: https://readthedocs.org/projects/lightgbm/builds/11019913/
• did not work: https://lightgbm.readthedocs.io/en/fix-remove-devtools/R/reference/agaricus.train.html

UPDATE 3: Tried completely removing URL and BugReports in R-package/DESCRIPTION, to see if that would force pkgdown to use the URLs I provided.

• commit: b160267
• build: https://readthedocs.org/projects/lightgbm/builds/11019973/
• worked!: https://lightgbm.readthedocs.io/en/fix-remove-devtools/R/reference/agaricus.train.html

UPDATE 4 Update 3 worked, but I originally thought it hadn't because my browser was caching RTD changes. Trying to revert the DESCRIPTION changes. The diff in https://github.com/r-lib/pkgdown/pull/1253/files makes it look like _pkgdown.yml takes precedence over DESCRIPTION fields for URLs

• commit: 4b18d3f
• build: https://readthedocs.org/projects/lightgbm/builds/11020037/
• worked! GitHub button and Source: links point to the right places. https://lightgbm.readthedocs.io/en/fix-remove-devtools/R/reference/agaricus.train.html

[jameslamb]

Ok I think this is fully working now and closes #3036 . This is ready for review.

[StrikerRUS]

the first few builds were very tough to debug on https://readthedocs.org/projects/lightgbm/builds/ because the logs from build_r.R were disabled by the sed call. Can we please remove it so the logs can be used for debugging? They aren't THAT long, and I think they're valuable

Are you speaking about this sed?

LightGBM/docs/conf.py

Line 243 in b60294b

sed -i'.bak' '/# Build the package (do not touch this line!)/q' build_r.R

This sed was used to prevent double building of LightGBM similarly to trick for CI with R cmd check and allows us to fit in RTD build time limits.

You can safely drop ca-certificates dependency now.

In build_reference() document argument is replaced by devel since 1.4.0 version.
https://github.com/r-lib/pkgdown/blob/c181e5c9ac09b28974a97f5e32deddc36690fcf2/R/build-reference.R#L139

Refer to https://github.com/r-lib/pkgdown/releases/tag/v1.4.0

build_site(), build_reference() and build_home() gain a parameter
devel which controls whether you're in deployment or development mode.
It generalises and replaces (with deprecation) the existing document
argument.

[jameslamb]

the first few builds were very tough to debug on https://readthedocs.org/projects/lightgbm/builds/ because the logs from build_r.R were disabled by the sed call. Can we please remove it so the logs can be used for debugging? They aren't THAT long, and I think they're valuable

Are you speaking about this sed?

LightGBM/docs/conf.py

Line 243 in b60294b

sed -i'.bak' '/# Build the package (do not touch this line!)/q' build_r.R

This sed was used to prevent double building of LightGBM similarly to trick for CI with R cmd check and allows us to fit in RTD build time limits.
You can safely drop ca-certificates dependency now.

In build_reference() document argument is replaced by devel since 1.4.0 version.
https://github.com/r-lib/pkgdown/blob/c181e5c9ac09b28974a97f5e32deddc36690fcf2/R/build-reference.R#L139

Refer to https://github.com/r-lib/pkgdown/releases/tag/v1.4.0

build_site(), build_reference() and build_home() gain a parameter
devel which controls whether you're in deployment or development mode.
It generalises and replaces (with deprecation) the existing document
argument.

Oh ok! Yes I misunderstood what that sed command was doing, thanks for the clarification. But I still think we should remove it. roxygen2::roxygenize(load = "installed") will say "don't try to rebuild the package, just assume it has already been built and installed".

I'll try removing ca-certificates and updating build_reference() call tonight.

[jameslamb]

Seems that removing ca-certificates worked!

• build: https://readthedocs.org/projects/lightgbm/builds/11027879/
• docs: https://lightgbm.readthedocs.io/en/fix-remove-devtools/R/reference/agaricus.train.html

[jameslamb]

close-reopen to re-trigger CI. Azure got stuck trying to download MiKTeX 😬

[StrikerRUS]

LGTM! Thanks a lot!

[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.