Bug in description handling

[Martijn Pieters]

On 14 Aug 2010, at 17:12, Dan Villiom Podlaski Christiansen wrote:
> I have two gripes with this patch:
>
> To begin with, checking if sys.platform is 'mac' is pointless. Only  
> Mac OS 9 has this in sys.platform, and I believe support for it was  
> dropped in Python 2.4. This is the earliest version that Mercurial  
> works with, and even *if* you had a compatible Python binary running  
> there, it wouldn't work, as osutil hasn't been implemented for that  
> system.

That's a holdover from the getpreferredencoding port; locale.py used  
to check for both 'darwin' and 'mac' before using the version that  
always returns 'mac-roman'. Since the fix was to remove those two  
platform names from the test in question, so the POSIX branch would be  
used instead, I simply used these in the test for when to install our  
own version.

> In addition, the version check doesn't deal with patched Python  
> binaries that don't need the override. I have 7 Python binaries on  
> my Mac:
>
> Python 2.4.6, 2.5.5, 2.6.5, 2.7 & 3.1.2 from MacPorts.
> Python 2.5.4 & 2.6.1 from Apple, included with Mac OS X 10.6.
>
> Of these, only the 2.4.6 & 2.5.5 binaries from MacPorts yields ‘mac- 
> roman’ for locale.getpreferredencoding(). The rest correctly yield  
> ‘UTF-8’. Thus, the assumption that any Python earlier than 2.7 is  
> broken isn't safe.

It looks like Apple backported the patch too then, as upstream  
certainly doesn't include this in 2.5.4 or 2.6.1. I didn't check if  
2.6.5 upstream has it, since the original bug report only mentions 2.7  
and 3.1 as receiving the fixes.

Note that the code executed if the version and platform tests succeed  
is lifted straight from locale.py. We are thus merely executing the  
same instructions elsewhere.

> I posted[1] a patch to this list yesterday, based on your earlier  
> patch. (Perhaps I should have Cc'ed you; I didn't think of that…)  
> Instead of always overriding getpreferredencoding(), only a value of  
> ‘mac-roman’ causes the fixed method to be invoked. After all, it's  
> rather unlikely that users actually want this encoding, and even in  
> this unlikely scenario, your method should be safe, right? I'm not  
> sure any of this actually affects users, though…
>
> [1] <http://selenic.com/pipermail/mercurial-devel/2010-August/023604.html 
> >


I kinda like your approach; clearly you are more familiar with the  
codebase; refitting _encodingfixup works nicely for this.

Patch works for me.

-- 
Martijn Pieters