sagemath/sage

make sagelib a script package

mkoeppe opened this issue · 78 comments

... 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

Depends on #29098
Depends on #29697
Depends on #29793

CC: @embray @videlec @orlitzky @dimpase @jhpalmieri @kiwifb

Component: build

Author: Matthias Koeppe

Branch: cc30471

Reviewer: Dima Pasechnik

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

Description changed:

--- 
+++ 
@@ -1 +1,10 @@
+... 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:
+- https://groups.google.com/d/msg/sage-devel/nTwhCV89FXE/_7GdzGy4BgAJ ("package sources")
+- https://groups.google.com/d/msg/sage-devel/nTwhCV89FXE/-obd4rMDBAAJ
+ 
+Related tickets:
+- #29386 Install script packages via sage-spkg
+
+

Changed dependencies from #29287 to #29098

Commit: 80cb1b9

New commits:

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

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

The clean target should perhaps be transformed into setup.py clean, reference: pypa/setuptools#1347

Author: Matthias Koeppe

Changed commit from 80cb1b9 to 91e5a3d

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

Replying to @mkoeppe:

The clean target should perhaps be transformed into setup.py clean, reference: pypa/setuptools#1347

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

comment:11

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

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

Changed commit from 91e5a3d to 08cbbf2

comment:15

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

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

Right, this is now #28815.

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

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

Changed commit from 08cbbf2 to 5e915b0

Changed dependencies from #29098 to #29098, #29697

Changed commit from 5e915b0 to f6e3fa1

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

Ready for review.

Changed commit from f6e3fa1 to fcf2cab

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

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

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

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

Changed commit from fcf2cab to a973fc4

comment:24

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.

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

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

Changed commit from a973fc4 to c1db7ed

comment:26

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

comment:27

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

comment:28

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

comment:29

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

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.

Reviewer: Dima Pasechnik

comment:31

OK, makes sense.

comment:32

Thanks!

comment:33

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

Changed commit from c1db7ed to 6ee66dd

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

Redone on top of new version of #29098

comment:36

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

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

Changed commit from 6ee66dd to 1cfed7c

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:40

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

comment:41

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

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.

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

Changed commit from 1cfed7c to f9a30f6

Changed dependencies from #29098, #29697 to #29098, #29697, #29793

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

Changed commit from f9a30f6 to 5372065

comment:46

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

Changed commit from 5372065 to cc30471

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

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

comment:49

Ready for review...

comment:50

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

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

comment:52

OK

comment:53

Thanks!

comment:54

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

Changed commit from cc30471 to 5790687

Changed commit from 5790687 to cc30471

Changed commit from cc30471 to 539c182

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

Pushed to wrong ticket, sorry

Changed commit from 539c182 to cc30471

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

comment:61

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 (0bcd001 make sagelib a script package) produces the error for me whereas doing so on the immediately previous commit (10fb624 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.

Changed commit from cc30471 to none

comment:62

This is now #30123

comment:63

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