Merge lp:~gz/bzr/add_two_in_unicode_cwd_686611 into lp:bzr/2.0
Status: | Merged |
---|---|
Approved by: | Vincent Ladeuil |
Approved revision: | no longer in the source branch. |
Merged at revision: | 4768 |
Proposed branch: | lp:~gz/bzr/add_two_in_unicode_cwd_686611 |
Merge into: | lp:bzr/2.0 |
Diff against target: |
55 lines (+17/-2) 3 files modified
NEWS (+3/-0) bzrlib/mutabletree.py (+3/-1) bzrlib/tests/blackbox/test_add.py (+11/-1) |
To merge this branch: | bzr merge lp:~gz/bzr/add_two_in_unicode_cwd_686611 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Vincent Ladeuil | Approve | ||
Martin Pool | Approve | ||
Review via email: mp+43152@code.launchpad.net |
Commit message
Avoid UnicodeDecodeError in ``bzr add` on windows
Description of the change
This fixes a regression from the fix for bug 192859 (hence the merge to 2.0) which breaks smart_add with certain unicode inputs, on windows only. Rather than launching on fixing all the windows path functions, this branch just gets bzr back to its previous state by making the problematic function call conditional on symlinks actually being supported.
This is is complex edge case involving multiple bugs, including an upstream issue† that has been fixed in recent Python versions, but the gist is:
* smart_add now calls the (undocumented, untested) normalizepath
* normalizepath uses osutils.realpath on the path dirname to resolve symlinks
* osutils.realpath on nix encodes to bytes and decodes from bytes so always returns unicode
* osutils.realpath on windows is basically ntpath.abspath
* ntpath.abspath returns os.getcwd (not u) with a boolean False path, until Python 2.6.5
* normalizepath tries to recombine that bytestring dirname with the unicode basename
Note, symlinks will never be supported on Python 2 on windows‡ so there's no future compatibility issue to worry about.
There's a whole bunch of extra weirdness here, like I have no idea why builtins.
I'm not sure what to do about NEWS, as this branch predates the system switch.
† os.path.abspath with unicode argument should call os.getcwdu
<http://
‡ Add os.symlink() and os.path.islink() support for Windows
MvL: "The patch has now missed the window for 2.7, so backporting it won't be necessary."
<http://
Since only windows is concerned here (as far as regressions go), I'd prefer that you target 2.2 instead. We won't release 2.0 and 2.1 for windows any more so no need to make our life harder when merging up.
For the news entry, just file it where appropriate in NEWS in the 2.2 branch. When merging up in trunk, don't duplicate the entry for 2.3 but make sure it's added to doc/en/ releases- notes/bzr- 2.2.txt (the merge should try to add it to bzr-2.3.txt with huge conflicts, just get rid of them).
The fix looks sane enough that I can pre-approve landing it to 2.2 and trunk, but feel free to make two proposals (one for 2.2 and one for trunk) if you prefer a further review.
So my vote is Disapprove for 2.0, but Approve for 2.2 and trunk