You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Niels de Vos <ni...@wincor-nixdorf.com> on 2008/06/27 10:12:22 UTC

[PATCH] hooks-scripts: fix svn2feed.py on Windows

Hello,

running svn2feed.py on Windows results in an error. It seems that 
popen() does not allow a list as first parameter. The Python-docs 
explicitly note that on UNIX a sequence can be passed, Windows is not 
mentioned. Obviously Python on Windows only accepts a string.

Best regards,
Niels de Vos

--
popen2.popen3 on Windows does not allow a list or sequence as first 
parameter.

* tools/hook-scripts/svn2feed.py: change the the cmd-list to a string

Re: [PATCH] hooks-scripts: fix svn2feed.py on Windows

Posted by Peter Samuelson <pe...@p12n.org>.
[Peter Samuelson]
> I suggest doing something along the lines of
> 
>   cmd = '"%s" info -r%s "%s"' % svnlook_cmd, revision, self.repos_path

Forgot to mention - this has a security hole if users are allowed to
create their own repositories which will then be served via svnserve or
mod_dav_svn.  Not a problem on Windows, perhaps, if " is not a legal
character in a filename, but on Unix it is, so I could run arbitrary
code as the svnserve or apache user by creating a repository called

  /var/lib/svn/repos/"; do whatever I want

Much better to pass a list to popen3.  I wonder why they don't bother
to support (emulate) that on Windows.
-- 
Peter Samuelson | org-tld!p12n!peter | http://p12n.org/

Re: [PATCH] hooks-scripts: fix svn2feed.py on Windows

Posted by Daniel Shahaf <d....@daniel.shahaf.co.il>.
Peter Samuelson wrote on Sat, 28 Jun 2008 at 16:15 -0500:
> I'm told you can get the same safety with the 'subprocess' module ...
> which is only provided on python2.4 (from December 2004) and up.  Did
> we ever decide whether we could ditch python 2.2 and 2.3 users yet?
> 

Jeremy Hinds had a patch to use subprocess.Popen in the Python tests.  
The patch hasn't been committed, but as I recall, the transition to 2.4 
was discussed then and agreed.



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] hooks-scripts: fix svn2feed.py on Windows

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Daniel Shahaf wrote on Wed, 13 Aug 2008 at 06:42 +0300:
> I'll commit in a few days if no one objects.
> 

r32513.  I wrote a log message; next time please remember to provide 
one.

> Thanks for the patch,
> 
> Daniel
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] hooks-scripts: fix svn2feed.py on Windows

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Niels de Vos wrote on Tue, 12 Aug 2008 at 15:10 +0200:
> Ups, I' very sorry. Somehow this slipped through :(
> You should not have any problems with the patch below. I only needed to
> add 'stdout=subprocess.PIPE' to subprocess.Popen().
> 

Yes, works now.  (Well, sort of; the <author><name> tag has a ^M
character that doesn't exist in the repository, and the 'Modified' list 
is printed as a repr()'d array -- with [] and without newlines -- but I 
assume that's not due to your patch.)

I'll commit in a few days if no one objects.

Thanks for the patch,

Daniel

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] hooks-scripts: fix svn2feed.py on Windows

Posted by Niels de Vos <ni...@wincor-nixdorf.com>.
Daniel Shahaf wrote:
> (Patch manager hat on.  I was about to file this, but then...)

Ups, I' very sorry. Somehow this slipped through :(
You should not have any problems with the patch below. I only needed to
add 'stdout=subprocess.PIPE' to subprocess.Popen().

Thanks again, Niels

> Niels de Vos wrote on Mon, 28 Jul 2008 at 10:39 +0200:
>> Okay, here it is. This patch replaces the non-Windows popen2 usage with
>> the platform independent subprocess module. As discussed, this requires
>> at least Pyhon-2.4 as earlier versions do not contain the subprocess
>> module.


Index: svn2feed.py
===================================================================
--- svn2feed.py	(revision 32448)
+++ svn2feed.py	(working copy)
@@ -78,7 +78,7 @@
 
 import getopt
 import os
-import popen2
+import subprocess
 import cPickle as pickle
 import datetime
 import time
@@ -127,18 +127,14 @@
         revision = str(revision)
 
         cmd = [self.svnlook_cmd, 'info', '-r', revision, self.repos_path]
-        child_out, child_in, child_err = popen2.popen3(cmd)
-        info_lines = child_out.readlines()
-        child_out.close()
-        child_in.close()
-        child_err.close()
+        proc = subprocess.Popen(cmd, stdout=subprocess.PIPE)
+        proc.wait()
+        info_lines = proc.stdout.readlines()
 
         cmd = [self.svnlook_cmd, 'changed', '-r', revision, self.repos_path]
-        child_out, child_in, child_err = popen2.popen3(cmd)
-        changed_data = child_out.read()
-        child_out.close()
-        child_in.close()
-        child_err.close()
+        proc = subprocess.Popen(cmd, stdout=subprocess.PIPE)
+        proc.wait()
+        changed_data = proc.stdout.readlines()
 
         desc = ("\nRevision: %s\nLog: %sModified: \n%s"
                 % (revision, info_lines[3], changed_data))
@@ -411,13 +407,10 @@
         svnlook_cmd = 'svnlook'
         if svn_path is not None:
             svnlook_cmd = os.path.join(svn_path, 'svnlook')
-        child_out, child_in, child_err = popen2.popen3([svnlook_cmd,
-                                                        'youngest',
-                                                        repos_path])
-        cmd_out = child_out.readlines()
-        child_out.close()
-        child_in.close()
-        child_err.close()
+        cmd = [svnlook_cmd, 'youngest', repos_path]
+        proc = subprocess.Popen(cmd, stdout=subprocess.PIPE)
+        proc.wait()
+        cmd_out = proc.stdout.readlines()
         try:
             revisions = [int(cmd_out[0])]
         except IndexError, msg:


Re: [PATCH] hooks-scripts: fix svn2feed.py on Windows

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
(Patch manager hat on.  I was about to file this, but then...)

Niels de Vos wrote on Mon, 28 Jul 2008 at 10:39 +0200:
> Okay, here it is. This patch replaces the non-Windows popen2 usage with
> the platform independent subprocess module. As discussed, this requires
> at least Pyhon-2.4 as earlier versions do not contain the subprocess
> module.
> 
> Thanks,
> Niels
> 
> Index: svn2feed.py
> ===================================================================
> --- svn2feed.py	(revision 32316)
> +++ svn2feed.py	(working copy)
> @@ -78,7 +78,7 @@
>  
>  import getopt
>  import os
> -import popen2
> +import subprocess
>  import cPickle as pickle
>  import datetime
>  import time
> @@ -127,18 +127,14 @@
>          revision = str(revision)
>  
>          cmd = [self.svnlook_cmd, 'info', '-r', revision, self.repos_path]
> -        child_out, child_in, child_err = popen2.popen3(cmd)
> -        info_lines = child_out.readlines()
> -        child_out.close()
> -        child_in.close()
> -        child_err.close()
> +        proc = subprocess.Popen(cmd, stdout=subprocess.PIPE)
> +        proc.wait()
> +        info_lines = proc.stdout.readlines()
>  
>          cmd = [self.svnlook_cmd, 'changed', '-r', revision, self.repos_path]
> -        child_out, child_in, child_err = popen2.popen3(cmd)
> -        changed_data = child_out.read()
> -        child_out.close()
> -        child_in.close()
> -        child_err.close()
> +        proc = subprocess.Popen(cmd, stdout=subprocess.PIPE)
> +        proc.wait()
> +        changed_data = proc.stdout.readlines()
>  
>          desc = ("\nRevision: %s\nLog: %sModified: \n%s"
>                  % (revision, info_lines[3], changed_data))
> @@ -411,13 +407,10 @@
>          svnlook_cmd = 'svnlook'
>          if svn_path is not None:
>              svnlook_cmd = os.path.join(svn_path, 'svnlook')
> -        child_out, child_in, child_err = popen2.popen3([svnlook_cmd,
> -                                                        'youngest',
> -                                                        repos_path])
> -        cmd_out = child_out.readlines()
> -        child_out.close()
> -        child_in.close()
> -        child_err.close()
> +        cmd = [svnlook_cmd, 'youngest', repos_path]
> +        proc = subprocess.Popen(cmd)
> +        proc.wait()
> +        cmd_out = proc.stdout.readlines()

I got an error with the patched version:

    tools\hook-scripts> python -V
    Python 2.5.1

    tools\hook-scripts> python svn2feed.py -F atom -u http://bar -U http://bar C:\tmp\svn\repos1
    1
    Traceback (most recent call last):
      File "svn2feed.py", line 452, in <module>
        main()
      File "svn2feed.py", line 413, in main
        cmd_out = proc.stdout.readlines()
    AttributeError: 'NoneType' object has no attribute 'readlines'

>          try:
>              revisions = [int(cmd_out[0])]
>          except IndexError, msg:
> 
> 
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] hooks-scripts: fix svn2feed.py on Windows

Posted by Niels de Vos <ni...@wincor-nixdorf.com>.
Daniel Shahaf wrote:
> Ping; this last iteration of the patch has received little response.
> 
> URL: http://thread.gmane.org/gmane.comp.version-control.subversion.devel/101880/focus=101933
> 
> (more below)
> 
> Niels de Vos wrote on Mon, 30 Jun 2008 at 12:14 +0200:
>> Here's a patch for review with popen.popen* replaced by the 
>> subprocess-module. It should work on both UNIX and Windows. Only tested 
>> on UNIX for now.
>>
>> Note that this patch also changes the output of the feed to match the 
>> format of viewvc more.
>>
>> I could update the patch to only reflect the popen-subprocess change if 
>> needed.
>>
> 
> Please do so; one patch should contain one logical change, and patches 
> that contain multiple unrelated changes are much less likely to be 
> applied.

Okay, here it is. This patch replaces the non-Windows popen2 usage with
the platform independent subprocess module. As discussed, this requires
at least Pyhon-2.4 as earlier versions do not contain the subprocess
module.

Thanks,
Niels

Index: svn2feed.py
===================================================================
--- svn2feed.py	(revision 32316)
+++ svn2feed.py	(working copy)
@@ -78,7 +78,7 @@
 
 import getopt
 import os
-import popen2
+import subprocess
 import cPickle as pickle
 import datetime
 import time
@@ -127,18 +127,14 @@
         revision = str(revision)
 
         cmd = [self.svnlook_cmd, 'info', '-r', revision, self.repos_path]
-        child_out, child_in, child_err = popen2.popen3(cmd)
-        info_lines = child_out.readlines()
-        child_out.close()
-        child_in.close()
-        child_err.close()
+        proc = subprocess.Popen(cmd, stdout=subprocess.PIPE)
+        proc.wait()
+        info_lines = proc.stdout.readlines()
 
         cmd = [self.svnlook_cmd, 'changed', '-r', revision, self.repos_path]
-        child_out, child_in, child_err = popen2.popen3(cmd)
-        changed_data = child_out.read()
-        child_out.close()
-        child_in.close()
-        child_err.close()
+        proc = subprocess.Popen(cmd, stdout=subprocess.PIPE)
+        proc.wait()
+        changed_data = proc.stdout.readlines()
 
         desc = ("\nRevision: %s\nLog: %sModified: \n%s"
                 % (revision, info_lines[3], changed_data))
@@ -411,13 +407,10 @@
         svnlook_cmd = 'svnlook'
         if svn_path is not None:
             svnlook_cmd = os.path.join(svn_path, 'svnlook')
-        child_out, child_in, child_err = popen2.popen3([svnlook_cmd,
-                                                        'youngest',
-                                                        repos_path])
-        cmd_out = child_out.readlines()
-        child_out.close()
-        child_in.close()
-        child_err.close()
+        cmd = [svnlook_cmd, 'youngest', repos_path]
+        proc = subprocess.Popen(cmd)
+        proc.wait()
+        cmd_out = proc.stdout.readlines()
         try:
             revisions = [int(cmd_out[0])]
         except IndexError, msg:


-- 
WINCOR NIXDORF International GmbH 
Sitz der Gesellschaft: Paderborn 
Registergericht Paderborn HRB 3507
Geschäftsführer: Eckard Heidloff (Vorsitzender), Stefan Auerbach, Dr. Jürgen Wunram
Vorsitzender des Aufsichtsrats: Karl-Heinz Stiller 
Steuernummer: 339/5884/0020 - Ust-ID Nr.: DE812927716 - WEEE-Reg.-Nr. DE44477193

Diese E-Mail enthält vertrauliche Informationen. Wenn Sie nicht der richtige Adressat sind oder diese E-Mail irrtümlich erhalten haben, informieren Sie bitte sofort den Absender und vernichten Sie diese E-Mail. Das unerlaubte Kopieren sowie die unbefugte Weitergabe dieser E-Mail ist nicht gestattet.

This e-mail may contain confidential information. If you are not the intended recipient (or have received this e-mail in error) please notify the sender immediately and destroy this e-mail. Any unauthorised copying, disclosure or distribution of the material in this e-mail is strictly forbidden. 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] hooks-scripts: fix svn2feed.py on Windows

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Ping; this last iteration of the patch has received little response.

URL: http://thread.gmane.org/gmane.comp.version-control.subversion.devel/101880/focus=101933

(more below)

Niels de Vos wrote on Mon, 30 Jun 2008 at 12:14 +0200:
> Here's a patch for review with popen.popen* replaced by the 
> subprocess-module. It should work on both UNIX and Windows. Only tested 
> on UNIX for now.
> 
> Note that this patch also changes the output of the feed to match the 
> format of viewvc more.
> 
> I could update the patch to only reflect the popen-subprocess change if 
> needed.
> 

Please do so; one patch should contain one logical change, and patches 
that contain multiple unrelated changes are much less likely to be 
applied.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] hooks-scripts: fix svn2feed.py on Windows

Posted by Niels de Vos <ni...@wincor-nixdorf.com>.
Martin Furter wrote:
> On Mon, 30 Jun 2008, Niels de Vos wrote:
> 
>> +        for f in changed_data:
>> +                desc += '<li>%s</li>\n' % re.split('\s', f, 1)[1]
> 
> Wouldn't the normal string split work too here?
>       desc += '<li>%s</li>\n' % f.split(None, 1)[1]

Yes, of course. That would not be a problem and 'import re' could be 
removed again.

Niels


Re: [PATCH] hooks-scripts: fix svn2feed.py on Windows

Posted by Martin Furter <mf...@rola.ch>.

On Mon, 30 Jun 2008, Niels de Vos wrote:

> +        for f in changed_data:
> +                desc += '<li>%s</li>\n' % re.split('\s', f, 1)[1]

Wouldn't the normal string split work too here?
       desc += '<li>%s</li>\n' % f.split(None, 1)[1]

Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] hooks-scripts: fix svn2feed.py on Windows

Posted by Niels de Vos <ni...@wincor-nixdorf.com>.
Peter Samuelson wrote:
> [Peter Samuelson]
>> How annoying.  By passing a list, you don't have to worry about
>> quoting your arguments, to account for spaces and such in filenames.
> 
> I'm told you can get the same safety with the 'subprocess' module ...
> which is only provided on python2.4 (from December 2004) and up.  Did
> we ever decide whether we could ditch python 2.2 and 2.3 users yet?

Here's a patch for review with popen.popen* replaced by the 
subprocess-module. It should work on both UNIX and Windows. Only tested 
on UNIX for now.

Note that this patch also changes the output of the feed to match the 
format of viewvc more.

I could update the patch to only reflect the popen-subprocess change if 
needed.

Thanks,
Niels

Re: [PATCH] hooks-scripts: fix svn2feed.py on Windows

Posted by Peter Samuelson <pe...@p12n.org>.
[Peter Samuelson]
> How annoying.  By passing a list, you don't have to worry about
> quoting your arguments, to account for spaces and such in filenames.

I'm told you can get the same safety with the 'subprocess' module ...
which is only provided on python2.4 (from December 2004) and up.  Did
we ever decide whether we could ditch python 2.2 and 2.3 users yet?
-- 
Peter Samuelson | org-tld!p12n!peter | http://p12n.org/

Re: [PATCH] hooks-scripts: fix svn2feed.py on Windows

Posted by Peter Samuelson <pe...@p12n.org>.
[Niels de Vos]
> It seems that popen() does not allow a list as first parameter. The
> Python-docs explicitly note that on UNIX a sequence can be passed,
> Windows is not mentioned.

How annoying.  By passing a list, you don't have to worry about quoting
your arguments, to account for spaces and such in filenames.  Your
patch will not work, for example, if a repository path is under
C:\Documents and Settings, or if svnlook_cmd is under C:\Program Files.

I suggest doing something along the lines of

  cmd = '"%s" info -r%s "%s"' % svnlook_cmd, revision, self.repos_path

-- 
Peter Samuelson | org-tld!p12n!peter | http://p12n.org/