Re: [rdiff-backup-users] rdiff-backup 4 windows (no cygwin needed!)

[Andrew Ferguson]

On Jun 6, 2008, at 12:44 PM, Josh Nisly wrote:

> Armando M. Baratti wrote:
> > Fred Gansevles escreveu:
> > > Hello,
> > >
> > >   I am currently working on a port for rdiff-backup that runs with
> > > ActivePython.
> > >
> > >   It is a work in progress, but the first builds of the  
> > > _librsync.pyd
> > > and C.pyd already work (mostly).
> > >
> > >   The next step is to build a posix-module that, when imported,
> > > magically provides a _working_ os.stat,
> > >   pwd and grp modules and whatever is needed.
> > >
> > >   Since I am new to the rdiff-backup scene, could somebody introduce
> > > me to someone that know more
> > >   about the rdiff-on-windows problems?
> > >
> > >   What I have build so far can be downloaded from
> > > http://www.betterbe.com/rdiff
> > >
> > >   I'd like to know if more people think this port is useful.
> > >
> > > Fred Gansevles.
> > > address@hidden
> > There is (was?) someone else ( Josh Nisly - rdiffbackup  at   
> > joshnisly  dot  com) working on a native Windows ports of rdiff- 
> > backup:
> > http://lists.gnu.org/archive/html/rdiff-backup-users/2008-04/msg00001.html
> >
> > Maybe you could join efforts.
> Yes, I have built a working rdiff-backup that builds into a native  
> win32 executable with py2exe. I've posted patches to the mailing  
> list, and am waiting for Andrew Ferguson to apply them.
>
> I think he said a month or so ago that he was going to apply them  
> soon, but I haven't heard from him.


Sorry about that -- didn't have any spare time for a few months there.  
Thanks for the reminder.


I've looked at all of your patches and applied each one except rdiff- 
backup-stat.patch and rdiff-backup-ids.patch.

Regarding rdiff-backup-ids.patch:

Moving the import statements from file-wide to function-specific  
concerns me. I don't know whether Python is smart enough to optimize  
that code path. It would be terrible if the pwd/grp modules were  
loaded & unloaded each  time one of those functions was called (since  
they are called for each file, I believe). Given that there are  
already several initialization functions in user_group.py, I think it  
makes sense to move the module importing to the proper init functions  
(you'll have to trace through and see where) and then setup some  
variable which handles the Windows special case (that is, the lack of  
grp and pwd modules).

Regarding rdiff-backup-stat.patch:

I agree that there's a long history of whole-sale copying the correct  
code into cmodule.c to fix platform-specific issues. However, the code  
copy in this patch is enormous, and I think there's a better way to do  
this. The calls to the stat code in cmodule.c originate from the  
function rpath.setdata(), rpath.py:789. If you look just below that  
function, you'll see an unused function 'make_file_dict_old'.

Before cmodule.c existed, rdiff-backup clearly used to rely on the  
os.lstat() function which Python provided. The change to use rdiff- 
backup's own stat functions was made before I got involved in this  
project, so I don't know why that was done. It may have been done to  
continue supporting Python 2.2 (now over 5 years old), but that's just  
a guess. Or perhaps there was a time when os.lstat() was jut not up to  
snuff.

At any rate, today, with a modern python, os.lstat() is clearly the  
way to go on Windows. Changing rpath.setdata() to call os.lstat() on  
windows (and then re-invigorating make_file_dict_old -- maybe  
'make_file_dict_windows' ?) seems like the best path for two reasons:  
1) It keeps cmodule.c smaller and 2) We benefit from improvements to  
Python's handling of os.lstat() on Windows.

If the reason for cmodule.c is to maintain support for Python 2.2,  
then I think it's fair to require Python 2.5 (or whatever version has  
good Windows support) for those running rdiff-backup on Windows. Your  
patches are truly making the port possible, so we can define  
requirements at this time.

When Python 3 comes along later this year, I think it might be a good  
time to 'clean house' and drop the legacy code on all platforms, but  
that's still down the road (and will of course be a new branch of the  
rdiff-backup code base).

Oh, and when you sent me rdiff-backup-stat.patch, you mentioned the  
lack of proper charset support. I think relying on Python's os.lstat()  
on Windows might help illuminate some of those bugs.


looking forward to the improved patches and MANY thanks,
Andrew