Run sage -ba instead of sage -b after upgrading Cython

[85eec1a4-3d04-4b4d-b711-d4db03337c41]

We should really run sage -ba when we upgrade Cython and not just sage -b.

CC: @robertwb

Component: build

Keywords: upgrade cython

Reviewer: Jeroen Demeyer

Issue created by migration from https://trac.sagemath.org/ticket/4797

[85eec1a4-3d04-4b4d-b711-d4db03337c41]

Description changed:

--- 
+++ 
@@ -1,4 +1,4 @@
-When upgrading Cython like at #4639 we should really run a -ba on upgrade and not just a -b since the new Cython version in this case does fix some fundamental issues the way exceptions are handled. In general I would be sleep much better if we do this in general since many potentially odd Heisenbugs that disappear after either a partial -b or a -ba would be avoided that way.
+When upgrading Cython like at #4639 we should really run a -ba on upgrade and not just a -b since the new Cython version in this case does fix some fundamental issues the way exceptions are handled. In general I would sleep much better if we do this in general since many potentially odd Heisenbugs that disappear after either a partial -b or a -ba would be avoided that way if a new Cython version is involved.

 Cheers,

[85eec1a4-3d04-4b4d-b711-d4db03337c41]

comment:3

I can live with this issue being fixed in 3.3 since we will not upgrade Cython in 3.2.2->3.2.3.

Cheers,

Michael

[85eec1a4-3d04-4b4d-b711-d4db03337c41]

comment:4

Ooops. Reassigned this time :)

Cheers,

Michael

[williamstein]

comment:5

Does anybody have any idea how to implement this? Here is one idea. We make it so there is a command like "sage -ba" that doesn't actually rebuild the sage library, then we make the cython spkg-install call that command. It could be called "sage -ba_nobuild" or something. This is way better, I think, than "sage -ba" trying to detect if cython was upgraded.

The disadvantage is that it might make testing installing the cython SPKG inconvenient.

[robertwb]

comment:6

Yes, this would make testing Cython spkgs a major pain. I think this probably best belongs in the upgrade script--it could touch all .pyx files after upgrading the Cython script.

[85eec1a4-3d04-4b4d-b711-d4db03337c41]

comment:7

Yes and no. When I test Cython releases I delete the build tree and then do a -ba anyway since that is the only reliable way to test. Obviously if someone is testing "just" the spkg this ought to be not enforced, so RobertWB's idea seems the way to go.

Cheers,

Michael

[robertwb]

comment:8

Yep, when you test a Cython release (assuming I've done my job) it should just work. That's different when I'm hunting down a bug and want to keep re-compiling a certain file (e.g. that last memory leak).

[williamstein]

comment:9

If we've released for months and months without fixing this, it doesn't make sense to keep it as a blocker.

[jdemeyer]

Changed keywords from none to upgrade cython

[jdemeyer]

Description changed:

--- 
+++ 
@@ -1,5 +1,3 @@
-When upgrading Cython like at #4639 we should really run a -ba on upgrade and not just a -b since the new Cython version in this case does fix some fundamental issues the way exceptions are handled. In general I would sleep much better if we do this in general since many potentially odd Heisenbugs that disappear after either a partial -b or a -ba would be avoided that way if a new Cython version is involved.
+We should really run `sage -ba` on upgrade and not just `sage -b`.  This is for example needed after the numpy upgrade in sage-4.6.1.alpha0, see #9808.

-Cheers,
-
-Michael
+(note: earlier versions of this ticket mentioned `sage -ba` only in the case of a *cython* upgrade, but it's probably better to do `sage -ba` always after upgrading -- Jeroen Demeyer).

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:11

This is just due to missing dependencies in module_list.py, so if everybody updating spkgs carefully checked them and added missing ones, this would be a non-issue. (And I consider it as such. Perhaps worth a work-around hint in the Developer's and / or Installation Guide.)

At least for upgrades to final versions, this should IMHO never be necessary.

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:12

P.S.: Explicitly touching some files in spkg-install that are contained in the extension modules' include_dirs can also avoid sage -ba, though of course a hack.

[jdemeyer]

Description changed:

--- 
+++ 
@@ -1,3 +1 @@
-We should really run `sage -ba` on upgrade and not just `sage -b`.  This is for example needed after the numpy upgrade in sage-4.6.1.alpha0, see #9808.
-
-(note: earlier versions of this ticket mentioned `sage -ba` only in the case of a *cython* upgrade, but it's probably better to do `sage -ba` always after upgrading -- Jeroen Demeyer).
+We should really run `sage -ba` when we upgrade Cython and not just `sage -b`.

[jdemeyer]

comment:13

Replying to @nexttime:

This is just due to missing dependencies in module_list.py, so if everybody updating spkgs carefully checked them and added missing ones, this would be a non-issue. (And I consider it as such. Perhaps worth a work-around hint in the Developer's and / or Installation Guide.)

I created ticket #10124 to implement this.

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:14

Replying to @jdemeyer:

I created ticket #10124 to implement this.

10214

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:15

Running sage -ba explicitly is not even necessary upon (true) Cython upgrades, though we could implement this in spkg/install.

We just have to make any Cython file / extension module depend on a single, distinct file of the Cython distribution (e.g. header) and preferably make sure this is only touched when really necessary.

Therefore it would make sense to use a Sage-specific file for such, which is created and managed by our spkg-install for Cython.

People upgrading the Cython package will best know if a complete rebuild will be necessary, depending on the Cython version found in the current installation subject to upgrade.

[jdemeyer]

Reviewer: Jeroen Demeyer

[jdemeyer]

comment:16

Closing this since I haven't seen this problem at all recently.