## #20210 closed enhancement (fixed)

# Move memory functions to cysignals

Reported by: | Jeroen Demeyer | Owned by: | |
---|---|---|---|

Priority: | major | Milestone: | sage-7.1 |

Component: | packages: standard | Keywords: | |

Cc: | Luca De Feo, Martin Albrecht | Merged in: | |

Authors: | Jeroen Demeyer | Reviewers: | Martin Albrecht |

Report Upstream: | N/A | Work issues: | |

Branch: | 4bb8337 (Commits, GitHub, GitLab) | Commit: | |

Dependencies: | Stopgaps: |

### Description (last modified by )

Move all memory-allocation functions like `sage_malloc`

to cysignals (renaming `sage_`

-> `sig_`

).

**Upstream**: https://github.com/sagemath/cysignals/releases/download/1.1.0/cysignals-1.1.0.tar.bz2

### Change History (20)

### comment:1 Changed 7 years ago by

Description: | modified (diff) |
---|

### comment:2 Changed 7 years ago by

Branch: | → u/jdemeyer/cysignals_memory |
---|

### comment:3 follow-up: 4 Changed 7 years ago by

Commit: | → 4bb8337295ed82c93d1a9c9f7173a4c36f97151d |
---|---|

Status: | new → needs_review |

### comment:4 Changed 7 years ago by

For easier reviewing, I separated the commits:

03458ea `Upgrade cysignals package`

This is literally just the upgrade of the package.

dce67fc `Move memory functions to cysignals`

This contains all changes to the Sage library (mostly changing include paths), except for renaming `sage_malloc`

-> `sig_malloc`

and similar.

4bb8337 `Rename sage_malloc -> sig_malloc and friends`

This was made with a simple `sed`

script, it deals only with renaming `sage_malloc`

-> `sig_malloc`

and similar.

### comment:5 follow-up: 6 Changed 7 years ago by

Why do we have to `include "cysignals/memory.pxi"`

and cannot `cimport`

? Patch looks good, but I didn't check all lines of 4bb8337.

### comment:6 Changed 7 years ago by

Replying to malb:

Why do we have to

`include "cysignals/memory.pxi"`

and cannot`cimport`

?

Because `cysignals/memory.pxi`

includes `cysignals/signals.pxi`

so it's really by transitivity.

### comment:7 Changed 7 years ago by

I am currently in Orsay discussing with Erik Bray and Nicolas Thiéry to see if we can use `cimport`

instead of `include`

for `cysignals/signals.pxi`

but it doesn't look easy.

### comment:8 Changed 7 years ago by

Reviewers: | → Martin Albrecht |
---|---|

Status: | needs_review → positive_review |

Okay, looks good then. I didn't run tests, though.

### comment:9 Changed 7 years ago by

Branch: | u/jdemeyer/cysignals_memory → 4bb8337295ed82c93d1a9c9f7173a4c36f97151d |
---|---|

Resolution: | → fixed |

Status: | positive_review → closed |

### comment:10 Changed 7 years ago by

Commit: | 4bb8337295ed82c93d1a9c9f7173a4c36f97151d |
---|

I've been working on ticket #14304 which now depends on this ticket. Since it's been rebased, it now has doctests failing that aren't touched by the ticket. So I ran all the doctests with just this ticket applied and the following one fails:

sage -t src/sage/tests/cmdline.py # 3 doctests failed

However, the following are failing for #14304

