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:

Status badges

Description (last modified by jdemeyer)

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 changed from 7100fa7c4e530c2e56656dc7f19541b2882954ab to ca7bfae4de3f00e828d6ee3eea9483e88cb26e19

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 jdemeyer

  • Description modified (diff)

comment:3 follow-up: Changed 3 years ago by embray

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 jdemeyer

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 follow-up: Changed 3 years ago by embray

  • 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 embray

  • Status changed from needs_review to positive_review

comment:7 in reply to: ↑ 5 Changed 3 years ago by jdemeyer

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 jdemeyer

  • 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 vbraun

  • Branch changed from u/jdemeyer/ticket-28349 to ca7bfae4de3f00e828d6ee3eea9483e88cb26e19
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.