[Svnmerge] svnmerge rewritten in Python

[Jim Fulton]

Giovanni Bajo wrote:
> Jim Fulton <jim at zope.com> wrote:
> 
> 
>>>- It's a straightforward conversion. It's *almost* 1:1 with the shell
>>>version, the only big difference being that revision lists are handled by
>>>a class RevisionList. At this point, I did not bother refactoring code or
>>>changing structure too much, it can be done in followups if there is
>>>interest to maintain and work on this Python version.
>>
>>I don't think a shell-based version can be robust and portable to windows.
>>In particular, there's no portable way to run a process and get both
> 
> output
> 
>>and a return code.
> 
> 
> It works for me, so I don't see where the problem is. I wrote many tools in
> python which parses shell outputs, and they all work under both Linux and
> Windows. If I wanted to use a 2.4-ism, I could even have imported
> subprocess, which was created exactly for this purpose.
 >
> 
>>This means you can't get output of svn commands and
>>get detect errors cleanly.
> 
> 
> It works already. Not in the version I posted, but this is the updated
> version
> 
> def launch(cmd):
>     """Launch a sub-process. Return its output (lines) and exit code."""
>     if os.name == 'posix':
>         p = popen2.Popen4(cmd)
>         p.tochild.close()
>         out = p.fromchild.readlines()
>         ret = p.wait()
>         if ret == 0:
>             ret = None
>         else:
>             ret >>= 8
>     else:
>         i,k = os.popen4(cmd)
>         i.close()
>         out = k.readlines()
>         ret = k.close()
> 
>     if ret is None:
>         return out
>     raise LaunchError(ret, cmd, out)
> 
> This works correctly under both Linux and Windows:

This is interesting. This behavior, getting the return status
as a result of a close of a file returned by popen4, is inconsistent
with the Python library documentation.  If we rely on this, we
need, IMO, to get the Python documentation fixed -- by bringing this
up on python-dev.  If we can rely on this, I retract my objections
based on inability to get exit status and output.

I still think it would be cleaner to get data back from
the apis than to parse output.

...

>>This is why I ultimately decided to use the
>>Python subversion bindings.  Using these is not easy though do to the
>>extremely sparse documentation.  I do think the result would be cleaner in
>>the long run.  For example, there is no need to parse svn command output.
>>The svn api returns real data.
> 
> 
> I thought about it as well. My opinion is that svnmerge is too simple a tool
> at the moment to use the bindings. I don't see how they can solve problem.
> Do you believe the code is going to get smaller or easier to follow? I fear
> it is going to be quite the opposite...

Understood.  I do think it would be cleaner.

BTW, one of the reasons I wanted this in Python is that I want to see
more functionality, especially to deal with 2-way merging.

But there are a number of ways the user interface can be improved, as
you've begun to notice.

> I think there is a gain in using small, self-contained scripts which spawn
> external processes, especially when those processes have been designed to be
> run like that. This is the case of almost anything under UNIX, and
> Subversion lists its "machine-parsable output" amongs its features. The fact
> that the script was born as a shell script tells everything.

Hm, if that is a goal, then that's a good thing.  I'd be more convinced
if more of the commands had uml output options.  From the amount that I've
played with the libraries, I do like dealing with the data better.

> Can you show us your working-in-progress script?

Sure.  It doesn't really work at all.  I was working on a doc test
and was in the middle of implementing init.

> 
> 
>>>- It's lightly tested. Testing and bugreports are welcome :)
>>
>>Right, my intent was to include fairly extensive doctests.
> 
> 
> Yeah, I did some basic testing of all the low-level functions, but I was
> thikning of testing the whole program. I can't see a way to do it without
> setting up a test repository, ecc, which is unfortunate. Anyway, having
> doctests for the simple functions is fine.

I strongly recommend automated tests included with the script --
preferably as doc tests.  You should be able to get an idea of the
testing approach I was following in my script.

Jim
-------------- next part --------------
A non-text attachment was scrubbed...
Name: svnmerge.py
Type: application/x-httpd-cgi
Size: 13510 bytes
Desc: not available
Url : /pipermail/svnmerge/attachments/20050923/f7addfee/attachment.bin