Opened 20 months ago
Closed 11 months ago
#28349 closed defect (duplicate)
Problem with SAGE_INC in module_list.py when using system packages
Reported by: | embray | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-duplicate/invalid/wontfix |
Component: | build | Keywords: | |
Cc: | dimpase | Merged in: | |
Authors: | Erik Bray | Reviewers: | Dima Pasechnik |
Report Upstream: | N/A | Work issues: | |
Branch: | u/embray/ticket-28349 (Commits, GitHub, GitLab) | Commit: | c775407d82a013e9c3b9ac72fe2f73f0d3d96527 |
Dependencies: | Stopgaps: |
Description (last modified by )
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...)
Change History (21)
comment:1 Changed 20 months ago by
- Description modified (diff)
comment:2 Changed 20 months ago by
comment:3 follow-up: ↓ 6 Changed 20 months ago by
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).
comment:4 Changed 20 months ago by
comment:5 Changed 20 months ago by
- Branch set to u/embray/ticket-28349
- Commit set to c775407d82a013e9c3b9ac72fe2f73f0d3d96527
comment:6 in reply to: ↑ 3 ; follow-up: ↓ 14 Changed 20 months ago by
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.
comment:7 Changed 20 months ago by
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).
comment:8 follow-up: ↓ 16 Changed 20 months ago by
+from sage.env import SAGE_LOCAL, SAGE_INC
would surely be inconvenient for distributions that don't use sage.env
.
comment:9 Changed 20 months ago by
- Branch changed from u/embray/ticket-28349 to u/jdemeyer/ticket-28349
comment:10 Changed 20 months ago by
- Commit changed from c775407d82a013e9c3b9ac72fe2f73f0d3d96527 to 7100fa7c4e530c2e56656dc7f19541b2882954ab
comment:11 Changed 20 months ago by
- Branch changed from u/jdemeyer/ticket-28349 to u/embray/ticket-28349
- Commit changed from 7100fa7c4e530c2e56656dc7f19541b2882954ab to c775407d82a013e9c3b9ac72fe2f73f0d3d96527
comment:12 Changed 20 months ago by
Heh, perhaps we can merge these two fixes? I think Jeroen's branch makes sense in its own right.
comment:13 Changed 20 months ago by
I created #28352 with my fix.
comment:14 in reply to: ↑ 6 Changed 20 months ago by
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
comment:15 Changed 20 months ago by
The gd
library does depend on the png
library, but that's independent of the header files.
comment:16 in reply to: ↑ 8 Changed 20 months ago by
Replying to dimpase:
+from sage.env import SAGE_LOCAL, SAGE_INCwould 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
.
comment:17 Changed 16 months ago by
- Milestone changed from sage-8.9 to sage-9.1
Ticket retargeted after milestone closed
comment:18 Changed 12 months ago by
- Milestone changed from sage-9.1 to sage-duplicate/invalid/wontfix
- Status changed from new to needs_review
If I'm not mistaken, this was fixed by #28352?
comment:19 Changed 12 months ago by
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
comment:20 Changed 12 months ago by
- Reviewers set to Dima Pasechnik
- Status changed from needs_review to positive_review
comment:21 Changed 11 months ago by
- Resolution set to duplicate
- Status changed from positive_review to closed
good catch. I noticed this too, and was wondering what it is...