[MERGE] win32-specific fixes for selftest (all blackbox tests pass!)

[Andrew Bennetts]

A few comments, but I'm basically happy for this to be merged.  Thanks so much
for doing this.

Alexander Belchenko wrote:
> === modified file bzr // last-changed:bialix at ukr.net-20070304001602-mhzsxzstjkx
> ... t50ao
> --- bzr
> +++ bzr
> @@ -106,6 +106,7 @@
>      # but our policy is to always close files from a finally block. -- mbp 20070215
>      try:
>          sys.stdout.flush()
> +        sys.stderr.flush()
>      except IOError, e:
>          import errno
>          if sys.platform != 'win32' or e.errno != errno.EINVAL:

You should update the comment saying "# stderr isn't buffered".  Probably just
delete that comment I guess; it seemed like a reasonable assumption at the time,
but apparently it isn't.

> === modified file bzrlib/tests/__init__.py
> --- bzrlib/tests/__init__.py
> +++ bzrlib/tests/__init__.py
> @@ -475,7 +475,14 @@
>                  else:
>                      test_root = test_root.encode(
>                          sys.getfilesystemencoding())
> -                osutils.rmtree(test_root)
> +                try:
> +                    osutils.rmtree(test_root)
> +                except OSError, e:
> +                    if sys.platform == 'win32' and e.errno == errno.EACCES:
> +                        print >>sys.stderr, ('Permission denied: '
> +                                             'unable to remove testing dir')
> +                    else:
> +                        raise

You do this several times.  I think it'd be better to add this behaviour to
osutils.rmtree.  In fact it almost has this feature already; see the default
value for the 'onerror' parameter.  Perhaps this should be done in an
alternative error handler you can pass to onerror.  Or perhaps just do
"ignore_errors=True"?

Also, the error message printed should include the value of 'test_root'.

Finally, I wonder if it would be better to use warnings.warn rather than
printing directly to stderr?

> === modified file bzrlib/tests/blackbox/test_serve.py // last-changed:bialix at uk
> ... r.net-20070304002950-zis61clb0i16ljvp
> --- bzrlib/tests/blackbox/test_serve.py
> +++ bzrlib/tests/blackbox/test_serve.py
[...]
>          orig_bzr_remote_path = os.environ.get('BZR_REMOTE_PATH')
> -        os.environ['BZR_REMOTE_PATH'] = self.get_bzr_path()
> +        bzr_remote_path = self.get_bzr_path()
> +        if sys.platform == 'win32':
> +            bzr_remote_path = sys.executable + ' ' + self.get_bzr_path()
> +        os.environ['BZR_REMOTE_PATH'] = bzr_remote_path

Interesting.  I was surprised that this was the only spot that needed this
change, but after looking around I see that get_bzr_path isn't used directly
much; most places use start_bzr_subprocess, which explicitly uses sys.executable
as well.

Anyway, this change is fine :)

>              branch = Branch.open(
> -                'bzr+ssh://fred:secret@localhost:%d%s' % (port, path_to_branch))
> -            
> +                'bzr+ssh://fred:secret@localhost:%d%s' % (port,path_to_branch))
> +

And thanks for fixing my code formatting :)

> === modified file bzrlib/tests/test_config.py // last-changed:bialix at ukr.net-20
> ... 070304003140-76mrqwec8e5zndeh
> --- bzrlib/tests/test_config.py
> +++ bzrlib/tests/test_config.py
> @@ -269,21 +269,12 @@
>  
>      def setUp(self):
>          super(TestConfigPath, self).setUp()
> -        self.old_home = os.environ.get('HOME', None)
> -        self.old_appdata = os.environ.get('APPDATA', None)
>          os.environ['HOME'] = '/home/bogus'
> -        os.environ['APPDATA'] = \
> -            r'C:\Documents and Settings\bogus\Application Data'
> +        if sys.platform == 'win32':
> +            os.environ['BZR_HOME'] = \
> +                r'C:\Documents and Settings\bogus\Application Data'
>  
>      def tearDown(self):
> -        if self.old_home is None:
> -            del os.environ['HOME']
> -        else:
> -            os.environ['HOME'] = self.old_home
> -        if self.old_appdata is None:
> -            del os.environ['APPDATA']
> -        else:
> -            os.environ['APPDATA'] = self.old_appdata
>          super(TestConfigPath, self).tearDown()

This tearDown doesn't do anything, so just delete it.

-Andrew.