Always publish (l)utimes in helpers when available and fix type conversions.

Previously, bup assumed a timeval was (long, long), which agrees with
glibc.info and (Linux) utimes(2), but POSIX says it's (time_t,
suseconds_t).

Since the types vary (and even if they didn't, Python doesn't provide
safe "time_t" conversions), handle the type conversions more
generically, via ASSIGN_PYLONG_TO_INTEGRAL() and
INTEGRAL_ASSIGNMENT_FITS().

Optimistically assume the latter is sufficient for tv_usec (where we
convert it to a long long) because POSIX guarantees that tv_usec will
be a signed integral.

Whenever available, publish utimes and lutimes independently in
helpers, and run the relevant t/txstat.py tests.  So now, we'll be
running at least some tests on utimensat, lutimes, and utimes whenever
they're available.

A more significant improvement would be to run some broader bup tests
twice, once with utimensat and once with utimes/lutimes, whenever both
are available.

cf. 9ce545703dda3c888081c504863308902ce8c56b

Signed-off-by: Rob Browning <rlb@defaultvalue.org>

diff --git a/lib/bup/_helpers.c b/lib/bup/_helpers.c
index 0fd20cc233db19fc0152f1a98aa692abdffbb6c4..673ae8ae4ff7dbb6bde11c937e06cc5227dabfe5 100644 (file)
--- a/lib/bup/_helpers.c
+++ b/lib/bup/_helpers.c
@@ -773,27 +773,6 @@ static PyObject *bup_set_linux_file_attr(PyObject *self, PyObject *args)
 #endif
 
 
