[Svnmerge] KeyError on svnmerge.py

[Giovanni Bajo]

On Sat, 2008-03-08 at 10:10 -0500, Raman Gupta wrote:

> > 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?
> 
> Ok, so it *was* the removal of the LC_ALL=C that caused this -- I
> suspected as much when I asked Hernan to test with rev 29665 yesterday.
> 
> The reason it was removed was discussed on the mailing list. I also
> asked explicitly on the list about the ramifications related to
> removing LC_ALL=C, and no one replied.

Sorry, I had missed that. Please, feel free to CC: me on any questions
which you feel I might have insights on :)

One question: is using LC_MESSAGES=C enough to fix the regression? In
that case, I would assume that LC_MESSAGES would only affect SVN's own
strings and not log messages. It would be a win-win.

> Regardless, since it is now clear what the purpose of the setting was,
> my suggestion would be to add the LC_ALL=C prefix explicitly to calls
> that require the svn output to be parseable by svnmerge.py, which
> leaves calls to svn log and the output of svn merge using the
> appropriate user locale. I think this could be done with a new
> parameter on the call to launchsvn called "localized" which would
> default to true.

Looks like a sensible design. I won't argue on the default even if I'd
think the opposite is more safe (given that it's the way svnmerge.py has
always been run).

> In UNIX, one is able to set a per-command environment variable by
> prefixing the command with the environment setting e.g. specifying
> "LC_ALL=C svn ...". Will that work on Windows also? 

No: this is not even a POSIX feature, but a shell feature.

> If not, we could
> set os.environ temporarily 

launch() has two implementations nowadays: subprocess and popen. With
subprocess, there is an explicit parameter called "environ" where you
can pass in the environment in which the subprocess will be run (and
it's of course portable among platforms). 

For popen(), setting and restoring the environ would seem the only
solution.

Anyway, let's first see if LC_MESSAGES=C is enough...

> but in that case I suppose we need to ensure that launchsvn is not
> called simultaneously by multiple threads.

Which threads? svnmerge.py does not use threads at all.
-- 
Giovanni Bajo