Re: [rdiff-backup-users] More windows patches

[Josh Nisly]

Andrew Ferguson wrote:

> On Apr 13, 2008, at 6:24 PM, Josh Nisly wrote:
> > Andrew Ferguson wrote:
> > > On Apr 11, 2008, at 5:09 PM, Josh Nisly wrote:
> > > > Here's another patch for rdiff-backup on Windows. os.popen does not 
> > > > work correctly on Windows, so this patch uses subprocess.Popen for 
> > > > windows platforms.
> > >
> > >
> > > Great. This patch and your previous one have been committed to CVS.
> >
> > Thanks! Here is the next round of patches. :-)
> >
> > The SIGTERM, etc. signals are not defined (and don't really make 
> > sense) on Windows, so rdiff-backup-signals.patch removes them
>
> Ok, is there some other representation of the Close/Quit/Abort event 
> that we should catch instead? The point of those checks is to 
> gracefully catch the user killing the process. If we don't do so 
> gracefully, then Python generates a rather verbose stack trace. Yuck.
Ok, SIGINT on Windows seems to work for this. I found that SIGTERM 
actually exists on Windows, but SIGQUIT and SIGHUP do not. SIGTERM 
doesn't catch Ctrl+C, though, which is why the attached patch add 
SIGINT. It seems like SIGINT would be the right thing to use on posix 
too, but I'll defer to you on that one. Let me know what you think of 
this patch.
> > os.kill is not available on Windows, so there's no way from Python to 
> > determine whether a process is running, without using the PyWin32 
> > package. (We check to make sure that another rdiff-backup process 
> > isn't already running, so that we don't stomp on each other's data.) 
> > The rdiff-backup-kill.patch simply ignores the check. So here is my 
> > question: is it acceptable to import and use platform-specific 
> > modules? We would obviously need to handle it correctly if they 
> > weren't there. Seems like we do something similar for Mac OS X 
> > specific functionality, but I wanted to check.
>
> Yes, please use the PyWin32 package to implement the functionality. It 
> is very important that we make sure that only 1 rdiff-backup instance 
> is accessing a repository at a time.
>
> As a general rule, the goal here is to have absolute parity between 
> the posix and nt platforms. In cases where nt truly does not support 
> it (eg, symlinks, hardlinks), it's ok for the feature to be dropped 
> (which you did in the appropriate place: fs_abilities.py). The 
> os.kill() case is like popen2() -- rdiff-backup should use the 
> Windows-specific code as necessary.
Ok, I'll look into this and report back once I have something that works.
> I will hold these patches until your reply (since I presume you will 
> want to improve them).
Thanks for your feedback on the patches. I appreciate the effort you've 
made to give feedback and commit them.
> Andrew

JoshN

rdiff-backup-signals.tar
Description: Unix tar archive