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

Bug#711402: RFS: scandir/0.1+git20130521-1 [ITP]



I don't intend to sponsor this package, but here's my review:

* Nicolas Delvaux <contact@nicolas-delvaux.org>, 2013-06-06, 20:57:
http://mentors.debian.net/debian/pool/main/s/scandir/scandir_0.1+git20130521-1.dsc

The source package name is a bit too generic. I'd rename it to python-scandir.

I wouldn't include guesswork like "It may integrate the Python Standard Library in a future version." in the description.

The watch files says that there has been "no formal upstream release yet", but the version number looks like it was a git snapshot newer than a 0.1 release.

The current contents of README.Debian really belongs in README.source.

This:

PY3REQUESTED := $(shell py3versions -r)
PY3DEFAULT := $(shell py3versions -d)
PYTHON3 := $(filter-out $(PY3DEFAULT),$(PY3REQUESTED)) python3

looks over-engineered. How about:

PYTHON3 = $(shell py3versions -r)

instead?

The package FTBFS when built twice in a row:
|  dpkg-source -b scandir-0.1+git20130521
| dpkg-source: info: using source format `3.0 (quilt)'
| dpkg-source: info: building scandir using existing ./scandir_0.1+git20130521.orig.tar.gz
| dpkg-source: error: cannot represent change to build/lib.linux-i686-3.3/_scandir.cpython-33m.so: binary file contents changed
| dpkg-source: error: add build/lib.linux-i686-3.3/_scandir.cpython-33m.so in debian/source/include-binaries if you want to store the modified binary in the debian tarball
| dpkg-source: error: cannot represent change to build/lib.linux-i686-3.2/_scandir.cpython-32mu.so: binary file contents changed
| dpkg-source: error: add build/lib.linux-i686-3.2/_scandir.cpython-32mu.so in debian/source/include-binaries if you want to store the modified binary in the debian tarball
| dpkg-source: error: cannot represent change to build/temp.linux-i686-3.3/_scandir.o: binary file contents changed
| dpkg-source: error: add build/temp.linux-i686-3.3/_scandir.o in debian/source/include-binaries if you want to store the modified binary in the debian tarball
| dpkg-source: error: cannot represent change to build/temp.linux-i686-3.2/_scandir.o: binary file contents changed
| dpkg-source: error: add build/temp.linux-i686-3.2/_scandir.o in debian/source/include-binaries if you want to store the modified binary in the debian tarball
| dpkg-source: error: unrepresentable changes to source
| dpkg-buildpackage: error: dpkg-source -b scandir-0.1+git20130521 gave error exit status 2

You don't need to pass --buildsystem=python_distutils to dh; dh will detect the correct build system automatically.

scandir.py says "See LICENSE.txt for the full license text" but there's no such file in .orig.tar. The tarball is missing also other goodies that exists in the git repository: changelog, README, tests.

In Python 2.X, the pure-Python implementation of scandir() cannot handle paths that are non-ASCII bytestrings; all I get is "UnicodeDecodeError: 'ascii' codec can't decode byte ...". See the attached test-case #1.

In Python >= 3.2, scandir() cannot list filenames that are not representable in locale encoding. It should probably use PyUnicode_DecodeFSDefault() instead of decoding with "strict" error handler. See test-case #2.

IMHO it's premature to upload it unstable. Experimental might be okay.

--
Jakub Wilk
#!/usr/bin/python

import sys

sys.modules['_scandir'] = None # force the pure-Python implementation
import scandir

for x in scandir.scandir('\xf1'): pass
#!/usr/bin/python3

import tempfile
import sys
import os
import shutil

import scandir

path = b'\xf1'
try:
	path.decode(sys.getfilesystemencoding(), 'strict')
except UnicodeError:
	pass
else:
	raise RuntimeError('please run this this under an UTF-8 locale')

tmpdir = tempfile.mkdtemp()
try:
	os.chdir(tmpdir)
	with open(path, 'w'): pass
	for x in scandir.scandir('.'): pass
finally:
	os.chdir('/')
	shutil.rmtree(tmpdir)

Reply to: