This is the mail archive of the libstdc++@gcc.gnu.org mailing list for the libstdc++ project.

Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]
Other format:	[Raw text]

Re: [PATCH] Five patches for std::experimental::filesystem

From: Jonathan Wakely <jwakely at redhat dot com>
To: Christophe Lyon <christophe dot lyon at linaro dot org>
Cc: "libstdc++ at gcc dot gnu dot org" <libstdc++ at gcc dot gnu dot org>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
Date: Thu, 27 Oct 2016 14:34:57 +0100
Subject: Re: [PATCH] Five patches for std::experimental::filesystem
Authentication-results: sourceware.org; auth=none
References: <20161024165010.GL2922@redhat.com> <20161025153232.GN2922@redhat.com> <CAKdteOZzp5_NYNHQ9wgECZUYT3RDsd37s8ppARpTkWn_ueyz4g@mail.gmail.com>

On 26/10/16 09:24 +0200, Christophe Lyon wrote:

Hi Jonathan,

On 25 October 2016 at 17:32, Jonathan Wakely <jwakely@redhat.com> wrote:

Two more fixes for the filesystem TS, and improved tests.

  Handle negative times in filesystem::last_write_time
          * src/filesystem/ops.cc
       (last_write_time(const path&, file_time_type, error_code&)): Handle
       negative times correctly.
       * testsuite/experimental/filesystem/operations/last_write_time.cc:
       Test writing file times.

   Fix error handling in copy_file and equivalent
          * src/filesystem/ops.cc (do_copy_file): Report an error if source
or
       destination is not a regular file (LWG 2712).
       (equivalent): Fix error handling and result when only one file
exists.
       * testsuite/experimental/filesystem/operations/copy.cc: Remove files
       created by tests. Test copying directories.
       * testsuite/experimental/filesystem/operations/copy_file.cc: Remove
       files created by tests.
       * testsuite/experimental/filesystem/operations/equivalent.cc: New.
       * testsuite/experimental/filesystem/operations/is_empty.cc: New.
       * testsuite/experimental/filesystem/operations/read_symlink.cc:
Remove
       file created by test.
       * testsuite/experimental/filesystem/operations/remove_all.cc: New.
       * testsuite/util/testsuite_fs.h (~scoped_file): Only try to remove
       file if path is non-empty, to support removal by other means.

Tested x86_64-linux, committed to trunk.

I can see failures in
experimental/filesystem/operations/last_write_time.cc after your
committed this patch:
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libstdc++-v3/testsuite/experimental/filesystem/operations/last_write_time.cc:127:
void test02(): Assertion 'last_write_time(f.path) == time' failed.
on arm*linux* and aarch64*linux* targets.


That test will fail for targets where _GLIBCXX_USE_UTIMENSAT is not
defined, as they use utime() instead which only supports second
granularity.

This should solve it, by only checking that the file times are within
one second of the expected value.

Tested x86_64-linux, committed to trunk.

commit 9b3f71e225ae45ccc1a5a78fee05d4ed9a969e8e
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Oct 27 11:48:00 2016 +0100

    Adjust precision of filesystem::last_write_time tests
    
    	* testsuite/experimental/filesystem/iterators/directory_iterator.cc:
    	Use end() function to get end iterator.
    	* testsuite/experimental/filesystem/iterators/pop.cc: Remove printf
    	statements that were present for debugging.
    	* testsuite/experimental/filesystem/iterators/
    	recursive_directory_iterator.cc: Use end() function to get end
    	iterator.
    	* testsuite/experimental/filesystem/operations/last_write_time.cc:
    	Only require file timestamps to be accurate to one second.

diff --git a/libstdc++-v3/testsuite/experimental/filesystem/iterators/directory_iterator.cc b/libstdc++-v3/testsuite/experimental/filesystem/iterators/directory_iterator.cc
index 5788700..d1f2c54 100644
--- a/libstdc++-v3/testsuite/experimental/filesystem/iterators/directory_iterator.cc
+++ b/libstdc++-v3/testsuite/experimental/filesystem/iterators/directory_iterator.cc
@@ -34,14 +34,14 @@ test01()
   const auto p = __gnu_test::nonexistent_path();
   fs::directory_iterator iter(p, ec);
   VERIFY( ec );
-  VERIFY( iter == fs::directory_iterator() );
+  VERIFY( iter == end(iter) );
 
   // Test empty directory.
   create_directory(p, fs::current_path(), ec);
   VERIFY( !ec );
   iter = fs::directory_iterator(p, ec);
   VERIFY( !ec );
-  VERIFY( iter == fs::directory_iterator() );
+  VERIFY( iter == end(iter) );
 
   // Test non-empty directory.
   create_directory_symlink(p, p / "l", ec);
@@ -51,20 +51,20 @@ test01()
   VERIFY( iter != fs::directory_iterator() );
   VERIFY( iter->path() == p/"l" );
   ++iter;
-  VERIFY( iter == fs::directory_iterator() );
+  VERIFY( iter == end(iter) );
 
   // Test inaccessible directory.
   permissions(p, fs::perms::none, ec);
   VERIFY( !ec );
   iter = fs::directory_iterator(p, ec);
   VERIFY( ec );
-  VERIFY( iter == fs::directory_iterator() );
+  VERIFY( iter == end(iter) );
 
   // Test inaccessible directory, skipping permission denied.
   const auto opts = fs::directory_options::skip_permission_denied;
   iter = fs::directory_iterator(p, opts, ec);
   VERIFY( !ec );
-  VERIFY( iter == fs::directory_iterator() );
+  VERIFY( iter == end(iter) );
 
   permissions(p, fs::perms::owner_all, ec);
   remove_all(p, ec);
@@ -82,12 +82,12 @@ test02()
   // Test post-increment (libstdc++/71005)
   auto iter = fs::directory_iterator(p, ec);
   VERIFY( !ec );
-  VERIFY( iter != fs::directory_iterator() );
+  VERIFY( iter != end(iter) );
   const auto entry1 = *iter;
   const auto entry2 = *iter++;
   VERIFY( entry1 == entry2 );
   VERIFY( entry1.path() == p/"l" );
-  VERIFY( iter == fs::directory_iterator() );
+  VERIFY( iter == end(iter) );
 
   remove_all(p, ec);
 }
diff --git a/libstdc++-v3/testsuite/experimental/filesystem/iterators/pop.cc b/libstdc++-v3/testsuite/experimental/filesystem/iterators/pop.cc
index d247ab4..9306c03 100644
--- a/libstdc++-v3/testsuite/experimental/filesystem/iterators/pop.cc
+++ b/libstdc++-v3/testsuite/experimental/filesystem/iterators/pop.cc
@@ -84,14 +84,10 @@ test03()
     std::advance(dir, i);
     int expected_depth = i;
     VERIFY( dir.depth() == expected_depth );
-    __builtin_printf("%d %d %s\n", i, dir.depth(), dir->path().c_str());
     dir.pop(ec);
     VERIFY( !ec );
     if (dir != end(dir))
-    {
-    __builtin_printf("%d %d %s\n", i, dir.depth(), dir->path().c_str());
       VERIFY( dir.depth() == (expected_depth - 1) );
-    }
 
     dir = fs::recursive_directory_iterator(p);
     std::advance(dir, i);
diff --git a/libstdc++-v3/testsuite/experimental/filesystem/iterators/recursive_directory_iterator.cc b/libstdc++-v3/testsuite/experimental/filesystem/iterators/recursive_directory_iterator.cc
index 79aa178..3dc7ba2 100644
--- a/libstdc++-v3/testsuite/experimental/filesystem/iterators/recursive_directory_iterator.cc
+++ b/libstdc++-v3/testsuite/experimental/filesystem/iterators/recursive_directory_iterator.cc
@@ -34,39 +34,39 @@ test01()
   const auto p = __gnu_test::nonexistent_path();
   fs::recursive_directory_iterator iter(p, ec);
   VERIFY( ec );
-  VERIFY( iter == fs::recursive_directory_iterator() );
+  VERIFY( iter == end(iter) );
 
   // Test empty directory.
   create_directory(p, fs::current_path(), ec);
   VERIFY( !ec );
   iter = fs::recursive_directory_iterator(p, ec);
   VERIFY( !ec );
-  VERIFY( iter == fs::recursive_directory_iterator() );
+  VERIFY( iter == end(iter) );
 
   // Test non-empty directory.
   create_directories(p / "d1/d2");
   VERIFY( !ec );
   iter = fs::recursive_directory_iterator(p, ec);
   VERIFY( !ec );
-  VERIFY( iter != fs::recursive_directory_iterator() );
+  VERIFY( iter != end(iter) );
   VERIFY( iter->path() == p/"d1" );
   ++iter;
   VERIFY( iter->path() == p/"d1/d2" );
   ++iter;
-  VERIFY( iter == fs::recursive_directory_iterator() );
+  VERIFY( iter == end(iter) );
 
   // Test inaccessible directory.
   permissions(p, fs::perms::none, ec);
   VERIFY( !ec );
   iter = fs::recursive_directory_iterator(p, ec);
   VERIFY( ec );
-  VERIFY( iter == fs::recursive_directory_iterator() );
+  VERIFY( iter == end(iter) );
 
   // Test inaccessible directory, skipping permission denied.
   const auto opts = fs::directory_options::skip_permission_denied;
   iter = fs::recursive_directory_iterator(p, opts, ec);
   VERIFY( !ec );
-  VERIFY( iter == fs::recursive_directory_iterator() );
+  VERIFY( iter == end(iter) );
 
   // Test inaccessible sub-directory.
   permissions(p, fs::perms::owner_all, ec);
@@ -75,24 +75,24 @@ test01()
   VERIFY( !ec );
   iter = fs::recursive_directory_iterator(p, ec);
   VERIFY( !ec );
-  VERIFY( iter != fs::recursive_directory_iterator() );
+  VERIFY( iter != end(iter) );
   VERIFY( iter->path() == p/"d1" );
   ++iter;              // should recurse into d1
   VERIFY( iter->path() == p/"d1/d2" );
   iter.increment(ec);  // should fail to recurse into p/d1/d2
   VERIFY( ec );
