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

Add "configure --disable-notebook"; show descriptions of optional packages in "configure --help" #30383

Closed
mkoeppe opened this issue Aug 17, 2020 · 62 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Aug 17, 2020

We refactor the code in sage_spkg_enable.m4 and sage_spkg_collect.m4 so that it becomes possible to add configure --disable-SPKG options to disable standard packages. This also simplifies build/make/Makefile.in slightly because it no longer has to make a distinction between standard and optional packages.

We demonstrate this by adding one such option, configure --disable-notebook, which disables building the Jupyter notebook package ... and all of its exclusive dependencies. This is useful for people who want to use the system Jupyter notebook -- the Jupyter kernel is still built and can be installed there.

The new option appears before the --enable... options for optional packages.

We also make the configure help a bit more informative (by including the 1-line descriptions of the optional packages) and prettier.

$ ./configure --help
...
Optional Features:
  --disable-option-checking  ignore unrecognized --enable/--with options
  --disable-FEATURE       do not include FEATURE (same as --enable-FEATURE=no)
  --enable-FEATURE[=ARG]  include FEATURE [ARG=yes]
...
  --enable-experimental-packages
                          allow installing experimental packages (default: no
                          = ask for user confirmation for each package)
  --enable-download-from-upstream-url
                          allow downloading packages from their upstream URL
                          if they cannot be found on the Sage mirrors
  --disable-notebook      disable build of the Jupyter notebook and related
                          packages
  --enable-4ti2={no|if_installed (default)|yes}
                          enable build and use of the optional package 4ti2: Algebraic, geometric
                          and combinatorial problems on linear spaces
                          * package info: ./sage -info 4ti2
  --disable-4ti2          disable build and uninstall if previously installed by Sage in PREFIX;
                          same as --enable-4ti2=no
  --enable-atlas={no|if_installed (default)|yes}
                          enable build and use of the optional package atlas: Automatically Tuned
                          Linear Algebra Software (BLAS implementation)
                          * package info: ./sage -info atlas
  --disable-atlas         disable build and uninstall if previously installed by Sage in PREFIX;
                          same as --enable-atlas=no
  --enable-awali={no|if_installed (default)|yes}
                          enable build and use of the experimental package awali: Computation of/with
                          finite state machines
                          * package info: ./sage -info awali
  --disable-awali         disable build and uninstall if previously installed by Sage in PREFIX;
                          same as --enable-awali=no

This is also preparation for #30556 (packages that will not work without openssl), and for testing modularized installs (#30778, #29864).

CC: @jhpalmieri @orlitzky @slel @seblabbe @dimpase @embray @kliem

Component: build

Keywords: sd111

Author: Matthias Koeppe

Branch/Commit: 4916415

Reviewer: John Palmieri, Dima Pasechnik

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

@mkoeppe mkoeppe added this to the sage-9.3 milestone Aug 17, 2020
@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 13, 2020

comment:3

Suggestions for a better name for this new package type are welcome

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 14, 2020

comment:4

Alternatively, we could also make it possible to disable any standard package.

@mkoeppe

This comment has been minimized.

@mkoeppe mkoeppe changed the title New package type: optional-enabled-by-default "configure --disable-SPKG" for standard packages Sep 15, 2020
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 15, 2020

Dependencies: #29363

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 15, 2020

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 15, 2020

New commits:

c73460cAdd options 'configure --disable-SPKG' for standard packages
22d7ae4m4/sage_spkg_*.m4: At the end of configure, indicate which optional/experimental packages are configured to be installed
bd6c109Merge branch 't/29363/at_the_end_of_configure__indicate_which_optional_experimental_packages_are_configured_to_be_installed' into t/30383/new_package_type__optional_enabled_by_default
3c98700Replace SAGE_STANDARD_PACKAGES by use of SAGE_OPTIONAL_INSTALLED_PACKAGES
182c1fdm4/sage_spkg_collect.m4: Reduce verbosity by removing 'does not support check for system package'

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 15, 2020

Commit: 182c1fd

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 15, 2020

Author: Matthias Koeppe

@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@orlitzky
Copy link
Contributor

comment:12

Doesn't this create a bajillion ./configure options that basically do nothing (because the packages are dependencies), and another bajillion that cause sage to function improperly (test suite failures, etc.)?

I think the original approach was more on-track, although I wouldn't have created a separate package type for it (we don't need to proliferate a new package type throughout the build system every time we tweak a bit of metadata). Instead I would have added an "enabled by default" (default: no) feature somewhere to the optional packages. Long-term, a way to control (and depend on) optional features is needed, but that's a much larger undertaking akin to USE flags in Gentoo.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 16, 2020

comment:13

Replying to @orlitzky:

Doesn't this create a bajillion ./configure options that basically do nothing (because the packages are dependencies), and another bajillion that cause sage to function improperly (test suite failures, etc.)?

Yes, a gazillion of "advanced options"

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Sep 16, 2020

comment:14

Most "standard" packages are not really required in any strict sense other than passing doctests that test particular features. Making it possible to disable them gives users a remedy when the build fails on a system

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 6, 2020

Changed keywords from none to sd111

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 6, 2020

comment:15

Hoping we can make progress on this ticket this week - https://wiki.sagemath.org/days111

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 15, 2020

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

e64ef1aMerge tag '9.3.beta4' into t/30383/new_package_type__optional_enabled_by_default

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 15, 2020

Changed commit from 182c1fd to e64ef1a

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 4, 2021

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

