-
-
Notifications
You must be signed in to change notification settings - Fork 663
Install built doc to SAGE_DOC #40807
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
base: develop
Are you sure you want to change the base?
Conversation
Documentation preview for this PR (built with commit f7cafd3; changes) is ready! 🎉 |
IMO it would be better to do this at the meson level (with That being said, this change only affects sage-the-distro so I have no objection as a temporary stopgap. |
src/sage_docbuild/builders.py
Outdated
@@ -145,11 +145,6 @@ def f(self, *args, **kwds): | |||
if build_options.ABORT_ON_ERROR: | |||
raise Exception("Non-exception during docbuild: %s" % (e,), e) | |||
|
|||
if type == 'latex': |
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.
why remove this? The files are in those places, or not?
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.
They show the absolute path to the built docs in the meson build directory. So they do not match with the install directory now, at least for sage-the-distro. They look redundant and misleading.
I guess the same is true in other scenarios. Do they show the correct directory for you? Then I may restore the logging.
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.
Do they show the correct directory for you? Then I may restore the logging.
Yes, they show the correct build directory (even for users of sage-the-distro).
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.
It is correct "build directory". But users should be notified the correct "install directory".
Sphinx already displays the relative paths to the built files. The additional display of the absolute paths to the "build directory" seems redundant and misleading at least for sage-the-distro users.
If the absolute paths to the build directory are still useful to users in other environment, I am okay to restore the deleted lines. Let me know.
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 may also add an echo command in the Makefile to display the install directory after copying.
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.
If the absolute paths to the build directory are still useful to users in other environment, I am okay to restore the deleted lines. Let me know.
Yes please restore it. There is no such thing as an install directory in the context of the sage_docbuild
commands.
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.
Okay.
By the way,
$ sage --docbuild en/tutorial html
[tutorial ] building [html]: targets for 23 source files that are out of date
[tutorial ] updating environment: [new config] 23 added, 0 changed, 0 removed
[tutorial ] Extension error!
...
[tutorial ] * matplotlib.sphinxext.plot_directive (3.10.5)
[tutorial ] * jupyter_sphinx (0.5.3)
[tutorial ] * furo (2024.07.18)
[tutorial ] * sphinx_basic_ng (1.0.0.beta2)
[tutorial ] Traceback
[tutorial ] =========
[tutorial ] File "/Users/kwankyu/GitHub/sage-dev/local/var/lib/sage/venv-python3.13/lib/python3.13/site-packages/jupyter_sphinx/utils.py", line 15, in blank_nb
[tutorial ] raise ExtensionError("Unable to find kernel", orig_exc=e)
[tutorial ] sphinx.errors.ExtensionError: Unable to find kernel (exception: No such kernel named sagemath)
[tutorial ] The full traceback has been saved in:
[tutorial ] /var/folders/t2/sj0m9bz56rlcy7kvx_hr4jjw0000gn/T/sphinx-err-lq7i4i2d.log
[tutorial ] To report this error to the developers, please open an issue at <https://github.com/sphinx-doc/sphinx/issues/>. Thanks!
[tutorial ] Please also report this if it was a user error, so that a better error message can be provided next time.
Build finished. The built documents can be found in /Users/kwankyu/GitHub/sage-dev/local/share/doc/sage/html/en/tutorial.
Does the command work for you?
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 restored the logging. For Makefile, the install directory is printed after copying.
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.
Thanks, and yes, it's finding the kernel for me.
|
||
doc-html: | ||
meson compile -C ../../build/sage-distro doc-html | ||
mkdir -p $(SAGE_DOC) | ||
cp -rf ../../build/sage-distro/src/doc/html $(SAGE_DOC) |
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.
where does SAGE_DOC points to on your system, i.e. where do those docs end up in?
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.
For sage-the-distro, it is local/share/doc
at sage root by default. The Makefile runs after environment variables for sage-the-distro are setup.
Yes. Then
Yes. |
13be91e
to
f7cafd3
Compare
Strangely, |
There must have been some confusion on my side. I tested again. It works well. Ready to go. |
This is a solution for https://groups.google.com/g/sage-devel/c/IVo1EI33CyU, alternative to #40798.
With this PR
installs the built doc to the expected location (set by SAGE_DOC). The same with
📝 Checklist
⌛ Dependencies