p_group_cohomology missing dependencies

[jdemeyer]

The API change of coercion_model (#27021) broke p_group_cohomology.

• patchbot report: ​https://patchbot.sagemath.org/log/27152/Ubuntu/18.04/x86_64/4.15.0-38-generic/quasar/2019-01-29%2023:36:43

Rebuilding p_group_cohomology is sufficient (an analogous problem occurred with giacpy_sage on #27021).

As experimental fix, we make p_group_cohomology depend on element.pxd and cython.

[jdemeyer]

New commits:

​eb9a9ff

Add missing dependencies of p_group_cohomology

[SimonKing]

Replying to jdemeyer:

The fix is simple: rebuilding p_group_cohomology is sufficient

That's actually good news. When you wrote "API change broke the spkg", I thought I need to change code again (by the way, I am not sure: What is the current official spkg version in Sage? v3.1?).

(an analogous problem occurred with giacpy_sage on #27021). There is no automatic way to do this, but because of the missing dependency on cython (which is what I grepped for) I missed this.

The current dependencies are

singular meataxe | matplotlib gap xz $(SAGERUNTIME)

Really the package does depend on sagelib. Namely, it cimports various Sage types (including Element, Parent).

In an ideal world, it would be possible to have a dependency list

singular meataxe sage.structure.element | matplotlib gap xz $(SAGERUNTIME)

[SimonKing]

I hate the trac interface. Writing my comment destroyed your branch.

Can you provide the branch again?

[chapoton]

New commits:

​eb9a9ff

Add missing dependencies of p_group_cohomology

[jdemeyer]

Replying to SimonKing:

Replying to jdemeyer:

The fix is simple: rebuilding p_group_cohomology is sufficient

That's actually good news. When you wrote "API change broke the spkg", I thought I need to change code again

No, what really happened is an incompatible change in element.pxd which means that every Cython file cimporting that must be recompiled.

[jdemeyer]

Replying to SimonKing:

In an ideal world, it would be possible to have a dependency list

singular meataxe sage.structure.element | matplotlib gap xz $(SAGERUNTIME)

Actually, why don't we just do that?

[SimonKing]

Replying to jdemeyer:

Replying to SimonKing:

In an ideal world, it would be possible to have a dependency list

singular meataxe sage.structure.element | matplotlib gap xz $(SAGERUNTIME)

Actually, why don't we just do that?

Well, when I wrote my comment, I thought that it isn't supported yet.

So, are you saying that a dependency on specific parts of the Sage library is possible? Or would that mean to extend Sage's build system?

[git]

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

​0b3ce61

Add missing dependencies of p_group_cohomology

[jdemeyer]

Note that this won't pick up transitive dependencies (the Makefile doesn't know that element.pxd depends on sage_object.pxd for example), but at least this should work when changing the file element.pxd.

[jdemeyer]

Replying to SimonKing:

So, are you saying that a dependency on specific parts of the Sage library is possible?

It's a Makefile, so it is possible to depend on specific filenames. It just never occurred to me to actually do that.

[SimonKing]

Replying to jdemeyer:

Replying to SimonKing:

So, are you saying that a dependency on specific parts of the Sage library is possible?

It's a Makefile, so it is possible to depend on specific filenames. It just never occurred to me to actually do that.

Nice! I thought the dependencies file may only contain package names.

Also good that you add pip - indeed spkg-install uses pip.

But how to test it? Pull the branch, build sage, touch element.pxd, do make and see if the spkg gets rebuilt?

[SimonKing]

I hate the trac interface.

When I write a comment, I certainly don't want that trac changes the ticket description back to what it was when I started to write my comment!

[jdemeyer]

Replying to SimonKing:

But how to test it? Pull the branch, build sage, touch element.pxd, do make and see if the spkg gets rebuilt?

Yes, something like that.

[SimonKing]

I think it would be possible to be a little more thorough. Here is the list of all cimports in the spkg that get stuff from outside the spkg (hence, that's dependencies):

src/pGroupCohomology/cochain.pxd:from sage.matrix.matrix_gfpn_dense cimport Matrix_gfpn_dense as MTX
src/pGroupCohomology/cochain.pxd:from sage.libs.meataxe cimport *
src/pGroupCohomology/cochain.pxd:from sage.structure.element cimport RingElement, ModuleElement, Element
src/pGroupCohomology/cochain.pxd:from sage.rings.morphism cimport RingHomomorphism
src/pGroupCohomology/cochain.pyx:from sage.structure.element cimport RingElement
src/pGroupCohomology/cochain.pyx:from sage.matrix.matrix_gfpn_dense cimport new_mtx
src/pGroupCohomology/cochain.pyx:from libc.string cimport memcpy
src/pGroupCohomology/cochain.pyx:from cysignals.signals cimport sig_check, sig_on, sig_off
src/pGroupCohomology/cohomology.pyx:from libc.string cimport memcpy
src/pGroupCohomology/cohomology.pyx:from sage.matrix.matrix_gfpn_dense cimport Matrix_gfpn_dense as MTX
src/pGroupCohomology/cohomology.pyx:from sage.matrix.matrix_gfpn_dense cimport new_mtx
src/pGroupCohomology/cohomology.pyx:from sage.libs.meataxe cimport *
src/pGroupCohomology/modular_cohomology.pyx:from sage.libs.meataxe cimport *
src/pGroupCohomology/modular_cohomology.pyx:from sage.matrix.matrix_gfpn_dense cimport Matrix_gfpn_dense as MTX
src/pGroupCohomology/modular_cohomology.pyx:from sage.matrix.matrix_gfpn_dense cimport new_mtx
src/pGroupCohomology/resolution_bindings.pxd:from sage.libs.meataxe cimport *
src/pGroupCohomology/resolution.pxd:from sage.libs.meataxe cimport *
src/pGroupCohomology/resolution.pxd:from sage.matrix.matrix_gfpn_dense cimport Matrix_gfpn_dense as MTX
src/pGroupCohomology/resolution.pyx:from libc.string cimport memcpy
src/pGroupCohomology/resolution.pyx:from cysignals.memory cimport sig_free
src/pGroupCohomology/resolution.pyx:from cysignals.signals cimport sig_check, sig_on, sig_off
src/pGroupCohomology/resolution.pyx:from sage.matrix.matrix_gfpn_dense cimport Matrix_gfpn_dense as MTX
src/pGroupCohomology/resolution.pyx:from sage.matrix.matrix_gfpn_dense cimport FieldConverter_class, FieldConverter, new_mtx
src/pGroupCohomology/resolution.pyx:from sage.matrix.matrix0 cimport Matrix as Matrix0

So, the complete lists of dependencies is

sage.matrix.matrix_gfpn_dense
sage.libs.meataxe
sage.structure.element
sage.rings.morphism
libc.string
cysignals.signals
cysignals.memory
sage.matrix.matrix0

So, I suggest to include all this in the list of dependencies. Which gives rise to the questions:

• Has the dependency list be in a single line, or can it be one dependency per line? Would look nicer!
• What to do about libc and cysignals? Is that covered by the dependencies python and cython?

[slelievre]

Replying to SimonKing:

Must the dependency list be in a single line, or can it be one dependency per line? Would look nicer!

Only the first line of the dependencies file is read, so yes, the dependencies must all be listed in the first line of that file.

[SimonKing]

I have added all dependencies in sagelib, i.e., all pxd-files from which p_group_cohomology cimports.

So, what's missing is libc and cysignals. How to name them in the dependencies?

---

New commits:

​46b2823

Add further p_group_cohomology dependencies from sagelib

[jdemeyer]

Replying to SimonKing:

So, the complete lists of dependencies is

sage.matrix.matrix_gfpn_dense
sage.libs.meataxe
sage.structure.element
sage.rings.morphism
libc.string
cysignals.signals
cysignals.memory
sage.matrix.matrix0

No, that's not the complete list of dependencies. Like I said, you should also consider implied dependencies: for example, sage.structure.element depends on sage.structure.parent and so on. So my choice of sage.structure.element and sage.matrix.matrix_gfpn_dense was meant to be a reasonably small but meaningful set of dependencies.

For cysignals, just add a dependency on the cysignals package.

[SimonKing]

Replying to jdemeyer:

No, that's not the complete list of dependencies. Like I said, you should also consider implied dependencies:

I understood that. But I think to the very least one should name all pxd files from which the package cimports. Everything else would be too short.

For cysignals, just add a dependency on the cysignals package.

Thanks. And for libc?

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​f39c9f1

Add cysignals to p_group_cohomology's dependencies list

[SimonKing]

I have added cysignals to the dependencies. What about libc? In any case, it is still "needs review", and since I added stuff (after searching for all cimports), I guess I qualify as one author.

[jdemeyer]

Replying to SimonKing:

What about libc?

That's just the system C library on which virtually everything depends. It's assumed to remain ABI-compatible, so recompiling isn't needed.