Code review comment for lp://staging/~jelmer/bzr-builddeb/gather-orig-files-non-utf8

Am 30/11/11 19:52, schrieb Martin Packman:
> Review: Needs Fixing
>
> The feature you want is BytestringNamedFilesystem which boils down to is-posix-system and yes, will always be true for bzr-builddeb.
>
> + prefix = "%s_%s.orig" % (osutils.safe_utf8(package),
> + osutils.safe_utf8(version))
>
> The alternative name of 'safe_utf8' is 'break_things_randomly', don't use it. You need to actually know the input types to the function and handle them appropriately. Here, it's dead easy, both package names and versions must be ascii:
>
> <http://www.debian.org/doc/debian-policy/ch-controlfields.html#s-f-Source>
> <http://www.debian.org/doc/debian-policy/ch-controlfields.html#s-f-Version>
>
> So, the correct spelling is just `.encode("ascii")` as python-debian gives us unicode (and will work regardless).
Makes sense. Thanks, fixed.
> I think that GetOrigSourceSource also suffers from this problem, as it has nearly the same code. It needs a test as well, and either the same fix or to share the core code between the two.
One of my other branches changes GetOrigSourceSource anyway, and
refactors it to use gather_orig_files, among other things:
lp:~jelmer/bzr-builddeb/get-packaged-orig-source.

Cheers,

Jelmer