bzr-builddeb

Merge lp://staging/~jelmer/bzr-builddeb/gather-orig-files-non-utf8 into lp://staging/bzr-builddeb

gather-orig-files-non-utf8
Merge into trunk

Proposed by Jelmer Vernooij on 2011-11-30

Status:

Merged

Approved by:

Jelmer Vernooij on 2011-12-01

Approved revision:

652

Merged at revision:

651

Proposed branch:

lp://staging/~jelmer/bzr-builddeb/gather-orig-files-non-utf8

Merge into:

lp://staging/bzr-builddeb

Prerequisite:

lp://staging/~jelmer/bzr-builddeb/location-alias-upstream

Diff against target:

60 lines (+28/-2)

3 files modified

debian/changelog (+3/-1)
tests/test_upstream.py (+18/-0)
upstream/__init__.py (+7/-1)

To merge this branch:

bzr merge lp://staging/~jelmer/bzr-builddeb/gather-orig-files-non-utf8

High

Fix Released

Link a bug report

Reviewer	Date Requested	Status
Martin Packman (community)		Approve on 2011-11-30
Bzr-builddeb-hackers	2011-11-30	Pending
Review via email: mp+83967@code.staging.launchpad.net

This proposal supersedes a proposal from 2011-11-30.

Description of the change

In gather_orig_files() we scan a directory for relevant orig tarballs. Cope with filenames that are not valid in the current file system locale - we are only interested in files with a particular prefix anyway.

The test isn't protected by a Feature. I'm not aware of a relevant feature, and bzr-builddeb is only used on POSIX-based systems anyway, which allow arbitrary byte filenames.

Revision history for this message

Martin Packman (gz) wrote on 2011-11-30:

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).

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.

review: Needs Fixing

Revision history for this message

Jelmer Vernooij (jelmer) wrote on 2011-11-30:

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

lp://staging/~jelmer/bzr-builddeb/gather-orig-files-non-utf8 updated on 2011-11-30

652. By Jelmer Vernooij on 2011-11-30: Encode package/version as ascii rather than utf8.

Revision history for this message

Martin Packman (gz) wrote on 2011-11-30:

> 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.

Aha, so you'd already handled that duplication it but in a different branch, good work.

review: Approve