Opened 3 years ago

Closed 3 years ago

#28352 closed defect (fixed)

Fix dependencies on png.h, m4ri.h, m4rie.h

Reported by: Jeroen Demeyer Owned by:
Priority: major Milestone: sage-8.9
Component: build Keywords:
Cc: Dima Pasechnik, Erik Bray Merged in:
Authors: Jeroen Demeyer Reviewers: Erik Bray
Report Upstream: N/A Work issues:
Branch: ca7bfae (Commits, GitHub, GitLab) Commit: ca7bfae4de3f00e828d6ee3eea9483e88cb26e19
Dependencies: Stopgaps:

Status badges

Description (last modified by Jeroen Demeyer)

The following dependencies were incorrect:

  • sage.matrix.matrix_mod2_dense does not use png.h
  • sage.modules.vector_mod2_dense does not use png.h
  • sage.matrix.matrix_rational_dense does not use m4ri.h

The following dependencies are in Cython files, so Cython picks them up automatically:

  • sage.matrix.matrix_mod2_dense dependency on m4ri.h
  • sage.modules.vector_mod2_dense dependency on m4ri.h
  • sage.matrix.matrix_gf2e_dense dependency on m4rie.h

The following include and dependency was removed:

  • pb_wrap.h included m4ri/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 git

Commit: 7100fa7c4e530c2e56656dc7f19541b2882954abca7bfae4de3f00e828d6ee3eea9483e88cb26e19

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

ca7bfaeFix dependencies on png.h, m4ri.h, m4rie.h

comment:2 Changed 3 years ago by Jeroen Demeyer

Description: modified (diff)

comment:3 Changed 3 years ago by Erik Bray

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 Jeroen Demeyer

Replying to embray:

If Cython can pick up such header dependencies automatically, why do we still also need the depends for sage.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 Changed 3 years ago by Erik Bray

Reviewers: Erik Bray
Status: newneeds_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 Erik Bray

Status: needs_reviewpositive_review

comment:7 in reply to:  5 Changed 3 years ago by Jeroen Demeyer

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 Jeroen Demeyer

Summary: Fix dependencies on png.h and m4ri.hFix dependencies on png.h, m4ri.h, m4rie.h

comment:9 Changed 3 years ago by Volker Braun

Branch: u/jdemeyer/ticket-28349ca7bfae4de3f00e828d6ee3eea9483e88cb26e19
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.