Merge lp://staging/~gz/bzr-builder/nonascii_maintainer_837741 into lp://staging/bzr-builder

Proposed by Martin Packman
Status: Merged
Approved by: Jelmer Vernooij
Approved revision: 154
Merged at revision: 149
Proposed branch: lp://staging/~gz/bzr-builder/nonascii_maintainer_837741
Merge into: lp://staging/bzr-builder
Prerequisite: lp://staging/~gz/bzr-builder/trivial_test_file_contents
Diff against target: 64 lines (+28/-1)
2 files modified
cmds.py (+10/-1)
tests/test_blackbox.py (+18/-0)
To merge this branch: bzr merge lp://staging/~gz/bzr-builder/nonascii_maintainer_837741
Reviewer Review Type Date Requested Status
Jelmer Vernooij (community) code Approve
Review via email: mp+80046@code.staging.launchpad.net

Description of the change

Finishes off the work by Michał Sawicz to correctly add non-ascii maintainer names to changelogs:

<https://code.launchpad.net/~saviq/bzr-builder/decode-maintainer/+merge/79596>

The significant differences from that proposal are:

* There's a test to ensure it works
* Older versions of python-debian before the api change in 0.1.20 are also fixed

On the specifics:

* The test requires something other than LANG=C which may be unfortunate give that's how the automated builders seem to run things, but there's no sensible way around.
* Adds the decoding to add_autobuild_changelog_entry rather than altering the get_maintainer function, which has a copy in python-debian that should be used in future.
* Only handles the maintainer name taken from the environment, not if it's passed in. If any callers do that, they still have the problem of not knowing if debian.changelog wants utf-8 bytes or unicode.
* Encodes the package name to ascii. This is the tweak needed to get the old way of doing things in debian.changelog to work, and upcasting to unicode will do the right thing for later versions.
* There's no __version__ attribute in the debian package, so detecting the api change means poking at the Changelog.__unicode__ method.

To post a comment you must log in.
153. By Martin Packman

Get comment about debian policy on package names pedantically correct

Revision history for this message
Martin Packman (gz) wrote :

I should note that I've tested this against the version of python-debian packaged on my system (0.1.18~twiddle) and the current git head (0.1.21).

Revision history for this message
Jelmer Vernooij (jelmer) wrote :

Shouldn't that be a TestSkipped rather than a knownFailure ?

Revision history for this message
Jelmer Vernooij (jelmer) :
review: Needs Information (code)
Revision history for this message
Martin Packman (gz) wrote :

> Shouldn't that be a TestSkipped rather than a knownFailure ?

Maybe, or perhaps should use a requireFeature call. I went with this spelling because there is a failure scenario here - having a non-ascii value in the environment and not having the a locale set that could decode it to unicode. Really bzr-builder isn't the right place to address that kind of problem though, as any user running into it should just fix their setup.

Revision history for this message
Jelmer Vernooij (jelmer) wrote :

Either TestSkipped or requireFeature seems reasonable to me for this.

I don't think this really is a failure scenario. If the terminal encoding doesn't allow encoding those characters, then we don't have the ability to test this, but it doesn't seem like a failure.

154. By Martin Packman

Switch test to using skip rather than knownFailure as suggested by Jelmer in review

Revision history for this message
Martin Packman (gz) wrote :

Looks like bzrlib needs a feature along these lines still, so went with skip and a message that includes the problem encoding.

Revision history for this message
Jelmer Vernooij (jelmer) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches