Get rid of "# optional - build"

[mkoeppe]

Most of its uses should be replaced by # optional - sage_spkg

Depends on #32759

CC: @jhpalmieri @kiwifb @seblabbe @antonio-rojas

Component: doctest framework

Author: Matthias Koeppe

Branch/Commit: 9f76647

Reviewer: John Palmieri, François Bissey, Antonio Rojas

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

[mkoeppe]

Branch: u/mkoeppe/get_rid_of____optional___build_

[mkoeppe]

Commit: b41bb1d

[mkoeppe]

New commits:

e943bdc	src/sage_setup/find.py: Remove unneeded # optional - build
c9b48a3	src/sage: Replace '# optional - build' by '# optional - sage_spkg'
b41bb1d	src/bin/sage-runtests: Remove detection of 'build' optional tag

[mkoeppe]

Author: Matthias Koeppe

[jhpalmieri]

comment:3

Also remove "build" from TESTALL_FLAGS in Makefile?

[mkoeppe]

comment:4

Ah, I knew I forgot something

[mkoeppe]

Dependencies: #32759

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

e4cb681	src/sage/misc/sagedoc.py: Fix doctest
be532a2	src/sage/misc/sphinxify.py: Use module-level # sage.doctest: optional - sphinx
86db757	src/sage/misc/bindable_class.py: In doctests, strip output of sage_getdoc
0df932d	src/sage/misc/sageinspect.py: Make doctest pass even if sphinx formatting is not available
6f8f8e0	src/sage/repl/ipython_tests.py: Make doctest pass even if sphinx formatting is not available
1807446	src/sage/misc/package.py: Use gmpy2 instead of alabaster as example in doctest
f4aaa81	README.md, build/make/install: Update - documentation no longer has special log file location
651c986	build/make/Makefile.in (doc): Revert to just doc-html (not both doc-html and doc-pdf)
1ac7455	Merge #32759
294b10c	Makefile (TESTALL_FLAGS): Remove build

[sagetrac-git]

Changed commit from b41bb1d to 294b10c

[jhpalmieri]

comment:9

The changes make sense to me. fbissey, arojas, et al.: can you please evaluate it from the distro point of view?

[kiwifb]

comment:10

It is always a bit difficult when a big dependency is merged :(

Anyway focusing on the individual commits at the beginning.

• From the detection pattern I see in sage-runtests, I would have never tested something with a tag "build" before the change.
• The stuff in sage_setup will pass, I checked it.
• I replace package.py with a minimal file with no affected tests.
• I am unsure what will happen in tests/cmdline.py, those tests would have been skipped before, I am not quite sure what happens to them now. I don't know how sage_spkg is triggered but if it is some of these tests will fail (sage --info ...).

By the way, I believe there is a space missing at https://github.com/sagemath/sagetrac-mirror/blob/develop/src/sage/tests/cmdline.py?id=c9b48a3c200d4f41c576e339ebe0a37cbac074e0#n210
at the very least it is inconsistent with the rest.

[mkoeppe]

comment:11

Replying to @kiwifb:

I don't know how sage_spkg is triggered

It is triggered by sage -p returning without error (see SagePackageSystem._is_present)

[sagetrac-git]

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

2c19bb9

src/sage/tests/cmdline.py: Add missing space in # optional - sage_spkg

[sagetrac-git]

Changed commit from 294b10c to 2c19bb9

[kiwifb]

comment:13

Replying to @mkoeppe:

Replying to @kiwifb:

I don't know how sage_spkg is triggered

It is triggered by sage -p returning without error (see SagePackageSystem._is_present)

Then I won't ever test that :)

fbissey@tarazed ~ $ sage -p
/usr/bin/sage: line 1087: exec: sage-spkg: not found

[mkoeppe]

comment:14

Excellent, this works as intended.

[kiwifb]

comment:15

OK, all good to me then. Don't know about Antonio.

[antonio-rojas]

comment:16

Looks good to me too.

[jhpalmieri]

comment:17

Let's merge it, then.

[jhpalmieri]

Reviewer: John Palmieri, François Bissey, Antonio Rojas

[mkoeppe]

comment:18

Thanks all for reviewing!

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

edaa3e6	Merge tag '9.5.beta8' into t/32759/__configure___disable_doc
577cf8c	Merge #32759

[sagetrac-git]

Changed commit from 2c19bb9 to 577cf8c

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

2a4a323	Merge tag '9.5.beta7' into t/29039/pip_installable_package_sage_bootstrap
a7fa5aa	Merge #29039
2dd7cf5	Merge tag '9.5.beta9' into t/32759/__configure___disable_doc
440aba1	Merge #32759

[sagetrac-git]

Changed commit from 577cf8c to 440aba1

[mkoeppe]

comment:22

Merged updated #32759.

[mkoeppe]

comment:24

Thank you.

[vbraun]

comment:25

tests...

[jhpalmieri]

comment:26

I'm having problems building this, in fact. There is a bad merge in doctest/control.py (<<<<<<<<< HEAD is present) and the new passage has from sage.features.sagemath import sage_features, but there is no sage_features. If I change to use all_features, then I get

TypeError: all_features() got an unexpected keyword argument 'logger'

[sagetrac-git]

Changed commit from 440aba1 to 24298d0

[sagetrac-git]

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

2e0b91d	src/sage/doctest/control.py: Fix up merge
24298d0	Merge #32759

[mkoeppe]

comment:28

Merged updated #32759.

[jhpalmieri]

comment:30

I think you're missing the most recent commit from #32759 — the one fixing output for a doctest.

[sagetrac-git]

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

43221d0	src/sage/features/sagemath.py: Fix doctest output
9f76647	Merge #32759

[sagetrac-git]

Changed commit from 24298d0 to 9f76647

[jhpalmieri]

comment:32

I think this is good now. All tests pass for me.

[mkoeppe]

comment:33

Thanks!

[vbraun]

Changed branch from u/mkoeppe/get_rid_of____optional___build_ to 9f76647