-#if defined(HAVE_UTIMENSAT) || defined(HAVE_FUTIMES) || defined(HAVE_LUTIMES)
-
-static int bup_parse_xutime_args(char **path,
-                                 long *access,
-                                 long *access_ns,
-                                 long *modification,
-                                 long *modification_ns,
-                                 PyObject *self, PyObject *args)
-{
-    if (!PyArg_ParseTuple(args, "s((ll)(ll))",
-                          path,
-                          access, access_ns,
-                          modification, modification_ns))
-        return 0;
-    return 1;
-}
-
-#endif /* defined(HAVE_UTIMENSAT) || defined(HAVE_FUTIMES)
-          || defined(HAVE_LUTIMES) */
-
-
 #define INTEGRAL_ASSIGNMENT_FITS(dest, src)                             \
     ({                                                                  \
         *(dest) = (src);                                                \
@@ -879,52 +858,73 @@ static PyObject *bup_utimensat(PyObject *self, PyObject *args)
 #endif /* def HAVE_UTIMENSAT */
 
 
+#if defined(HAVE_UTIMES) || defined(HAVE_LUTIMES)
+
+static int bup_parse_xutimes_args(char **path,
+                                  struct timeval tv[2],
+                                  PyObject *args)
+{
+    PyObject *access_py, *modification_py;
+    long long access_us, modification_us; // POSIX guarantees tv_usec is signed.
+
+    if (!PyArg_ParseTuple(args, "s((OL)(OL))",
+                          path,
+                          &access_py, &access_us,
+                          &modification_py, &modification_us))
+        return 0;
+
+    int overflow;
+    if (!ASSIGN_PYLONG_TO_INTEGRAL(&(tv[0].tv_sec), access_py, &overflow))
+    {
+        if (overflow)
+            PyErr_SetString(PyExc_ValueError, "unable to convert access time seconds to timeval");
+        return 0;
+    }
+    if (!INTEGRAL_ASSIGNMENT_FITS(&(tv[0].tv_usec), access_us))
+    {
+        PyErr_SetString(PyExc_ValueError, "unable to convert access time nanoseconds to timeval");
+        return 0;
+    }
+    if (!ASSIGN_PYLONG_TO_INTEGRAL(&(tv[1].tv_sec), modification_py, &overflow))
+    {
+        if (overflow)
+            PyErr_SetString(PyExc_ValueError, "unable to convert modification time seconds to timeval");
+        return 0;
+    }
+    if (!INTEGRAL_ASSIGNMENT_FITS(&(tv[1].tv_usec), modification_us))
+    {
+        PyErr_SetString(PyExc_ValueError, "unable to convert modification time nanoseconds to timeval");
+        return 0;
+    }
+    return 1;
+}
+
+#endif /* defined(HAVE_UTIMES) || defined(HAVE_LUTIMES) */
+
+
 #ifdef HAVE_UTIMES
-#define BUP_HAVE_BUP_UTIME_NS 1
-static PyObject *bup_utime_ns(PyObject *self, PyObject *args)
+static PyObject *bup_utimes(PyObject *self, PyObject *args)
 {
-    int rc;
     char *path;
-    long access, access_ns, modification, modification_ns;
     struct timeval tv[2];
-
-    if (!bup_parse_xutime_args(&path, &access, &access_ns,
-                               &modification, &modification_ns,
-                               self, args))
-       return NULL;
-
-    tv[0].tv_sec = access;
-    tv[0].tv_usec = access_ns / 1000;
-    tv[1].tv_sec = modification;
-    tv[1].tv_usec = modification_ns / 1000;
-    rc = utimes(path, tv);
+    if (!bup_parse_xutimes_args(&path, tv, args))
+        return NULL;
+    int rc = utimes(path, tv);
     if (rc != 0)
         return PyErr_SetFromErrnoWithFilename(PyExc_OSError, path);
-
     return Py_BuildValue("O", Py_None);
 }
 #endif /* def HAVE_UTIMES */
 
 
 #ifdef HAVE_LUTIMES
-#define BUP_HAVE_BUP_LUTIME_NS 1
-static PyObject *bup_lutime_ns(PyObject *self, PyObject *args)
+static PyObject *bup_lutimes(PyObject *self, PyObject *args)
 {
-    int rc;
     char *path;
-    long access, access_ns, modification, modification_ns;
     struct timeval tv[2];
-
-    if (!bup_parse_xutime_args(&path, &access, &access_ns,
-                               &modification, &modification_ns,
-                               self, args))
-       return NULL;
-
-    tv[0].tv_sec = access;
-    tv[0].tv_usec = access_ns / 1000;
-    tv[1].tv_sec = modification;
-    tv[1].tv_usec = modification_ns / 1000;
-    rc = lutimes(path, tv);
+    if (!bup_parse_xutimes_args(&path, tv, args))
+        return NULL;
+    int rc = lutimes(path, tv);
     if (rc != 0)
         return PyErr_SetFromErrnoWithFilename(PyExc_OSError, path);
 
@@ -1121,13 +1121,13 @@ static PyMethodDef helper_methods[] = {
     { "bup_utimensat", bup_utimensat, METH_VARARGS,
       "Change path timestamps with nanosecond precision (POSIX)." },
 #endif
-#ifdef BUP_HAVE_BUP_UTIME_NS
-    { "bup_utime_ns", bup_utime_ns, METH_VARARGS,
-      "Change path timestamps with up to nanosecond precision." },
+#ifdef HAVE_UTIMES
+    { "bup_utimes", bup_utimes, METH_VARARGS,
+      "Change path timestamps with microsecond precision." },
 #endif
-#ifdef BUP_HAVE_BUP_LUTIME_NS
-    { "bup_lutime_ns", bup_lutime_ns, METH_VARARGS,
-      "Change path timestamps with up to nanosecond precision;"
+#ifdef HAVE_LUTIMES
+    { "bup_lutimes", bup_lutimes, METH_VARARGS,
+      "Change path timestamps with microsecond precision;"
       " don't follow symlinks." },
 #endif
     { "stat", bup_stat, METH_VARARGS,

diff --git a/lib/bup/t/txstat.py b/lib/bup/t/txstat.py
index 7cb144b911bbccc39c8a50408a2995dc77d05d3b..1d4bad35d08b814eadfe9dcc332b84c7d5fe0f3a 100644 (file)
--- a/lib/bup/t/txstat.py
+++ b/lib/bup/t/txstat.py
@@ -30,6 +30,20 @@ def test_fstime():
     WVPASSEQ(type(x[0]), type(0))
     WVPASSEQ(type(x[1]), type(0))
 
+    WVPASSEQ(xstat.nsecs_to_timeval(0), (0, 0))
+    WVPASSEQ(xstat.nsecs_to_timeval(10**9), (1, 0))
+    WVPASSEQ(xstat.nsecs_to_timeval(500000000), (0, (10**9 / 2) / 1000))
+    WVPASSEQ(xstat.nsecs_to_timeval(1500000000), (1, (10**9 / 2) / 1000))
+    WVPASSEQ(xstat.nsecs_to_timeval(-10**9), (-1, 0))
+    WVPASSEQ(xstat.nsecs_to_timeval(-500000000), (-1, (10**9 / 2) / 1000))
+    WVPASSEQ(xstat.nsecs_to_timeval(-1500000000), (-2, (10**9 / 2) / 1000))
+    x = xstat.nsecs_to_timeval(1977777778)
+    WVPASSEQ(type(x[0]), type(0))
+    WVPASSEQ(type(x[1]), type(0))
+    x = xstat.nsecs_to_timeval(-1977777778)
+    WVPASSEQ(type(x[0]), type(0))
+    WVPASSEQ(type(x[1]), type(0))
+
     WVPASSEQ(xstat.fstime_floor_secs(0), 0)
     WVPASSEQ(xstat.fstime_floor_secs(10**9 / 2), 0)
     WVPASSEQ(xstat.fstime_floor_secs(10**9), 1)
@@ -60,27 +74,43 @@ def test_bup_utimensat():
         subprocess.call(['rm', '-rf', tmpdir])
 
 
-try:
-    _have_bup_utime_ns = _helpers.bup_utime_ns
-except AttributeError, e:
-    _have_bup_utime_ns = False
+@wvtest
+def test_bup_utimes():
+    if not xstat._bup_utimes:
+        return
+    tmpdir = tempfile.mkdtemp(prefix='bup-tmetadata-')
+    try:
+        path = tmpdir + '/foo'
+        open(path, 'w').close()
+        frac_ts = (0, 10**6 / 2)
+        xstat._bup_utimes(path, (frac_ts, frac_ts))
+        st = _helpers.stat(path)
+        atime_ts = st[8]
+        mtime_ts = st[9]
+        WVPASSEQ(atime_ts[0], 0)
+        WVPASS(atime_ts[1] == 0 or atime_ts[1] == frac_ts[1] * 1000)
+        WVPASSEQ(mtime_ts[0], 0)
+        WVPASS(mtime_ts[1] == 0 or mtime_ts[1] == frac_ts[1] * 1000)
+    finally:
+        subprocess.call(['rm', '-rf', tmpdir])
+
 
 @wvtest
-def test_timespec_behavior():
-    if not _have_bup_utime_ns:
+def test_bup_lutimes():
+    if not xstat._bup_lutimes:
         return
     tmpdir = tempfile.mkdtemp(prefix='bup-tmetadata-')
     try:
         path = tmpdir + '/foo'
         open(path, 'w').close()
-        frac_ts = (0, 10**9 / 2)
-        _helpers.bup_utime_ns(path, (frac_ts, frac_ts))
+        frac_ts = (0, 10**6 / 2)
+        xstat._bup_lutimes(path, (frac_ts, frac_ts))
         st = _helpers.stat(path)
         atime_ts = st[8]
         mtime_ts = st[9]
         WVPASSEQ(atime_ts[0], 0)
-        WVPASS(atime_ts[1] == 0 or atime_ts[1] == frac_ts[1])
+        WVPASS(atime_ts[1] == 0 or atime_ts[1] == frac_ts[1] * 1000)
         WVPASSEQ(mtime_ts[0], 0)
-        WVPASS(mtime_ts[1] == 0 or mtime_ts[1] == frac_ts[1])
+        WVPASS(mtime_ts[1] == 0 or mtime_ts[1] == frac_ts[1] * 1000)
     finally:
         subprocess.call(['rm', '-rf', tmpdir])

diff --git a/lib/bup/xstat.py b/lib/bup/xstat.py
index f715d1fb4c2d1af5bedaa982b9bc892784294613..23483f5991f6d0c226b672a03ce4445722dcf156 100644 (file)
--- a/lib/bup/xstat.py
+++ b/lib/bup/xstat.py
@@ -8,6 +8,17 @@ try:
 except AttributeError, e:
     _bup_utimensat = False
 
+try:
+    _bup_utimes = _helpers.bup_utimes
+except AttributeError, e:
+    _bup_utimes = False
+
+try:
+    _bup_lutimes = _helpers.bup_lutimes
+except AttributeError, e:
+    _bup_lutimes = False
+
+
 def timespec_to_nsecs((ts_s, ts_ns)):
     # c.f. _helpers.c: timespec_vals_to_py_ns()
     if ts_ns < 0 or ts_ns > 999999999:
@@ -22,6 +33,13 @@ def nsecs_to_timespec(ns):
     return (ns / 10**9, ns % 10**9)
 
 
+def nsecs_to_timeval(ns):
+    """Return (s, us) where ns is always non-negative
+    and t = s + us / 10e5"""
+    ns = int(ns)
+    return (ns / 10**9, (ns % 10**9) / 1000)
+
+
 def fstime_floor_secs(ns):
     """Return largest integer not greater than ns / 10e8."""
     return int(ns) / 10**9;
@@ -40,6 +58,7 @@ def fstime_to_sec_str(fstime):
     else:
         return '%d.%09d' % (s, ns)
 
+
 if _bup_utimensat:
     def utime(path, times):
         """Times must be provided as (atime_ns, mtime_ns)."""
@@ -52,17 +71,17 @@ if _bup_utimensat:
         mtime = nsecs_to_timespec(times[1])
         _bup_utimensat(_helpers.AT_FDCWD, path, (atime, mtime),
                        _helpers.AT_SYMLINK_NOFOLLOW)
-else:
+else: # Must have these if utimensat isn't available.
     def utime(path, times):
         """Times must be provided as (atime_ns, mtime_ns)."""
-        atime = nsecs_to_timespec(times[0])
-        mtime = nsecs_to_timespec(times[1])
-        _helpers.bup_utime_ns(path, (atime, mtime))
+        atime = nsecs_to_timeval(times[0])
+        mtime = nsecs_to_timeval(times[1])
+        _bup_utimes(path, (atime, mtime))
     def lutime(path, times):
         """Times must be provided as (atime_ns, mtime_ns)."""
-        atime = nsecs_to_timespec(times[0])
-        mtime = nsecs_to_timespec(times[1])
-        _helpers.bup_lutime_ns(path, (atime, mtime))
+        atime = nsecs_to_timeval(times[0])
+        mtime = nsecs_to_timeval(times[1])
+        _bup_lutimes(path, (atime, mtime))
 
 
 class stat_result: