Skip to content

[SPARK-56015][INFRA][DOCS] Cleanup docs container, remove unused R deps, and fix x86 build.#54974

Open
holdenk wants to merge 11 commits intoapache:masterfrom
holdenk:SPARK-56015-remove-r-deps-from-docs-container-r2
Open

[SPARK-56015][INFRA][DOCS] Cleanup docs container, remove unused R deps, and fix x86 build.#54974
holdenk wants to merge 11 commits intoapache:masterfrom
holdenk:SPARK-56015-remove-r-deps-from-docs-container-r2

Conversation

@holdenk
Copy link
Contributor

@holdenk holdenk commented Mar 24, 2026

What changes were proposed in this pull request?

Update the documentation build README, container, and steps. Takes over from #54838

Why are the changes needed?

  • SparkR is deprecated and docs are built outside the container anyways
  • Current docs build is hard coded to assume ARM
  • For building the deprecated R docs people need to install an extra package
  • We shouldn't be creating a bunch of files as root in the users spark directtory if we can avoid it.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Built docs docker container locally, built docs with the doc build shell script locally, built website with docs container locally.

Was this patch authored or co-authored using generative AI tooling?

  • Asked chat gpt about the different CRAN install methods.
  • copilot in autocomplete mode for some of it
  • Various asking claude about R & Jekyll stuff I'm not super up to date on
  • case/switch in shell script generated by claude

sfc-gh-hkarau and others added 11 commits March 24, 2026 13:15
Co-authored-by: Holden Karau <holden@pigscanfly.ca>
…th root permissions, fix we use Python 3.12 not 3.11 anymore, handle amd64/arm64 builds

Co-authored-by: Holden Karau <holden@pigscanfly.ca>
… bind mount permissions

Co-authored-by: Holden Karau <holden@pigscanfly.ca>
… out of scope for fixing that here today lets just get the current system working again on x86.

Co-authored-by: Holden Karau <holden@pigscanfly.ca>
Co-authored-by: Holden Karau <holden@pigscanfly.ca>
Co-authored-by: Holden Karau <holden@pigscanfly.ca>
Co-authored-by: Holden Karau <holden@pigscanfly.ca>
…witch on uname -m

Co-authored-by: Holden Karau <holden@pigscanfly.ca>
…structed in the dockerfile.

Co-authored-by: Holden Karau <holden@pigscanfly.ca>
…ols install.

Co-authored-by: Holden Karau <holden@pigscanfly.ca>
…is to run on my machine, add bundle install

Co-authored-by: Holden Karau <holden@pigscanfly.ca>
@holdenk holdenk force-pushed the SPARK-56015-remove-r-deps-from-docs-container-r2 branch from 0af964a to b28b08a Compare March 24, 2026 20:16
@holdenk holdenk changed the title [WIP][SPARK-56015][INFRA][DOCS] Cleanup docs container, remove unused R deps, and fix x86 build. [SPARK-56015][INFRA][DOCS] Cleanup docs container, remove unused R deps, and fix x86 build. Mar 24, 2026
@holdenk holdenk marked this pull request as ready for review March 24, 2026 20:16
@holdenk holdenk requested a review from dongjoon-hyun March 24, 2026 20:17
@dongjoon-hyun
Copy link
Member

@holdenk ?

Screenshot 2026-03-24 at 22 02 59

@holdenk
Copy link
Contributor Author

holdenk commented Mar 25, 2026

I'll rebase that's weird


# 3.Build docs on container: `error docs`, `scala doc`, `python doc`, `sql doc`.
docker run \
--user "$(id -u):$(id -g)" \
Copy link
Member

Choose a reason for hiding this comment

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

Is this required additionally?

set -ex
_arch="$(uname -m)"
case "$_arch" in
"aarch64") export JAVA_HOME=/usr/lib/jvm/java-17-openjdk-arm64 ;;
Copy link
Member

Choose a reason for hiding this comment

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

This is a nice improvement but looks orthogonal to this topic. Could you proceed this independently as a spin-off? Then, we can merge aarch64 support quickly on ubuntu-24.04-arm runner later.

Rscript -e "devtools::install_version('preferably', version='0.4', repos='https://cloud.r-project.org')"

# See more in SPARK-39735
ENV R_LIBS_SITE="/usr/local/lib/R/site-library:${R_LIBS_SITE}:/usr/lib/R/library"
Copy link
Member

Choose a reason for hiding this comment

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

Shall we focus on this removal only in this PR?

@@ -4,7 +4,6 @@ GEM
addressable (2.8.7)
Copy link
Member

Choose a reason for hiding this comment

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

I believe we need to proceed Gemfile update independently to be safe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants