Re: utimensat round 3

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Jim Meyering on 10/16/2009 6:34 AM:
>>> This failure may also be due to the way utimecmp works,
>>> but I haven't delved into that code today.
>> So yes, I'm starting to think we need to teach utimecmp about
>> quantization, and declare two mtimes as equal if they would both quantize
>> to the same value (even if they have distinct values due to higher
>> resolution).  But how to determine quantization of a given filesystem?
> 
> I wouldn't worry about that right now (or maybe ever ;-).
> Short term, I'd change the test so it exercises UTIME_NOW,
> but keeping st1 and st2 far enough apart that sub-second
> effects aren't an issue.
> Then there's no need for UTIMECMP_TRUNCATE_SOURCE.

Well, from your output, your system had a quantization at 10ms when done
by the system, so a usleep(20000) should be enough.  I've worked that into
my test, as shown below, and pushed the series.  Please try it to see if
it solved that particular failure for you.  At least for me on cygwin, it
was enough of a workaround to bypass cygwin's clock_gettime drift as well.

- --
Don't work too hard, make some time for fun as well!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkrYknkACgkQ84KuGfSFAYAatACgjvKQZ56Qxm8QaNtH9ZNDZwzS
u9UAn1HJRDLaJfOmXlu7O4p1cRsJZraN
=psdu
-----END PGP SIGNATURE-----

>From 4d8f229535aef794199fa6dd0eb29e852c35d73d Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Mon, 12 Oct 2009 10:42:35 -0600
Subject: [PATCH] test-stat-time, test-utimens: improve portability

ext4 on an alpha system has a quantization of about 10 ms but
a resolution of 1ns; utimecmp does not know about quantization,
so tests were failing when comparing timestamps that fall
within the same quantization window.  Add strategic usleeps
throughout to minimize this issue, whether or not we later
improve utimecmp to account for quantization.

Windows (and hence cygwin) is documented as having a default
clock quantization of 15.25 milliseconds (although it can be
reduced to 1 millisecond); file timestamps are quantized to this
boundary even though more accurate timing can be obtained.
However, this means that 15 milliseconds is too short for any
test that wants to guarantee crossing a file timestamp boundary.
Cygwin, however, still has bugs where clock_gettime can lag
behind file timestamps, which is not fixed by this patch.

Solaris 9 with NFS exposed the same problem for futimes that was
previously fixed for utimes on Solaris 8, where futimens(f,NULL)
uses a different time source than futimes(,{,UTIME_NOW}).

* tests/test-stat-time.c (nap): Lengthen delay to 20ms, for
ext4 on alpha, and for cygwin.
* tests/test-utimens-common.h: New file.
(nap): Factor delays into single function.
* tests/test-lutimens.h (test_lutimens): Use new header.
* tests/test-futimens.h (test_futimens): Likewise.
* tests/test-utimens.h (test_utimens): Likewise.  Also, force NFS
timestamps to occur from same machine, as was done previously for
test_utimens.
* modules/utimens-tests (Files): Ship new file.
* modules/futimens-tests (Files): Likewise.
Reported in part by Jim Meyering.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog                   |   14 +++++++++
 modules/futimens-tests      |    1 +
 modules/utimens-tests       |    1 +
 tests/test-futimens.h       |   64 ++++++++++++++++++----------------------
 tests/test-lutimens.h       |   49 +++++--------------------------
 tests/test-stat-time.c      |    8 +++--
 tests/test-utimens-common.h |   68 +++++++++++++++++++++++++++++++++++++++++++
 tests/test-utimens.h        |   41 ++++---------------------
 8 files changed, 133 insertions(+), 113 deletions(-)
 create mode 100644 tests/test-utimens-common.h

diff --git a/ChangeLog b/ChangeLog
index ab545dc..8b2edc3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,19 @@
 2009-10-16  Eric Blake  <address@hidden>

+       test-stat-time, test-utimens: improve portability
+       * tests/test-stat-time.c (nap): Lengthen delay to 20ms, for
+       ext4 on alpha, and for cygwin.
+       * tests/test-utimens-common.h: New file.
+       (nap): Factor delays into single function.
+       * tests/test-lutimens.h (test_lutimens): Use new header.
+       * tests/test-futimens.h (test_futimens): Likewise.
+       * tests/test-utimens.h (test_utimens): Likewise.  Also, force NFS
+       timestamps to occur from same machine, as was done previously for
+       test_utimens.
+       * modules/utimens-tests (Files): Ship new file.
+       * modules/futimens-tests (Files): Likewise.
+       Reported in part by Jim Meyering.
+
        sys_stat: sort replacement declarations
        * lib/sys_stat.in.h: Sort declarations.
        * lib/futimens.c (futimens): Fix typo.
diff --git a/modules/futimens-tests b/modules/futimens-tests
index 50756da..3a92524 100644
--- a/modules/futimens-tests
+++ b/modules/futimens-tests
@@ -1,5 +1,6 @@
 Files:
 tests/test-futimens.h
+tests/test-utimens-common.h
 tests/test-futimens.c

 Depends-on:
diff --git a/modules/utimens-tests b/modules/utimens-tests
index 746b3ac..25e7476 100644
--- a/modules/utimens-tests
+++ b/modules/utimens-tests
@@ -2,6 +2,7 @@ Files:
 tests/test-futimens.h
 tests/test-lutimens.h
 tests/test-utimens.h
+tests/test-utimens-common.h
 tests/test-utimens.c

 Depends-on:
diff --git a/tests/test-futimens.h b/tests/test-futimens.h
index 0335845..7c05bbf 100644
--- a/tests/test-futimens.h
+++ b/tests/test-futimens.h
@@ -14,41 +14,12 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */

-/* This file assumes that BASE and ASSERT are already defined.  */
+#include "test-utimens-common.h"

-#ifndef GL_TEST_UTIMENS
-# define GL_TEST_UTIMENS
-
-#include <fcntl.h>
-#include <errno.h>
-#include <string.h>
-#include <unistd.h>
-
-#include "stat-time.h"
-#include "timespec.h"
-#include "utimecmp.h"
-
-enum {
-  BILLION = 1000 * 1000 * 1000,
-
-  Y2K = 946684800, /* Jan 1, 2000, in seconds since epoch.  */
-
-  /* Bogus positive and negative tv_nsec values closest to valid
-     range, but without colliding with UTIME_NOW or UTIME_OMIT.  */
-  UTIME_BOGUS_POS = BILLION + ((UTIME_NOW == BILLION || UTIME_OMIT == BILLION)
-                               ? (1 + (UTIME_NOW == BILLION + 1)
-                                  + (UTIME_OMIT == BILLION + 1))
-                               : 0),
-  UTIME_BOGUS_NEG = -1 - ((UTIME_NOW == -1 || UTIME_OMIT == -1)
-                          ? (1 + (UTIME_NOW == -2) + (UTIME_OMIT == -2))
-                          : 0)
-};
-
-#endif /* GL_TEST_UTIMENS */
-
-/* This function is designed to test both gl_futimens(a,NULL,b) and
-   futimens(a,b).  FUNC is the function to test.  If PRINT, warn
-   before skipping tests with status 77.  */
+/* This file is designed to test both gl_futimens(a,NULL,b) and
+   futimens(a,b).  FUNC is the function to test.  Assumes that BASE
+   and ASSERT are already defined.  If PRINT, warn before skipping
+   tests with status 77.  */
 static int
 test_futimens (int (*func) (int, struct timespec const *),
                bool print)
@@ -60,6 +31,8 @@ test_futimens (int (*func) (int, struct timespec const *),
   ASSERT (0 <= fd);

   /* Sanity check.  */
+  ASSERT (fstat (fd, &st1) == 0);
+  nap ();
   errno = 0;
   result = func (fd, NULL);
   if (result == -1 && errno == ENOSYS)
@@ -73,7 +46,28 @@ test_futimens (int (*func) (int, struct timespec const *),
       return 77;
     }
   ASSERT (!result);
-  ASSERT (fstat (fd, &st1) == 0);
+  ASSERT (fstat (fd, &st2) == 0);
+  /* If utimens truncates to less resolution than the file system
+     supports, then time can appear to go backwards between now and a
+     follow-up utimens with UTIME_NOW or a NULL timespec.  Use
+     UTIMECMP_TRUNCATE_SOURCE to compensate, with st1 as the
+     source.  */
+  ASSERT (0 <= utimecmp (BASE "file", &st2, &st1, UTIMECMP_TRUNCATE_SOURCE));
+  {
+    /* On some NFS systems, the 'now' timestamp of creat or a NULL
+       timespec is determined by the server, but the 'now' timestamp
+       determined by gettime() (as is done when using UTIME_NOW) is
+       determined by the client; since the two machines are not
+       necessarily on the same clock, this is another case where time
+       can appear to go backwards.  The rest of this test cares about
+       client time, so manually use gettime() to set both times.  */
+    struct timespec ts[2];
+    gettime (&ts[0]);
+    ts[1] = ts[0];
+    ASSERT (func (fd, ts) == 0);
+    ASSERT (fstat (fd, &st1) == 0);
+    nap ();
+  }

   /* Invalid arguments.  */
   errno = 0;
diff --git a/tests/test-lutimens.h b/tests/test-lutimens.h
index 18cf75f..632b103 100644
--- a/tests/test-lutimens.h
+++ b/tests/test-lutimens.h
@@ -14,41 +14,12 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */

-/* This file assumes that BASE and ASSERT are already defined.  */
+#include "test-utimens-common.h"

-#ifndef GL_TEST_UTIMENS
-# define GL_TEST_UTIMENS
-
-#include <fcntl.h>
-#include <errno.h>
-#include <string.h>
-#include <unistd.h>
-
-#include "stat-time.h"
-#include "timespec.h"
-#include "utimecmp.h"
-
-enum {
-  BILLION = 1000 * 1000 * 1000,
-
-  Y2K = 946684800, /* Jan 1, 2000, in seconds since epoch.  */
-
-  /* Bogus positive and negative tv_nsec values closest to valid
-     range, but without colliding with UTIME_NOW or UTIME_OMIT.  */
-  UTIME_BOGUS_POS = BILLION + ((UTIME_NOW == BILLION || UTIME_OMIT == BILLION)
-                               ? (1 + (UTIME_NOW == BILLION + 1)
-                                  + (UTIME_OMIT == BILLION + 1))
-                               : 0),
-  UTIME_BOGUS_NEG = -1 - ((UTIME_NOW == -1 || UTIME_OMIT == -1)
-                          ? (1 + (UTIME_NOW == -2) + (UTIME_OMIT == -2))
-                          : 0)
-};
-
-#endif /* GL_TEST_UTIMENS */
-
-/* This function is designed to test both lutimens(a,b) and
+/* This file is designed to test both lutimens(a,b) and
    utimensat(AT_FDCWD,a,b,AT_SYMLINK_NOFOLLOW).  FUNC is the function
-   to test.  If PRINT, warn before skipping tests with status 77.  */
+   to test.  Assumes that BASE and ASSERT are already defined.  If
+   PRINT, warn before skipping tests with status 77.  */
 static int
 test_lutimens (int (*func) (char const *, struct timespec const *), bool print)
 {
@@ -77,17 +48,13 @@ test_lutimens (int (*func) (char const *, struct timespec 
const *), bool print)
     }
   ASSERT (!result);
   ASSERT (lstat (BASE "link", &st1) == 0);
-#if HAVE_USLEEP
-  /* On Cygwin, the mere act of lstat changes symlink atime, even
-     though POSIX says that only readlink is allowed to do that.
-     Sleeping for one millisecond is enough to expose this.  Platforms
-     without usleep either don't have symlinks, or are immune.  */
-  usleep (1000);
-#endif
+  /* On cygwin, lstat() changes atime of symlinks, so that lutimens
+     can only effectively modify mtime.  */
+  nap ();
   ASSERT (lstat (BASE "link", &st2) == 0);
   if (st1.st_atime != st2.st_atime
       || get_stat_atime_ns (&st1) != get_stat_atime_ns (&st2))
-     atime_supported = false;
+    atime_supported = false;

   /* Invalid arguments.  */
   errno = 0;
diff --git a/tests/test-stat-time.c b/tests/test-stat-time.c
index 3a0aafc..7fb2e93 100644
--- a/tests/test-stat-time.c
+++ b/tests/test-stat-time.c
@@ -104,9 +104,11 @@ nap (void)
   static long delay;
   if (!delay)
     {
-      /* Initialize only once, by sleeping for 15 milliseconds (needed
+      /* Initialize only once, by sleeping for 20 milliseconds (needed
          since xfs has a quantization of about 10 milliseconds, even
-         though it has a granularity of 1 nanosecond).  If the seconds
+         though it has a granularity of 1 nanosecond, and since NTFS
+         has a default quantization of 15.25 milliseconds, even though
+         it has a granularity of 100 nanoseconds).  If the seconds
          differ, repeat the test one more time (in case we crossed a
          quantization boundary on a file system with 1 second
          resolution).  If we can't observe a difference in only the
@@ -116,7 +118,7 @@ nap (void)
       struct stat st2;
       ASSERT (stat ("t-stt-stamp1", &st1) == 0);
       ASSERT (force_unlink ("t-stt-stamp1") == 0);
-      delay = 15000;
+      delay = 20000;
       usleep (delay);
       create_file ("t-stt-stamp1");
       ASSERT (stat ("t-stt-stamp1", &st2) == 0);
diff --git a/tests/test-utimens-common.h b/tests/test-utimens-common.h
new file mode 100644
index 0000000..1945f23
--- /dev/null
+++ b/tests/test-utimens-common.h
@@ -0,0 +1,68 @@
+/* Test of file timestamp modification functions.
+   Copyright (C) 2009 Free Software Foundation, Inc.
+
+   This program is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* This file defines some prerequisites useful to utime-related tests.  */
+
+#ifndef GL_TEST_UTIMENS_COMMON
+# define GL_TEST_UTIMENS_COMMON
+
+#include <fcntl.h>
+#include <errno.h>
+#include <string.h>
+#include <unistd.h>
+
+#include "stat-time.h"
+#include "timespec.h"
+#include "utimecmp.h"
+
+enum {
+  BILLION = 1000 * 1000 * 1000,
+
+  Y2K = 946684800, /* Jan 1, 2000, in seconds since epoch.  */
+
+  /* Bogus positive and negative tv_nsec values closest to valid
+     range, but without colliding with UTIME_NOW or UTIME_OMIT.  */
+  UTIME_BOGUS_POS = BILLION + ((UTIME_NOW == BILLION || UTIME_OMIT == BILLION)
+                               ? (1 + (UTIME_NOW == BILLION + 1)
+                                  + (UTIME_OMIT == BILLION + 1))
+                               : 0),
+  UTIME_BOGUS_NEG = -1 - ((UTIME_NOW == -1 || UTIME_OMIT == -1)
+                          ? (1 + (UTIME_NOW == -2) + (UTIME_OMIT == -2))
+                          : 0)
+};
+
+/* Sleep long enough to cross a timestamp quantization boundary on
+   most known systems with subsecond timestamp resolution.  For
+   example, ext4 has a quantization of 10 milliseconds, but a
+   resolution of 1 nanosecond.  Likewise, NTFS has a quantization as
+   slow as 15.25 milliseconds, but a resolution of 100 nanoseconds.
+   This is necessary on systems where creat or utimens with NULL
+   rounds down to the quantization boundary, but where gettime and
+   hence utimensat can inject timestamps between quantization
+   boundaries.  By ensuring we cross a boundary, we are less likely to
+   confuse utimecmp for two times that would round to the same
+   quantization boundary but are distinct based on resolution.  */
+static void
+nap (void)
+{
+  /* Systems that lack usleep also lack subsecond timestamps.  Our
+     usage of utimecmp allows equality, so we don't need to sleep.  */
+#if HAVE_USLEEP
+  usleep (20 * 1000); /* 20 milliseconds.  */
+#endif
+}
+
+#endif /* GL_TEST_UTIMENS_COMMON */
diff --git a/tests/test-utimens.h b/tests/test-utimens.h
index 08d3ac0..d9676f2 100644
--- a/tests/test-utimens.h
+++ b/tests/test-utimens.h
@@ -14,40 +14,11 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */

-/* This file assumes that BASE and ASSERT are already defined.  */
+#include "test-utimens-common.h"

-#ifndef GL_TEST_UTIMENS
-# define GL_TEST_UTIMENS
-
-#include <fcntl.h>
-#include <errno.h>
-#include <string.h>
-#include <unistd.h>
-
-#include "stat-time.h"
-#include "timespec.h"
-#include "utimecmp.h"
-
-enum {
-  BILLION = 1000 * 1000 * 1000,
-
-  Y2K = 946684800, /* Jan 1, 2000, in seconds since epoch.  */
-
-  /* Bogus positive and negative tv_nsec values closest to valid
-     range, but without colliding with UTIME_NOW or UTIME_OMIT.  */
-  UTIME_BOGUS_POS = BILLION + ((UTIME_NOW == BILLION || UTIME_OMIT == BILLION)
-                               ? (1 + (UTIME_NOW == BILLION + 1)
-                                  + (UTIME_OMIT == BILLION + 1))
-                               : 0),
-  UTIME_BOGUS_NEG = -1 - ((UTIME_NOW == -1 || UTIME_OMIT == -1)
-                          ? (1 + (UTIME_NOW == -2) + (UTIME_OMIT == -2))
-                          : 0)
-};
-
-#endif /* GL_TEST_UTIMENS */
-
-/* This function is designed to test both utimens(a,b) and
-   utimensat(AT_FDCWD,a,b,0).  FUNC is the function to test.  */
+/* This file is designed to test both utimens(a,b) and
+   utimensat(AT_FDCWD,a,b,0).  FUNC is the function to test.  Assumes
+   that BASE and ASSERT are already defined.  */
 static int
 test_utimens (int (*func) (char const *, struct timespec const *))
 {
@@ -61,13 +32,14 @@ test_utimens (int (*func) (char const *, struct timespec 
const *))
      UTIMECMP_TRUNCATE_SOURCE to compensate, with st1 as the
      source.  */
   ASSERT (stat (BASE "file", &st1) == 0);
+  nap ();
   ASSERT (func (BASE "file", NULL) == 0);
   ASSERT (stat (BASE "file", &st2) == 0);
   ASSERT (0 <= utimecmp (BASE "file", &st2, &st1, UTIMECMP_TRUNCATE_SOURCE));
   {
     /* On some NFS systems, the 'now' timestamp of creat or a NULL
        timespec is determined by the server, but the 'now' timestamp
-       determined by gettime() (as is done when using UTIME_OMIT) is
+       determined by gettime() (as is done when using UTIME_NOW) is
        determined by the client; since the two machines are not
        necessarily on the same clock, this is another case where time
        can appear to go backwards.  The rest of this test cares about
@@ -77,6 +49,7 @@ test_utimens (int (*func) (char const *, struct timespec 
const *))
     ts[1] = ts[0];
     ASSERT (func (BASE "file", ts) == 0);
     ASSERT (stat (BASE "file", &st1) == 0);
+    nap ();
   }

   /* Invalid arguments.  */
-- 
1.6.5.rc1