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

[SPARK-46626][DOCS] Bump jekyll to 4.3.3 to enable support for Ruby 3.3.0 #44628

Closed
wants to merge 3 commits into from

Conversation

nchammas
Copy link
Contributor

@nchammas nchammas commented Jan 8, 2024

What changes were proposed in this pull request?

  1. Bump jekyll to 4.3.3.
  2. Loosen the dependency spec for jekyll to make updates easier.
  3. Don't mention Ruby 1 or 2 in the docs.
  4. Don't use sudo with gem in the docs.

Why are the changes needed?

  1. Jekyll 4.3.2 is broken on Ruby 3.3.0. Jekyll 4.3.3 fixes the issue.

  2. There is no need to pin Jekyll in the Gemfile since it gets pinned automatically for us in the lock file. This makes updating dependencies via bundle update easier.

  3. Both Ruby 1 and 2 are EOL. We should not use or reference them in the docs.

  4. Installing stuff as the superuser is explicitly discouraged by both pip and gem. Pip issues this warning:

    WARNING: Running pip as the 'root' user can result in broken permissions and conflicting behaviour with the system package manager. It is recommended to use a virtual environment instead: https://pip.pypa.io/warnings/venv

    And bundler issues this warning:

    Don't run Bundler as root. Installing your bundle as root will break this application for all non-root users on this machine.

    We should not encourage this pattern in our docs.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Building the docs against Ruby 3.2.2 and 3.3.0.

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

No.

@github-actions github-actions bot added the DOCS label Jan 8, 2024
gem "ffi", "1.15.5"
gem "jekyll", "4.3.2"
gem "jekyll", "~> 4.3"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would like to migrate all of the dependency specifications here to use the pessimistic operator, but I leave that to a future PR.

@nchammas
Copy link
Contributor Author

nchammas commented Jan 8, 2024

cc @srowen since you reviewed #42669.

Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

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

Looks OK if it works

@nchammas
Copy link
Contributor Author

nchammas commented Jan 8, 2024

The build appears to be passing. Here is the build of the docs for this PR.

Is there some other test you'd like me to run or scenario to check out? Happy to do it.

@HyukjinKwon
Copy link
Member

Merged to master.

@nchammas nchammas deleted the SPARK-46626-jekyll-ruby branch January 9, 2024 01:51
HyukjinKwon pushed a commit that referenced this pull request Jan 11, 2024
### What changes were proposed in this pull request?

As [promised here][1], this change loosens our Ruby dependency specification so that Bundler can update transitive dependencies more easily.

Other changes included:
- Remove the direct dependency on webrick, because Jekyll [fixed the problem][2] that caused us to add it in the first place.
- Add explanatory comments to parts of the document generation process that are not obvious.

[1]: #44628 (comment)
[2]: jekyll/jekyll#8524

We can still build our docs using Ruby 2.7, but we should push devs to install Ruby 3 since Ruby 2 is [EOL][3] and we are unable to upgrade some of our doc dependencies until we're running Ruby 3.

[3]: https://www.ruby-lang.org/en/news/2022/04/12/ruby-2-7-6-released/

### Why are the changes needed?

Make the document building process more robust to future updates coming from the Ruby ecosystem.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

I built and reviewed the docs on both Ruby 2.7.8 and Ruby 3.3.0 using the following command:

```sh
SKIP_SCALADOC=1 SKIP_PYTHONDOC=1 SKIP_RDOC=1 bundle exec jekyll build
```

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

No.

Closes #44667 from nchammas/SPARK-46658-ruby-deps.

Authored-by: Nicholas Chammas <nicholas.chammas@gmail.com>
Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
rb-fsevent (0.11.2)
rb-inotify (0.10.1)
ffi (~> 1.0)
rexml (3.2.6)
rouge (3.26.0)
safe_yaml (1.0.5)
sass-embedded (1.63.6)
google-protobuf (~> 3.23)
sass-embedded (1.69.7)
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @nchammas , I hit an error when building docs for the 4.0 preview release

Your bundle is locked to sass-embedded (1.69.7) from rubygems repository https://rubygems.org/ or installed locally, but that version can no longer be found in that
source. That means the author of sass-embedded (1.69.7) has removed it. You'll need to update your bundle to a version other than sass-embedded (1.69.7) that hasn't been
removed in order to install.

I don't know the context of picking sass-embedded version here, which version should we use?

cc @dongjoon-hyun

Copy link
Contributor

Choose a reason for hiding this comment

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

We have doc building job in PR pre-merge tests, I'm wondering how this can be broken. Or is it because the test environment is different from the release environment defined by https://github.com/apache/spark/blob/master/dev/create-release/spark-rm/Dockerfile ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This version was selected by Bundler. Try running bundle update to bump the versions in the lock file and try again.

Would you like me to open a PR to bump the Ruby dependencies?

Copy link
Contributor

Choose a reason for hiding this comment

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

oh, we upgraded bundle at 1e3892f#diff-78c95b3441eb17ae275e1651c656a3614738ea96c7a13df92c95b0b987b6c497R77 but not in the docker file. let me try it.

Copy link
Contributor Author

@nchammas nchammas May 9, 2024

Choose a reason for hiding this comment

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

Ruby 2 is EOL, so we should not be using Ruby 2.7 anywhere. I can also address this if you like, though it should not be a blocker. A simple refresh of the Gem lock file should be enough for now.

Copy link
Contributor

Choose a reason for hiding this comment

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

it's a blocker for producing a 4.0 release...

@nchammas can you help to make necessary updates to the docker file so that it can build docs? Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure thing. Will look now.

I will need your help with testing, however, since non-committers cannot run all the steps of the release image.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yea I'll test directly with your PR. Just ping me when it's ready. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants