-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
add how to build the doc with devcontainer. #4757
add how to build the doc with devcontainer. #4757
Conversation
Signed-off-by: Tomoya Fujita <[email protected]>
@@ -5,7 +5,7 @@ | |||
}, | |||
"workspaceMount": "source=${localWorkspaceFolder},target=/tmp/doc_repository,type=bind", | |||
"workspaceFolder": "/tmp/doc_repository", | |||
"postCreateCommand": "pip3 install --no-warn-script-location --user -r requirements.txt -c constraints.txt", | |||
"postCreateCommand": "pip3 install --no-warn-script-location --user -r requirements.txt -c constraints.txt --break-system-packages", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is required for ubuntu 24.04 or later, see more details for https://peps.python.org/pep-0668/
and that should be no problem, since this option is applied in the container environment only.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, I'm a bit confused about why this is needed here.
The current Dockerfile uses Ubuntu 22.04:
FROM ubuntu:jammy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the version of setuptools in Ubuntu 22.04 doesn't understand this flag
This PR also changes the Dockerfile to use ubuntu:24.04
? or did you mean something else?? btw, i verified that this works with no problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR also changes the Dockerfile to use
ubuntu:24.04
?
No, sorry, I just looked too quickly.
One thing we have to be careful about with changing the Dockerfile is that the Dockerfile is used to build and deploy the documentation. So as soon as we merge this, we'll start using 24.04 for the deployment. That is the correct thing going forward, but I'm going to hold off on merging this until I can merge it and immediately run a doc deployment job.
HTML artifacts: https://github.com/ros2/ros2_documentation/actions/runs/10986104386/artifacts/1964121223. To view the resulting site:
|
source/The-ROS2-Project/Contributing/Contributing-To-ROS-2-Documentation.rst
Outdated
Show resolved
Hide resolved
Signed-off-by: Tomoya Fujita <[email protected]>
Signed-off-by: Tomoya Fujita <[email protected]>
.. code-block:: console | ||
|
||
pip3 install --user --upgrade -r requirements.txt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this command fails with venv,
(ros2doc) root@tomoyafujita:~/ros2_ws/work/ros2_documentation# pip3 install --user --upgrade -r requirements.txt
ERROR: Can not perform a '--user' install. User site-packages are not visible in this virtualenv.
|
||
.. group-tab:: macOS | ||
|
||
.. code-block:: console | ||
|
||
pip3 install --user --upgrade -r requirements.txt | ||
pip install -r requirements.txt -c constraints.txt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am almost sure this works, but i do not have the dev environment with macOS and Windows, so i would like someone to help to make sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@clalancette i tried to address all comments, can you take a look at this again?
@clalancette friendly ping. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me. I'm going to go ahead and merge this, and then immediately run https://build.ros.org/job/doc_ros2doc/ . If that fails, I'll have to go ahead and revert it.
(also, I won't merge the backport PRs until that job completes successfully) |
* add how to build the doc with devcontainer. * use venv instead of virtualenv. * add venv to the official doc build procedure. Signed-off-by: Tomoya Fujita <[email protected]> (cherry picked from commit fcf9bc5)
* add how to build the doc with devcontainer. * use venv instead of virtualenv. * add venv to the official doc build procedure. Signed-off-by: Tomoya Fujita <[email protected]> (cherry picked from commit fcf9bc5) # Conflicts: # .devcontainer/devcontainer.json # source/The-ROS2-Project/Contributing/Contributing-To-ROS-2-Documentation.rst
* add how to build the doc with devcontainer. * use venv instead of virtualenv. * add venv to the official doc build procedure. Signed-off-by: Tomoya Fujita <[email protected]> (cherry picked from commit fcf9bc5) # Conflicts: # .devcontainer/devcontainer.json # source/The-ROS2-Project/Contributing/Contributing-To-ROS-2-Documentation.rst
Looks successful! https://build.ros.org/job/doc_ros2doc/1479/ |
* add how to build the doc with devcontainer. * use venv instead of virtualenv. * add venv to the official doc build procedure. Signed-off-by: Tomoya Fujita <[email protected]> (cherry picked from commit fcf9bc5) Co-authored-by: Tomoya Fujita <[email protected]>
* add how to build the doc with devcontainer. * use venv instead of virtualenv. * add venv to the official doc build procedure. Signed-off-by: Tomoya Fujita <[email protected]> (cherry picked from commit fcf9bc5)
* add how to build the doc with devcontainer. * use venv instead of virtualenv. * add venv to the official doc build procedure. Signed-off-by: Tomoya Fujita <[email protected]> (cherry picked from commit fcf9bc5) Co-authored-by: Tomoya Fujita <[email protected]>
* add how to build the doc with devcontainer. * use venv instead of virtualenv. * add venv to the official doc build procedure. Signed-off-by: Tomoya Fujita <[email protected]> (cherry picked from commit fcf9bc5)
* add how to build the doc with devcontainer. * use venv instead of virtualenv. * add venv to the official doc build procedure. Signed-off-by: Tomoya Fujita <[email protected]> (cherry picked from commit fcf9bc5) Co-authored-by: Tomoya Fujita <[email protected]>
closes #4497