Bug#903579: basemap: Generate _geoslib.c with Cython 0.28.2 for Python 3.7 transition
On Wed, 11 Jul 2018 10:38:51 -0400 Sandro Tosi wrote: > control: tags -1 - patch > > On Wed, Jul 11, 2018 at 10:29 AM Bas Couwenberg wrote: > > > > On 2018-07-11 16:23, Sandro Tosi wrote: > > > On Wed, Jul 11, 2018 at 10:03 AM Bas Couwenberg > > > wrote: > > >> The _geoslib.c file needs to regenerated with a recent Cython to be > > >> compatible with Python 3.7. > > > > > > the attached patch looks it's including only the regenerated files, > > > wouldnt it be better to b-d on cython and actual regenerate the .c > > > file at debian package build time? > > > > That would be better, but not something I'm willing to spend my time on. > > fair enough, but as is this patch cannot be accepted. Don't let the perfect be the enemy of good. The patch is fine: your package already ships a pre-built file, so updating it is a fine solution to fix this bug and let us move on with the python3 transition. Sure, regenerating it at build time would be better, but that can be done later if you don't have the time to do it now. For the time being, this patch is no different than when we had to manually run autoreconf and ship the results in a patch to get an updated config.guess, for example. I'd propose to ship this now and file a bug upstream asking to generate the file during the build. Then you can drop the patch when that happens (or when they just update the file to a new version, whichever happens first). Cheers, Emilio
Bug#903579: basemap: Generate _geoslib.c with Cython 0.28.2 for Python 3.7 transition
On 2018-07-11 16:38, Sandro Tosi wrote: control: tags -1 - patch On Wed, Jul 11, 2018 at 10:29 AM Bas Couwenberg wrote: On 2018-07-11 16:23, Sandro Tosi wrote: > On Wed, Jul 11, 2018 at 10:03 AM Bas Couwenberg > wrote: >> The _geoslib.c file needs to regenerated with a recent Cython to be >> compatible with Python 3.7. > > the attached patch looks it's including only the regenerated files, > wouldnt it be better to b-d on cython and actual regenerate the .c > file at debian package build time? That would be better, but not something I'm willing to spend my time on. fair enough, but as is this patch cannot be accepted. Why not? It works just fine. cythonizing will just need to be done again in the future. At least apply the README.md change to debian/docs, that should be uncontroversial. I only fixed this issue because it also prevented the rebuild of basemap with GEOS 3.7.0 from experimental. >> Upstream should generate the .c files as part of the build instead of >> including a pre-generated file in the distribution as a more permanent >> solution. > > could you open an issue upstream for this? No, please do that yourself as the maintainer of this package. Kind Regards, Bas
Bug#903579: basemap: Generate _geoslib.c with Cython 0.28.2 for Python 3.7 transition
control: tags -1 - patch On Wed, Jul 11, 2018 at 10:29 AM Bas Couwenberg wrote: > > On 2018-07-11 16:23, Sandro Tosi wrote: > > On Wed, Jul 11, 2018 at 10:03 AM Bas Couwenberg > > wrote: > >> The _geoslib.c file needs to regenerated with a recent Cython to be > >> compatible with Python 3.7. > > > > the attached patch looks it's including only the regenerated files, > > wouldnt it be better to b-d on cython and actual regenerate the .c > > file at debian package build time? > > That would be better, but not something I'm willing to spend my time on. fair enough, but as is this patch cannot be accepted. > > I only fixed this issue because it also prevented the rebuild of basemap > with GEOS 3.7.0 from experimental. > > >> Upstream should generate the .c files as part of the build instead of > >> including a pre-generated file in the distribution as a more permanent > >> solution. > > > > could you open an issue upstream for this? > > No, please do that yourself as the maintainer of this package. > > Kind Regards, > > Bas -- Sandro "morph" Tosi My website: http://sandrotosi.me/ Me at Debian: http://wiki.debian.org/SandroTosi G+: https://plus.google.com/u/0/+SandroTosi
Bug#903579: basemap: Generate _geoslib.c with Cython 0.28.2 for Python 3.7 transition
On 2018-07-11 16:23, Sandro Tosi wrote: On Wed, Jul 11, 2018 at 10:03 AM Bas Couwenberg wrote: The _geoslib.c file needs to regenerated with a recent Cython to be compatible with Python 3.7. the attached patch looks it's including only the regenerated files, wouldnt it be better to b-d on cython and actual regenerate the .c file at debian package build time? That would be better, but not something I'm willing to spend my time on. I only fixed this issue because it also prevented the rebuild of basemap with GEOS 3.7.0 from experimental. Upstream should generate the .c files as part of the build instead of including a pre-generated file in the distribution as a more permanent solution. could you open an issue upstream for this? No, please do that yourself as the maintainer of this package. Kind Regards, Bas
Bug#903579: basemap: Generate _geoslib.c with Cython 0.28.2 for Python 3.7 transition
On Wed, Jul 11, 2018 at 10:03 AM Bas Couwenberg wrote: > The _geoslib.c file needs to regenerated with a recent Cython to be > compatible with Python 3.7. the attached patch looks it's including only the regenerated files, wouldnt it be better to b-d on cython and actual regenerate the .c file at debian package build time? > Upstream should generate the .c files as part of the build instead of > including a pre-generated file in the distribution as a more permanent > solution. could you open an issue upstream for this? Thanks, -- Sandro "morph" Tosi My website: http://sandrotosi.me/ Me at Debian: http://wiki.debian.org/SandroTosi G+: https://plus.google.com/u/0/+SandroTosi
