Andrew Ferguson wrote:
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.
[EMAIL PROTECTED]
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).
Fine. I'm pretty sure that Python is smart enough, but it is a little
weird. I'll fix this.
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'.
Yes. My understanding is that rdiff-backup used to use that function,
but in old versions of Python it didn't work well.
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.
I don't know if Python 2.2's lstat function is good enough, but I would
be very worried about switching over to it for everyone. I agree that
we need a platform specific check.
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.
Ok, agreed. What if I would make the cmodule throw an AttributeError on
windows, and we would catch that in make_file_dict, and fall back to the
os.lstat()? I'm trying to avoid checking os.name every single time the
function is called, because it is, IIUC, pretty much the heart of
rdiff-backup. I don't want to introduce an extra round-trip to the
remote end every time. I also like doing it this way because it feels
cleaner to not specifically check for os.name in python code if possible.
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).
I agree. A new branch would definitely be needed, because enterprise
platforms (RHEL, for example) still use old versions of python (I
occasionally still get users running Python 2.3!
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.
Yes, Python's os.lstat() function would handle charsets perfectly. I
think this is the right way to go.
looking forward to the improved patches and MANY thanks,
Andrew
Thanks for your feedback. I'll submit updated patches as soon as I can.
JoshN
_______________________________________________
rdiff-backup-users mailing list at rdiff-backup-users@nongnu.org
http://lists.nongnu.org/mailman/listinfo/rdiff-backup-users
Wiki URL: http://rdiff-backup.solutionsfirst.com.au/index.php/RdiffBackupWiki
