Problem with SAGE_INC in module_list.py when using system packages

[embray]

I noticed in a recent build of Sage on a Debian buster VM, using many system packages, a problem:

Every time I try to re-build, whether I run make build, sage -b, etc. two Python modules get recompiled (from their Cython-generated C sources, though Cython itself is not being re-run). Specifically,

src/build/cythonized/sage/modules/vector_mod2_dense.c
src/build/cythonized/sage/matrix/matrix_mod2_dense.c

One of the things that these two modules have in common, and unique from other modules (as listed in src/module_list.py) is their dependency on SAGE_INC + '/png.h'. But since I'm using the system libpng, this file does not exist, so those modules constantly get rebuilt.

I'm not really sure the best thing to do about this right now. Maybe we should only add those headers to the depends list if they actually exist (and just assume that system headers won't be updated often...)

CC: @dimpase

Component: build

Author: Erik Bray

Branch/Commit: u/embray/ticket-28349 @ c775407

Reviewer: Dima Pasechnik

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

[dimpase]

comment:2

good catch. I noticed this too, and was wondering what it is...

[jdemeyer]

comment:3

This file doesn't actually depend on png.h in any way that I can see, so we should just drop that dependency (regardless of whether it fixes the issue on this ticket).

[jdemeyer]

Author: Jeroen Demeyer

[embray]

Branch: u/embray/ticket-28349

[embray]

New commits:

ec2a458	Trac #28349: Move include files from Extension.depends into kwds
c775407	Trac #28349: Don't explicit use SAGE_INC for most header paths

[embray]

Changed author from Jeroen Demeyer to Erik Bray

[embray]

Commit: c775407

[embray]

comment:6

Replying to @jdemeyer:

This file doesn't actually depend on png.h in any way that I can see, so we should just drop that dependency (regardless of whether it fixes the issue on this ticket).

It does depend indirectly through gd.h, which it does depend on directly. But I agree it's a bit odd to include png.h in that list explicitly.

[embray]

comment:7

Sorry--pressed submit without seeing you had added to this. Maybe you were going to change something?

Nevertheless, the problem is still more general than png.h. It would be better not to use SAGE_INC in this way (e.g. the same problem applies for m4ri, I just don't have a system package for that yet).

[dimpase]

comment:8

+from sage.env import SAGE_LOCAL, SAGE_INC

would surely be inconvenient for distributions that don't use sage.env.

[jdemeyer]

Changed branch from u/embray/ticket-28349 to u/jdemeyer/ticket-28349

[embray]

Changed commit from c775407 to 7100fa7

[embray]

comment:10

Apparently in the depends = ['png.h] traces back to #5265, and didn't really make sense then either. When vector_mod2_dense was added it was just carried over as cargo cult.

---

New commits:

7100fa7

Fix dependencies on png.h and m4ri.h

[jdemeyer]

New commits:

ec2a458	Trac #28349: Move include files from Extension.depends into kwds
c775407	Trac #28349: Don't explicit use SAGE_INC for most header paths

[jdemeyer]

Changed branch from u/jdemeyer/ticket-28349 to u/embray/ticket-28349

[jdemeyer]

Changed commit from 7100fa7 to c775407

[embray]

comment:12

Heh, perhaps we can merge these two fixes? I think Jeroen's branch makes sense in its own right.

[jdemeyer]

comment:13

I created #28352 with my fix.

[jdemeyer]

comment:14

Replying to @embray:

It does depend indirectly through gd.h, which it does depend on directly.

My gd.h does not depend on png.h

[jdemeyer]

comment:15

The gd library does depend on the png library, but that's independent of the header files.

[kiwifb]

comment:16

Replying to @dimpase:

+from sage.env import SAGE_LOCAL, SAGE_INC

would surely be inconvenient for distributions that don't use sage.env.

Some of us are phasing out sage-env but we do love sage.env.

[embray]

comment:17

Ticket retargeted after milestone closed

[mkoeppe]

comment:18

If I'm not mistaken, this was fixed by #28352?

[dimpase]

comment:19

this one has been fixed, but similar problems pop up elsewhere, e.g. I saw similar parasite rebuilds with
the branch of #29369 (currently disabled) - apparently

    Extension('sage.rings.polynomial.pbori',
              sources = ['sage/rings/polynomial/pbori.pyx'],
              libraries=['brial', 'brial_groebner'] + m4ri_libs + png_libs,
              library_dirs = m4ri_library_dirs + png_library_dirs,
              include_dirs = m4ri_include_dirs + png_include_dirs,
              depends = [SAGE_INC + "/polybori/" + hd + ".h" for hd in ["polybori", "config"]],
              extra_compile_args = m4ri_extra_compile_args),

is to blame

[dimpase]

Reviewer: Dima Pasechnik