32-bit hash fix for Cython update

sagemath / sage

Main repository of SageMath

https://www.sagemath.org

Other

1.44k stars 480 forks source link

32-bit hash fix for Cython update #15863

Closed vbraun closed 10 years ago

vbraun commented 10 years ago

This fixes a hash failure due to the Cython upgrade #15755

Note: I'm curently not testing every ticket on 32-bit since we don't have a buildslave that can do it in a reasonable amount of time.

Depends on #15755

CC: @ohanar @jpflori

Component: packages: standard

Branch/Commit: u/vbraun/cython_32bit @ d698040

Reviewer: Jeroen Demeyer

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

vbraun commented 10 years ago

Description changed:

--- 
+++ 
@@ -1 +1,3 @@
 This fixes a hash failure due to the Cython upgrade #15755
+
+Note: I'm curently not testing every ticket on 32-bit since we don't have a buildslave that can do it in a reasonable amount of time.

vbraun commented 10 years ago

Branch: u/vbraun/cython_32bit

vbraun commented 10 years ago

Commit: d698040

vbraun commented 10 years ago

New commits:

`cf1bb52`	`Cython: upgrade to 0.20.1`
`b17f7c8`	`Fix various declarations`
`413ab85`	`Make sure PolyBoRi hashes return Py_ssize_t`
`115c8fc`	`Fix version and checksum for Cython 0.20.1`
`faf410f`	`Merge branch 'u/jdemeyer/ticket/15755' of git://trac.sagemath.org/sage into ticket/15755`
`d698040`	`32-bit fix for hashes`

jdemeyer commented 10 years ago

comment:6

As you sure 32-bit doctest pass?

jdemeyer commented 10 years ago

Changed author from Volker Braun to none

jdemeyer commented 10 years ago

Reviewer: Volker Braun, Jeroen Demeyer

jdemeyer commented 10 years ago

comment:7

Normally, this should be fixed by #15755, hashes should never return a Python long. Proposal: close as invalid.

vbraun commented 10 years ago

comment:8

This is caused by #15755. Also, there is nothing wrong with returning long for hashes on obsolete platforms imho. In this particular case Polybori returns the hash as size_t.

vbraun commented 10 years ago

Changed reviewer from Volker Braun, Jeroen Demeyer to Jeroen Demeyer

vbraun commented 10 years ago

Author: Volker Braun

jdemeyer commented 10 years ago

comment:10

Replying to @vbraun:

This is caused by #15755.

Can you confirm then that doctests do not pass with #15755 and they do pass with #15863?

Also, there is nothing wrong with returning long for hashes on obsolete platforms imho.

Who says it only happens on "obsolete" platforms?

jdemeyer commented 10 years ago

Changed reviewer from Jeroen Demeyer to none

vbraun commented 10 years ago

comment:11

Replying to @jdemeyer:

Can you confirm then that doctests do not pass with #15755 and they do pass with #15863?

Yes.

Who says it only happens on "obsolete" platforms?

It works on LP64. Nobody in their right mind is going to use 32-bit or Windows 64 for scientific computations nowadays.

jdemeyer commented 10 years ago

comment:12

Replying to @vbraun:

It works on LP64.

Why are you sure?

vbraun commented 10 years ago

comment:13

I've already tested #15755 and this ticket on the buildbot. On LP64 both size_t and C long have the same size, so it obviously works. With #15755 and 32-bit size_t now converts to a Python (arbitrary precision) long, but that is fine too as long as the doctest is fixed.

vbraun commented 10 years ago

comment:14

I didn't pull the added commits to #15755, trying again...

vbraun commented 10 years ago

Reviewer: Jeroen Demeyer

vbraun commented 10 years ago

comment:15

Ok, works. Sorry for the noise...

vbraun commented 10 years ago

Changed author from Volker Braun to none