-  VERIFY( iter == fs::recursive_directory_iterator() );
+  VERIFY( iter == end(iter) );
 
   // Test inaccessible sub-directory, skipping permission denied.
   iter = fs::recursive_directory_iterator(p, opts, ec);
   VERIFY( !ec );
-  VERIFY( iter != fs::recursive_directory_iterator() );
+  VERIFY( iter != end(iter) );
   VERIFY( iter->path() == p/"d1" );
   ++iter;              // should recurse into d1
   VERIFY( iter->path() == p/"d1/d2" );
   iter.increment(ec);  // should fail to recurse into p/d1/d2, so skip it
   VERIFY( !ec );
-  VERIFY( iter == fs::recursive_directory_iterator() );
+  VERIFY( iter == end(iter) );
 
   permissions(p/"d1/d2", fs::perms::owner_all, ec);
   remove_all(p, ec);
@@ -109,7 +109,7 @@ test02()
   // Test post-increment (libstdc++/71005)
   auto iter = fs::recursive_directory_iterator(p, ec);
   VERIFY( !ec );
-  VERIFY( iter != fs::recursive_directory_iterator() );
+  VERIFY( iter != end(iter) );
   const auto entry1 = *iter;
   const auto entry2 = *iter++;
   VERIFY( entry1 == entry2 );
@@ -118,7 +118,7 @@ test02()
   const auto entry4 = *iter++;
   VERIFY( entry3 == entry4 );
   VERIFY( entry3.path() == p/"d1/d2" );
-  VERIFY( iter == fs::recursive_directory_iterator() );
+  VERIFY( iter == end(iter) );
 
   remove_all(p, ec);
 }
@@ -145,7 +145,7 @@ test04()
 {
   // libstdc++/71004
   const fs::recursive_directory_iterator it;
-  VERIFY( it == fs::recursive_directory_iterator() );
+  VERIFY( it == end(it) );
 }
 
 void
diff --git a/libstdc++-v3/testsuite/experimental/filesystem/operations/last_write_time.cc b/libstdc++-v3/testsuite/experimental/filesystem/operations/last_write_time.cc
index 74be4f9..a60a25f 100644
--- a/libstdc++-v3/testsuite/experimental/filesystem/operations/last_write_time.cc
+++ b/libstdc++-v3/testsuite/experimental/filesystem/operations/last_write_time.cc
@@ -32,13 +32,13 @@
 # include <utime.h>
 #endif
 
+using time_type = std::experimental::filesystem::file_time_type;
+
 void
 test01()
 {
   // read times
 
-  using time_type = std::experimental::filesystem::file_time_type;
-
   auto p = __gnu_test::nonexistent_path();
   std::error_code ec;
   time_type mtime = last_write_time(p, ec);
@@ -105,13 +105,19 @@ test01()
 #endif
 }
 
+bool approx_equal(time_type file_time, time_type expected)
+{
+  auto delta = expected - file_time;
+  if (delta < delta.zero())
+    delta = -delta;
+  return delta < std::chrono::seconds(1);
+}
+
 void
 test02()
 {
   // write times
 
-  using time_type = std::experimental::filesystem::file_time_type;
-
   __gnu_test::scoped_file f;
   std::error_code ec;
   time_type time;
@@ -119,27 +125,27 @@ test02()
   time = last_write_time(f.path);
   last_write_time(f.path, time, ec);
   VERIFY( !ec );
-  VERIFY( last_write_time(f.path) == time );
+  VERIFY( approx_equal(last_write_time(f.path), time) );
 
   time -= std::chrono::milliseconds(1000 * 60 * 10 + 15);
   last_write_time(f.path, time, ec);
   VERIFY( !ec );
-  VERIFY( last_write_time(f.path) == time );
+  VERIFY( approx_equal(last_write_time(f.path), time) );
 
   time += std::chrono::milliseconds(1000 * 60 * 20 + 15);
   last_write_time(f.path, time, ec);
   VERIFY( !ec );
-  VERIFY( last_write_time(f.path) == time );
+  VERIFY( approx_equal(last_write_time(f.path), time) );
 
   time = time_type();
   last_write_time(f.path, time, ec);
   VERIFY( !ec );
-  VERIFY( last_write_time(f.path) == time );
+  VERIFY( approx_equal(last_write_time(f.path), time) );
 
   time -= std::chrono::milliseconds(1000 * 60 * 10 + 15);
   last_write_time(f.path, time, ec);
   VERIFY( !ec );
-  VERIFY( last_write_time(f.path) == time );
+  VERIFY( approx_equal(last_write_time(f.path), time) );
 }
 
 int

References:
- [PATCH] Five patches for std::experimental::filesystem
  - From: Jonathan Wakely
- Re: [PATCH] Five patches for std::experimental::filesystem
  - From: Jonathan Wakely
- Re: [PATCH] Five patches for std::experimental::filesystem
  - From: Christophe Lyon

Index Nav:	[Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav:	[Date Prev] [Date Next]	[Thread Prev] [Thread Next]