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

[Josh Nisly]

Josh Nisly wrote:
> 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.
> > > > > 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).
> 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.

Attached is a patch that implements the make_file_dict as described 
above. Let me know what you think.

Thanks,
JoshN

--- rdiff_backup/cmodule.c      10 Jun 2008 12:55:59 -0000      1.24
+++ rdiff_backup/cmodule.c      11 Jun 2008 06:48:44 -0000
@@ -48,10 +48,6 @@
 /* This code taken from Python's posixmodule.c */
 #undef STAT
 #if defined(MS_WIN64) || defined(MS_WIN32)
-#      define LSTAT _stati64
-#      define STAT _stati64
-#      define FSTAT _fstati64
-#      define STRUCT_STAT struct _stati64
 #      define SYNC _flushall
 #else
 #      define LSTAT lstat
@@ -77,15 +73,6 @@
 #define S_ISFIFO(mode)        (((mode) & S_IFMT) == S_IFIFO)
 #endif
 
-#if defined(MS_WIN64) || defined(MS_WIN32)
-#define S_ISSOCK(mode) (0)
-#define S_ISFIFO(mode) (0)
-#define S_ISLNK(mode) (0)
-#define S_ISLNK(mode) (0)
-#define S_ISCHR(mode) (0)
-#define S_ISBLK(mode) (0)
-#endif
-
 static PyObject *UnknownFileTypeError;
 static PyObject *c_make_file_dict(PyObject *self, PyObject *args);
 static PyObject *long2str(PyObject *self, PyObject *args);
@@ -98,6 +85,10 @@
         PyObject *self;
         PyObject *args;
 {
+#if defined(MS_WINDOWS)
+       PyErr_SetString(PyExc_AttributeError, "This function is not implemented 
on Windows.");
+       return NULL;
+#else
   PyObject *size, *inode, *mtime, *atime, *ctime, *devloc, *return_val;
   char *filename, filetype[5];
   STRUCT_STAT sbuf;
@@ -118,10 +109,8 @@
          return NULL;
        }
   }
-#if defined(MS_WINDOWS)
   size = PyLong_FromLongLong((PY_LONG_LONG)sbuf.st_size);
   inode = PyLong_FromLongLong((PY_LONG_LONG)-1);
-#else
 #ifdef HAVE_LARGEFILE_SUPPORT
   size = PyLong_FromLongLong((PY_LONG_LONG)sbuf.st_size);
   inode = PyLong_FromLongLong((PY_LONG_LONG)sbuf.st_ino);
@@ -129,7 +118,6 @@
   size = PyInt_FromLong(sbuf.st_size);
   inode = PyInt_FromLong((long)sbuf.st_ino);
 #endif /* HAVE_LARGEFILE_SUPPORT */
-#endif /* defined(MS_WINDOWS) */
   mode = (long)sbuf.st_mode;
   perms = mode & 07777;
 #if defined(HAVE_LONG_LONG) && !defined(MS_WINDOWS)
@@ -223,6 +211,7 @@
   Py_DECREF(atime);
   Py_DECREF(ctime);
   return return_val;
+#endif /* defined(MS_WINDOWS) */
 }
 
 /* Convert python long into 7 byte string */
Index: rdiff_backup/rpath.py
===================================================================
RCS file: /sources/rdiff-backup/rdiff-backup/rdiff_backup/rpath.py,v
retrieving revision 1.120
diff -u -r1.120 rpath.py
--- rdiff_backup/rpath.py       10 Jun 2008 13:14:52 -0000      1.120
+++ rdiff_backup/rpath.py       11 Jun 2008 06:59:04 -0000
@@ -788,10 +788,13 @@
 
        def setdata(self):
                """Set data dictionary using C extension"""
-               self.data = self.conn.C.make_file_dict(self.path)
+               try:
+                       self.data = self.conn.C.make_file_dict(self.path)
+               except AttributeError:
+                       self.data = self.make_file_dict_native()
                if self.lstat(): self.conn.rpath.setdata_local(self)
 
-       def make_file_dict_old(self):
+       def make_file_dict_native(self):
                """Create the data dictionary"""
                statblock = self.conn.rpath.tupled_lstat(self.path)
                if statblock is None: