From patchwork Thu Oct 27 13:34:57 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Wakely X-Patchwork-Id: 79656 Delivered-To: patch@linaro.org Received: by 10.140.97.247 with SMTP id m110csp627140qge; Thu, 27 Oct 2016 06:35:37 -0700 (PDT) X-Received: by 10.107.12.88 with SMTP id w85mr6746040ioi.198.1477575337183; Thu, 27 Oct 2016 06:35:37 -0700 (PDT) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id h9si6658036pap.255.2016.10.27.06.35.36 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 27 Oct 2016 06:35:37 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-return-439744-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org; spf=pass (google.com: domain of gcc-patches-return-439744-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=gcc-patches-return-439744-patch=linaro.org@gcc.gnu.org DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; q=dns; s=default; b=m+omqgE24sOIddoWN s8M+oOuLGB19mUN9CdS09I6SgTmvq1fyy3fXWPZCIWGKvXA/C3MNlVHHv5zYRg/O Z6ljIn+aX2jhsaO2mNdcP+BCzy4OCqr0ufT0EbPUyNvWvpZEnFdwG1an8UwqHCM1 4hM1ygcd7nz66/nzqE2SBwGWSc= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=default; bh=Wwy/JlHHMJUmJTMd1xXp0vd 7nAI=; b=aia8MDq8LDxuX8bWpS1qji/GkMU+Rh0InCify2+P5u9wnfKMc0d6DfK R8FZlE1mVBpBgvceBYDmrh9zmo41GHsM/raE/i1CgubbnO/BSXcRT6Qk7qTdbnne /APJfo6C6T0fExq7v499tTBJ7mx8oMxA9hn7ibRpTvKLg4/6ZeJ0= Received: (qmail 14962 invoked by alias); 27 Oct 2016 13:35:13 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 14920 invoked by uid 89); 27 Oct 2016 13:35:10 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.3 required=5.0 tests=BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=Lyon, lyon, Christophe, Five X-Spam-User: qpsmtpd, 2 recipients X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 27 Oct 2016 13:35:00 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 08162335EA9; Thu, 27 Oct 2016 13:34:59 +0000 (UTC) Received: from localhost (ovpn-116-41.ams2.redhat.com [10.36.116.41]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u9RDYwxS006001; Thu, 27 Oct 2016 09:34:58 -0400 Date: Thu, 27 Oct 2016 14:34:57 +0100 From: Jonathan Wakely To: Christophe Lyon Cc: "libstdc++@gcc.gnu.org" , "gcc-patches@gcc.gnu.org" Subject: Re: [PATCH] Five patches for std::experimental::filesystem Message-ID: <20161027110139.GY2922@redhat.com> References: <20161024165010.GL2922@redhat.com> <20161025153232.GN2922@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-Clacks-Overhead: GNU Terry Pratchett User-Agent: Mutt/1.7.0 (2016-08-17) On 26/10/16 09:24 +0200, Christophe Lyon wrote: >Hi Jonathan, > >On 25 October 2016 at 17:32, Jonathan Wakely 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 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 #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