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

Fix Documentation CI + General CI Cleanup #7105

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

timohl
Copy link
Contributor

@timohl timohl commented Dec 19, 2024

Type

  • Bug fix (non-breaking change which fixes an issue): Fixes #
  • New feature (non-breaking change which adds functionality). Resolves #
  • Breaking change (fix or feature that would cause existing functionality to not work as expected) Resolves #

Motivation and Context

Documentation CI is failing due to missing dependency for cmake.
The issue is that the dependency install script assumes Ubuntu 20.04 (Focal) is used and tries to install CMake from the Kitware APT Repository for Focal.
The github runner uses ubuntu-latest thought, which is currently Ubuntu 22.04 (Jammy) (actually just changed to 24.04 which produces more issues).

(See edit at bottom for a description of the general CI cleanups)

Checklist:

  • I have run python util/check_style.py --apply to apply Open3D code style
    to my code.
  • This PR changes Open3D behavior or adds new functionality.
    • Both C++ (Doxygen) and Python (Sphinx / Google style) documentation is
      updated accordingly.
    • I have added or updated C++ and / or Python unit tests OR included test
      results
      (e.g. screenshots or numbers) here.
  • I will follow up and update the code if CI fails.
  • For fork PRs, I have selected Allow edits from maintainers.

Description

Ubuntu 22.04. (Jammy) already comes with cmake 3.22.1, so using the Kitware APT Repository is not neccessary anymore.
This PR removes the explicit cmake update and thereby fixes the Documentation CI.

Also, the Ubuntu version is pinned to 22.04 because github is changing ubuntu-latest to 24.04.

Edit:
I went through the CI and cleaned up a bit.
If this is too much change, feel free to tell me, so I can revert to just add the documentation CI fix.

Centralized versions to requirements*.txt

There was a TODO comment in util/ci_utils.sh:
# TODO: modify other locations to use requirements.txt
So, I went ahead and centralized pip package versions to requirements files where it was possible.

Updated pip version

Pip version used in CI was updated from 23.2.1 to 24.3.1.

Removed outdated version pinning of jedi and idna

Both issues were solved upstream a couple of years ago:
JEDI_VER: "0.17.2" ipython/ipython#12740
IDNA_VER: "2.8" # psf/requests#5710

Synchronized pyproject.toml with requirements*.txt files

There were version mismatches between those files.

@timohl
Copy link
Contributor Author

timohl commented Dec 19, 2024

Strangely, the Documentation fails here, but in my fork repo with the same commit it runs without problems:
https://github.com/timohl/Open3D/actions/runs/12409485496/job/34644832179

Also, I do not know what this error means:

 make[1]: *** [CMakeFiles/Makefile2:4155: cpp/pybind/CMakeFiles/python-package.dir/rule] Error 2
make: *** [Makefile:1060: python-package] Error 2

Maybe somebody can restart the Documentation CI job?

@timohl timohl changed the title Fix Documentation CI Fix Documentation CI + General CI Cleanup Dec 19, 2024
@timohl
Copy link
Contributor Author

timohl commented Dec 19, 2024

I went through the CI and cleaned up a bit.
If this is too much change, feel free to tell me, so I can revert to just add the documentation CI fix.

Centralized versions to requirements*.txt

There was a TODO comment in util/ci_utils.sh:
# TODO: modify other locations to use requirements.txt
So, I went ahead and centralized pip package versions to requirements files where it was possible.

Updated pip version

Pip version used in CI was updated from 23.2.1 to 24.3.1.

Removed outdated version pinning of jedi and idna

Both issues were solved upstream a couple of years ago:
JEDI_VER: "0.17.2" ipython/ipython#12740
IDNA_VER: "2.8" # psf/requests#5710

Synchronized pyproject.toml with requirements*.txt files

There were version mismatches between those files.

@timohl
Copy link
Contributor Author

timohl commented Dec 19, 2024

Ok, I think I found the issue why the Documentation CI was successful on my fork but failing here:
My fork CI was using Ubuntu 22.04 while the CI here is already using Ubuntu 24.04 for ubuntu-latest.
As a temporary fix, I pinned the Ubuntu version to 22.04, so hopefully the Documentation will build correctly now.
Other pipelines kept working with Ubuntu 24.04 since they use Docker with 20.04/22.04 internally, while the Documentation pipeline was building directly in the github runner.

In order to support building on 24.04, probably a docker build should be added for that (future work though).

@benjaminum
Copy link
Contributor

This problem right?

In file included from /home/runner/work/Open3D/Open3D/3rdparty/glew/src/glew.c:41:
/usr/include/GL/osmesa.h:125:1: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘OSMesaCreateContext’
  125 | OSMesaCreateContext( GLenum format, OSMesaContext sharelist );
      | ^~~~~~~~~~~~~~~~~~~

I think it is ubuntu-latest on purpose to detect problems with new versions. That's how I understand the comment in the workflow file.
For now we can pin the version to make the documentation build and we can make a note to change it back when we address 24.04. @ssheorey what do you think?

@timohl
Copy link
Contributor Author

timohl commented Dec 19, 2024

Yes, that is my thought as well.
Just pinning to 22.04 temporarily to still update documentation in the meantime.
I will add a comment in like an hour.

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.

2 participants