ea182d7Copy changes from build/pkgs/sagelib/src to src
a1a10b9src/VERSION.txt: New
5697335src/setup.cfg: Add license_file=LICENSE.txt
deb9eb3Merge tag '9.3.beta3' into t/30912/sagelib__update_metadata_for_pypi_deployment
7ad4c0eMerge tag '9.3.beta4' into t/30912/sagelib__update_metadata_for_pypi_deployment
27b589bMerge branch 't/30912/sagelib__update_metadata_for_pypi_deployment' into t/31362/make_all_sage_local__make_all_sage_venv
8796008Put pynac into SAGE_LOCAL, sagelib into SAGE_VENV
d6831b1build/make/Makefile.in: Add all-build-local, all-build-venv, which include dependency on toolchain
61f6ba6Makefile: Add top-level targets build-local, build-venv
af2e1fbMerge #31362

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 4, 2021

Changed commit from e64ef1a to af2e1fb

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 4, 2021

Changed dependencies from #29363 to #31362

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 5, 2021

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

340fbb7bootstrap: Emit SAGE_SPKG_ENABLE calls for standard packages

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 10, 2021

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

74a83fcbuild/pkgs/matplotlib/dependencies: Undo removal of tornado

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 10, 2021

comment:33

Replying to @jhpalmieri:

Why the change to matplotlib dependencies?

Thanks for catching this - I had thought I could get rid of tornado at some point but jupyterlab_widgets also needs it as a dependency, so that wouldn't help.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 10, 2021

comment:34

Replying to @jhpalmieri:

the change in line 66 of m4/sage_spkg_configure.m4 renders very strangely (strange font for "default") when I click on the branch here for example or view the diff on my computer. Any ideas what that's about?

Sorry, I was playing with Unicode superscripts. If it does not render nice, I can just go back to using ascii

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 10, 2021

Changed commit from 74a83fc to 93d31b9

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 10, 2021

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

93d31b9Remove use of unicode superscripts in configure --help

@mkoeppe

This comment has been minimized.

@jhpalmieri
Copy link
Member

comment:37

Typo in the comments? In m4/sage_spkg_collect.m4, the descriptions for SAGE_OPTIONAL_INSTALLED_PACKAGES and SAGE_OPTIONAL_CLEANED_PACKAGES are the same. Should installed be changed to uninstalled as the last word in the description for the CLEANED version?

Configuring with --disable-notebook does indeed disable the notebook, as advertised. Sage builds, and so does the documentation. I was a little surprised that all doctests passed, too.

I don't know the syntax well enough to be able to thoroughly review this, unfortunately. Anyone else?

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 11, 2021

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

18fbb85m4/sage_spkg_collect.m4: Fix description of SAGE_OPTIONAL_CLEANED_PACKAGES

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 11, 2021

Changed commit from 93d31b9 to 18fbb85

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 11, 2021

comment:39

Replying to @jhpalmieri:

Configuring with --disable-notebook does indeed disable the notebook, as advertised. Sage builds, and so does the documentation. I was a little surprised that all doctests passed, too.

This is, in fact, a weakness in our doctest coverage. We have nothing at all that tests whether the Jupyter notebook works.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 11, 2021

Changed commit from 18fbb85 to c3e4093

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 11, 2021

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

c3e4093Rename SAGE_OPTIONAL_CLEANED_PACKAGES to SAGE_OPTIONAL_UNINSTALLED_PACKAGES

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 15, 2021

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

4916415Merge tag '9.3.beta9' into t/30383/new_package_type__optional_enabled_by_default

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 15, 2021

Changed commit from c3e4093 to 4916415

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 15, 2021

Changed dependencies from #31362, #31278 to none

@jhpalmieri
Copy link
Member

comment:46

Replying to @mkoeppe:

Replying to @jhpalmieri:

Configuring with --disable-notebook does indeed disable the notebook, as advertised. Sage builds, and so does the documentation. I was a little surprised that all doctests passed, too.

This is, in fact, a weakness in our doctest coverage. We have nothing at all that tests whether the Jupyter notebook works.

Maybe related to the fact that you can't start the notebook once you are running Sage from the command line? With SageNB you could run notebook() from within Sage, but this is not possible any more, or at least not in an obvious way, or maybe it just hasn't been implemented by us.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 17, 2021

comment:47

I think it's just that nobody has worked on writing such tests. I guess there must be technologies for automatically testing web apps but I don't know about them or if Jupyter already uses them.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 17, 2021

comment:48

Replying to @jhpalmieri:

With SageNB you could run notebook() from within Sage, but this is not possible any more, or at least not in an obvious way

This is certainly a feature that I certainly could have used if it was available. Transforming an existing Python process to a Jupyter kernel just amounts to running sage.repl.ipython_kernel.__main__, I think. But to my understanding no feature in the Jupyter notebook allows to connect to an existing running kernel. I found some discussions in this direction, for example jupyter/help#298, ipython/ipython#4066

@dimpase
Copy link
Member

dimpase commented Mar 17, 2021

Reviewer: Dima Pasechnik

@dimpase
Copy link
Member

dimpase commented Mar 17, 2021

comment:49

this looks good to go.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 17, 2021

Changed reviewer from Dima Pasechnik to John Palmieri, Dima Pasechnik

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 17, 2021

comment:50

Thanks!

@vbraun
Copy link
Member

vbraun commented Mar 20, 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

5 participants