You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by jeremy hinds <je...@gmail.com> on 2008/04/07 05:50:19 UTC
[PATCH] Use subprocess.Popen for executing commands in cmdline tests
This was suggested by Branko Čibej here:
http://subversion.tigris.org/servlets/ReadMsg?listName=dev&msgNo=136742
I don't have a normal Windows development environment set up, so I
haven't actually verified that exit-code testing now works correctly
in non-posix environments. And as mentioned in the log message, this
pushes the Python version required to run the test suite up to 2.4.
[[[
In cmdline tests, use subprocess.Popen for executing commands to allow
support for exit-code checks on both Windows and posix systems. This makes
the test suite require Python version >= 2.4.
* subversion/tests/cmdline/svntest/main.py
(global): Import subprocess instead of popen2. Remove variable
"platform_with_popen3_class".
(open_pipe): Remove "binary_mode" parameter. Use subprocess.Popen for
spawning the child process.
(wait_on_pipe): Interpret the return value of wait() according to
subprocess.Popen semantics.
(run_command, run_command_stdin, spawn_process): Remove "binary_mode"
parameter, and do not pass that parameter to run_command_stdin, open_pipe.
(run_svn, run_svnadmin, run_svnlook, run_svnsync, run_svnversion,
create_repos, copy_repos, TestSpawningThread.run_one): Do not pass
"binary_mode" parameter to run_command, open_pipe, or spawn_process.
* subversion/tests/cmdline/svntest/actions.py
(run_and_verify_svnlook, run_and_verify_svnlook2, run_and_verify_svnadmin,
run_and_verify_svnadmin2, run_and_verify_svnversion,
run_and_verify_svnversion2, run_and_verify_svn, run_and_verify_svn2,
run_and_verify_svn_match_any, run_and_verify_svn_match_any2):
Remove the caveat comment stating that exit-code checks are skipped
for some platforms.
(run_and_verify_load, check_prop): Do not pass the "binary_mode" parameter
to run_command_stdin or run_command.
* subversion/tests/cmdline/svnadmin_tests.py,
subversion/tests/cmdline/svnlook_tests.py,
subversion/tests/cmdline/svndumpfilter_tests.py,
subversion/tests/cmdline/getopt_tests.py:
Do not pass the "binary_mode" parameter to run_command or
run_command_stdin.
Suggested by: brane
Patch by: Jeremy Hinds <je...@gmail.com>
]]]
Re: [PATCH] Use subprocess.Popen for executing commands in cmdline tests
Posted by Eric Hanchrow <of...@blarg.net>.
I'd be extra careful with this on Windows -- I can't give specifics,
but I've seen Popen in 2.4 fail mysteriously on Windows (on a
different project), forcing me to use os.popen instead. Make sure you
test it.
--
I'd like to see Rush Limbaugh tell Omar he can't get
married.
--Alex Kotlowitz. You hadda be there
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Use subprocess.Popen for executing commands in cmdline
tests
Posted by Daniel Shahaf <d....@daniel.shahaf.co.il>.
Daniel Shahaf wrote on Sat, 26 Apr 2008 at 13:34 +0300:
> jeremy hinds wrote on Fri, 25 Apr 2008 at 22:58 -0600:
> > My plan was to
> > get a Windows build working to test this before submitting the updated
> > patch, but I think I'm going to have to throw in the towel on that for
> > now. I did at least extract the Popen functionality into a separate
> > script and ran that on Windows (I should have done this before). The
> > pertinent lines in the patch are
> >
> > + close_fds=False
> > + if is_posix_os():
> > + close_fds=True
> > + # (use universal newlines for text mode)
> > + kid = Popen(command, shell=False, bufsize=-1,
> > + universal_newlines=(not binary_mode),
> > + stdin=PIPE, stdout=PIPE, stderr=PIPE, close_fds=close_fds)
> > + return kid.stdin, kid.stdout, kid.stderr, (kid, command)
> >
> > The only other difference between this and the previous iteration is
> > the removal of another comment in svntest/main.py.
> >
>
> Yes, the tests run now against the latest svn binary I have, and pass
> (except for some failures unrelated to your patch). I'll try and test the
> patch against HEAD this week, if no one beats me to it.
Your patch breaks merge_tests 43 64 76 86 for me. The output from #43 is:
=============================================================
Expected 'E1' and actual 'E1' in disk tree are different!
=============================================================
EXPECTED NODE TO BE:
=============================================================
* Node name: E1
Path: __SVN_ROOT_NODE\F\E1
Contents: N/A (node is a directory)
Properties: {'svn:mergeinfo': '/A/B/F/E:5\n/A/B/F/E1:5-8\n'}
Attributes: {}
Children: 2
=============================================================
ACTUAL NODE FOUND:
=============================================================
* Node name: E1
Path: F\E1
Contents: N/A (node is a directory)
Properties: {'svn:mergeinfo': '/A/B/F/E:5\n\n/A/B/F/E1:5-8\n'}
Attributes: {}
Children: 2
Unequal at node E1
Unequal at node F
EXCEPTION: SVNTreeUnequal
I think I saw earlier a FAIL in one of the #1 tests as well (i.e.,
log_tests 1 or lock_tests 1, etc.), but I can't reproduce it now.
I have not finished the test run after I verified that the merge_tests
failure was caused by the patch.
HTH,
Daniel
Re: [PATCH] Use subprocess.Popen for executing commands in cmdline
tests
Posted by Daniel Shahaf <d....@daniel.shahaf.co.il>.
jeremy hinds wrote on Fri, 25 Apr 2008 at 22:58 -0600:
> 2008/4/19 Daniel Shahaf <d....@daniel.shahaf.co.il>:
> > jeremy hinds wrote on Fri, 18 Apr 2008 at 19:01 -0600:
> >
> > > I started working on getting a Windows dev environment set up so I can
> > > at least make sure things can be expected to work before asking for
> > > anybody else's time to test it. But I'm not quite there yet. In the
> > > meantime, if someone else happens to get a chance to look at it, that
> > > would be great.
> > >
> >
> > I'm getting this error with your patch on Windows (ra_svn fsfs
> > VC9Express):
> >
> > START: changelist_tests.py
> > CMD: svnadmin.exe create svn-test-work\local_tmp\repos --bdb-txn-nosync --fs-type=fsfs
> > Traceback (most recent call last):
...
> > File "E:\Python\lib\subprocess.py", line 551, in __init__
> > raise ValueError("close_fds is not supported on Windows "
> > ValueError: close_fds is not supported on Windows platforms
> > END: changelist_tests.py
> >
> > (The error repeats for every *_tests.py file.)
> >
> > Python docs agree: (http://docs.python.org/lib/node528.html)
> >
> > "If close_fds is true, all file descriptors except 0, 1 and 2 will
> > be closed before the child process is executed. (Unix only)"
> > ^^^^^^^^^^^
> >
> > Daniel
>
>
> Thanks for giving it a try, Daniel. I saw that "Unix only" thing in
> the documentation, but it seemed unclear what the behavior would be on
> non-Unix.
Agreed, it's unclear. In Python trunk the paragraph reads:
(http://svn.python.org/view/python/trunk/Doc/library/subprocess.rst?rev=62427&view=auto):
If *close_fds* is true, all file descriptors except :const:`0`, :const:`1` and
:const:`2` will be closed before the child process is executed. (Unix only).
Or, on Windows, if *close_fds* is true then no handles will be inherited by the
child process. Note that on Windows, you cannot set *close_fds* to true and
also redirect the standard handles by setting *stdin*, *stdout* or *stderr*.
but doesn't mention throwing exceptions in older versions.
> I was rather hoping for "silently ignored." My plan was to
> get a Windows build working to test this before submitting the updated
> patch, but I think I'm going to have to throw in the towel on that for
> now. I did at least extract the Popen functionality into a separate
> script and ran that on Windows (I should have done this before). The
> pertinent lines in the patch are
>
> + close_fds=False
> + if is_posix_os():
> + close_fds=True
> + # (use universal newlines for text mode)
> + kid = Popen(command, shell=False, bufsize=-1,
> + universal_newlines=(not binary_mode),
> + stdin=PIPE, stdout=PIPE, stderr=PIPE, close_fds=close_fds)
> + return kid.stdin, kid.stdout, kid.stderr, (kid, command)
>
> The only other difference between this and the previous iteration is
> the removal of another comment in svntest/main.py.
>
Yes, the tests run now against the latest svn binary I have, and pass
(except for some failures unrelated to your patch). I'll try and test the
patch against HEAD this week, if no one beats me to it.
Daniel
> [[[
> In cmdline tests, use subprocess.Popen for executing commands to allow
> support for exit-code checks on both Windows and posix systems. This makes
> the test suite require Python version >= 2.4.
>
> * subversion/tests/cmdline/svntest/main.py
> (global): Import subprocess instead of popen2. Remove variable
> "platform_with_popen3_class".
> (_quote_arg): Removed, since commands + args to open_pipe are now lists.
> (open_pipe): Replace "mode" character parameter with a "binary_mode" boolean
> value. Use subprocess.Popen for spawning the child process.
> (wait_on_pipe): Interpret the return value of wait() according to
> subprocess.Popen semantics.
> (spawn_process, copy_repos): When calling open_pipe, pass the command as
> a list and binary_mode as a boolean.
> (run_svn2): New, like run_svn but with a "binary_mode" boolean param.
> (TestSpawningThread.run_one): Remove the caveat comment stating that
> result (exit-code) is None on Windows.
>
> * subversion/tests/cmdline/svntest/actions.py
> (run_and_verify_svnlook, run_and_verify_svnlook2, run_and_verify_svnadmin,
> run_and_verify_svnadmin2, run_and_verify_svnversion,
> run_and_verify_svnversion2,
> run_and_verify_svn_match_any, run_and_verify_svn_match_any2):
> Remove the caveat comment stating that exit-code checks are skipped
> for some platforms.
> (run_and_verify_svn): Remove the caveat comment stating that exit-code
> checks are skipped for some platforms. Pass binary_mode=0 to
> run_and_verify_svn2.
> (run_and_verify_svn2): Remove the caveat comment stating that exit-code
> checks are skipped for some platforms. Add a binary_mode boolean
> parameter. Replace call to run_svn with run_svn2.
>
> * subversion/tests/cmdline/import_tests.py
> (import_eol_style): Call run_and_verify_svn2 with binary_mode=1.
>
> * subversion/tests/cmdline/cat_tests.py,
> subversion/tests/cmdline/lock_tests.py,
> subversion/tests/cmdline/stat_tests.py:
> Pass binary_mode parameter to run_and_verify_svn2.
>
> ]]]
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Use subprocess.Popen for executing commands in cmdline
tests
Posted by Daniel Shahaf <d....@daniel.shahaf.co.il>.
jeremy hinds wrote on Sun, 18 May 2008 at 11:10 -0600:
> On Thu, May 15, 2008 at 1:32 PM, Daniel Shahaf <d....@daniel.shahaf.co.il> wrote:
> > [ Summary: patch below works on Linux and breaks four regression test on
> > Windows. ]
> >
> > jeremy hinds wrote on Fri, 25 Apr 2008 at 22:58 -0600:
> >> [[[
> >> In cmdline tests, use subprocess.Popen for executing commands to allow
> >> support for exit-code checks on both Windows and posix systems. This makes
> >> the test suite require Python version >= 2.4.
...
> >> ]]]
> >
> > Jeremy, what's the status of this patch? Are you looking into the
> > Windows test failures?
> >
>
> Sorry - I haven't forgotten or given up on this, but I had to
> side-line it for a while. I'm hoping to pick it up again soon.
>
Sounds good; I've filed it as issue #3201 so it doesn't get forgotten.
(I didn't CC you because I don't know your tigris username.)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Use subprocess.Popen for executing commands in cmdline tests
Posted by jeremy hinds <je...@gmail.com>.
On Thu, May 15, 2008 at 1:32 PM, Daniel Shahaf <d....@daniel.shahaf.co.il> wrote:
> [ Summary: patch below works on Linux and breaks four regression test on
> Windows. ]
>
> jeremy hinds wrote on Fri, 25 Apr 2008 at 22:58 -0600:
>> [[[
>> In cmdline tests, use subprocess.Popen for executing commands to allow
>> support for exit-code checks on both Windows and posix systems. This makes
>> the test suite require Python version >= 2.4.
>>
>> * subversion/tests/cmdline/svntest/main.py
>> (global): Import subprocess instead of popen2. Remove variable
>> "platform_with_popen3_class".
>> (_quote_arg): Removed, since commands + args to open_pipe are now lists.
>> (open_pipe): Replace "mode" character parameter with a "binary_mode" boolean
>> value. Use subprocess.Popen for spawning the child process.
>> (wait_on_pipe): Interpret the return value of wait() according to
>> subprocess.Popen semantics.
>> (spawn_process, copy_repos): When calling open_pipe, pass the command as
>> a list and binary_mode as a boolean.
>> (run_svn2): New, like run_svn but with a "binary_mode" boolean param.
>> (TestSpawningThread.run_one): Remove the caveat comment stating that
>> result (exit-code) is None on Windows.
>>
>> * subversion/tests/cmdline/svntest/actions.py
>> (run_and_verify_svnlook, run_and_verify_svnlook2, run_and_verify_svnadmin,
>> run_and_verify_svnadmin2, run_and_verify_svnversion,
>> run_and_verify_svnversion2,
>> run_and_verify_svn_match_any, run_and_verify_svn_match_any2):
>> Remove the caveat comment stating that exit-code checks are skipped
>> for some platforms.
>> (run_and_verify_svn): Remove the caveat comment stating that exit-code
>> checks are skipped for some platforms. Pass binary_mode=0 to
>> run_and_verify_svn2.
>> (run_and_verify_svn2): Remove the caveat comment stating that exit-code
>> checks are skipped for some platforms. Add a binary_mode boolean
>> parameter. Replace call to run_svn with run_svn2.
>>
>> * subversion/tests/cmdline/import_tests.py
>> (import_eol_style): Call run_and_verify_svn2 with binary_mode=1.
>>
>> * subversion/tests/cmdline/cat_tests.py,
>> subversion/tests/cmdline/lock_tests.py,
>> subversion/tests/cmdline/stat_tests.py:
>> Pass binary_mode parameter to run_and_verify_svn2.
>>
>> ]]]
>
> Jeremy, what's the status of this patch? Are you looking into the
> Windows test failures?
>
Sorry - I haven't forgotten or given up on this, but I had to
side-line it for a while. I'm hoping to pick it up again soon.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Use subprocess.Popen for executing commands in cmdline
tests
Posted by Daniel Shahaf <d....@daniel.shahaf.co.il>.
[ Summary: patch below works on Linux and breaks four regression test on
Windows. ]
jeremy hinds wrote on Fri, 25 Apr 2008 at 22:58 -0600:
> [[[
> In cmdline tests, use subprocess.Popen for executing commands to allow
> support for exit-code checks on both Windows and posix systems. This makes
> the test suite require Python version >= 2.4.
>
> * subversion/tests/cmdline/svntest/main.py
> (global): Import subprocess instead of popen2. Remove variable
> "platform_with_popen3_class".
> (_quote_arg): Removed, since commands + args to open_pipe are now lists.
> (open_pipe): Replace "mode" character parameter with a "binary_mode" boolean
> value. Use subprocess.Popen for spawning the child process.
> (wait_on_pipe): Interpret the return value of wait() according to
> subprocess.Popen semantics.
> (spawn_process, copy_repos): When calling open_pipe, pass the command as
> a list and binary_mode as a boolean.
> (run_svn2): New, like run_svn but with a "binary_mode" boolean param.
> (TestSpawningThread.run_one): Remove the caveat comment stating that
> result (exit-code) is None on Windows.
>
> * subversion/tests/cmdline/svntest/actions.py
> (run_and_verify_svnlook, run_and_verify_svnlook2, run_and_verify_svnadmin,
> run_and_verify_svnadmin2, run_and_verify_svnversion,
> run_and_verify_svnversion2,
> run_and_verify_svn_match_any, run_and_verify_svn_match_any2):
> Remove the caveat comment stating that exit-code checks are skipped
> for some platforms.
> (run_and_verify_svn): Remove the caveat comment stating that exit-code
> checks are skipped for some platforms. Pass binary_mode=0 to
> run_and_verify_svn2.
> (run_and_verify_svn2): Remove the caveat comment stating that exit-code
> checks are skipped for some platforms. Add a binary_mode boolean
> parameter. Replace call to run_svn with run_svn2.
>
> * subversion/tests/cmdline/import_tests.py
> (import_eol_style): Call run_and_verify_svn2 with binary_mode=1.
>
> * subversion/tests/cmdline/cat_tests.py,
> subversion/tests/cmdline/lock_tests.py,
> subversion/tests/cmdline/stat_tests.py:
> Pass binary_mode parameter to run_and_verify_svn2.
>
> ]]]
Jeremy, what's the status of this patch? Are you looking into the
Windows test failures?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Use subprocess.Popen for executing commands in cmdline tests
Posted by jeremy hinds <je...@gmail.com>.
2008/4/19 Daniel Shahaf <d....@daniel.shahaf.co.il>:
> jeremy hinds wrote on Fri, 18 Apr 2008 at 19:01 -0600:
>
> > I started working on getting a Windows dev environment set up so I can
> > at least make sure things can be expected to work before asking for
> > anybody else's time to test it. But I'm not quite there yet. In the
> > meantime, if someone else happens to get a chance to look at it, that
> > would be great.
> >
>
> I'm getting this error with your patch on Windows (ra_svn fsfs
> VC9Express):
>
> START: changelist_tests.py
> CMD: svnadmin.exe create svn-test-work\local_tmp\repos --bdb-txn-nosync --fs-type=fsfs
> Traceback (most recent call last):
> File "F:\Data\Unzip\svn\subversion/tests/cmdline/changelist_tests.py", line 885, in <module>
> svntest.main.run_tests(test_list)
> File "F:\Data\Unzip\svn\subversion\tests\cmdline\svntest\main.py", line 1386, in run_tests
> actions.setup_pristine_repository()
> File "F:\Data\Unzip\svn\subversion\tests\cmdline\svntest\actions.py", line 44, in setup_pristine_repository
> main.create_repos(main.pristine_dir)
> File "F:\Data\Unzip\svn\subversion\tests\cmdline\svntest\main.py", line 581, in create_repos
> path, *opts)
> File "F:\Data\Unzip\svn\subversion\tests\cmdline\svntest\main.py", line 309, in run_command
> None, *varargs)
> File "F:\Data\Unzip\svn\subversion\tests\cmdline\svntest\main.py", line 398, in run_command_stdin
> *varargs)
> File "F:\Data\Unzip\svn\subversion\tests\cmdline\svntest\main.py", line 365, in spawn_process
> infile, outfile, errfile, kid = open_pipe(cmd_argv, binary_mode)
> File "F:\Data\Unzip\svn\subversion\tests\cmdline\svntest\main.py", line 326, in open_pipe
> stdin=PIPE, stdout=PIPE, stderr=PIPE, close_fds=True)
> File "E:\Python\lib\subprocess.py", line 551, in __init__
> raise ValueError("close_fds is not supported on Windows "
> ValueError: close_fds is not supported on Windows platforms
> END: changelist_tests.py
>
> (The error repeats for every *_tests.py file.)
>
> Python docs agree: (http://docs.python.org/lib/node528.html)
>
> "If close_fds is true, all file descriptors except 0, 1 and 2 will
> be closed before the child process is executed. (Unix only)"
> ^^^^^^^^^^^
>
> Daniel
Thanks for giving it a try, Daniel. I saw that "Unix only" thing in
the documentation, but it seemed unclear what the behavior would be on
non-Unix. I was rather hoping for "silently ignored." My plan was to
get a Windows build working to test this before submitting the updated
patch, but I think I'm going to have to throw in the towel on that for
now. I did at least extract the Popen functionality into a separate
script and ran that on Windows (I should have done this before). The
pertinent lines in the patch are
+ close_fds=False
+ if is_posix_os():
+ close_fds=True
+ # (use universal newlines for text mode)
+ kid = Popen(command, shell=False, bufsize=-1,
+ universal_newlines=(not binary_mode),
+ stdin=PIPE, stdout=PIPE, stderr=PIPE, close_fds=close_fds)
+ return kid.stdin, kid.stdout, kid.stderr, (kid, command)
The only other difference between this and the previous iteration is
the removal of another comment in svntest/main.py.
[[[
In cmdline tests, use subprocess.Popen for executing commands to allow
support for exit-code checks on both Windows and posix systems. This makes
the test suite require Python version >= 2.4.
* subversion/tests/cmdline/svntest/main.py
(global): Import subprocess instead of popen2. Remove variable
"platform_with_popen3_class".
(_quote_arg): Removed, since commands + args to open_pipe are now lists.
(open_pipe): Replace "mode" character parameter with a "binary_mode" boolean
value. Use subprocess.Popen for spawning the child process.
(wait_on_pipe): Interpret the return value of wait() according to
subprocess.Popen semantics.
(spawn_process, copy_repos): When calling open_pipe, pass the command as
a list and binary_mode as a boolean.
(run_svn2): New, like run_svn but with a "binary_mode" boolean param.
(TestSpawningThread.run_one): Remove the caveat comment stating that
result (exit-code) is None on Windows.
* subversion/tests/cmdline/svntest/actions.py
(run_and_verify_svnlook, run_and_verify_svnlook2, run_and_verify_svnadmin,
run_and_verify_svnadmin2, run_and_verify_svnversion,
run_and_verify_svnversion2,
run_and_verify_svn_match_any, run_and_verify_svn_match_any2):
Remove the caveat comment stating that exit-code checks are skipped
for some platforms.
(run_and_verify_svn): Remove the caveat comment stating that exit-code
checks are skipped for some platforms. Pass binary_mode=0 to
run_and_verify_svn2.
(run_and_verify_svn2): Remove the caveat comment stating that exit-code
checks are skipped for some platforms. Add a binary_mode boolean
parameter. Replace call to run_svn with run_svn2.
* subversion/tests/cmdline/import_tests.py
(import_eol_style): Call run_and_verify_svn2 with binary_mode=1.
* subversion/tests/cmdline/cat_tests.py,
subversion/tests/cmdline/lock_tests.py,
subversion/tests/cmdline/stat_tests.py:
Pass binary_mode parameter to run_and_verify_svn2.
]]]
>
>
>
> >
> >
> > On Fri, Apr 18, 2008 at 6:35 AM, Branko Čibej <br...@xbc.nu> wrote:
> > > Daniel Shahaf wrote:
> > >
> > > > jeremy hinds wrote on Wed, 9 Apr 2008 at 22:49 -0600:
> > > >
> > > >
> > > > > 2008/4/8 jeremy hinds <je...@gmail.com>:
> > > > >
> > > > >
> > > > > > Thanks for clearing that up. While rewriting the patch to handle
> > > > > > universal_newlines and shell=False, another question came up.
> > > > > >
> > > > > > I made universal_newlines apply on any platform rather than just
> > > > > > Windows, and that seemed to work fine on everything except
> > > > > > import_tests.import_eol_style(). The problem is that
> > > > > > run_and_verify_svn() has text mode hard-coded in it, so the "\r\n"
> > > > > > sequences were converted to "\n". (Honestly, I don't understand how
> > > > > > that test worked accurately on Windows before.)
> > > > > >
> > > > > > The question is whether it is better to preserve old behavior by
> > > never
> > > > > > using universal_newlines on posix, or should I add a "binary_mode"
> > > > > > parameter to run_and_verify_svn2()?
> > > > > >
> > > > > >
> > > > > >
> > > > > I decided err on the side of flexibility and add a "binary_mode" param
> > > > > to run_and_verify_svn2(). So here is my new and improved (and
> > > > > hopefully good) patch. Again, I've only been able to test this on
> > > > > Linux.
> > > > >
> > > > > [[[
> > > > > In cmdline tests, use subprocess.Popen for executing commands to allow
> > > > > support for exit-code checks on both Windows and posix systems. This
> > > makes
> > > > > the test suite require Python version >= 2.4.
> > > > >
> > > > >
> > > > >
> > > >
> > > > Ping. Has anyone reviewed the binary_mode changes? Could someone verify
> > > that the patch works on Windows too?
> > > >
> > > >
> > >
> > > Oh sorry. Looks good to me. I can't test on Windows these days, sorry.
> > >
> > > -- Brane
> > >
> > >
> >
Re: [PATCH] Use subprocess.Popen for executing commands in cmdline
tests
Posted by Daniel Shahaf <d....@daniel.shahaf.co.il>.
jeremy hinds wrote on Fri, 18 Apr 2008 at 19:01 -0600:
> I started working on getting a Windows dev environment set up so I can
> at least make sure things can be expected to work before asking for
> anybody else's time to test it. But I'm not quite there yet. In the
> meantime, if someone else happens to get a chance to look at it, that
> would be great.
>
I'm getting this error with your patch on Windows (ra_svn fsfs
VC9Express):
START: changelist_tests.py
CMD: svnadmin.exe create svn-test-work\local_tmp\repos --bdb-txn-nosync --fs-type=fsfs
Traceback (most recent call last):
File "F:\Data\Unzip\svn\subversion/tests/cmdline/changelist_tests.py", line 885, in <module>
svntest.main.run_tests(test_list)
File "F:\Data\Unzip\svn\subversion\tests\cmdline\svntest\main.py", line 1386, in run_tests
actions.setup_pristine_repository()
File "F:\Data\Unzip\svn\subversion\tests\cmdline\svntest\actions.py", line 44, in setup_pristine_repository
main.create_repos(main.pristine_dir)
File "F:\Data\Unzip\svn\subversion\tests\cmdline\svntest\main.py", line 581, in create_repos
path, *opts)
File "F:\Data\Unzip\svn\subversion\tests\cmdline\svntest\main.py", line 309, in run_command
None, *varargs)
File "F:\Data\Unzip\svn\subversion\tests\cmdline\svntest\main.py", line 398, in run_command_stdin
*varargs)
File "F:\Data\Unzip\svn\subversion\tests\cmdline\svntest\main.py", line 365, in spawn_process
infile, outfile, errfile, kid = open_pipe(cmd_argv, binary_mode)
File "F:\Data\Unzip\svn\subversion\tests\cmdline\svntest\main.py", line 326, in open_pipe
stdin=PIPE, stdout=PIPE, stderr=PIPE, close_fds=True)
File "E:\Python\lib\subprocess.py", line 551, in __init__
raise ValueError("close_fds is not supported on Windows "
ValueError: close_fds is not supported on Windows platforms
END: changelist_tests.py
(The error repeats for every *_tests.py file.)
Python docs agree: (http://docs.python.org/lib/node528.html)
"If close_fds is true, all file descriptors except 0, 1 and 2 will
be closed before the child process is executed. (Unix only)"
^^^^^^^^^^^
Daniel
>
>
> On Fri, Apr 18, 2008 at 6:35 AM, Branko Čibej <br...@xbc.nu> wrote:
> > Daniel Shahaf wrote:
> >
> > > jeremy hinds wrote on Wed, 9 Apr 2008 at 22:49 -0600:
> > >
> > >
> > > > 2008/4/8 jeremy hinds <je...@gmail.com>:
> > > >
> > > >
> > > > > Thanks for clearing that up. While rewriting the patch to handle
> > > > > universal_newlines and shell=False, another question came up.
> > > > >
> > > > > I made universal_newlines apply on any platform rather than just
> > > > > Windows, and that seemed to work fine on everything except
> > > > > import_tests.import_eol_style(). The problem is that
> > > > > run_and_verify_svn() has text mode hard-coded in it, so the "\r\n"
> > > > > sequences were converted to "\n". (Honestly, I don't understand how
> > > > > that test worked accurately on Windows before.)
> > > > >
> > > > > The question is whether it is better to preserve old behavior by
> > never
> > > > > using universal_newlines on posix, or should I add a "binary_mode"
> > > > > parameter to run_and_verify_svn2()?
> > > > >
> > > > >
> > > > >
> > > > I decided err on the side of flexibility and add a "binary_mode" param
> > > > to run_and_verify_svn2(). So here is my new and improved (and
> > > > hopefully good) patch. Again, I've only been able to test this on
> > > > Linux.
> > > >
> > > > [[[
> > > > In cmdline tests, use subprocess.Popen for executing commands to allow
> > > > support for exit-code checks on both Windows and posix systems. This
> > makes
> > > > the test suite require Python version >= 2.4.
> > > >
> > > >
> > > >
> > >
> > > Ping. Has anyone reviewed the binary_mode changes? Could someone verify
> > that the patch works on Windows too?
> > >
> > >
> >
> > Oh sorry. Looks good to me. I can't test on Windows these days, sorry.
> >
> > -- Brane
> >
> >
>
Re: [PATCH] Use subprocess.Popen for executing commands in cmdline tests
Posted by jeremy hinds <je...@gmail.com>.
I started working on getting a Windows dev environment set up so I can
at least make sure things can be expected to work before asking for
anybody else's time to test it. But I'm not quite there yet. In the
meantime, if someone else happens to get a chance to look at it, that
would be great.
On Fri, Apr 18, 2008 at 6:35 AM, Branko Čibej <br...@xbc.nu> wrote:
> Daniel Shahaf wrote:
>
> > jeremy hinds wrote on Wed, 9 Apr 2008 at 22:49 -0600:
> >
> >
> > > 2008/4/8 jeremy hinds <je...@gmail.com>:
> > >
> > >
> > > > Thanks for clearing that up. While rewriting the patch to handle
> > > > universal_newlines and shell=False, another question came up.
> > > >
> > > > I made universal_newlines apply on any platform rather than just
> > > > Windows, and that seemed to work fine on everything except
> > > > import_tests.import_eol_style(). The problem is that
> > > > run_and_verify_svn() has text mode hard-coded in it, so the "\r\n"
> > > > sequences were converted to "\n". (Honestly, I don't understand how
> > > > that test worked accurately on Windows before.)
> > > >
> > > > The question is whether it is better to preserve old behavior by
> never
> > > > using universal_newlines on posix, or should I add a "binary_mode"
> > > > parameter to run_and_verify_svn2()?
> > > >
> > > >
> > > >
> > > I decided err on the side of flexibility and add a "binary_mode" param
> > > to run_and_verify_svn2(). So here is my new and improved (and
> > > hopefully good) patch. Again, I've only been able to test this on
> > > Linux.
> > >
> > > [[[
> > > In cmdline tests, use subprocess.Popen for executing commands to allow
> > > support for exit-code checks on both Windows and posix systems. This
> makes
> > > the test suite require Python version >= 2.4.
> > >
> > >
> > >
> >
> > Ping. Has anyone reviewed the binary_mode changes? Could someone verify
> that the patch works on Windows too?
> >
> >
>
> Oh sorry. Looks good to me. I can't test on Windows these days, sorry.
>
> -- Brane
>
>
Re: [PATCH] Use subprocess.Popen for executing commands in cmdline
tests
Posted by Branko Čibej <br...@xbc.nu>.
Daniel Shahaf wrote:
> jeremy hinds wrote on Wed, 9 Apr 2008 at 22:49 -0600:
>
>> 2008/4/8 jeremy hinds <je...@gmail.com>:
>>
>>> Thanks for clearing that up. While rewriting the patch to handle
>>> universal_newlines and shell=False, another question came up.
>>>
>>> I made universal_newlines apply on any platform rather than just
>>> Windows, and that seemed to work fine on everything except
>>> import_tests.import_eol_style(). The problem is that
>>> run_and_verify_svn() has text mode hard-coded in it, so the "\r\n"
>>> sequences were converted to "\n". (Honestly, I don't understand how
>>> that test worked accurately on Windows before.)
>>>
>>> The question is whether it is better to preserve old behavior by never
>>> using universal_newlines on posix, or should I add a "binary_mode"
>>> parameter to run_and_verify_svn2()?
>>>
>>>
>> I decided err on the side of flexibility and add a "binary_mode" param
>> to run_and_verify_svn2(). So here is my new and improved (and
>> hopefully good) patch. Again, I've only been able to test this on
>> Linux.
>>
>> [[[
>> In cmdline tests, use subprocess.Popen for executing commands to allow
>> support for exit-code checks on both Windows and posix systems. This makes
>> the test suite require Python version >= 2.4.
>>
>>
>
> Ping. Has anyone reviewed the binary_mode changes? Could someone verify
> that the patch works on Windows too?
>
Oh sorry. Looks good to me. I can't test on Windows these days, sorry.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Use subprocess.Popen for executing commands in cmdline
tests
Posted by Daniel Shahaf <d....@daniel.shahaf.co.il>.
jeremy hinds wrote on Wed, 9 Apr 2008 at 22:49 -0600:
> 2008/4/8 jeremy hinds <je...@gmail.com>:
> > Thanks for clearing that up. While rewriting the patch to handle
> > universal_newlines and shell=False, another question came up.
> >
> > I made universal_newlines apply on any platform rather than just
> > Windows, and that seemed to work fine on everything except
> > import_tests.import_eol_style(). The problem is that
> > run_and_verify_svn() has text mode hard-coded in it, so the "\r\n"
> > sequences were converted to "\n". (Honestly, I don't understand how
> > that test worked accurately on Windows before.)
> >
> > The question is whether it is better to preserve old behavior by never
> > using universal_newlines on posix, or should I add a "binary_mode"
> > parameter to run_and_verify_svn2()?
> >
>
> I decided err on the side of flexibility and add a "binary_mode" param
> to run_and_verify_svn2(). So here is my new and improved (and
> hopefully good) patch. Again, I've only been able to test this on
> Linux.
>
> [[[
> In cmdline tests, use subprocess.Popen for executing commands to allow
> support for exit-code checks on both Windows and posix systems. This makes
> the test suite require Python version >= 2.4.
>
Ping. Has anyone reviewed the binary_mode changes? Could someone verify
that the patch works on Windows too?
Thanks,
Daniel
> * subversion/tests/cmdline/svntest/main.py
> (global): Import subprocess instead of popen2. Remove variable
> "platform_with_popen3_class".
> (_quote_arg): Removed, since commands + args to open_pipe are now lists.
> (open_pipe): Replace "mode" character parameter with a "binary_mode" boolean
> value. Use subprocess.Popen for spawning the child process.
> (wait_on_pipe): Interpret the return value of wait() according to
> subprocess.Popen semantics.
> (spawn_process, copy_repos): When calling open_pipe, pass the command as
> a list and binary_mode as a boolean.
> (run_svn2): New, like run_svn but with a "binary_mode" boolean param.
>
> * subversion/tests/cmdline/svntest/actions.py
> (run_and_verify_svnlook, run_and_verify_svnlook2, run_and_verify_svnadmin,
> run_and_verify_svnadmin2, run_and_verify_svnversion,
> run_and_verify_svnversion2,
> run_and_verify_svn_match_any, run_and_verify_svn_match_any2):
> Remove the caveat comment stating that exit-code checks are skipped
> for some platforms.
> (run_and_verify_svn): Remove the caveat comment stating that exit-code
> checks are skipped for some platforms. Pass binary_mode=0 to
> run_and_verify_svn2.
> (run_and_verify_svn2): Remove the caveat comment stating that exit-code
> checks are skipped for some platforms. Add a binary_mode boolean
> parameter. Replace call to run_svn with run_svn2.
>
> * subversion/tests/cmdline/import_tests.py
> (import_eol_style): Call run_and_verify_svn2 with binary_mode=1.
>
> * subversion/tests/cmdline/cat_tests.py,
> subversion/tests/cmdline/lock_tests.py,
> subversion/tests/cmdline/stat_tests.py:
> Pass binary_mode parameter to run_and_verify_svn2.
>
> ]]]
>
>
> Thanks,
> Jeremy
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Use subprocess.Popen for executing commands in cmdline tests
Posted by jeremy hinds <je...@gmail.com>.
2008/4/8 jeremy hinds <je...@gmail.com>:
>
> On Mon, Apr 7, 2008 at 9:18 AM, Branko Čibej <br...@xbc.nu> wrote:
> > jeremy hinds wrote:
> >
> > >
> > > > * I /think/ i recall that we have the binary-mode parameter for a
> > > > reason. I don't remember the reason offhand, but just killing off
> > > > that option doesn't seem safe. (FWIW, the reason is most likely
> > > > related to text and binary being different on Windows.)
> > > >
> > > >
> > >
> > > I think you're right, in that the binary-mode param was only used when
> > > invoking os.popen3() on platforms without the Popen3 class (Windows).
> > > But having replaced both of those with subprocess.Popen, there's
> > > nowhere to use that parameter anymore. At least, I don't see anything
> > > that looks analogous in creationflags or anywhere like that.
> > >
> > >
> >
> > universal_newlines=True is text (non-binary) mode.
> >
> > -- Brane
>
> Thanks for clearing that up. While rewriting the patch to handle
> universal_newlines and shell=False, another question came up.
>
> I made universal_newlines apply on any platform rather than just
> Windows, and that seemed to work fine on everything except
> import_tests.import_eol_style(). The problem is that
> run_and_verify_svn() has text mode hard-coded in it, so the "\r\n"
> sequences were converted to "\n". (Honestly, I don't understand how
> that test worked accurately on Windows before.)
>
> The question is whether it is better to preserve old behavior by never
> using universal_newlines on posix, or should I add a "binary_mode"
> parameter to run_and_verify_svn2()?
>
I decided err on the side of flexibility and add a "binary_mode" param
to run_and_verify_svn2(). So here is my new and improved (and
hopefully good) patch. Again, I've only been able to test this on
Linux.
[[[
In cmdline tests, use subprocess.Popen for executing commands to allow
support for exit-code checks on both Windows and posix systems. This makes
the test suite require Python version >= 2.4.
* subversion/tests/cmdline/svntest/main.py
(global): Import subprocess instead of popen2. Remove variable
"platform_with_popen3_class".
(_quote_arg): Removed, since commands + args to open_pipe are now lists.
(open_pipe): Replace "mode" character parameter with a "binary_mode" boolean
value. Use subprocess.Popen for spawning the child process.
(wait_on_pipe): Interpret the return value of wait() according to
subprocess.Popen semantics.
(spawn_process, copy_repos): When calling open_pipe, pass the command as
a list and binary_mode as a boolean.
(run_svn2): New, like run_svn but with a "binary_mode" boolean param.
* subversion/tests/cmdline/svntest/actions.py
(run_and_verify_svnlook, run_and_verify_svnlook2, run_and_verify_svnadmin,
run_and_verify_svnadmin2, run_and_verify_svnversion,
run_and_verify_svnversion2,
run_and_verify_svn_match_any, run_and_verify_svn_match_any2):
Remove the caveat comment stating that exit-code checks are skipped
for some platforms.
(run_and_verify_svn): Remove the caveat comment stating that exit-code
checks are skipped for some platforms. Pass binary_mode=0 to
run_and_verify_svn2.
(run_and_verify_svn2): Remove the caveat comment stating that exit-code
checks are skipped for some platforms. Add a binary_mode boolean
parameter. Replace call to run_svn with run_svn2.
* subversion/tests/cmdline/import_tests.py
(import_eol_style): Call run_and_verify_svn2 with binary_mode=1.
* subversion/tests/cmdline/cat_tests.py,
subversion/tests/cmdline/lock_tests.py,
subversion/tests/cmdline/stat_tests.py:
Pass binary_mode parameter to run_and_verify_svn2.
]]]
Thanks,
Jeremy
Re: [PATCH] Use subprocess.Popen for executing commands in cmdline tests
Posted by jeremy hinds <je...@gmail.com>.
On Mon, Apr 7, 2008 at 9:18 AM, Branko Čibej <br...@xbc.nu> wrote:
> jeremy hinds wrote:
>
> >
> > > * I /think/ i recall that we have the binary-mode parameter for a
> > > reason. I don't remember the reason offhand, but just killing off
> > > that option doesn't seem safe. (FWIW, the reason is most likely
> > > related to text and binary being different on Windows.)
> > >
> > >
> >
> > I think you're right, in that the binary-mode param was only used when
> > invoking os.popen3() on platforms without the Popen3 class (Windows).
> > But having replaced both of those with subprocess.Popen, there's
> > nowhere to use that parameter anymore. At least, I don't see anything
> > that looks analogous in creationflags or anywhere like that.
> >
> >
>
> universal_newlines=True is text (non-binary) mode.
>
> -- Brane
Thanks for clearing that up. While rewriting the patch to handle
universal_newlines and shell=False, another question came up.
I made universal_newlines apply on any platform rather than just
Windows, and that seemed to work fine on everything except
import_tests.import_eol_style(). The problem is that
run_and_verify_svn() has text mode hard-coded in it, so the "\r\n"
sequences were converted to "\n". (Honestly, I don't understand how
that test worked accurately on Windows before.)
The question is whether it is better to preserve old behavior by never
using universal_newlines on posix, or should I add a "binary_mode"
parameter to run_and_verify_svn2()?
Re: [PATCH] Use subprocess.Popen for executing commands in cmdline
tests
Posted by Branko Čibej <br...@xbc.nu>.
jeremy hinds wrote:
>> * I /think/ i recall that we have the binary-mode parameter for a
>> reason. I don't remember the reason offhand, but just killing off
>> that option doesn't seem safe. (FWIW, the reason is most likely
>> related to text and binary being different on Windows.)
>>
>
> I think you're right, in that the binary-mode param was only used when
> invoking os.popen3() on platforms without the Popen3 class (Windows).
> But having replaced both of those with subprocess.Popen, there's
> nowhere to use that parameter anymore. At least, I don't see anything
> that looks analogous in creationflags or anywhere like that.
>
universal_newlines=True is text (non-binary) mode.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Use subprocess.Popen for executing commands in cmdline tests
Posted by Eric Hanchrow <of...@blarg.net>.
>>>>> "jeremy" == jeremy hinds <je...@gmail.com> writes:
jeremy> I could have sworn that I saw at least one instance of a
jeremy> pipeline being executed through this, but I'm not having
jeremy> any luck finding it now.
If you are simply referring to an example of how to create your own
pipeline with subprocess.Popen (as opposed to using the shell to do
it), here's the relevant example from the documentation:
17.1.3.2 Replacing shell pipe line
output=`dmesg | grep hda`
==>
p1 = Popen(["dmesg"], stdout=PIPE)
p2 = Popen(["grep", "hda"], stdin=p1.stdout, stdout=PIPE)
output = p2.communicate()[0]
--
I'd like to see Rush Limbaugh tell Omar he can't get
married.
--Alex Kotlowitz. You hadda be there
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Use subprocess.Popen for executing commands in cmdline tests
Posted by jeremy hinds <je...@gmail.com>.
On Mon, Apr 7, 2008 at 3:35 AM, Branko Čibej <br...@xbc.nu> wrote:
> jeremy hinds wrote:
>
> > [[[
> > In cmdline tests, use subprocess.Popen for executing commands to allow
> > support for exit-code checks on both Windows and posix systems. This
> makes
> > the test suite require Python version >= 2.4.
> >
> >
>
> I do have a couple questions.
>
> * Why run Popen with shell=True? I don't think we need that and can
> avoid all kinds of quoting magic that we do now, since Popen does
> the system-specific argument quoting for us.
I could have sworn that I saw at least one instance of a pipeline
being executed through this, but I'm not having any luck finding it
now. I might have been imagining things. I'll take a closer look and
see if there's any reason not to pass the command & args to Popen as a
list.
> * I /think/ i recall that we have the binary-mode parameter for a
> reason. I don't remember the reason offhand, but just killing off
> that option doesn't seem safe. (FWIW, the reason is most likely
> related to text and binary being different on Windows.)
I think you're right, in that the binary-mode param was only used when
invoking os.popen3() on platforms without the Popen3 class (Windows).
But having replaced both of those with subprocess.Popen, there's
nowhere to use that parameter anymore. At least, I don't see anything
that looks analogous in creationflags or anywhere like that.
>
> -- Brane
>
>
Re: [PATCH] Use subprocess.Popen for executing commands in cmdline
tests
Posted by Branko Čibej <br...@xbc.nu>.
jeremy hinds wrote:
> [[[
> In cmdline tests, use subprocess.Popen for executing commands to allow
> support for exit-code checks on both Windows and posix systems. This makes
> the test suite require Python version >= 2.4.
>
I do have a couple questions.
* Why run Popen with shell=True? I don't think we need that and can
avoid all kinds of quoting magic that we do now, since Popen does
the system-specific argument quoting for us.
* I /think/ i recall that we have the binary-mode parameter for a
reason. I don't remember the reason offhand, but just killing off
that option doesn't seem safe. (FWIW, the reason is most likely
related to text and binary being different on Windows.)
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org