[WIP][SPARK-56015][INFRA][DOCS] Cleanup docs container, remove unused R deps, and fix x86 build.#54838
Conversation
…tm_source=chatgpt.com devtools install_version deprecated. Left devtools for downstream usage. Co-authored-by: Holden Karau <holden@pigscanfly.ca>
…ike archived projects. Also temp put roxygen2 back to 7.2 Co-authored-by: Holden Karau <holden@pigscanfly.ca>
Co-authored-by: Holden Karau <holden@pigscanfly.ca>
Co-authored-by: Holden Karau <holden@pigscanfly.ca>
…-build Co-authored-by: Holden Karau <holden@pigscanfly.ca>
dev/spark-test-image/docs/Dockerfile
Outdated
| Rscript -e "devtools::install_version('preferably', version='0.4', repos='https://cloud.r-project.org')" | ||
| RUN Rscript -e "pak::pak('roxygen2@7.2.0')" && \ | ||
| Rscript -e "pak::pak('pkgdown@2.2.0')" | ||
| RUN Rscript -e "devtools::install_version('preferably', version='0.4.1', repos='https://cloud.r-project.org')" |
There was a problem hiding this comment.
Why this is not using pak, @holdenk ? For the consistency, I guess this PR is supposed to remove devtools::install_version completely.
There was a problem hiding this comment.
So preferably is a deprecated/archived package and we need to use legacy install to install it. In others where we don't build docs and don't use the preferably template we could avoid devtools entirely. We could also drop the template since it's been removed from CRAN but that feels like a bigger conversation. See https://cran.r-project.org/web/packages/preferably/index.htm
dev/spark-test-image/docs/Dockerfile
Outdated
| && rm -rf /var/lib/apt/lists/* | ||
|
|
||
| # Broken up for caching since CRAN can change. | ||
| RUN Rscript -e "install.packages(c('devtools', 'knitr', 'markdown', 'rmarkdown', 'testthat', 'remotes', 'pak'), repos='https://cloud.r-project.org/')" |
There was a problem hiding this comment.
I'm wondering if we can remove devtools installation completely. Does pak depend on devtools?
There was a problem hiding this comment.
So the reason is the https://cran.r-project.org/web/packages/preferably/index.html package needs to be installed for the current docs and it's deprecated/archived on CRAN so we use the legacy installer.
Co-authored-by: Holden Karau <holden@pigscanfly.ca>
20cf4e0 to
f5aa4bb
Compare
dongjoon-hyun
left a comment
There was a problem hiding this comment.
Thank you for updating, @holdenk .
There is no side-effect from pkgdown change (from 2.0.1 to 2.2.0)?
|
IIUC, this simply aims to remove the warning instead of fixing any outage (as of now). So, maybe, did you verify the generated Rdoc manually, @holdenk ? If there is no noticeable change, this sounds okay to me. |
|
@dongjoon-hyun it started out as unblocking the docs build but in the meantime you fixed the blocking issue so it's less critical now :) I've got a local build, going to do another one with the rebase on top of 54164e9 |
|
Actually looking at this, I think we can / should drop R from the docs container see the |
+1 for the direction to drop R from the docs containers (for Apache Spark 4.2+) since it's deprecated already. |
What changes were proposed in this pull request?
Bump package versions in Docs Docker file to currently working versions & update to non-deprecated installer.
Why are the changes needed?
Docs container uses deprecated installation methods (also failed but failure was also fixed in 54164e9 )
Does this PR introduce any user-facing change?
No
How was this patch tested?
Built docs docker container locally.
Was this patch authored or co-authored using generative AI tooling?
Asked chat gpt about the different CRAN install methods.