[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Fixes for python-pyopencl and new upsteam release



On Fri, 2010-11-05 at 15:44 +0100, Tomasz Rybak wrote:
> Dnia 2010-11-03, śro o godzinie 20:44 +0000, Adam D. Barratt pisze:
> > On Sat, 2010-10-23 at 20:47 +0200, Tomasz Rybak wrote:
> > > Dnia 2010-10-23, sob o godzinie 16:37 +0100, Adam D. Barratt pisze:
> > > > On Thu, 2010-10-21 at 21:47 +0200, Tomasz Rybak wrote:
> > [ print -> print() ]
> > > > That won't work with python 2.6 or earlier; well, it'll work with 2.6 if
> > > > you import it from future, but the package claims to support python 2.5
> > > > and above.  There's a bunch of other occurrences of this in the diff,
> > > > but the rest all appear to be patched out.
> > > 
> > > Removed all 9 lines containing print.
> > 
> > Hmmm, might the messages in them be useful?  I'm not sure if there's a
> > nice way of handling both the print keyword and function without a try
> > except.  Having said that, as the package doesn't ship a python3 binary
> > package anyway, you only actually need to be compatible with 2.6 and
> > earlier...
> 
> Those were lines run when build fails because of configuration file
> (either missing or trying to run ./configure with configuration file
> already present). They would be executed only if something would go
> wrong during package build.

Which is exactly the sort of situation where they might be useful during
a Debian package build, no? :-)

> According to Piotr Ożarowski (sponsor of python-pyopencl) python2.5
> copes with print() called as function. I have checked on python2.5 and
> he is right - it does not require importing anything from __future__.
> Therefore I have reverted this patch.

According to the documentation, the function was only introduced in 2.6.
In any case "print ('x')" in pre-2.6 versions isn't calling print(),
it's passing 'x' to the print statement, so the lines are fine; I should
have spotted that earlier. (The parentheses are a red herring here as
they're simply acting as no-op grouping; the print function takes a set
of arguments beyond the string to print, and indeed doesn't work in
2.5).

> > > > +  * Forced build scripts not to use boost libraries included in the source
> > > > 
> > > > Could you point out where that fix is in the diff?  I couldn't
> > > > immediately spot it, but might be missing something obvious.
> > > 
> > > Upstream package (available from pypi) includes Boost.
> > > I am using source downloaded using git (debian/rules get-orig-source),
> > > so it does not contain Boost (I do not want waste space), but I also
> > > disabled ability to use shipped boost in case someone wants to build
> > > using pypi source.
[...]
> > That doesn't actually appear to disable anything, afaics.
> > USE_SHIPPED_BOOST will be set to False when building the Debian package,
> > but only because bpl-subset/bpl_subset/boost/version.hpp won't exist.
> 
> You were right. Now I have changed build script not to check presence
> of boost files and always set USE_SHIPPED_BOOST to false.

Sounds good.

> > > IMO having at least one OpenCL implementation and supporting
> > > libraries would make Debian more attractive.
> > 
> > Having an implementation that's not widely tested (popcon reports a
> > total of six installations, fwiw) also has the potential to have
> > precisely the opposite effect, I suspect.
> > 
> > If people are particularly interested in things that are "gaining
> > momentum" whilst a new release of any distribution is in progress then
> > isn't the likelihood that they'll either use newer versions of the
> > distribution or at least supplement the base with much more up-to-date
> > backports?
[...]
> Project is used on different operating systems and bugs are found
> on Mac, Windows, and Linux - but for now there has been no official
> packages, so there are questions on mailing list about problems
> with compilation. I believe providing packages will help to solve
> at least those problems.
> 
> I also believe that having at least one (currently only one, provided
> by NVIDIA) implementation of OpenCL and bindings to another languages
> will serve as "foot in the door" for Debian and OpenCL.

The diff's still horribly noisy, but most of the changes seem sane
enough or isolated to sections of the code which aren't used in the
package (e.g. the setuptools changes) so please feel free to upload to
unstable so we can look at getting the fixes in to testing.

One quick comment on the changelog:

+  * Fixed Maintainer field in debian/copyrigth (Closes: #588873)

s/copyrigth/copyright/

Regards,

Adam


Reply to: