[Svnmerge] KeyError on svnmerge.py

[Giovanni Bajo]

On Sat, 2008-03-08 at 15:28 +0100, Laurent PETIT wrote:

> 
>         This is not required. It should be sufficient what svnmerge.py
>         already
>         does:
>         
>         # We expect non-localized output from SVN
>         os.environ["LC_ALL"] = "C"
>         
>         You might want to try changing that into "LC_MESSAGES" to see
>         if it
>         makes any difference.
> 
> 
> Sorry, but I've the latest version of svnmerge.py from trunk, and I 
> can't see the two above lines.

You are right. It's been broken lately by this patch:

------------------------------------------------------------------------
r29666 | rocketraman | 2008-03-02 01:48:59 +0100 (Sun, 02 Mar 2008) | 19
lines
Changed paths:
   M /trunk/contrib/client-side/svnmerge/svnmerge.py
   M /trunk/contrib/client-side/svnmerge/svnmerge_test.py

Resolve issue with encoding commit log messages as described by
Romulo Ceccon at:

http://article.gmane.org/gmane.comp.version-control.subversion.svnmerge.devel/872

* contrib/client-side/svnmerge.py: Import locale, set locale to user
default.
  (recode_stdout_to_file): New method to decode standard output and
encode using
    the user's default locale encoding.
  (get_commit_log): Call recode_stdout_to_file to change the encoding of
svn
    log output.

* contrib/client-side/svnmerge_test.py
  (testCommitMessageEncoding): New test case to verify the commit log
message
    encoding.

Patch by:  Romulo A. Cesson <romulocesson at yahoo.com.br>
           me
Review by: Thomas Heller <theller at ctypes.org>

------------------------------------------------------------------------

I'm CC'ing the authors and reviewers. This patch contains this hunk:

================================================================================
@@ -89,8 +89,10 @@
 # Each line of the embedded log messages will be prefixed by
LOG_LINE_PREFIX.
 LOG_LINE_PREFIX = 2 * ' '
 
-# We expect non-localized output from SVN
-os.environ["LC_ALL"] = "C"
+# Set python to the default locale as per environment settings, same as
svn
+# TODO we should really parse config and if log-encoding is specified,
set
+# the locale to match that encoding
+locale.setlocale(locale.LC_ALL, '')
 

###############################################################################
 # Support for older Python versions
================================================================================


which removes the LC_ALL=C from the environment. This is needed to force
SVN to produce non localized strings, as explained by the comment. Why
has it been removed?

> Anyway, for the moment, I'm unable to use svnmerge with my desktop in
> French.

Try restoring those two lines.

> By the way, the patch I've submitted in another reply to this thread
> does handle the problem by not depending anymore on strings
> localization but rather on xml output structure, which, as I guess,
> may be (but maybe not ?) more stable than relying on i18n labels ?

The output of svn is meant to be machine parsable and it is very stable.
The LC_ALL=C setting ensures that the English strings are used, which
are not subject to i18n issues.

> Please find in attachment a new version of my patch, which verifies
> svn's client version : if it is older than 1.3.x, then it falls back
> to the current way of resolving svninfo, and if it is newer or equal
> to 1.3.0, then it uses the --xml option.
> 
> Please find in attachment the new patch version.

As I said, this patch is unnecessary, in that the same bug can be fixed
re-adding those two lines that have been there since the inception of
svnmerge.sh (before even svnmerge.py was born).

Moreover, your patch introduces a new dependency on xml.minidom which is
unnecessary; at the very least you should use pulldom, which is already
used to parse the output of "svn log --xml" (see get_copyfrom() and the
SvnLogParser class); theoretically, you would write a SvnInfoParser
class using the same structure and use it in the same way that
get_copyfrom() uses.

Anyway, I'm *not* suggesting you to write this code, since the problem
at hand can be solved by fixing a one-liner regression. There's
absolutely no reason to complicate the code with the xml parser unless
there's a real-world problem we need to solve; usage of xml for svn log
was really the only way to do it correctly.
-- 
Giovanni Bajo