Skip to content

Conversation

@zhengruifeng
Copy link
Contributor

@zhengruifeng zhengruifeng commented Jan 15, 2024

What changes were proposed in this pull request?

Pin

  • sphinxcontrib-applehelp==1.0.4
  • sphinxcontrib-devhelp==1.0.2
  • sphinxcontrib-htmlhelp==2.0.1
  • sphinxcontrib-qthelp==1.0.3
  • sphinxcontrib-serializinghtml==1.1.5

previously,
Install Python linter dependencies install sphinxcontrib-applehelp-1.0.7, and then Install dependencies for documentation generation reinstall it with sphinxcontrib-applehelp-1.0.4;

now,
Install Python linter dependencies install sphinxcontrib-applehelp-1.0.8, and Install dependencies for documentation generation keep this intallation:
Requirement already satisfied: sphinxcontrib-applehelp in /usr/local/lib/python3.9/dist-packages (from sphinx==4.5.0) (1.0.8)

Why are the changes needed?

doc build is failing with:

Sphinx version error:
The sphinxcontrib.applehelp extension used by this project needs at least Sphinx v5.0; it therefore cannot be built with this version.
make: *** [Makefile:35: html] Error 2
                    ------------------------------------------------
      Jekyll 4.3.3   Please append `--trace` to the `build` command 
                     for any additional information or backtrace. 
                    ------------------------------------------------
/__w/spark/spark/docs/_plugins/copy_api_dirs.rb:131:in `<top (required)>': Python doc generation failed (RuntimeError)
	from /__w/spark/spark/docs/.local_ruby_bundle/ruby/2.7.0/gems/jekyll-4.3.3/lib/jekyll/external.rb:57:in `require'
	from /__w/spark/spark/docs/.local_ruby_bundle/ruby/2.7.0/gems/jekyll-4.3.3/lib/jekyll/external.rb:57:in `block in require_with_graceful_fail'
Sphinx version error:
The sphinxcontrib.devhelp extension used by this project needs at least Sphinx v5.0; it therefore cannot be built with this version.
make: *** [Makefile:35: html] Error 2
                    ------------------------------------------------
      Jekyll 4.3.3   Please append `--trace` to the `build` command 
                     for any additional information or backtrace. 
                    ------------------------------------------------
/__w/spark/spark/docs/_plugins/copy_api_dirs.rb:131:in `<top (required)>': Python doc generation failed (RuntimeError)

Does this PR introduce any user-facing change?

no

How was this patch tested?

ci

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

no

test
@github-actions github-actions bot added the INFRA label Jan 15, 2024
@yaooqinn
Copy link
Member

Mind creating a JIRA ticket @zhengruifeng?

@zhengruifeng
Copy link
Contributor Author

Mind creating a JIRA ticket @zhengruifeng?

sure, it's still under test, maybe we need to pin more deps.

@zhengruifeng zhengruifeng changed the title [INFRA] Pin sphinxcontrib-applehelp==1.0.4 [INFRA] Pin sphinxcontrib-* Jan 15, 2024
@zhengruifeng zhengruifeng changed the title [INFRA] Pin sphinxcontrib-* [SPARK-46715][INFRA] Pin sphinxcontrib-* Jan 15, 2024
@LuciferYang
Copy link
Contributor

How about upgrade sphinx to 5.0.0 directly? I am conducting tests, and it seems ok:

#44729

https://github.com/LuciferYang/spark/actions/runs/7523170642/job/20476201559

image

@zhengruifeng
Copy link
Contributor Author

How about upgrade sphinx to 5.0.0 directly? I am conducting tests, and it seems ok:

#44729

https://github.com/LuciferYang/spark/actions/runs/7523170642/job/20476201559

image

I remember @itholic had compared multiple versions of sphinx, and chose 4.5.0.
Other versions cause some render issues without failing in CI.

@LuciferYang
Copy link
Contributor

How about upgrade sphinx to 5.0.0 directly? I am conducting tests, and it seems ok:
#44729
https://github.com/LuciferYang/spark/actions/runs/7523170642/job/20476201559
image

I remember @itholic had compared multiple versions of sphinx, and chose 4.5.0. Other versions cause some render issues without failing in CI.

OK, let's continue using 4.5.0 for now

@zhengruifeng
Copy link
Contributor Author

zhengruifeng commented Jan 15, 2024

FYI, 4.2.0 is chosen in #44012 (comment) and then 4.5.0 for performance in 5eec99d

let's stick to 4.x before more investigation on the built docs with higher versions.

@zhengruifeng
Copy link
Contributor Author

thanks all

merged to master to restore CI

@zhengruifeng zhengruifeng deleted the infra_pin_sphinxcontrib-applehelp branch January 15, 2024 04:01
@nchammas
Copy link
Contributor

nchammas commented Jan 15, 2024

The proximate reason for this failure is that we install pydata_sphinx_theme without a version qualifier here:

python3.9 -m pip install 'flake8==3.9.0' pydata_sphinx_theme 'mypy==0.982' 'pytest==7.1.3' 'pytest-mypy-plugins==1.9.3' numpydoc jinja2 'black==23.9.1'

Note that this is separate from the constraint that sphinx==4.5.0, so pip installs pydata_sphinx_theme at version 0.15.1. This in turn requires sphinx>=5.0, which pulls in the incompatible Sphinx extensions mentioned in the error you are seeing.

Collecting pydata_sphinx_theme
  Downloading pydata_sphinx_theme-0.15.1-py3-none-any.whl.metadata (7.3 kB)
Collecting sphinx>=5.0 (from pydata_sphinx_theme)
  Downloading sphinx-7.2.6-py3-none-any.whl.metadata (5.9 kB)
Collecting sphinxcontrib-applehelp (from sphinx>=5.0->pydata_sphinx_theme)
  Downloading sphinxcontrib_applehelp-1.0.8-py3-none-any.whl.metadata (2.3 kB)

A longer term solution would be to compile all of our requirements per Python version and install them in one go. That's the only way pip is going to have the information it needs to prevent this kind of problem from occurring.

@wbo4958
Copy link
Contributor

wbo4958 commented Jan 15, 2024

@zhengruifeng, seems this issue is till in branch-3.5, is it possible to back-port to branch 3.5?

@zhengruifeng
Copy link
Contributor Author

@wbo4958 It seems the pydata_sphinx_theme installations is not changed:
it installs pydata_sphinx_theme-0.15.1 in Install Python linter dependencies, and then pydata_sphinx_theme-0.13.3 in Install dependencies for documentation generation, in both a successful run (https://github.com/apache/spark/actions/runs/7507799677/job/20442119109) and a failed one (https://github.com/apache/spark/actions/runs/7522706520/job/20475105206)

The reason I think is pydata_sphinx_theme itself (both 0.15.1 and 0.13.3) doesn't specify the versions of sphinxcontrib-*:

│   ├── sphinxcontrib-applehelp               sphinxcontrib-applehelp is a Sphinx extension which outputs Apple help books
│   ├── sphinxcontrib-devhelp                 sphinxcontrib-devhelp is a sphinx extension which outputs Devhelp documents
│   ├── sphinxcontrib-htmlhelp>=2.0.0         sphinxcontrib-htmlhelp is a sphinx extension which renders HTML help files
│   ├── sphinxcontrib-jsmath                  A sphinx extension which renders display math in HTML via JavaScript
│   ├── sphinxcontrib-qthelp                  sphinxcontrib-qthelp is a sphinx extension which outputs QtHelp documents
│   └── sphinxcontrib-serializinghtml>=1.1.9  sphinxcontrib-serializinghtml is a sphinx extension which outputs "serialized"

then latest sphinxcontrib-* is not compatabile with sphinx.

As to 3.5, I am not sure, since it uses sphinx<3.1.0, probably needs a separate fix

@wbo4958
Copy link
Contributor

wbo4958 commented Jan 16, 2024

Thx for your fix.

zhengruifeng added a commit that referenced this pull request Jan 16, 2024
### What changes were proposed in this pull request?
backport #44727 to branch-3.5

### Why are the changes needed?
to restore doc build

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

### How was this patch tested?
ci

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

Closes #44744 from zhengruifeng/infra_pin_shinxcontrib.

Authored-by: Ruifeng Zheng <[email protected]>
Signed-off-by: Ruifeng Zheng <[email protected]>
dongjoon-hyun pushed a commit that referenced this pull request Jan 17, 2024
### What changes were proposed in this pull request?
1, Combine pip installations for lint and doc together
2, pip list before run tests

### Why are the changes needed?
1, to avoid potential conflicts, for example:
existing `sphinx==4.5.0` requires `docutils<0.18,>=0.14`, while unpinned `sphinx` requires `docutils<0.21,>=0.18.1`. If we install them with different commands, upgrade of `sphinx` might be broken.

2, to make it easier to debug, for example: #44727 (comment)

`sphinxcontrib-*` were installed twice with different versions, which is confusing.

### Does this PR introduce _any_ user-facing change?
no, infra-only

### How was this patch tested?
ci

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

Closes #44754 from zhengruifeng/infra_combine_pip_installation.

Authored-by: Ruifeng Zheng <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants