search

sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.38k stars 470 forks source link

Move SAGE_ROOT/build/pkgs/*/src to SAGE_ROOT/pkgs/* #31577

Closed mkoeppe closed 3 years ago

mkoeppe commented 3 years ago

In the current layout, these embedded source trees are a little difficult to find / complicated to type.

The proposed location is:

SAGE_ROOT
- pkgs
  - sage-conf
    - setup.py
    - bin/
    - sage_conf.py.in
  - sage-docbuild
    - setup.py
    - sage_docbuild   -> symlinks to SAGE_ROOT/sage_docbuild/
  - sage-sws2rst
    - setup.py
    - bin/
    - sage_sws2rst/
  - sagemath-standard
    - setup.py
    - bin             -> symlinks to SAGE_ROOT/src/bin/        
    - sage            -> symlinks to SAGE_ROOT/src/sage/
    - sage_setup      -> symlinks to SAGE_ROOT/src/sage_setup/ (removed in #29847)

that is, the new directory pkgs will be on the same level as 

SAGE_ROOT
- src

the unchanged monolithic sagelib source tree, which will continue to contain subdirectories

  - bin/ 
  - doc/
  - sage/
  - sage_docbuild/      # was sage_setup/docbuild/ in Sage 9.2
  - sage_setup/

We add symlinks from the previous locations SAGE_ROOT/build/pkgs/sage[math]_*/src to the new locations SAGE_ROOT/pkgs/sage[math]-*

29847 will add:

SAGE_ROOT
- pkgs
  - sage-setup
    - setup.py
    - sage_setup         -> symlinks to src/sage_setup/

and remove sage_setup from the sagemath-standard distribution.

Other modularization tickets (#29705) will add

SAGE_ROOT
- pkgs
  - sagemath-core/
  - sagemath-brial/
  - sagemath-giac/
  - sagemath-meataxe/
  - sagemath-tdlib/

etc.

29868 will add

SAGE_ROOT
- pkgs
  - sagemath-doc-html/
  - sagemath-doc-pdf/

All files that contain Sage doctests will remain in the monolithic src/ source tree; the source trees of the distributions symlink there. This may be changed in a follow-up ticket regarding the modularization of doctesting.

Before merging this branch into a non-distclean worktree, it is useful to use 

  git clean -f -x build/pkgs/*/src

CC: @jhpalmieri @dimpase @kiwifb

Component: build

Author: Matthias Koeppe

Branch/Commit: 7568dc6

Reviewer: Dima Pasechnik

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

mkoeppe commented 3 years ago
comment:1

Alternatively, these directories could just be put into SAGE_ROOT:

SAGE_ROOT
 - sage_conf
 - sage_docbuild
 - sage_sws2rst
 - sagemath_standard
mkoeppe commented 3 years ago
comment:2

(made this change in the ticket description)

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -1,14 +1,18 @@
-In the current layout, these embedded source trees are a little difficult to find.
+In the current layout, these embedded source trees are a little difficult to find / complicated to type.

 The proposed location is:

SAGE_ROOT

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -12,7 +12,14 @@
 that is, on the same level as
mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -5,21 +5,51 @@

SAGE_ROOT

-

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -18,7 +18,7 @@
     - sage            -> symlinks to SAGE_ROOT/src/sage
     - sage_setup      -> symlinks to SAGE_ROOT/src/sage_setup

-that is, these new directories on the same level as +that is, these new directories will be on the same level as 

   - src
@@ -36,20 +36,29 @@
 Also, we rename the confusingly named directory `SAGE_ROOT/build` (Python users expect a directory named `build` to contains build artifacts only) to

+Other modularization tickets (#29705) will add

+```

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -33,7 +33,7 @@
     - sage_setup

-Also, we rename the confusingly named directory SAGE_ROOT/build (Python users expect a directory named build to contains build artifacts only) to +In follow-up ticket #31662, we rename the confusingly named directory SAGE_ROOT/build (Python users expect a directory named build to contains build artifacts only) to

   - sage_bootstrap
mkoeppe commented 3 years ago

Dependencies: #30913

mkoeppe commented 3 years ago

Branch: u/mkoeppe/move_sage_root_build_pkgs_src_to_sage_root__rename_sage_root_build_to_sage_root_sage_bootstrap

mkoeppe commented 3 years ago

Last 10 new commits:

0283da5build/make/Makefile.in: Add wheel, setuptools_wheel to PYTHON_TOOLCHAIN to make sure that PEP 517 packages have a complete build system
f720722build/pkgs/sagelib/src/tox.ini: Add factor nobuildisolation
c451b31src/setup.cfg.m4 (install_requires): Add sage_conf
6700223Merge tag '9.3.rc0' into t/30913/sagelib__add_setup_cfg__install_requires_
04da2c6build/pkgs/ipywidgets: Patch out declaring install-requires of nbformat and jupyterlab-widgets
68dc845Merge branch 't/30913/sagelib__add_setup_cfg__install_requires_' into t/31577/move_sage_root_build_pkgs___src_to_sage_root____rename_sage_root_build_to_sage_root_sage_bootstrap
9181571Move /build/pkgs/sage_conf/src to /sage_conf, leave symlink behind
4f480b4Move /build/pkgs/sage_docbuild/src to /sage_docbuild, leave symlink behind
cc04151Move /build/pkgs/sage_sws2rst/src to /sage_sws2rst, leave symlink behind
8862b55Move /build/pkgs/sagelib/src to /sagemath_standard, leave symlink behind
mkoeppe commented 3 years ago

Commit: 8862b55

mkoeppe commented 3 years ago

Author: Matthias Koeppe

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 8862b55 to 73d83c2

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

73d83c2Move /build/pkgs/sagelib/src to /sagemath_standard, leave symlink behind
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

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

223e2baconfigure.ac: Use shorter path to sage_conf/
9bbb57bFixup: Add build/pkgs/sage_sws2rst/src symlink
ee0a21eFixup: Add build/pkgs/sagelib/src symlink
6ff4ba1.gitignore: Update for new source tree locations
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 73d83c2 to 6ff4ba1

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 6ff4ba1 to ef2fee8

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

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

ef2fee8WIP: Fix up docker build
jhpalmieri commented 3 years ago
comment:13
sage_docbuild   -> symlinks to SAGE_ROOT/src/sage_docbuild

Would it make sense to reverse this: put the files in SAGE_ROOT/sage_docbuild and then make SAGE_ROOT/src/sage_docbuild into a link? Is there any reason to keep SAGE_ROOT/src/sage_docbuild? If so, why also create the top-level directory?

What about SAGE_ROOT/src/sage_setup, along the same lines?

mkoeppe commented 3 years ago
comment:14

This is a great question.

There is no technical reason for keeping SAGE_ROOT/src/sage_docbuild at all. Likewise, after #29847, there will be no technical reason for keeping SAGE_ROOT/src/sage_setup.

But in #29705, I promised not to change the structure of the source tree, which I interpret as keeping the monolithic tree SAGE_ROOT/src unchanged. It is so that we don't overwhelm the developer community by making too many changes at the same time.

Of course, it could be argued that sage_docbuild is a new name anyway, and very few people will know or care about this package.

So if you think that's better, I can just move the sage_docbuild sources to SAGE_ROOT/sage_docbuild/sage_docbuild on this ticket.

(I'd rather not put reverse symlinks.)

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

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

12df870No more src/sage_docbuild
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from ef2fee8 to 12df870

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -9,10 +9,10 @@
     - sage_conf.py.in
   - sage_docbuild
     - setup.py
-    - sage_docbuild   -> symlinks to SAGE_ROOT/src/sage_docbuild
+    - sage_docbuild   # moved here from src/sage_docbuild - in Sage 9.2 was src/sage_setup/docbuild
   - sage_sws2rst
     - setup.py
-    - ...
+    - sage_sws2rst
   - sagemath_standard
     - setup.py
     - sage            -> symlinks to SAGE_ROOT/src/sage
@@ -49,7 +49,7 @@
mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -23,13 +23,12 @@
mkoeppe commented 3 years ago
comment:18

I will make another change, renaming the top-level directories from using underscores to using dashes. This will emphasize that these are the names of distribution packages (PyPI normalizes to dashes).

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -4,16 +4,16 @@

SAGE_ROOT

mkoeppe commented 3 years ago
comment:20

(This change is making the kludge that I had to add to the top-level Makefile unnecessary. The command for building make sage_conf, will no longer be fooled by the existence of a top-level directory of that name.)

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 12df870 to 1fe5f3d

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

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

1fe5f3dSpell distribution source directories sage-conf, sage-docbuild, sage-sws2rst, sagemath-standard with dashes, not underscores
mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -6,17 +6,20 @@
 SAGE_ROOT
   - sage-conf
     - setup.py
+    - bin/
     - sage_conf.py.in
   - sage-docbuild
     - setup.py
-    - sage_docbuild   # moved here from src/sage_docbuild - in Sage 9.2 was src/sage_setup/docbuild
+    - sage_docbuild   # moved here from src/sage_docbuild/ - in Sage 9.2 was src/sage_setup/docbuild/
   - sage-sws2rst
     - setup.py
-    - sage_sws2rst
+    - bin/
+    - sage_sws2rst/
   - sagemath-standard
     - setup.py
-    - sage            -> symlinks to SAGE_ROOT/src/sage
-    - sage_setup      -> symlinks to SAGE_ROOT/src/sage_setup
+    - bin             -> symlinks to SAGE_ROOT/src/bin/        
+    - sage            -> symlinks to SAGE_ROOT/src/sage/
+    - sage_setup      -> symlinks to SAGE_ROOT/src/sage_setup/

that is, these new directories will be on the same level as

@@ -26,10 +29,10 @@ the monolithic sagelib source tree, unchanged compared to Sage 9.2, which will continue to contain subdirectories

-    - bin 
-    - doc
-    - sage
-    - sage_setup 
+    - bin/ 
+    - doc/
+    - sage/
+    - sage_setup/

In follow-up ticket #31662, we rename the confusingly named directory SAGE_ROOT/build (Python users expect a directory named build to contains build artifacts only) to @@ -37,10 +40,10 @@

   - sage-bootstrap
     - setup.py
-    - sage_bootstrap
-    - bin
-    - make
-    - pkgs
+    - sage_bootstrap/
+    - bin/
+    - make/
+    - pkgs/

29847 will add:

@@ -48,17 +51,17 @@

   - sage-setup
     - setup.py
-    - sage_setup       # SAGE_ROOT/src/sage_setup will be moved here
+    - sage_setup/      # SAGE_ROOT/src/sage_setup will be moved here

Other modularization tickets (#29705) will add

-  - sagemath-core
-  - sagemath-brial
-  - sagemath-giac
-  - sagemath-meataxe
-  - sagemath-tdlib
+  - sagemath-core/
+  - sagemath-brial/
+  - sagemath-giac/
+  - sagemath-meataxe/
+  - sagemath-tdlib/

etc.

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -10,7 +10,7 @@
     - sage_conf.py.in
   - sage-docbuild
     - setup.py
-    - sage_docbuild   # moved here from src/sage_docbuild/ - in Sage 9.2 was src/sage_setup/docbuild/
+    - sage_docbuild/   # moved here from src/sage_docbuild/ - in Sage 9.2 was src/sage_setup/docbuild/
   - sage-sws2rst
     - setup.py
     - bin/
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 1fe5f3d to 8ad9b3e

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

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

8ad9b3eFixup renames
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 8ad9b3e to 4c0d38d

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

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

4c0d38dAlso standardize on using dashes in distribution names in setup.cfg
mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -65,3 +65,10 @@

etc.

+#29868 will add + +```

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

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

47b890eMerge tag '9.3.rc3' into t/31577/move_sage_root_build_pkgs___src_to_sage_root____rename_sage_root_build_to_sage_root_sage_bootstrap
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 4c0d38d to 47b890e

jhpalmieri commented 3 years ago
comment:28

make ptestlong fails to run any doctests because of this error:

Using --optional=build,dochtml,homebrew,pip,sage,sage_spkg
Doctesting entire Sage library.
Traceback (most recent call last):
  File "/Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.3.rc2/src/bin/sage-runtests", line 144, in <module>
    err = DC.run()
  File "/Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.3.rc2/local/lib/python3.9/site-packages/sage/doctest/control.py", line 1204, in run
    self.expand_files_into_sources()
  File "/Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.3.rc2/local/lib/python3.9/site-packages/sage/doctest/control.py", line 788, in expand_files_into_sources
    self.sources = [FileDocTestSource(path, self.options) for path in expand()]
  File "/Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.3.rc2/local/lib/python3.9/site-packages/sage/doctest/control.py", line 788, in <listcomp>
    self.sources = [FileDocTestSource(path, self.options) for path in expand()]
  File "/Users/palmieri/Desktop/Sage/sage_builds/TESTING/sage-9.3.rc2/local/lib/python3.9/site-packages/sage/doctest/sources.py", line 528, in __init__
    raise ValueError("unknown file extension %r"%ext)
ValueError: unknown file extension ''
make: *** [ptestlong] Error 1

I believe the problem is that it trying to doctest src/sage_docbuild.

mkoeppe commented 3 years ago
comment:29

OK, I guess I have to retract my claim that there is no technical reason for having src/sage_docbuild. In part that's #31352 though

jhpalmieri commented 3 years ago
comment:30

I'm currently testing this change:

diff --git a/src/sage/doctest/control.py b/src/sage/doctest/control.py
index afb0d6cbcf..45ec2263fb 100644
--- a/src/sage/doctest/control.py
+++ b/src/sage/doctest/control.py
@@ -695,7 +695,7 @@ class DocTestController(SageObject):
             # don't make sense to run outside a build environment
             if have_git:
                 self.files.append(opj(SAGE_SRC, 'sage_setup'))
-                self.files.append(opj(SAGE_SRC, 'sage_docbuild'))
+                self.files.append(opj(SAGE_ROOT, 'sage-docbuild'))
             self.files.append(SAGE_DOC_SRC)

         if self.options.all or (self.options.new and not have_git):
mkoeppe commented 3 years ago
comment:31

I think I'd rather undo the change in 12df870 and keep it for another ticket.

I think it's better to keep everything that is subject to Sage doctesting in one physical tree

mkoeppe commented 3 years ago
comment:32

(Modularization of doctesting is a separate issue.)

kiwifb commented 3 years ago
comment:33

Replying to @jhpalmieri:

I'm currently testing this change:

diff --git a/src/sage/doctest/control.py b/src/sage/doctest/control.py
index afb0d6cbcf..45ec2263fb 100644
--- a/src/sage/doctest/control.py
+++ b/src/sage/doctest/control.py
@@ -695,7 +695,7 @@ class DocTestController(SageObject):
             # don't make sense to run outside a build environment
             if have_git:
                 self.files.append(opj(SAGE_SRC, 'sage_setup'))
-                self.files.append(opj(SAGE_SRC, 'sage_docbuild'))
+                self.files.append(opj(SAGE_ROOT, 'sage-docbuild'))
             self.files.append(SAGE_DOC_SRC)

         if self.options.all or (self.options.new and not have_git):

I really don't like that kind of changes. In sage-on-distro SAGE_SRC becomes SAGE_LIB at runtime when SAGE_ROOT is undefined. That would break things there. Not to mention I am against introducing more SAGE_ROOT outside of the build system as a matter of principle. As Matthias says modularization of doctesting is a separate issue and that's what would be most appropriate there.

jhpalmieri commented 3 years ago
comment:34

Replying to @kiwifb:

Replying to @jhpalmieri:

I'm currently testing this change:

diff --git a/src/sage/doctest/control.py b/src/sage/doctest/control.py
index afb0d6cbcf..45ec2263fb 100644
--- a/src/sage/doctest/control.py
+++ b/src/sage/doctest/control.py
@@ -695,7 +695,7 @@ class DocTestController(SageObject):
             # don't make sense to run outside a build environment
             if have_git:
                 self.files.append(opj(SAGE_SRC, 'sage_setup'))
-                self.files.append(opj(SAGE_SRC, 'sage_docbuild'))
+                self.files.append(opj(SAGE_ROOT, 'sage-docbuild'))
             self.files.append(SAGE_DOC_SRC)

         if self.options.all or (self.options.new and not have_git):

I really don't like that kind of changes. In sage-on-distro SAGE_SRC becomes SAGE_LIB at runtime when SAGE_ROOT is undefined. That would break things there. Not to mention I am against introducing more SAGE_ROOT outside of the build system as a matter of principle. As Matthias says modularization of doctesting is a separate issue and that's what would be most appropriate there.

I think that on distros, have_git will be false (it tests whether SAGE_ROOT/.git exists), so this block, and the resulting doctests, will be skipped entirely.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 47b890e to 08b8306

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

a24fe5cMerge #30913
ca9935eMove /build/pkgs/sage_conf/src to /sage_conf, leave symlink behind
f81216aMove /build/pkgs/sage_docbuild/src to /sage_docbuild, leave symlink behind
b5e30f4Move /build/pkgs/sage_sws2rst/src to /sage_sws2rst, leave symlink behind
25e6d14Move /build/pkgs/sagelib/src to /sagemath_standard, leave symlink behind
e253a2b.gitignore: Update for new source tree locations
1cfeb06Spell distribution source directories sage-conf, sage-docbuild, sage-sws2rst, sagemath-standard with dashes, not underscores
8d4ea4eAlso standardize on using dashes in distribution names in setup.cfg
08b8306WIP: Fix up docker build
kiwifb commented 3 years ago
comment:36

You are right, it will skipped. Although technically they could be run if present.

mkoeppe commented 3 years ago
comment:37

Nevertheless I have removed that change. Too complicated for 1 ticket.

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -10,7 +10,7 @@
     - sage_conf.py.in
   - sage-docbuild
     - setup.py
-    - sage_docbuild/   # moved here from src/sage_docbuild/ - in Sage 9.2 was src/sage_setup/docbuild/
+    - sage_docbuild   -> symlinks to SAGE_ROOT/sage_docbuild/
   - sage-sws2rst
     - setup.py
     - bin/
@@ -26,12 +26,13 @@

@@ -51,7 +52,7 @@

   - sage-setup
     - setup.py
-    - sage_setup/      # SAGE_ROOT/src/sage_setup will be moved here
+    - sage_setup         -> symlinks to src/sage_setup/

Other modularization tickets (#29705) will add @@ -72,3 +73,7 @@

+All files that contain Sage doctests will remain in the monolithic src/ source tree; the source trees of the distributions symlink there. This may be changed in a follow-up ticket regarding the modularization of doctesting. + + +