From e6470e37093084251b7ee60a904a78e54d13e31b Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Mon, 25 May 2020 22:04:28 +0300 Subject: Use dir_name_view for script environment working and sandbox directories --- doc/testscript.cli | 4 +- libbuild2/build/script/script.cxx | 4 +- libbuild2/script/run.cxx | 86 +++++++++++++++++++---------- libbuild2/script/run.hxx | 13 +++++ libbuild2/script/script.cxx | 2 +- libbuild2/script/script.hxx | 24 ++++---- libbuild2/test/script/runner.cxx | 23 ++++---- libbuild2/test/script/script.cxx | 22 +++++--- libbuild2/test/script/script.hxx | 2 +- libbuild2/types.hxx | 2 +- tests/test/script/builtin/mv.testscript | 8 +-- tests/test/script/builtin/rm.testscript | 6 +- tests/test/script/builtin/rmdir.testscript | 6 +- tests/test/script/runner/cleanup.testscript | 12 ++-- 14 files changed, 129 insertions(+), 85 deletions(-) diff --git a/doc/testscript.cli b/doc/testscript.cli index 5b36c5e..4c78e18 100644 --- a/doc/testscript.cli +++ b/doc/testscript.cli @@ -545,7 +545,7 @@ complete picture: \ $* 'World' >'Hello, World!' : command-name -$* 'John' 'Jane' >>EOO : command-names +$* 'John' 'Jane' >>EOO : command-names Hello, Jane! Hello, John! EOO @@ -943,7 +943,7 @@ Alternatively, we can use an absolute path: \ Inside the scope working directory filesystem names that start with \c{stdin}, -\c{stdout}, and \c{stderr}. +\c{stdout}, and \c{stderr} are reserved. To execute a test scope its commands (including variable assignments) are executed sequentially and in the order specified. If any of the commands diff --git a/libbuild2/build/script/script.cxx b/libbuild2/build/script/script.cxx index c258c4c..d27a30a 100644 --- a/libbuild2/build/script/script.cxx +++ b/libbuild2/build/script/script.cxx @@ -17,14 +17,14 @@ namespace build2 { // environment // - static const string wd_name ("current directory"); + static const optional wd_name ("current directory"); environment:: environment (action a, const target_type& t) : build2::script::environment ( t.ctx, cast (t.ctx.global_scope["build.host"]), - work, wd_name, + dir_name_view (&work, &wd_name), temp_dir.path, false /* temp_dir_keep */, redirect (redirect_type::none), redirect (redirect_type::merge, 2), diff --git a/libbuild2/script/run.cxx b/libbuild2/script/run.cxx index 870a70f..a48421c 100644 --- a/libbuild2/script/run.cxx +++ b/libbuild2/script/run.cxx @@ -24,6 +24,35 @@ namespace build2 { namespace script { + string + diag_path (const path& d) + { + string r ("'"); + + r += stream_verb_map ().path < 1 + ? diag_relative (d) + : d.representation (); + + r += '\''; + return r; + } + + string + diag_path (const dir_name_view& dn) + { + string r; + if (dn.name != nullptr && *dn.name) + { + r += **dn.name; + r += ' '; + } + + assert (dn.path != nullptr); + + r += diag_path (*dn.path); + return r; + } + // Normalize a path. Also make the relative path absolute using the // specified directory unless it is already absolute. // @@ -260,7 +289,7 @@ namespace build2 path eop; if (rd.type == redirect_type::file) - eop = normalize (rd.file.path, env.work_dir, ll); + eop = normalize (rd.file.path, *env.work_dir.path, ll); else { eop = path (op + ".orig"); @@ -894,19 +923,19 @@ namespace build2 for (const auto& cl: c.cleanups) { const path& p (cl.path); - path np (normalize (p, env.work_dir, ll)); + path np (normalize (p, *env.work_dir.path, ll)); const string& ls (np.leaf ().string ()); bool wc (ls == "*" || ls == "**" || ls == "***"); const path& cp (wc ? np.directory () : np); - const dir_path& sd (env.sandbox_dir); + const dir_path* sd (env.sandbox_dir.path); - if (!sd.empty () && !cp.sub (sd)) + if (sd != nullptr && !cp.sub (*sd)) fail (ll) << (wc ? "wildcard" : p.to_directory () ? "directory" : "file") << " cleanup " << p << " is out of " - << env.sandbox_dir_name << " " << sd; + << diag_path (env.sandbox_dir); env.clean ({cl.type, move (np)}, false); } @@ -993,6 +1022,8 @@ namespace build2 // auto std_path = [&env, &ci, &li, &ll] (const char* n) -> path { + using std::to_string; + path p (n); // 0 if belongs to a single-line script, otherwise is the command line @@ -1078,7 +1109,7 @@ namespace build2 } case redirect_type::file: { - isp = normalize (in.file.path, env.work_dir, ll); + isp = normalize (in.file.path, *env.work_dir.path, ll); open_stdin (); break; @@ -1193,7 +1224,7 @@ namespace build2 // p = r.file.mode == redirect_fmode::compare ? std_path (what) - : normalize (r.file.path, env.work_dir, ll); + : normalize (r.file.path, *env.work_dir.path, ll); m |= r.file.mode == redirect_fmode::append ? fdopen_mode::at_end @@ -1378,21 +1409,20 @@ namespace build2 if (pre) { - const dir_path& wd (env.work_dir); - const dir_path& sd (env.sandbox_dir); + const dir_path& wd (*env.work_dir.path); + const dir_path* sd (env.sandbox_dir.path); auto fail = [] (const string& d) {throw runtime_error (d);}; - if (!sd.empty () && !from.sub (sd) && !force) - fail ("'" + from.representation () + - "' is out of " + env.sandbox_dir_name + " '" + - sd.string () + "'"); + if (sd != nullptr && !from.sub (*sd) && !force) + fail (diag_path (from) + " is out of " + + diag_path (env.sandbox_dir)); auto check_wd = [&wd, &env, fail] (const path& p) { if (wd.sub (path_cast (p))) - fail ("'" + p.string () + "' contains " + - env.work_dir_name + " '" + wd.string () + "'"); + fail (diag_path (p) + " contains " + + diag_path (env.work_dir)); }; check_wd (from); @@ -1404,7 +1434,7 @@ namespace build2 // if (cln->enabled) cln->move = !butl::entry_exists (to) && - (sd.empty () || to.sub (sd)); + (sd == nullptr || to.sub (*sd)); } else if (cln->enabled) { @@ -1469,20 +1499,18 @@ namespace build2 { if (pre) { - const dir_path& wd (env.work_dir); - const dir_path& sd (env.sandbox_dir); + const dir_path& wd (*env.work_dir.path); + const dir_path* sd (env.sandbox_dir.path); auto fail = [] (const string& d) {throw runtime_error (d);}; - if (!sd.empty () && !p.sub (sd) && !force) - fail ("'" + p.representation () + - "' is out of " + env.sandbox_dir_name + " '" + - sd.string () + "'"); + if (sd != nullptr && !p.sub (*sd) && !force) + fail (diag_path (p) + " is out of " + + diag_path (env.sandbox_dir)); if (wd.sub (path_cast (p))) - fail ("'" + p.string () + - "' contains " + env.work_dir_name + - " '" + wd.string () + "'"); + fail (diag_path (p) + " contains " + + diag_path (env.work_dir)); } }, @@ -1520,7 +1548,7 @@ namespace build2 builtin b (bf (r, c.arguments, move (ifd), move (ofd.out), move (efd), - env.work_dir, + *env.work_dir.path, bcs)); success = run_pipe (env, @@ -1573,7 +1601,7 @@ namespace build2 program (path (s, 1, s.size () - 1)); } else - program (env.work_dir / p); + program (*env.work_dir.path / p); } } catch (const invalid_path& e) @@ -1594,7 +1622,7 @@ namespace build2 pp, args.data (), {ifd.get (), -1}, process::pipe (ofd), {-1, efd.get ()}, - env.work_dir.string ().c_str ()); + env.work_dir.path->string ().c_str ()); ifd.reset (); ofd.out.reset (); @@ -1783,7 +1811,7 @@ namespace build2 clean (environment& env, const location& ll) { context& ctx (env.context); - const dir_path& wdir (env.work_dir); + const dir_path& wdir (*env.work_dir.path); // Note that we operate with normalized paths here. // diff --git a/libbuild2/script/run.hxx b/libbuild2/script/run.hxx index 3f73eed..477dd88 100644 --- a/libbuild2/script/run.hxx +++ b/libbuild2/script/run.hxx @@ -56,6 +56,19 @@ namespace build2 // void print_dir (diag_record&, const dir_path&, const location&); + + // Return the quoted path representation with the preserved trailing + // directory separator. The path is relative if the verbosity level is + // less than 3. + // + string + diag_path (const path&); + + // Same as above, but prepends the path with a name, if present. The path + // must be not NULL. + // + string + diag_path (const dir_name_view&); } } diff --git a/libbuild2/script/script.cxx b/libbuild2/script/script.cxx index 9e8780e..c85bfd3 100644 --- a/libbuild2/script/script.cxx +++ b/libbuild2/script/script.cxx @@ -633,7 +633,7 @@ namespace build2 const path& p (c.path); - if (!sandbox_dir.empty () && !p.sub (sandbox_dir)) + if (sandbox_dir.path != nullptr && !p.sub (*sandbox_dir.path)) { if (implicit) return; diff --git a/libbuild2/script/script.hxx b/libbuild2/script/script.hxx index 359bb36..b887df6 100644 --- a/libbuild2/script/script.hxx +++ b/libbuild2/script/script.hxx @@ -363,16 +363,14 @@ namespace build2 // using the rm or mv builtins will fail the script execution. Must be // an absolute path. // - const dir_path& work_dir; // @@ dir_path_name - const string& work_dir_name; // Directory name for diagnostics. + const dir_name_view work_dir; - // If non-empty, then any attempt to remove or move a filesystem entry - // outside this directory using an explicit cleanup or the rm/mv + // If path is not NULL, then any attempt to remove or move a filesystem + // entry outside this directory using an explicit cleanup or the rm/mv // builtins will fail the script execution, unless the --force option is - // specified for the builtin. Must be an absolute path, unless is empty. + // specified for the builtin. Must be an absolute path, unless is NULL. // - const dir_path& sandbox_dir; // @@ dir_path_name - const string& sandbox_dir_name; // Directory name for diagnostics. + const dir_name_view sandbox_dir; // Used by the script running machinery to create special files in it. // Must be an absolute path. @@ -396,8 +394,8 @@ namespace build2 environment (build2::context& ctx, const target_triplet& pt, - const dir_path& wd, const string& wn, - const dir_path& sd, const string& sn, + const dir_name_view& wd, + const dir_name_view& sd, const dir_path& td, bool tk, redirect&& i = redirect (redirect_type::pass), redirect&& o = redirect (redirect_type::pass), @@ -405,9 +403,7 @@ namespace build2 : context (ctx), platform (pt), work_dir (wd), - work_dir_name (wn), sandbox_dir (sd), - sandbox_dir_name (sn), temp_dir (td), temp_dir_keep (tk), in (move (i)), @@ -420,15 +416,15 @@ namespace build2 // environment (build2::context& ctx, const target_triplet& pt, - const dir_path& wd, const string& wn, + const dir_name_view& wd, const dir_path& td, bool tk, redirect&& i = redirect (redirect_type::pass), redirect&& o = redirect (redirect_type::pass), redirect&& e = redirect (redirect_type::pass)) : environment (ctx, pt, - wd, wn, - empty_dir_path, empty_string, + wd, + dir_name_view (), td, tk, move (i), move (o), move (e)) { diff --git a/libbuild2/test/script/runner.cxx b/libbuild2/test/script/runner.cxx index 6e722de..03a1f0e 100644 --- a/libbuild2/test/script/runner.cxx +++ b/libbuild2/test/script/runner.cxx @@ -13,6 +13,8 @@ namespace build2 { namespace script { + using namespace build2::script; + bool default_runner:: test (scope& s) const { @@ -49,20 +51,20 @@ namespace build2 sp.parent == nullptr ? mkdir_buildignore ( ctx, - sp.work_dir, + *sp.work_dir.path, sp.root.target_scope.root_scope ()->root_extra->buildignore_file, 2) - : mkdir (sp.work_dir, 2)); + : mkdir (*sp.work_dir.path, 2)); if (r == mkdir_status::already_exists) - fail << "working directory " << sp.work_dir << " already exists" << + fail << diag_path (sp.work_dir) << " already exists" << info << "are tests stomping on each other's feet?"; // We don't change the current directory here but indicate that the // scope test commands will be executed in that directory. // if (verb >= 2) - text << "cd " << sp.work_dir; + text << "cd " << *sp.work_dir.path; } void default_runner:: @@ -88,22 +90,23 @@ namespace build2 rmdir_status r ( sp.parent == nullptr ? rmdir_buildignore (ctx, - sp.work_dir, + *sp.work_dir.path, sp.root.target_scope.root_scope ()-> root_extra->buildignore_file, 2) - : rmdir (ctx, sp.work_dir, 2)); + : rmdir (ctx, *sp.work_dir.path, 2)); if (r != rmdir_status::success) { diag_record dr (fail (ll)); - dr << "working directory " << sp.work_dir + + dr << diag_path (sp.work_dir) << (r == rmdir_status::not_exist ? " does not exist" : " is not empty"); if (r == rmdir_status::not_empty) - build2::script::print_dir (dr, sp.work_dir, ll); + print_dir (dr, *sp.work_dir.path, ll); } } @@ -112,8 +115,8 @@ namespace build2 // if (verb >= 2) text << "cd " << (sp.parent != nullptr - ? sp.parent->work_dir - : sp.work_dir.directory ()); + ? *sp.parent->work_dir.path + : sp.work_dir.path->directory ()); } void default_runner:: diff --git a/libbuild2/test/script/script.cxx b/libbuild2/test/script/script.cxx index 9f8cb0b..b56da1b 100644 --- a/libbuild2/test/script/script.cxx +++ b/libbuild2/test/script/script.cxx @@ -28,10 +28,10 @@ namespace build2 vars.assign (root.wd_var) = dir_path (); } - const dir_path& scope_base:: + const dir_path* scope_base:: wd_path () const { - return cast (vars [root.wd_var]); + return &cast (vars[root.wd_var]); } const target_triplet& scope_base:: @@ -49,8 +49,8 @@ namespace build2 // scope // - static const string wd_name ("test working directory"); - static const string sd_name ("working directory"); + static const optional wd_name ("test working directory"); + static const optional sd_name ("working directory"); scope:: scope (const string& id, scope* p, script& r) @@ -62,9 +62,11 @@ namespace build2 // environment (root.test_target.ctx, test_tt (), - wd_path (), wd_name, - p != nullptr ? root.work_dir : wd_path (), sd_name, - wd_path (), true /* temp_dir_keep */, + dir_name_view (wd_path (), &wd_name), + dir_name_view ( + p != nullptr ? root.work_dir.path : wd_path (), + &sd_name), + *wd_path (), true /* temp_dir_keep */, redirect (redirect_type::none), redirect (redirect_type::none), redirect (redirect_type::none)), @@ -89,7 +91,8 @@ namespace build2 // (handled in an ad hoc way). // if (p != nullptr) - const_cast (work_dir) = dir_path (p->work_dir) /= id; + const_cast (*work_dir.path) = + dir_path (*p->work_dir.path) /= id; } void scope:: @@ -171,7 +174,8 @@ namespace build2 // Set the script working dir ($~) to $out_base/test/ (id_path // for root is just the id which is empty if st is 'testscript'). // - const_cast (work_dir) = dir_path (rwd) /= id_path.string (); + const_cast (*work_dir.path) = + dir_path (rwd) /= id_path.string (); // Set the test variable at the script level. We do it even if it's // set in the buildfile since they use different types. diff --git a/libbuild2/test/script/script.hxx b/libbuild2/test/script/script.hxx index 93846df..7bdb2ac 100644 --- a/libbuild2/test/script/script.hxx +++ b/libbuild2/test/script/script.hxx @@ -67,7 +67,7 @@ namespace build2 protected: scope_base (script&); - const dir_path& + const dir_path* wd_path () const; const target_triplet& diff --git a/libbuild2/types.hxx b/libbuild2/types.hxx index d9f222b..6582c3a 100644 --- a/libbuild2/types.hxx +++ b/libbuild2/types.hxx @@ -230,6 +230,7 @@ namespace build2 using butl::path_name_view; using butl::path_name_value; using butl::dir_path; + using butl::dir_name_view; using butl::path_cast; using butl::basic_path; using butl::invalid_path; @@ -377,7 +378,6 @@ namespace build2 LIBBUILD2_SYMEXPORT ostream& operator<< (ostream&, run_phase); // utility.cxx - } // In order to be found (via ADL) these have to be either in std:: or in diff --git a/tests/test/script/builtin/mv.testscript b/tests/test/script/builtin/mv.testscript index 31e2603..764c1e0 100644 --- a/tests/test/script/builtin/mv.testscript +++ b/tests/test/script/builtin/mv.testscript @@ -21,8 +21,8 @@ : Moving path outside the testscript working directory fails. : $c <>/~%EOE% == 1 - %mv: '.+/fail/a/b/c' is out of working directory '.+/fail/test'% + mv ../../a/b/c ./c 2>>/EOE == 1 + mv: 'a/b/c' is out of working directory 'test/' EOE EOI @@ -84,14 +84,14 @@ : src : $c <"mv: '$~' contains test working directory '$~'" != 0 + mv $~ b 2>/"mv: 'test/1/' contains test working directory 'test/1/'" != 0 EOI : dst : $c <"mv: '$~' contains test working directory '$~'" != 0 + mv a "$~" 2>/"mv: 'test/1' contains test working directory 'test/1/'" != 0 EOI } diff --git a/tests/test/script/builtin/rm.testscript b/tests/test/script/builtin/rm.testscript index 5b00042..21ec2a9 100644 --- a/tests/test/script/builtin/rm.testscript +++ b/tests/test/script/builtin/rm.testscript @@ -11,7 +11,7 @@ : Removing scope directory fails. : $c <"rm: '$~' contains test working directory '$~'" == 1 + rm -r ./ 2>/"rm: 'test/1/' contains test working directory 'test/1/'" == 1 EOI } @@ -28,8 +28,8 @@ : Removing path outside the testscript working directory fails. : $c <>/~%EOE% == 1 - %rm: '.+/path/outside-scope/fail/a/b/c' is out of working directory '.+/path/outside-scope/fail/test'% + rm ../../a/b/c 2>>/EOE == 1 + rm: 'a/b/c' is out of working directory 'test/' EOE EOI diff --git a/tests/test/script/builtin/rmdir.testscript b/tests/test/script/builtin/rmdir.testscript index 269dd58..a63a5dd 100644 --- a/tests/test/script/builtin/rmdir.testscript +++ b/tests/test/script/builtin/rmdir.testscript @@ -11,7 +11,7 @@ : Removing scope directory fails. : $c <"rmdir: '$~' contains test working directory '$~'" == 1 + rmdir ./ 2>/"rmdir: 'test/1/' contains test working directory 'test/1/'" == 1 EOI : outside-scope @@ -24,8 +24,8 @@ : Removing directory outside the testscript working directory fails. : $c <>/~%EOE% == 1 - %rmdir: '.+/dir/outside-scope/fail/a/b/c/' is out of working directory '.+/dir/outside-scope/fail/test'% + rmdir ../../a/b/c 2>>/EOE == 1 + rmdir: 'a/b/c/' is out of working directory 'test/' EOE EOI diff --git a/tests/test/script/runner/cleanup.testscript b/tests/test/script/runner/cleanup.testscript index fcbdfc2..03153e4 100644 --- a/tests/test/script/runner/cleanup.testscript +++ b/tests/test/script/runner/cleanup.testscript @@ -56,7 +56,7 @@ b += --no-column : Test explicit cleanup of a file out of the testscript working directory. : $c <'$* &../../a' && $b 2>>/EOE != 0 - testscript:1: error: file cleanup ../../a is out of working directory test/ + testscript:1: error: file cleanup ../../a is out of working directory 'test/' info: test id: 1 EOE @@ -116,7 +116,7 @@ b += --no-column : Test cleanup of a directory out of the testscript working directory. : $c <'$* &../../a/' && $b 2>>/EOE != 0 - testscript:1: error: directory cleanup ../../a/ is out of working directory test/ + testscript:1: error: directory cleanup ../../a/ is out of working directory 'test/' info: test id: 1 EOE @@ -150,7 +150,7 @@ b += --no-column : dir : $c <'$* -d a/b' && $b 2>>/EOE != 0 - testscript:1: error: working directory test/1/ is not empty + testscript:1: error: test working directory 'test/1/' is not empty a/ info: test id: 1 EOE @@ -220,7 +220,7 @@ b += --no-column : 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/ + testscript:1: error: wildcard cleanup ../../a/*** is out of working directory 'test/' info: test id: 1 EOE @@ -346,7 +346,7 @@ EOI : Test an implicit cleanup being overwritten with the explicit one, : $c <'$* -o foo >=a &!a' && $b 2>>/EOE != 0 -testscript:1: error: working directory test/1/ is not empty +testscript:1: error: test working directory 'test/1/' is not empty a info: test id: 1 EOE @@ -359,7 +359,7 @@ $c <>/EOE != 0 $* &!a; $* -o foo >=a EOO -testscript:2: error: working directory test/1/ is not empty +testscript:2: error: test working directory 'test/1/' is not empty a info: test id: 1 EOE -- cgit v1.1