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

docbuild: Restore main website build - icons linking to pdf files #32043

Closed
mkoeppe opened this issue Jun 23, 2021 · 26 comments
Closed

docbuild: Restore main website build - icons linking to pdf files #32043

mkoeppe opened this issue Jun 23, 2021 · 26 comments

Comments

@mkoeppe
Copy link
Member

mkoeppe commented Jun 23, 2021

(from #31948 comment:83)

Apparently lost in #31948

CC: @kwankyu @jhpalmieri @haraldschilly

Component: documentation

Author: Kwankyu Lee

Branch/Commit: 0a25439

Reviewer: John Palmieri, Matthias Koeppe

Issue created by migration from https://trac.sagemath.org/ticket/32043

@mkoeppe mkoeppe added this to the sage-9.4 milestone Jun 23, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.4, sage-9.5 Aug 22, 2021
@kwankyu
Copy link
Collaborator

kwankyu commented Aug 28, 2021

Branch: u/klee/32043

@kwankyu
Copy link
Collaborator

kwankyu commented Aug 28, 2021

Author: Kwankyu Lee

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 28, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

b255863Restore website for pdf documentation

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 28, 2021

Commit: b255863

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 28, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

53f9938Edits of headings of external packages list

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 28, 2021

Changed commit from b255863 to 53f9938

@jhpalmieri
Copy link
Member

comment:6

make doc-html fails for me with the error

OSError: /Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.4.rc2/src/doc/en/tutorial/tour_numtheory.rst:129: WARNING: undefined label: sage.rings.padics.tutorial

I think this was the point of having doc-html-other depend on doc-html-reference.

Can you explain the other changes you've made?

@mkoeppe
Copy link
Member Author

mkoeppe commented Aug 28, 2021

comment:7

Pretty sure also doc-pdf-other needs a prerequisite.

Any changes really need to be tested with a parallel build, for example use export MAKE="make -j20"

@jhpalmieri
Copy link
Member

comment:8

I hit the problem with make doc-html right away, but after making the change

diff --git a/src/doc/Makefile b/src/doc/Makefile
index ff2c6037c1..bef4b3a212 100644
--- a/src/doc/Makefile
+++ b/src/doc/Makefile
@@ -47,7 +47,7 @@ doc-html-reference: doc-inventory-reference
        $(MAKE) SAGE_DOCBUILD_OPTS="$(SAGE_DOCBUILD_OPTS) --no-prune-empty-dirs" doc-html--reference_top
 
 # other documentation, html
-doc-html-other:
+doc-html-other: doc-html-reference
        $(MAKE) SAGE_DOCBUILD_OPTS="$(SAGE_DOCBUILD_OPTS) --no-prune-empty-dirs" $(foreach doc, $(wordlist 2, 100, $(shell cd $(SAGE_ROOT) && ./sage --docbuild --all-documents all)), doc-html--$(subst /,-,$(doc)))
 
 doc-html: doc-html-reference doc-html-other

make doc-clean && make doc-pdf has succeeded several times in a row. Same with make doc-clean && make doc && make doc-pdf. So I don't know if doc-pdf-other needs a prerequisite. (This is with MAKE='make -j6'.)

@kwankyu
Copy link
Collaborator

kwankyu commented Aug 29, 2021

comment:9

Replying to @jhpalmieri:

Can you explain the other changes you've made?

The website was not built because ReferenceTopBuilder had no chance to do its work.

Changes related with documentation of external packages were made to fix a link error in the reference top page and to make it read better (I think) for pdf files.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 29, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

681e5e6Revert harmful changes

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 29, 2021

Changed commit from 53f9938 to 681e5e6

@kwankyu
Copy link
Collaborator

kwankyu commented Aug 29, 2021

comment:11

Restored harmful and possibly harmful changes.

@kwankyu
Copy link
Collaborator

kwankyu commented Aug 29, 2021

comment:12

I had removed _output_dir method because I thought that is not necessary. But as I worry it may introduce a backward-incompatible side effect, I restored it.

@kwankyu
Copy link
Collaborator

kwankyu commented Aug 29, 2021

comment:13

Replying to @jhpalmieri:

So I don't know if doc-pdf-other needs a prerequisite. (This is with MAKE='make -j6'.)

Adding doc-html-reference does no harm.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 29, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

d486306Fix unintended new typos

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 29, 2021

Changed commit from 681e5e6 to d486306

@jhpalmieri
Copy link
Member

comment:16

Looks good to me. Thanks for fixing this.

@kwankyu
Copy link
Collaborator

kwankyu commented Aug 29, 2021

Reviewer: John Palmieri

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 29, 2021

Changed commit from d486306 to 0a25439

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 29, 2021

Branch pushed to git repo; I updated commit sha1. New commits:

0a25439Enroll doc-pdf-other as a target

@kwankyu
Copy link
Collaborator

kwankyu commented Aug 29, 2021

comment:20

Though it seems this .PHONY thing is not absolutely necessary.

@mkoeppe
Copy link
Member Author

mkoeppe commented Aug 29, 2021

Changed reviewer from John Palmieri to John Palmieri, Matthias Koeppe

@mkoeppe
Copy link
Member Author

mkoeppe commented Aug 29, 2021

comment:22

Replying to @kwankyu:

Changes related with documentation of external packages were made to fix a link error in the reference top page and to make it read better (I think) for pdf files.

Thanks, this is an improvement.

@jhpalmieri
Copy link
Member

comment:23

Let's merge it!

@vbraun
Copy link
Member

vbraun commented Sep 1, 2021

Changed branch from u/klee/32043 to 0a25439

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

No branches or pull requests

4 participants