Opened 3 years ago
Closed 3 years ago
#28352 closed defect (fixed)
Fix dependencies on png.h, m4ri.h, m4rie.h
Reported by: | jdemeyer | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.9 |
Component: | build | Keywords: | |
Cc: | dimpase, embray | Merged in: | |
Authors: | Jeroen Demeyer | Reviewers: | Erik Bray |
Report Upstream: | N/A | Work issues: | |
Branch: | ca7bfae (Commits, GitHub, GitLab) | Commit: | ca7bfae4de3f00e828d6ee3eea9483e88cb26e19 |
Dependencies: | Stopgaps: |
Description (last modified by )
The following dependencies were incorrect:
sage.matrix.matrix_mod2_dense
does not usepng.h
sage.modules.vector_mod2_dense
does not usepng.h
sage.matrix.matrix_rational_dense
does not usem4ri.h
The following dependencies are in Cython files, so Cython picks them up automatically:
sage.matrix.matrix_mod2_dense dependency
onm4ri.h
sage.modules.vector_mod2_dense dependency
onm4ri.h
sage.matrix.matrix_gf2e_dense dependency
onm4rie.h
The following include and dependency was removed:
pb_wrap.h
includedm4ri/m4ri.h
for no apparent reason, everything still works after removing that include (and therefore also the dependency)
Change History (9)
comment:1 Changed 3 years ago by
- Commit changed from 7100fa7c4e530c2e56656dc7f19541b2882954ab to ca7bfae4de3f00e828d6ee3eea9483e88cb26e19
comment:2 Changed 3 years ago by
- Description modified (diff)
comment:3 follow-up: ↓ 4 Changed 3 years ago by
If Cython can pick up such header dependencies automatically, why do we still also need the depends
for sage.rings.polynomial.pbori
?
comment:4 in reply to: ↑ 3 Changed 3 years ago by
Replying to embray:
If Cython can pick up such header dependencies automatically, why do we still also need the
depends
forsage.rings.polynomial.pbori
?
Cython picks up dependencies automatically if they appear in a cdef extern from "header.h"
But in this case, the dependency is through pb_wrap.h
.
comment:5 follow-up: ↓ 7 Changed 3 years ago by
- Reviewers set to Erik Bray
- Status changed from new to needs_review
I see. I suppose in principle one could put an empty cdef extern from "polybori/polybori.h"
in the Cython file then. I would still like to see the use of SAGE_INC
there go away, but that can be handled in #28349 or a different ticket.
comment:6 Changed 3 years ago by
- Status changed from needs_review to positive_review
comment:7 in reply to: ↑ 5 Changed 3 years ago by
Replying to embray:
I suppose in principle one could put an empty
cdef extern from "polybori/polybori.h"
in the Cython file then.
Indeed, I was thinking the same thing.
comment:8 Changed 3 years ago by
- Summary changed from Fix dependencies on png.h and m4ri.h to Fix dependencies on png.h, m4ri.h, m4rie.h
comment:9 Changed 3 years ago by
- Branch changed from u/jdemeyer/ticket-28349 to ca7bfae4de3f00e828d6ee3eea9483e88cb26e19
- Resolution set to fixed
- Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
Fix dependencies on png.h, m4ri.h, m4rie.h