From 7e289b3c2788325b11571d2b1bebe7f413cebd37 Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Wed, 31 May 2017 18:57:41 +0300 Subject: Implement new testscript cleanup wildcards semantics --- build2/test/script/runner.cxx | 182 +++++++++++++++++++--------------- tests/test/config-test/testscript | 2 +- tests/test/script/runner/cleanup.test | 106 ++++++++++++-------- 3 files changed, 168 insertions(+), 122 deletions(-) diff --git a/build2/test/script/runner.cxx b/build2/test/script/runner.cxx index f4c54a9..c1bf2d4 100644 --- a/build2/test/script/runner.cxx +++ b/build2/test/script/runner.cxx @@ -746,99 +746,108 @@ namespace build2 if (t == cleanup_type::never) continue; - const path& p (c.path); - const path& l (p.leaf ()); - const string& ls (l.string ()); + const path& cp (c.path); - // Remove the directory recursively if not current. Fail otherwise. - // Recursive removal of non-existing directory is not an error for - // 'maybe' cleanup type. + // Wildcard with the last component being '***' (without trailing + // separator) matches all files and sub-directories recursively as + // well as the start directories itself. So we will recursively + // remove the directories that match the parent (for the original + // path) directory wildcard. // - if (ls == "***") - { - // Cast to uint16_t to avoid ambiguity with libbutl::rmdir_r(). - // - - dir_path d (p.directory ()); - - // Don't remove the working directory (it will be removed by the - // dedicated cleanup). - // - rmdir_status r ( - rmdir_r (d, d != sp.wd_path, static_cast (3))); - - if (r == rmdir_status::success || - (r == rmdir_status::not_exist && t == cleanup_type::maybe)) - continue; + bool recursive (cp.leaf ().representation () == "***"); + const path& p (!recursive ? cp : cp.directory ()); - // The directory is unlikely to be current but let's keep for - // completeness. - // - fail (ll) << "registered for cleanup wildcard " << p - << (r == rmdir_status::not_empty - ? " matches the current directory" - : " doesn't match a directory"); - } - - // Remove files or directories using wildcard. Removal of - // sub-entries of non-existing directory is not an error for - // 'maybe' cleanup type. + // Remove files or directories using wildcard. // - // Note that only the leaf part of the cleanup wildcard is a - // pattern in terms of libbutl::path_search(). - // - if (ls == "*" || ls == "**") + if (p.string ().find_first_of ("?*") != string::npos) { - dir_path d (p.directory ()); - - if (t == cleanup_type::always && !dir_exists (d)) - fail (ll) << "registered for cleanup wildcard " << p - << " doesn't match a directory"; + bool removed (false); - if (l.to_directory ()) + auto rm = [&cp, recursive, &removed, &sp, &ll] (path&& pe, + const string&, + bool interm) { - auto rm = - [&p, &d, &ll] (path&& de, const string&, bool interm) -> bool + if (!interm) { - if (!interm) - { - dir_path sd (path_cast (d / de)); - - // We can get not_exist here due to racing conditions, but - // that's ok if somebody did our job. - // - rmdir_status r (rmdir (sd, 3)); - - if (r != rmdir_status::not_empty) - return true; - - diag_record dr (fail (ll)); - dr << "registered for cleanup directory " << sd - << " is not empty"; + // While removing the entry we can get not_exist due to + // racing conditions, but that's ok if somebody did our job. + // Note that we still set the removed flag to true in this + // case. + // + removed = true; // Will be meaningless on failure. - print_dir (dr, sd, ll); - dr << info << "wildcard: '" << p << "'"; + if (pe.to_directory ()) + { + dir_path d (path_cast (pe)); + + if (!recursive) + { + rmdir_status r (rmdir (d, 3)); + + if (r != rmdir_status::not_empty) + return true; + + diag_record dr (fail (ll)); + dr << "registered for cleanup directory " << d + << " is not empty"; + + print_dir (dr, d, ll); + dr << info << "wildcard: '" << cp << "'"; + } + else + { + // Don't remove the working directory (it will be removed + // by the dedicated cleanup). + // + // Cast to uint16_t to avoid ambiguity with + // libbutl::rmdir_r(). + // + rmdir_status r (rmdir_r (d, + d != sp.wd_path, + static_cast (3))); + + if (r != rmdir_status::not_empty) + return true; + + // The directory is unlikely to be current but let's keep + // for completeness. + // + fail (ll) << "registered for cleanup wildcard " << cp + << " matches the current directory"; + } } + else + rmfile (pe, 3); + } - return true; - }; + return true; + }; - path_search (l, rm, d); + // Note that here we rely on the fact that recursive iterating + // goes depth-first (which make sense for the cleanup). + // + try + { + path_search (p, rm); } - else + catch (const system_error& e) { - auto rm = [&d] (path&& p, const string&, bool interm) -> bool - { - if (!interm) - rmfile (d / p, 3); // That's ok if not exists. - - return true; - }; - - path_search (l, rm, d); + fail (ll) << "unable to cleanup wildcard " << cp << ": " << e; } - continue; + // Removal of no filesystem entries is not an error for 'maybe' + // cleanup type. + // + if (removed || t == cleanup_type::maybe) + continue; + + fail (ll) << "registered for cleanup wildcard " << cp + << " doesn't match any " + << (recursive + ? "path" + : p.to_directory () + ? "directory" + : "file"); } // Remove the directory if exists and empty. Fail otherwise. @@ -853,13 +862,22 @@ namespace build2 // level 2 (that was used for its creation). For other // directories use level 3 (as for other cleanups). // + int v (d == sp.wd_path ? 2 : 3); + + // Don't remove the working directory for the recursive cleanup + // (it will be removed by the dedicated one). + // // @@ If 'd' is a file then will fail with a diagnostics having // no location info. Probably need to add an optional location // parameter to rmdir() function. The same problem exists for // a file cleanup when try to rmfile() directory instead of // file. // - rmdir_status r (rmdir (d, d == sp.wd_path ? 2 : 3)); + rmdir_status r (!recursive + ? rmdir (d, v) + : rmdir_r (d, + d != sp.wd_path, + static_cast (v))); if (r == rmdir_status::success || (r == rmdir_status::not_exist && t == cleanup_type::maybe)) @@ -867,9 +885,11 @@ namespace build2 diag_record dr (fail (ll)); dr << "registered for cleanup directory " << d - << (r == rmdir_status::not_empty - ? " is not empty" - : " does not exist"); + << (r == rmdir_status::not_exist + ? " does not exist" + : !recursive + ? " is not empty" + : " is current"); if (r == rmdir_status::not_empty) print_dir (dr, d, ll); diff --git a/tests/test/config-test/testscript b/tests/test/config-test/testscript index d918a3b..0d08eb0 100644 --- a/tests/test/config-test/testscript +++ b/tests/test/config-test/testscript @@ -7,7 +7,7 @@ test.options = --serial-stop --quiet test.arguments = 'test(../proj/@./)' # Test out-of-src (for parallel). -test.cleanups = &**/ # Cleanup out directory structure. +test.cleanups = &?**/ # Cleanup out directory structure. +mkdir proj +mkdir proj/build diff --git a/tests/test/script/runner/cleanup.test b/tests/test/script/runner/cleanup.test index a26c1af..68d2014 100644 --- a/tests/test/script/runner/cleanup.test +++ b/tests/test/script/runner/cleanup.test @@ -163,47 +163,73 @@ b += --no-column : self : { - : always + : dirs : - $c <'$* -d a/b -f a/b/c &a/***' && $b + { + : always + : + $c <'$* -d a/b -d a/b/c &a/***/' && $b - : maybe - : - $c <'$* &?a/***' && $b + : maybe + : + $c <'$* &?a/***/' && $b - : not-exists - : - : Test cleanup of a wildcard not matching any directory. - : - $c <'$* &a/***' && $b 2>>/EOE != 0 - testscript:1: error: registered for cleanup wildcard test/1/a/*** doesn't match a directory - EOE + : not-empty + : + $c <'$* -d a/b -d a/b/c -f a/c &a/***/' && $b 2>>/EOE != 0 + testscript:1: error: registered for cleanup directory test/1/a/ is not empty + c + info: wildcard: 'test/1/a/***/' + EOE + } - : out-wd + : all-entries : - : Test cleanup of a wildcard out of the testscript working directory. + : Test the trailing triple-star special case. : - $c <'$* &../../a/***' && $b 2>>/EOE != 0 - testscript:1: error: wildcard cleanup ../../a/*** is out of working directory test/ - EOE + { + : always + : + $c <'$* -d a1/b -f a1/b/c -d a2/b -f a2/b/c &a?/***' && $b - : in-wd - : - : Test cleanup registration of a wildcard matching the directory that being - : outside the test working directory is inside the testscript working - : directory. - : - $c <'$* &../a/***' && $b 2>>/EOE != 0 - testscript:1: error: registered for cleanup wildcard test/a/*** doesn't match a directory - EOE + : maybe + : + $c <'$* &?a/***' && $b - : not-dir - : - : Test cleanup of a file as a wildcard. - : - $c <'$* -f a &a/***' && $b 2>>/~%EOE% != 0 - %error: unable to remove directory test/1/a/: .+% - EOE + : not-exists + : + : Test cleanup of a wildcard not matching any directory. + : + $c <'$* &a/***' && $b 2>>/EOE != 0 + testscript:1: error: registered for cleanup directory test/1/a/ does not exist + EOE + + : out-wd + : + : Test cleanup of a wildcard out of the testscript working directory. + : + $c <'$* &../../a/***' && $b 2>>/EOE != 0 + testscript:1: error: wildcard cleanup ../../a/*** is out of working directory test/ + EOE + + : in-wd + : + : Test cleanup registration of a wildcard matching the directory that being + : outside the test working directory is inside the testscript working + : directory. + : + $c <'$* &../a/***' && $b 2>>/EOE != 0 + testscript:1: error: registered for cleanup directory test/a/ does not exist + EOE + + : not-dir + : + : Test cleanup of a file as a wildcard. + : + $c <'$* -f a &a/***' && $b 2>>/~%EOE% != 0 + %error: unable to remove directory test/1/a/: .*% + EOE + } } : dir @@ -214,11 +240,11 @@ b += --no-column { : immediate : - $c <'$* -d a/b &a/ &a/*/' && $b + $c <'$* -d aa/b &aa/ &a*/*/' && $b : recursive : - $c <'$* -d a/b/c &a/ &a/**/' && $b + $c <'$* -d aa/b/c &aa/ &a?/**/' && $b } : maybe @@ -230,7 +256,7 @@ b += --no-column : Test cleanup of a wildcard that doesn't match any directory. : $c <'$* &a/**/' && $b 2>>/EOE != 0 - testscript:1: error: registered for cleanup wildcard test/1/a/**/ doesn't match a directory + testscript:1: error: registered for cleanup wildcard test/1/a/**/ doesn't match any directory EOE : not-dir @@ -238,7 +264,7 @@ b += --no-column : Test cleanup of a file as a directory wildcard. : $c <'$* -f a &a/**/' && $b 2>>/EOE != 0 - testscript:1: error: registered for cleanup wildcard test/1/a/**/ doesn't match a directory + testscript:1: error: registered for cleanup wildcard test/1/a/**/ doesn't match any directory EOE : not-empty @@ -260,11 +286,11 @@ b += --no-column { : immediate : - $c <'$* -d a -f a/c &a/ &a/*' && $b + $c <'$* -d aa -f aa/c &aa/ &a?/*' && $b : recursive : - $c <'$* -d a/b -f a/c -f a/b/e &a/ &a/b/ &a/**' && $b + $c <'$* -d aa/b -f aa/c -f aa/b/e &aa/ &aa/b/ &a*/**' && $b } : maybe @@ -286,7 +312,7 @@ $c <'$* -d a/b &a/ &a/b/' && $b : $c <'foo'; -true &* +$* -f bar &* EOI : wd-wildcard -- cgit v1.1