Opened 19 months ago

Closed 16 months ago

Last modified 15 months ago

#29411 closed enhancement (fixed)

make sagelib a script package

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-9.2
Component: build Keywords:
Cc: embray, vdelecroix, mjo, dimpase, jhpalmieri, fbissey Merged in:
Authors: Matthias Koeppe Reviewers: Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: cc30471 (Commits, GitHub, GitLab) Commit:
Dependencies: #29098, #29697, #29793 Stopgaps:

Status badges

Description (last modified by mkoeppe)

... so that sagelib is just an ordinary script package whose source tree is included (hence, no checksums.ini file; see #29287).

Related sage-devel discussions:

Related tickets:

  • #29386 Install script packages via sage-spkg

Change History (63)

comment:1 Changed 18 months ago by mkoeppe

  • Milestone changed from sage-9.1 to sage-9.2

comment:2 Changed 17 months ago by mkoeppe

  • Cc embray vdelecroix added
  • Description modified (diff)

comment:3 Changed 17 months ago by mkoeppe

  • Dependencies changed from #29287 to #29098

comment:4 Changed 17 months ago by mkoeppe

  • Branch set to u/mkoeppe/make_sagelib_a_script_package

comment:5 Changed 17 months ago by mkoeppe

  • Cc mjo added
  • Commit set to 80cb1b9dc4d0658d23ff075e9354d49b6ceac356

New commits:

768a314Merge build/make/deps into build/make/Makefile.in
80cb1b9Make sagelib a script package

comment:6 Changed 17 months ago by mkoeppe

Not quite done yet. Next step is to get rid of src/Makefile.in, moving the contents into the spkg-install and spkg-clean scripts.

comment:7 follow-up: Changed 17 months ago by mkoeppe

The clean target should perhaps be transformed into setup.py clean, reference: https://github.com/pypa/setuptools/issues/1347

comment:8 Changed 17 months ago by mkoeppe

  • Authors set to Matthias Koeppe

comment:9 Changed 17 months ago by git

  • Commit changed from 80cb1b9dc4d0658d23ff075e9354d49b6ceac356 to 91e5a3dd01722c50e21dc02152316d397e70ebb9

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

91e5a3dsrc/Makefile.in: Merge into build/make/Makefile.in, build/pkgs/sagelib/spkg-install

comment:10 in reply to: ↑ 7 Changed 17 months ago by mkoeppe

Replying to mkoeppe:

The clean target should perhaps be transformed into setup.py clean, reference: https://github.com/pypa/setuptools/issues/1347

That's already noted in #21508, will not happen on the present ticket

comment:11 Changed 17 months ago by mkoeppe

TBD: sagelib rebuilds are no longer triggered because it now goes through a non-phony target

comment:12 Changed 17 months ago by git

  • Commit changed from 91e5a3dd01722c50e21dc02152316d397e70ebb9 to 08cbbf200f2c5edc1bee4cac4372335f1bdf7168

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

08cbbf2build/make/Makefile.in, build/pkgs/sagelib/dependencies: Make sure sagelib spkg-install is always run

comment:13 Changed 17 months ago by mkoeppe

  • Cc dimpase jhpalmieri added

comment:14 Changed 17 months ago by mkoeppe

  • Cc fbissey added
  • Status changed from new to needs_review

comment:15 Changed 17 months ago by fbissey

Not quite what I had in mind but this is a step in the right direction. It makes sagelib less special in the build system. I believe the dependency part is OK. The makefile bits deserve a bit scrutiny and field testing.

comment:16 Changed 17 months ago by jhpalmieri

The old src/Makefile.in and now the new build/pkgs/sagelib/spkg-install has this in it:

Hoping that #20382 will make this unnecessary.

#20382 has been merged, so perhaps this should be clarified, although not necessarily on this ticket.

comment:17 Changed 17 months ago by mkoeppe

Right, this is now #28815.

comment:18 Changed 17 months ago by git

  • Commit changed from 08cbbf200f2c5edc1bee4cac4372335f1bdf7168 to 5e915b0031876052a7138ea3c862d643bab39e29

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

5e915b0build/pkgs/sagelib/spkg-install: Update references to tickets

comment:19 Changed 17 months ago by mkoeppe

  • Dependencies changed from #29098 to #29098, #29697

comment:20 Changed 17 months ago by git

  • Commit changed from 5e915b0031876052a7138ea3c862d643bab39e29 to f6e3fa191fdcc19b6bad34a46ddaea8648b62a30

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

53f0359src/sage/env.py (sage_include_directories): Do not put SAGE_INC in front of the sage source include directories
16247adsrc/setup.py: Do not put SAGE_LOCAL/lib in front of the library directories
9a50cbasrc/sage/env.py (sage_include_directories): Fixup doctest
73fb13aMerge branch 't/29697/src_setup_py__src_sage_env_py__sage_include_directories___do_not_add_another_copy_of_sage_inc__sage_local_lib_to_include_dirs__library_dirs' into t/29411/make_sagelib_a_script_package
f6e3fa1build/pkgs/sagelib/spkg-install: Also poison environment variables SAGE_LOCAL etc.

comment:21 Changed 17 months ago by mkoeppe

Ready for review.

comment:22 Changed 17 months ago by git

  • Commit changed from f6e3fa191fdcc19b6bad34a46ddaea8648b62a30 to fcf2cabede2195ab056c9220bc4db5c3a032560b

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

fcf2cabbuild/pkgs/sagelib/spkg-install: Add comment regarding SAGE_SPKG_INST

comment:23 Changed 17 months ago by git

  • Commit changed from fcf2cabede2195ab056c9220bc4db5c3a032560b to a973fc499fa531efecffe34284701ce2d8632c52

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

a973fc4build/pkgs/sagelib/spkg-install: Also poison SAGE_PKGCONFIG, SAGE_SPKG_SCRIPTS

comment:24 Changed 17 months ago by dimpase

please remove gcc from the modified comment in src/setup.py - it's a misleading artefact from the past, where gcc was needed to build Sage.

comment:25 Changed 17 months ago by git

  • Commit changed from a973fc499fa531efecffe34284701ce2d8632c52 to c1db7ed4ac41daebbdce7d7777abe86dae96fa4a

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

c1db7edsrc/setup.py: Update comment (not specific to gcc)

comment:26 Changed 17 months ago by dimpase

What is the point of FORCE target - to always (re)build sagelib, right? But is it needed?

comment:27 Changed 17 months ago by mkoeppe

Yes, without it, sagelib would not be rebuilt when source files change.

comment:28 Changed 17 months ago by dimpase

hmm, but why not honestly list deps in a makefile?

comment:29 Changed 17 months ago by mkoeppe

Well, instead of the dependency on FORCE that is listed in build/pkgs/sagelib/dependencies, we could make it a dependency on $(SAGE_SRC) $(SAGE_SRC)/* $(SAGE_SRC)/*/* $(SAGE_SRC)/*/*/* $(SAGE_SRC)/*/*/*/*, but I don't think this is better

comment:30 Changed 17 months ago by mkoeppe

In other words, we do not have dependency tracking at all for sagelib. Neither of distutils, setuptools, cython seem to have a mechanism for generating dependencies like gcc -M. This would be certainly be nice to have; in particular if it included dependencies on "system" headers and libraries, so that sagelib could automatically rebuild when system headers/libraries change (as in, most recently, https://groups.google.com/d/msg/sage-release/A53tGGsJNLg/uA01oUugAQAJ). But that's outside of the scope of this ticket.

comment:31 Changed 17 months ago by dimpase

  • Reviewers set to Dima Pasechnik
  • Status changed from needs_review to positive_review

OK, makes sense.

comment:32 Changed 17 months ago by mkoeppe

Thanks!

comment:33 Changed 17 months ago by mkoeppe

I have created #29711 (sagelib setup.py: Dependencies on header files of packages).

comment:34 Changed 17 months ago by git

  • Commit changed from c1db7ed4ac41daebbdce7d7777abe86dae96fa4a to 6ee66dddccadf36476af10720f72cddaa158a501
  • Status changed from positive_review to needs_review

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

e1a9413Merge build/make/deps into build/make/Makefile.in
da9538eMake sagelib a script package
9fd3f7esrc/Makefile.in: Merge into build/make/Makefile.in, build/pkgs/sagelib/spkg-install
9ea77e5build/make/Makefile.in, build/pkgs/sagelib/dependencies: Make sure sagelib spkg-install is always run
c3908e9build/pkgs/sagelib/spkg-install: Update references to tickets
62f2841Merge branch 't/29697/src_setup_py__src_sage_env_py__sage_include_directories___do_not_add_another_copy_of_sage_inc__sage_local_lib_to_include_dirs__library_dirs' into t/29411/make_sagelib_a_script_package
048ce5bbuild/pkgs/sagelib/spkg-install: Also poison environment variables SAGE_LOCAL etc.
4590c4cbuild/pkgs/sagelib/spkg-install: Add comment regarding SAGE_SPKG_INST
ed312c5build/pkgs/sagelib/spkg-install: Also poison SAGE_PKGCONFIG, SAGE_SPKG_SCRIPTS
6ee66ddsrc/setup.py: Update comment (not specific to gcc)

comment:35 Changed 17 months ago by mkoeppe

  • Status changed from needs_review to positive_review

Redone on top of new version of #29098

comment:36 Changed 17 months ago by embray

Oh, I was just thinking about this. Thanks for picking up the work. This is something I've been wanting to do for years.

comment:37 Changed 17 months ago by vbraun

  • Status changed from positive_review to needs_work

I'm getting this on kucalk buildbot

make[3]: Entering directory '/var/lib/buildbot/slave/sage_git/build/build/make'
cd '/var/lib/buildbot/slave/sage_git/build' && source '/var/lib/buildbot/slave/sage_git/build/src/bin/sage-env' && source '/var/lib/buildbot/slave/sage_git/build/build/bin/sage-build-env-config' && sage-logger -p '/var/lib/buildbot/slave/sage_git/build/build/pkgs/sagelib/spkg-install' '/var/lib/buildbot/slave/sage_git/build/logs/pkgs/sagelib-9.1.rc5.log'
[sagelib-9.1.rc5] /var/lib/buildbot/slave/sage_git/build/local/bin/pkg-config: line 16: /doesnotexist/bin/pkgconf: No such file or directory
[sagelib-9.1.rc5] ************************************************************************
[sagelib-9.1.rc5] Traceback (most recent call last):
[sagelib-9.1.rc5]   File "setup.py", line 72, in <module>
[sagelib-9.1.rc5]     from module_list import ext_modules, library_order
[sagelib-9.1.rc5]   File "/var/lib/buildbot/slave/sage_git/build/src/module_list.py", line 14, in <module>
[sagelib-9.1.rc5]     cblas_pc = pkgconfig.parse('cblas')
[sagelib-9.1.rc5]   File "/var/lib/buildbot/slave/sage_git/build/local/lib/python3.7/site-packages/pkgconfig/pkgconfig.py", line 248, in parse
[sagelib-9.1.rc5]     _raise_if_not_exists(package)
[sagelib-9.1.rc5]   File "/var/lib/buildbot/slave/sage_git/build/local/lib/python3.7/site-packages/pkgconfig/pkgconfig.py", line 103, in _raise_if_not_exists
[sagelib-9.1.rc5]     raise PackageNotFoundError(package)
[sagelib-9.1.rc5] pkgconfig.pkgconfig.PackageNotFoundError: cblas not found
[sagelib-9.1.rc5] ************************************************************************
[sagelib-9.1.rc5] Error building the Sage library
[sagelib-9.1.rc5] ************************************************************************
[sagelib-9.1.rc5] Please email sage-devel (http://groups.google.com/group/sage-devel)
[sagelib-9.1.rc5] explaining the problem and including the relevant part of the log file
[sagelib-9.1.rc5]   /var/lib/buildbot/slave/sage_git/build/logs/pkgs/sagelib-9.2.beta0.log
[sagelib-9.1.rc5] Describe your computer, operating system, etc.
[sagelib-9.1.rc5] ************************************************************************
[sagelib-9.1.rc5] 
[sagelib-9.1.rc5] real	0m0.117s
[sagelib-9.1.rc5] user	0m0.100s
[sagelib-9.1.rc5] sys	0m0.012s

comment:38 Changed 17 months ago by git

  • Commit changed from 6ee66dddccadf36476af10720f72cddaa158a501 to 1cfed7c676bd72db6100d81df7f397cd8d2442f3

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

1cfed7cbuild/pkgs/sagelib/spkg-install: Do not poison SAGE_LOCAL, used in script installed by spkg pkgconf

comment:39 Changed 17 months ago by mkoeppe

  • Status changed from needs_work to needs_review

comment:40 Changed 17 months ago by mkoeppe

(simplification in follow-up ticket: #29779 pkg-config installed from SPKG pkgconf should not depend on environment variable SAGE_LOCAL)

Last edited 17 months ago by mkoeppe (previous) (diff)

comment:41 follow-up: Changed 17 months ago by dimpase

kucalk buildbot -> I read on sage-devel that people suspect it to be running with --with-python=2, which of course is meaningless with Sage 9.2+. Perhaps we should make --with-python=2 trigger an error?

comment:42 in reply to: ↑ 41 Changed 17 months ago by mkoeppe

Replying to dimpase:

kucalk buildbot -> I read on sage-devel that people suspect it to be running with --with-python=2, which of course is meaningless with Sage 9.2+. Perhaps we should make --with-python=2 trigger an error?

#29669 makes it an error already.

But the above is unrelated to python 2 vs python 3. This error showed up on systems where no system pkg-config is available. Fixed by 1cfed7c.

comment:43 Changed 17 months ago by git

  • Commit changed from 1cfed7c676bd72db6100d81df7f397cd8d2442f3 to f9a30f60f7bcd47b5e3c781f0ad625e315765cea

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

38b6bcfMerge tag '9.2.beta0' into t/29411/make_sagelib_a_script_package
f9a30f6build/pkgs/sagelib/spkg-install: Fix up error exits

comment:44 Changed 17 months ago by mkoeppe

  • Dependencies changed from #29098, #29697 to #29098, #29697, #29793
  • Status changed from needs_review to needs_work

comment:45 Changed 17 months ago by git

  • Commit changed from f9a30f60f7bcd47b5e3c781f0ad625e315765cea to 5372065710668ae1b36d059c2cbf9b6f325239c7

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

305d8cfTrac #29345: replace a bash array with something portable.
d0dff56Trac #29345: replace a few uses of "source" with "."
5ac420bTrac #29345: fix some bashisms in sage-env's resolvelinks() function.
0a61795Trac #29345: don't force SHELL=bash any longer.
5db5318Trac #29345: remove "break" statements from AC_SEARCH_LIBS.
e810ad1Trac #29345: don't use sage's config.status for the lrcalc build.
93c9921Trac #29345: replace the function that populates the CVXOPT_* variables.
0e66a0aTrac #29345: add Dima's SPKG patches for ksh compatibility.
df3f05ebuild/make/Makefile.in [SCRIPT_PACKAGE_templ]: cd into the SPKG directory; adjust spkg-install scripts
5372065Merge branch 't/29793/script_packages_should_cd_into_the_spkg_directory' into t/29411/make_sagelib_a_script_package

comment:46 Changed 17 months ago by mkoeppe

  • Status changed from needs_work to needs_review

Merged in #29793 to resolve a merge conflict. Needs review

Last edited 17 months ago by mkoeppe (previous) (diff)

comment:47 Changed 17 months ago by git

  • Commit changed from 5372065710668ae1b36d059c2cbf9b6f325239c7 to cc304711ef42f25733f1ca861801d87f58fa1118

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

cc30471build/bin/write-dockerfile.sh: Do not ADD removed file src/Makefile.in

comment:48 Changed 17 months ago by mkoeppe

Ah, that's why this was failing on GitHub runs!

comment:49 Changed 17 months ago by mkoeppe

Ready for review...

comment:50 Changed 16 months ago by dimpase

why do I see [sagelib-9.1.rc5] while building, even though

$ cat VERSION.txt 
SageMath version 9.2.beta0, Release Date: 2020-05-28

comment:51 Changed 16 months ago by mkoeppe

When this ticket is merged, src/bin/sage-update-version will update build/pkgs/sagelib/package-version.txt, where this comes from

comment:52 Changed 16 months ago by dimpase

  • Status changed from needs_review to positive_review

OK

comment:53 Changed 16 months ago by mkoeppe

Thanks!

comment:54 Changed 16 months ago by dimpase

  • Branch changed from u/mkoeppe/make_sagelib_a_script_package to public/make_sagelib_a_script_package
  • Commit changed from cc304711ef42f25733f1ca861801d87f58fa1118 to 5790687b9dbf244e1c6d49b42481d552e2d5537f

rebased over rebased #29793


Last 10 new commits:

aa206bfsrc/Makefile.in: Merge into build/make/Makefile.in, build/pkgs/sagelib/spkg-install
e052741build/make/Makefile.in, build/pkgs/sagelib/dependencies: Make sure sagelib spkg-install is always run
1e19861build/pkgs/sagelib/spkg-install: Update references to tickets
d6ee516build/pkgs/sagelib/spkg-install: Also poison environment variables SAGE_LOCAL etc.
8a4b60dbuild/pkgs/sagelib/spkg-install: Add comment regarding SAGE_SPKG_INST
4071eebbuild/pkgs/sagelib/spkg-install: Also poison SAGE_PKGCONFIG, SAGE_SPKG_SCRIPTS
1707161src/setup.py: Update comment (not specific to gcc)
985bfcabuild/pkgs/sagelib/spkg-install: Do not poison SAGE_LOCAL, used in script installed by spkg pkgconf
b212ce8build/pkgs/sagelib/spkg-install: Fix up error exits
5790687build/bin/write-dockerfile.sh: Do not ADD removed file src/Makefile.in

comment:55 Changed 16 months ago by mkoeppe

  • Branch changed from public/make_sagelib_a_script_package to u/mkoeppe/make_sagelib_a_script_package
  • Commit changed from 5790687b9dbf244e1c6d49b42481d552e2d5537f to cc304711ef42f25733f1ca861801d87f58fa1118

comment:56 Changed 16 months ago by git

  • Commit changed from cc304711ef42f25733f1ca861801d87f58fa1118 to 539c1828dd17c90e5baa63d814f2c84419146f90
  • Status changed from positive_review to needs_review

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

539c182build/make/install: Do not depend on src/bin/sage-version.sh

comment:57 Changed 16 months ago by mkoeppe

  • Status changed from needs_review to positive_review

Pushed to wrong ticket, sorry


New commits:

539c182build/make/install: Do not depend on src/bin/sage-version.sh
Version 0, edited 16 months ago by mkoeppe (next)

comment:58 Changed 16 months ago by git

  • Commit changed from 539c1828dd17c90e5baa63d814f2c84419146f90 to cc304711ef42f25733f1ca861801d87f58fa1118
  • Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. This was a forced push. New commits:

comment:59 Changed 16 months ago by mkoeppe

  • Status changed from needs_review to positive_review

comment:60 Changed 16 months ago by vbraun

  • Branch changed from u/mkoeppe/make_sagelib_a_script_package to cc304711ef42f25733f1ca861801d87f58fa1118
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:61 Changed 15 months ago by dimpase

  • Commit cc304711ef42f25733f1ca861801d87f58fa1118 deleted

this has caused a breakage of jupyter notebook/server. Apparently some of its dependencies get "blessed" by ./sage -b, but no longer get blessed by make build.


Running 'make build' on that ticket's commit in the develop branch (0bcd001f760ff724832c32b4f65fa575fd82e000 make sagelib a script package) produces the error for me whereas doing so on the immediately previous commit (10fb62408a9761bf91a95d9eb7d7f16a104924c7 Trac #29289: Remove support for installing old-style SPKGs, deprecated in Sage 6.9) does not. Interestingly, it's been sort of hit-and-miss on whether the error shows up when doing just a './sage -b'.

On Saturday, July 11, 2020 at 5:26:02 PM UTC-7, Paul Masson wrote:

Just built a ticket based on 9.2.beta3 and the same problem occurs.

On Saturday, July 11, 2020 at 4:09:56 PM UTC-7, Paul Masson wrote:

After a successful incremental build from 9.2.beta2, trying to start a new notebook results in the browser console error "Failed to load resource: the server responded with a status of 404 (Not Found)" when trying to fetch Mathjax from http://localhost:8888/nbextensions/mathjax/MathJax.js. Has anyone other than Dima seen this also?

This appears to be a serious server configuration error. I don't see any ticket in either this beta or the previous one that would cause this. Am I missing some new configuration step? Sage built and runs just fine.

comment:62 Changed 15 months ago by mkoeppe

This is now #30123

comment:63 Changed 15 months ago by jhpalmieri

This broke sage -b: it wants to run make in a directory with no Makefile.

Note: See TracTickets for help on using tickets.