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

Split sage_setup.docbuild out to a separate distribution sage_docbuild #30010

Closed
mkoeppe opened this issue Jun 28, 2020 · 80 comments
Closed

Split sage_setup.docbuild out to a separate distribution sage_docbuild #30010

mkoeppe opened this issue Jun 28, 2020 · 80 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Jun 28, 2020

sage_setup.docbuild, created in #19127, has very different dependencies compared to sage_setup:

It depends on sagelib and sphinx,
whereas the rest of sage_setup is for building sagelib.

We remove the nesting within sage_setup, creating a new top-level package sage_docbuild.

We split it out as a separate pip-installable distribution package sage_docbuild.

Using the spkg-src script, a pip-installable tarball can be generated, which could be uploaded to PyPI -- for the use by external packages that want to build documentation.

This is preparation for #29868 (pip-installable packages sagemath-doc-src, sagemath-doc-inventory, sagemath-doc-html, sagemath-doc-pdf)

Depends on #31344
Depends on #31353

CC: @kiwifb @jhpalmieri @isuruf @mwageringel @haraldschilly @mezzarobba

Component: refactoring

Keywords: sd111

Author: Matthias Koeppe

Branch/Commit: aaab1d3

Reviewer: John Palmieri

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

@mkoeppe mkoeppe added this to the sage-9.2 milestone Jun 28, 2020
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 28, 2020

Dependencies: #29847

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 13, 2020

Changed dependencies from #29847 to none

@mkoeppe

This comment has been minimized.

@mkoeppe mkoeppe changed the title Split sage_setup.docbuild out to a separate package Split sage_setup.docbuild out to a separate distribution (distutils package) Jul 13, 2020
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 13, 2020

Dependencies: #29950

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 13, 2020

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 13, 2020

Last 10 new commits:

b8e366ebuild/pkgs/sagelib/spkg-src: chmod +x
0473ef3Merge branch 't/21559/change-src-bin-installation' into t/29950/build_sagelib_using_installed_sage_setup
06a3609sage_setup.find.find_python_sources: Add benchmark doctest
174626cMerge branch 't/29786/move_extension_options_from_src_module_list_py_to__distutils___directives_in_the_individual_files__part_4__sage_rings_' into t/29701/replace_use_of_module_list_optionalextension
034a7f7Merge branch 'public/move_all_code_from_src_setup_py__src_fpickle_setup_py_to_sage_setup' of git://trac.sagemath.org/sage into t/29701/replace_use_of_module_list_optionalextension
d3c608bMerge tag '9.2.beta3' into t/29701/replace_use_of_module_list_optionalextension
7244371Merge branch 't/29701/replace_use_of_module_list_optionalextension' into t/29950/build_sagelib_using_installed_sage_setup
4344f89Merge branch 't/21559/change-src-bin-installation' into t/29950/build_sagelib_using_installed_sage_setup
72c5040Merge branch 't/29950/build_sagelib_using_installed_sage_setup' into t/30010/split_sage_setup_docbuild_out_to_a_separate_package
ce79954build/pkgs/sage_setup_docbuild: New

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 13, 2020

Author: Matthias Koeppe

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 13, 2020

Commit: ce79954

@mkoeppe

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 15, 2020

Changed commit from ce79954 to e6b5f75

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 15, 2020

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

e6b5f75build/pkgs/sagelib/src/MANIFEST.in: Prune sage_setup/docbuild

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 15, 2020

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

cbfe485build/pkgs/sage_setup_docbuild: Set version

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 15, 2020

Changed commit from e6b5f75 to cbfe485

@jhpalmieri
Copy link
Member

comment:13

In the new spkg-install file, why

. sage-dist-helpers

Why do this at all, and if you're going to do it, why not use source instead of ., to be consistent with how sage-spkg does things?

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 20, 2020

Changed commit from cbfe485 to 39781d1

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 20, 2020

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

39781d1build/pkgs/sage_setup_docbuild/spkg-install: Remove useless use of sage-dist-helpers

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 20, 2020

comment:15

Thanks. This was a leftover from when I was using sdh_pip_install.

@jhpalmieri
Copy link
Member

comment:16

Can you please add sage_setup_docbuild to DOC_DEPENDENCIES in build/make/Makefile.in?

Edit: No, sorry, it's already there. I just built Sage and it build sage_setup_docbuild at the end, after the documentation. I don't know why.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 21, 2020

comment:17

Hm... it's possible that it is always built because its dependency sagelib is always built...

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 10, 2020

Changed commit from 39781d1 to 0237faa

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 10, 2020

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

fcad518Merge tag '9.2.beta7' into t/29701/replace_use_of_module_list_optionalextension
55c3fbcsrc/sage_setup/clean.py: Fix doctest
01b96b0Merge branch 't/29701/replace_use_of_module_list_optionalextension' into t/29950/build_sagelib_using_installed_sage_setup
8a19fe2build/make/Makefile.in (sagelib-clean): Clean the new build location
ccc67b0src/sage_setup: Update cythonized_dir in doctests
0237faaMerge branch 't/29950/build_sagelib_using_installed_sage_setup' into t/30010/split_sage_setup_docbuild_out_to_a_separate_package

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 10, 2020

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

7d1af54build/pkgs/sage_setup_docbuild: Make sagelib an order-only dep

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 10, 2020

Changed commit from 0237faa to 7d1af54

@jhpalmieri
Copy link
Member

Changed commit from b6abe5e to b42920b

@jhpalmieri
Copy link
Member

comment:47

The directory structure looks very strange: inside SAGE_ROOT/build/pkgs/sage_docbuild/src/ I see

build
  lib
     build
        lib
           build
              lib
                 build
                    lib
                       sage_docbuild
                 sage_docbuild
           sage_docbuild
     sage_docbuild

where indentations indicate levels of subdirectories. For example, I see SAGE_ROOT/build/pkgs/sage_docbuild/src/build/lib/build/lib/build/lib/build/lib/sage_docbuild. All of the sage_docbuild directories look identical.


New commits:

b42920btrac 30010: (typo) change sage_doctest -> sage_docbuild

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 23, 2021

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 23, 2021

comment:49

Thanks for catching this! This commit should fix it


New commits:

0a472f2build/pkgs/sage_docbuild/src/setup.py: Restrict find_namespace_packages to sage_docbuild

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 23, 2021

Changed commit from b42920b to 0a472f2

@jhpalmieri
Copy link
Member

comment:50

Perhaps for another ticket, but shouldn't make distclean clean up build artifacts like those subdirectories of build/pkgs/PKG/src/? To test the most recent change, I had to delete the extraneous stuff by hand, or alternatively start with a fresh tarball. Since I'm testing with various branches combined (to allow docbuilding to succeed on my Mac, for example), I would rather not use a fresh tarball in these situations.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 23, 2021

comment:51

Yes, a target sage_docbuild-clean is definitely missing, I'll add one.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 23, 2021

comment:52

Replying to @jhpalmieri:

I would rather not use a fresh tarball in these situations.

What works really well for me is git worktree - easy to create a new "distclean" copy of the branch with this

@jhpalmieri
Copy link
Member

comment:53

I get this at the start of doctesting:

    ModuleNotFoundError in doctesting framework
**********************************************************************
Traceback (most recent call last):
  File "/Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.3.beta7/local/lib/python3.9/site-packages/sage/doctest/forker.py", line 2508, in __call__
    doctests, extras = self._run(runner, options, results)
  File "/Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.3.beta7/local/lib/python3.9/site-packages/sage/doctest/forker.py", line 2551, in _run
    doctests, extras = self.source.create_doctests(sage_namespace)
  File "/Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.3.beta7/local/lib/python3.9/site-packages/sage/doctest/sources.py", line 733, in create_doctests
    load(filename, namespace) # errors raised here will be caught in DocTestTask
  File "/Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.3.beta7/local/lib/python3.9/site-packages/sage/repl/load.py", line 252, in load
    exec(code, globals)
  File "./clean.py", line 18, in <module>
ModuleNotFoundError: No module named 'sage_setup.find'

leading to

sage -t --long --random-seed=0 src/sage_setup/clean.py  # ModuleNotFoundError in doctesting framework

and then I get other doctest failures:

sage -t --long --random-seed=0 src/sage_setup/find.py  # 6 doctests failed
sage -t --long --random-seed=0 src/sage_setup/util.py  # 2 doctests failed
sage -t --long --random-seed=0 src/sage_setup/optional_extension.py  # 2 doctests failed

These all have a similar form:

**********************************************************************
File "src/sage_setup/find.py", line 38, in sage_setup.find.read_distribution
Failed example:
    from sage_setup.find import read_distribution
Exception raised:
    Traceback (most recent call last):
      File "/Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.3.beta7/local/lib/python3.9/site-packages/sage/doctest/forker.py", line 714, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.3.beta7/local/lib/python3.9/site-packages/sage/doctest/forker.py", line 1133, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage_setup.find.read_distribution[1]>", line 1, in <module>
        from sage_setup.find import read_distribution
    ModuleNotFoundError: No module named 'sage_setup.find'
**********************************************************************
File "src/sage_setup/find.py", line 92, in sage_setup.find.find_python_sources
Failed example:
    from sage_setup.find import find_python_sources
Exception raised:
    Traceback (most recent call last):
      File "/Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.3.beta7/local/lib/python3.9/site-packages/sage/doctest/forker.py", line 714, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.3.beta7/local/lib/python3.9/site-packages/sage/doctest/forker.py", line 1133, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage_setup.find.find_python_sources[1]>", line 1, in <module>
        from sage_setup.find import find_python_sources
    ModuleNotFoundError: No module named 'sage_setup.find'

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 24, 2021

Changed commit from 0a472f2 to 1dfeb84

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 24, 2021

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

1dfeb84build/make/Makefile.in (sage_docbuild-clean): New, run it from build-clean

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 24, 2021

comment:55

I don't know how to reproduce the ModuleNotFoundError... could you run make sagelib-clean sagelib and test again?

@jhpalmieri
Copy link
Member

comment:56

Replying to @mkoeppe:

I don't know how to reproduce the ModuleNotFoundError... could you run make sagelib-clean sagelib and test again?

I did that and got the same failures. This was with Sage's own Python and #18272. Then I switched to just this branch and used homebrew's Python and got the same errors.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 24, 2021

Changed commit from 1dfeb84 to 881c4af

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 24, 2021

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

881c4afsrc/sage_setup/__init__.py: Restore

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 24, 2021

comment:58

Thanks for testing - I've found the problem now. As a leftover from the previous version of the branch, sage_setup had lost its __init__.py file, causing an incomplete installation of it in site-packages

@jhpalmieri
Copy link
Member

comment:59

Now the ext directory is missing from build/pkgs/sage_docbuild/src/build/lib/sage_docbuild and similarly for local/lib/python3.9/site-packages/sage_docbuild, so the documentation fails to build. It thinks it succeeds — no error — but the log file says things like

Building ja/a_tour_of_sage.

[a_tour_of] Extension error:
[a_tour_of] Could not import extension sage_docbuild.ext.inventory_builder (exception: No module named 'sage_docbuild.ext')
Build finished. The built documents can be found in /Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.3.beta7/local/share/doc/sage/html/ja/a_tour_of_sage

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 24, 2021

Changed commit from 881c4af to aaab1d3

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 24, 2021

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

4eca937build/pkgs/sage_docbuild/src/setup.{cfg,py}: Use an explicit list of packages, add install_requires
aaab1d3build/pkgs/sage_docbuild/dependencies: Add dependencies on .py files

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 24, 2021

comment:61

Sorry, thanks for your patience with this. I was using find_namespace_packages wrong, now I have just replaced it with an explicit list of packages.

@jhpalmieri
Copy link
Member

comment:62

Can you explain the dependencies file?

$(PYTHON) sphinx ../pkgs/sage_docbuild/src/sage_docbuild/*.py ../pkgs/sage_docbuild/src/sage_docbuild/ext/*.py | $(PYTHON_TOOLCHAIN) sagelib

I don't know what .../*.py is supposed to accomplish: how does make know what py files are supposed to be there or what to do with those targets?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 25, 2021

comment:63

These dependencies point to $SAGE_SRC/docbuild (going through the symbolic link at $SAGE_ROOT/build/pkgs/sage_docbuild/src). make expands the wildcard pattern to the source file names there. When they are newer than the timestamp file $SAGE_LOCAL/var/lib/sage/installed/sage_docbuild-9.3.beta7, this triggers a rebuild/reinstallation of the package sage_docbuild.

@jhpalmieri
Copy link
Member

comment:64

Okay, thanks, that makes sense. This is working for me now, both building from scratch and upgrading from #31344.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 25, 2021

comment:65

Thank you!

@vbraun
Copy link
Member

vbraun commented Mar 9, 2021

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

3 participants