[MERGE] avoids preexec_fn on win32

Alexander Belchenko bialix at ukr.net
Thu Sep 7 11:03:46 BST 2006


John Arbash Meinel пишет:
> Alexander Belchenko wrote:
> ...
> 
>> After successfully applying your bundle to bzr.dev revno.1986 I got the
>> same error with preexec_fn.
>>
>> -- 
>> Alexander
> 
> I assume you only got those errors in the diff tests, and not in all the
> rest of the tests. (You may have also gotten that error in some of the
> gpg tests).

Yes, it is.

> The attached patch should clean up the all of the cases where I wanted
> to use preexec_fn. In general, where we were using it, it was to clean
> up environment variables, which wouldn't be useful anyway on win32.
> 
> I tested it on a windows machine, and tests that used preexec_fn seem to
> be passing now. (I still have problems with \r\n versus plain \n, but
> the rest seem okay).
> 
> I'm putting together a patch to handle the $HOME issues, since we want
> that fixed anyway. Which should fix all of the _set_user_ignores failures.

Your patch again seems incomplete a bit but only in the new introduced
changes in gpg.py: you forgot to 'import sys' but try to use sys name
later.
This small issue leads to gpg tests failed because of error raised:

======================================================================
ERROR: test_clears_progress (bzrlib.tests.test_gpg.TestCommandLine)

vvvv[log from
bzrlib.tests.test_gpg.TestCommandLine.test_clears_progress]-----

^^^^[log from
bzrlib.tests.test_gpg.TestCommandLine.test_clears_progress]-----
----------------------------------------------------------------------
Traceback (most recent call last):
   File "D:\Bazaar\sandbox\jam.no_preexec\bzrlib\tests\test_gpg.py",
line 78, in test_clears_progress
     self.assertProduces(content)
   File "D:\Bazaar\sandbox\jam.no_preexec\bzrlib\tests\test_gpg.py",
line 60, in assertProduces
     self.assertEqual(new_content, my_gpg.sign(content))
   File "D:\Bazaar\sandbox\jam.no_preexec\bzrlib\gpg.py", line 77, in sign
     if sys.platform == 'win32':
NameError: global name 'sys' is not defined

======================================================================

Please, fix this.

Also some blackbox tests broken because test expected to see output with
LF line-endings but instead got CRLF endings on win32. I don't know is
someone broke this part or this behaviour there was from old ages. But
in 0.9 release there is no such failing tests (it was added by John
Meinel in the end of August, as annotate says).

======================================================================
FAIL: test_run_bzr_subprocess_env
(bzrlib.tests.blackbox.test_selftest.TestRunBzrCaptured)

vvvv[log from
bzrlib.tests.blackbox.test_selftest.TestRunBzrCaptured.test_run_bzr_subprocess_env]

^^^^[log from
bzrlib.tests.blackbox.test_selftest.TestRunBzrCaptured.test_run_bzr_subprocess_env]
----------------------------------------------------------------------
Traceback (most recent call last):
   File
"D:\Bazaar\sandbox\jam.no_preexec\bzrlib\tests\blackbox\test_selftest.py",
line 190, in test_run_bzr_subprocess_env
     self.assertEqual('Joe Foo <joe at foo.com>\n', out)
AssertionError: 'Joe Foo <joe at foo.com>\n' != 'Joe Foo <joe at foo.com>\r\n'

======================================================================


In general this patch will get my +1.

--
Alexander

> ------------------------------------------------------------------------
> 
> # Bazaar revision bundle v0.8
> #
> # message:
> #   Update NEWS that we shouldn't use preexec_fn on win32
> # committer: John Arbash Meinel <john at arbash-meinel.com>
> # date: Wed 2006-09-06 10:33:36.621999979 -0500
> 
> === modified file bzrlib/gpg.py // last-changed:john at arbash-meinel.com-20060906
> ... 153211-9519b9dfdf3daf60
> --- bzrlib/gpg.py
> +++ bzrlib/gpg.py
> @@ -72,11 +72,16 @@
>  
>      def sign(self, content):
>          ui.ui_factory.clear_term()
> +
> +        preexec_fn = _set_gpg_tty
> +        if sys.platform == 'win32':
> +            # Win32 doesn't support preexec_fn, but wouldn't support TTY anyway.
> +            preexec_fn = None
>          try:
>              process = subprocess.Popen(self._command_line(),
>                                         stdin=subprocess.PIPE,
>                                         stdout=subprocess.PIPE,
> -                                       preexec_fn=_set_gpg_tty)
> +                                       preexec_fn=preexec_fn)
>              try:
>                  result = process.communicate(content)[0]
>                  if process.returncode is None:
> 

need to add 'import sys' at the top of file.





More information about the bazaar mailing list