sage -t src/sage/interacts/debugger.py # 1 doctest failed sage -t src/sage/tests/cmdline.py # 14 doctests failed sage -t src/sage/tests/french_book/nonlinear_doctest.py # 1 doctest failed sage -t src/sage/doctest/forker.py # 1 doctest failed sage -t src/sage/doctest/test.py # 1 doctest failed sage -t src/sage/repl/attach.py # 2 doctests failed sage -t src/sage/repl/interpreter.py # 22 doctests failed sage -t src/sage/repl/inputhook.pyx # 2 doctests failed sage -t src/sage/repl/ipython_extension.py # 9 doctests failed sage -t src/sage/repl/ipython_tests.py # 4 doctests failed sage -t src/sage/repl/interface_magic.py # 3 doctests failed sage -t src/sage/repl/display/formatter.py # 10 doctests failed sage -t src/sage/repl/rich_output/backend_ipython.py # 3 doctests failed sage -t src/sage/repl/ipython_kernel/install.py # 1 doctest failed sage -t src/sage/repl/ipython_kernel/kernel.py # 4 doctests failed sage -t src/sage/quivers/algebra_elements.pxi # 1 doctest failed sage -t src/sage/dev/trac_interface.py # 1 doctest failed sage -t src/sage/typeset/ascii_art.py # 2 doctests failed sage -t src/sage/misc/temporary_file.py # 1 doctest failed sage -t src/sage/structure/misc.pyx # 1 doctest failed sage -t src/sage/structure/sage_object.pyx # 4 doctests failed sage -t src/sage/combinat/shard_order.py # 1 doctest failed sage -t src/sage/combinat/root_system/root_lattice_realizations.py # 1 doctest failed sage -t src/sage/combinat/rigged_configurations/rigged_configurations.py # 1 doctest failed sage -t src/sage/combinat/rigged_configurations/rigged_configuration_element.py # 1 doctest failed sage -t src/sage/combinat/rigged_configurations/rc_crystal.py # 1 doctest failed sage -t src/sage/combinat/rigged_configurations/rc_infinity.py # 1 doctest failed sage -t src/sage/interfaces/sage0.py # 1 doctest failed

Any idea what's up?

### comment:12 Changed 7 years ago by

They are:

aly@aly-laptop:~/Sage/sage$ ./sage -t src/sage/tests/cmdline.py Running doctests with ID 2016-03-22-16-00-06-1cfb21fa. Git branch: ticket/20210 Using --optional=ccache,mpir,python2,sage Doctesting 1 file. sage -t --warn-long 18.2 src/sage/tests/cmdline.py ********************************************************************** File "src/sage/tests/cmdline.py", line 106, in sage.tests.cmdline.test_executable Failed example: out.find(version()) >= 0 Expected: True Got: False ********************************************************************** File "src/sage/tests/cmdline.py", line 114, in sage.tests.cmdline.test_executable Failed example: out.find(version()) >= 0 Expected: True Got: False ********************************************************************** File "src/sage/tests/cmdline.py", line 186, in sage.tests.cmdline.test_executable Failed example: out.find(version()) >= 0 Expected: True Got: False ********************************************************************** 1 item had failures: 3 of 235 in sage.tests.cmdline.test_executable [234 tests, 3 failures, 30.61 s] ---------------------------------------------------------------------- sage -t --warn-long 18.2 src/sage/tests/cmdline.py # 3 doctests failed ---------------------------------------------------------------------- Total time for all tests: 30.7 seconds cpu time: 0.3 seconds cumulative wall time: 30.6 seconds

### comment:13 Changed 7 years ago by

Well, that doesn't look related to this ticket... Are you sure you ran `make`

?

### comment:14 Changed 7 years ago by

Ugg. I think you're right. I'm running make now and will rerun the doctests.

### comment:15 Changed 7 years ago by

Do you remember which commands you did run and why you ran them instead of something involving `make`

? Did somebody tell you to run those commands or did you read it somewhere in the documentation? How could we make it more clear that people really should run `make`

before testing or running Sage?

### comment:16 follow-up: 17 Changed 7 years ago by

I've been running

./sage -b

for most tickets. I thought I only needed to run make when spkg changed, and just forgot to do so on this ticket out of habit of running the Sage build. Should I more generally run make?

### comment:17 Changed 7 years ago by

Replying to aly.deines:

I thought I only needed to run make when spkg changed

That is still true.

However: most of the time this spkg change is implicit when changing branches. When you move from a branch based on one Sage version to a branch based on a different Sage version, chances are high that some spkg has changed between those branches.

Should I more generally run make?

At least when changing branches, yes. Once you did this and you then make changes only to the Sage library, then `./sage -b`

should be sufficient. But run `make`

when in doubt.

Note that you can also use `make build`

instead of `make`

to skip the documentation build which can take a while.

### comment:18 Changed 7 years ago by

An easy way to run all doctests safely is to use `make ptest`

or `make ptestlong`

. That will do the build and run all doctests in one simple command.

### comment:19 follow-up: 20 Changed 7 years ago by

Awesome! Thanks! I might update some of the developer's guide to make this a bit clearer. :)

### comment:20 Changed 7 years ago by

Replying to aly.deines:

Awesome! Thanks! I might update some of the developer's guide to make this a bit clearer. :)

That would be really great! Still, the hard part is making people read it.

**Note:**See TracTickets for help on using tickets.

New commits:

`Upgrade cysignals package`

`Move memory functions to cysignals`

`Rename sage_malloc -> sig_malloc and friends`