diff options
| -rw-r--r-- | libbuild2/build/script/runner.cxx | 97 | ||||
| -rw-r--r-- | libbuild2/build/script/script.cxx | 4 | ||||
| -rw-r--r-- | libbuild2/build/script/script.hxx | 14 | ||||
| -rw-r--r-- | libbuild2/script/run.cxx | 107 | ||||
| -rw-r--r-- | libbuild2/script/script.hxx | 24 | ||||
| -rw-r--r-- | libbuild2/test/script/script.cxx | 1 |
6 files changed, 200 insertions, 47 deletions
diff --git a/libbuild2/build/script/runner.cxx b/libbuild2/build/script/runner.cxx index 5535bbb..c4a93fd 100644 --- a/libbuild2/build/script/runner.cxx +++ b/libbuild2/build/script/runner.cxx @@ -3,8 +3,12 @@ #include <libbuild2/build/script/runner.hxx> +#include <libbutl/filesystem.mxx> + #include <libbuild2/script/run.hxx> +using namespace butl; + namespace build2 { namespace build @@ -12,15 +16,104 @@ namespace build2 namespace script { void default_runner:: - enter (environment&, const location&) + enter (environment& env, const location& ll) { - // Noop. + // Create the temporary directory for this run regardless of the + // dry-run mode, since some commands still can be executed (see run() + // for details). This also a reason for not using the build2 + // filesystem API that considers the dry-run mode. + // + // Note that the directory auto-removal is active. + // + dir_path& td (env.temp_dir.path); + + try + { + td = dir_path::temp_path ("build2-build-script"); + } + catch (const system_error& e) + { + // While there can be no fault of the script being currently + // executed let's add the location anyway to ease the + // troubleshooting. And let's stick to that principle down the road. + // + fail (ll) << "unable to obtain temporary directory for buildscript " + << "execution" << e; + } + + mkdir_status r; + + try + { + r = try_mkdir (td); + } + catch (const system_error& e) + { + fail(ll) << "unable to create temporary directory '" << td << "': " + << e << endf; + } + + // Note that the temporary directory can potentially stay after some + // abnormally terminated script run. Clean it up and reuse if that's + // the case. + // + if (r == mkdir_status::already_exists) + try + { + butl::rmdir_r (td, false /* dir */); + } + catch (const system_error& e) + { + fail (ll) << "unable to cleanup temporary directory '" << td + << "': " << e; + } + + if (verb >= 3) + text << "mkdir " << td; } void default_runner:: leave (environment& env, const location& ll) { clean (env, ll); + + // Note that since the temporary directory may only contain special + // files that are created and registered for cleanup by the script + // running machinery and should all be removed by the above clean() + // function call, its removal failure may not be the script fault but + // potentially a bug or a filesystem problem. Thus, we don't ignore + // the errors and report them. + // + env.temp_dir.cancel (); + + const dir_path& td (env.temp_dir.path); + + try + { + // Note that the temporary directory must be empty to date. + // + rmdir_status r (try_rmdir (td)); + + if (r != rmdir_status::success) + { + diag_record dr (fail (ll)); + dr << "temporary directory '" << td + << (r == rmdir_status::not_exist + ? "' does not exist" + : "' is not empty"); + + if (r == rmdir_status::not_empty) + build2::script::print_dir (dr, td, ll); + } + } + catch (const system_error& e) + { + fail (ll) << "unable to remove temporary directory '" << td << "': " + << e; + } + + if (verb >= 3) + text << "rmdir " << td; } void default_runner:: diff --git a/libbuild2/build/script/script.cxx b/libbuild2/build/script/script.cxx index cbd41c7..b43203c 100644 --- a/libbuild2/build/script/script.cxx +++ b/libbuild2/build/script/script.cxx @@ -24,8 +24,8 @@ namespace build2 : build2::script::environment ( pt.ctx, cast<target_triplet> (pt.ctx.global_scope["build.host"]), - work, - wd_name, + work, wd_name, + temp_dir.path, false /* temp_dir_keep */, redirect (redirect_type::none), redirect (redirect_type::merge, 2), redirect (redirect_type::pass)), diff --git a/libbuild2/build/script/script.hxx b/libbuild2/build/script/script.hxx index 29d62aa..f8306e1 100644 --- a/libbuild2/build/script/script.hxx +++ b/libbuild2/build/script/script.hxx @@ -9,6 +9,7 @@ #include <libbuild2/utility.hxx> #include <libbuild2/variable.hxx> +#include <libbuild2/filesystem.hxx> // auto_rmdir #include <libbuild2/script/script.hxx> @@ -58,9 +59,6 @@ namespace build2 location end_loc; }; - //@@ Does environment need script? Can't we just pass it to parser along - // with environment. - // class environment: public build2::script::environment { public: @@ -83,6 +81,16 @@ namespace build2 // variable_map vars; + // Temporary directory for the script run (see build2::script:: + // environment::temp_dir for details). + // + // Currently this directory is removed regardless of the script + // execution success or failure. Later, to ease the troubleshooting, + // we may invent the build2 option suppressing the directory removal + // on failure. + // + auto_rmdir temp_dir; + virtual void set_variable (string&& name, names&&, const string& attrs) override; diff --git a/libbuild2/script/run.cxx b/libbuild2/script/run.cxx index b53fbc5..3474ccb 100644 --- a/libbuild2/script/run.cxx +++ b/libbuild2/script/run.cxx @@ -25,12 +25,12 @@ namespace build2 namespace script { // Normalize a path. Also make the relative path absolute using the - // environment's working directory unless it is already absolute. + // specified directory unless it is already absolute. // static path - normalize (path p, environment& env, const location& l) + normalize (path p, const dir_path& d, const location& l) { - path r (p.absolute () ? move (p) : env.work_dir / move (p)); + path r (p.absolute () ? move (p) : d / move (p)); try { @@ -178,6 +178,15 @@ namespace build2 return r; } + // Return true if a path is not under the script temporary directory or + // this directory will not be removed on failure. + // + static inline bool + avail_on_failure (const path& p, const environment& env) + { + return env.temp_dir_keep || !p.sub (env.temp_dir); + } + // Check if the script command output matches the expected result // (redirect value). Noop for redirect types other than none, here_*. // @@ -191,19 +200,22 @@ namespace build2 bool diag, const char* what) { - auto input_info = [&ip, &ll] (diag_record& d) + auto input_info = [&ip, &ll, &env] (diag_record& d) { - if (non_empty (ip, ll)) + if (non_empty (ip, ll) && avail_on_failure (ip, env)) d << info << "stdin: " << ip; }; - auto output_info = [&what, &ll] (diag_record& d, - const path& p, - const char* prefix = "", - const char* suffix = "") + auto output_info = [&what, &ll, &env] (diag_record& d, + const path& p, + const char* prefix = "", + const char* suffix = "") { if (non_empty (p, ll)) - d << info << prefix << what << suffix << ": " << p; + { + if (avail_on_failure (p, env)) + d << info << prefix << what << suffix << ": " << p; + } else d << info << prefix << what << suffix << " is empty"; }; @@ -220,8 +232,10 @@ namespace build2 if (diag) { diag_record d (error (ll)); - d << pr << " unexpectedly writes to " << what << - info << what << ": " << op; + d << pr << " unexpectedly writes to " << what; + + if (avail_on_failure (op, env)) + d << info << what << ": " << op; input_info (d); @@ -246,7 +260,7 @@ namespace build2 path eop; if (rd.type == redirect_type::file) - eop = normalize (rd.file.path, env, ll); + eop = normalize (rd.file.path, env.work_dir, ll); else { eop = path (op + ".orig"); @@ -263,7 +277,17 @@ namespace build2 path dp ("diff"); process_path pp (run_search (dp, true)); - cstrings args {pp.recall_string (), "-u"}; + cstrings args {pp.recall_string ()}; + + // If both files being compared won't be available on failure, then + // instruct diff not to print the file paths. It seems that the only + // way to achieve this is to abandon the output unified format in the + // favor of the minimal output, which normally is still informative + // enough for the troubleshooting (contains the difference line + // numbers, etc). + // + if (avail_on_failure (eop, env) || avail_on_failure (op, env)) + args.push_back ("-u"); // Ignore Windows newline fluff if that's what we are running on. // @@ -317,6 +341,8 @@ namespace build2 diag_record d (fail (ll)); print_process (d, args); d << " " << pe; + + print_file (d, ep, ll); } // Output doesn't match the expected result. @@ -568,7 +594,11 @@ namespace build2 // d << "invalid " << what << " regex redirect" << e; - output_info (d, save_regex (), "", " regex"); + // It would be a waste to save the regex into the file just to + // remove it. + // + if (env.temp_dir_keep) + output_info (d, save_regex (), "", " regex"); } // Parse the output into the literal line string. @@ -621,13 +651,17 @@ namespace build2 if (regex_match (ls, regex)) // Doesn't throw. return true; - // Output doesn't match the regex. We save the regex to file for - // troubleshooting regardless of whether we print the diagnostics or - // not. We, however, register it for cleanup in the later case (the - // expression may still succeed, we can be evaluating the if - // condition, etc). + // Output doesn't match the regex. // - path rp (save_regex ()); + // Unless the temporary directory is removed on failure, we save the + // regex to file for troubleshooting regardless of whether we print + // the diagnostics or not. We, however, register it for cleanup in the + // later case (the expression may still succeed, we can be evaluating + // the if condition, etc). + // + optional<path> rp; + if (env.temp_dir_keep) + rp = save_regex (); if (diag) { @@ -635,15 +669,18 @@ namespace build2 d << pr << " " << what << " doesn't match regex"; output_info (d, op); - output_info (d, rp, "", " regex"); + + if (rp) + output_info (d, *rp, "", " regex"); + input_info (d); // Print cached output. // print_file (d, op, ll); } - else - env.clean_special (rp); + else if (rp) + env.clean_special (*rp); // Fall through (to return false). // @@ -856,7 +893,7 @@ namespace build2 for (const auto& cl: c.cleanups) { const path& p (cl.path); - path np (normalize (p, env, ll)); + path np (normalize (p, env.work_dir, ll)); const string& ls (np.leaf ().string ()); bool wc (ls == "*" || ls == "**" || ls == "***"); @@ -973,7 +1010,7 @@ namespace build2 if (ci > 0) p += "-" + to_string (ci); - return normalize (move (p), env, ll); + return normalize (move (p), env.temp_dir, ll); }; // If this is the first pipeline command, then open stdin descriptor @@ -1040,7 +1077,7 @@ namespace build2 } case redirect_type::file: { - isp = normalize (in.file.path, env, ll); + isp = normalize (in.file.path, env.work_dir, ll); open_stdin (); break; @@ -1155,7 +1192,7 @@ namespace build2 // p = r.file.mode == redirect_fmode::compare ? std_path (what) - : normalize (r.file.path, env, ll); + : normalize (r.file.path, env.work_dir, ll); m |= r.file.mode == redirect_fmode::append ? fdopen_mode::at_end @@ -1520,10 +1557,10 @@ namespace build2 if (p.relative ()) { auto program = [&p, &args] (path pp) - { - p = move (pp); - args[0] = p.string ().c_str (); - }; + { + p = move (pp); + args[0] = p.string ().c_str (); + }; if (p.simple ()) { @@ -1636,13 +1673,13 @@ namespace build2 assert (false); } - if (non_empty (esp, ll)) + if (non_empty (esp, ll) && avail_on_failure (esp, env)) d << info << "stderr: " << esp; - if (non_empty (osp, ll)) + if (non_empty (osp, ll) && avail_on_failure (osp, env)) d << info << "stdout: " << osp; - if (non_empty (isp, ll)) + if (non_empty (isp, ll) && avail_on_failure (isp, env)) d << info << "stdin: " << isp; // Print cached stderr. diff --git a/libbuild2/script/script.hxx b/libbuild2/script/script.hxx index 120cd1d..abb2fd7 100644 --- a/libbuild2/script/script.hxx +++ b/libbuild2/script/script.hxx @@ -353,6 +353,7 @@ namespace build2 // an absolute path. // const dir_path& work_dir; + const string& work_dir_name; // Directory name for diagnostics. // If non-empty, then any attempt to remove or move a filesystem entry // outside this directory using an explicit cleanup or the rm/mv @@ -360,11 +361,18 @@ namespace build2 // specified for the builtin. Must be an absolute path, unless is empty. // const dir_path& sandbox_dir; + const string& sandbox_dir_name; // Directory name for diagnostics. - // Directory names for diagnostics. + // Used by the script running machinery to create special files in it. + // Must be an absolute path. // - const string& work_dir_name; - const string& sandbox_dir_name; + const dir_path& temp_dir; + + // The temporary directory will not be removed on the script failure, + // which allows the script running machinery to refer to the special + // files in the diagnostics. + // + const bool temp_dir_keep; // Process streams default redirects. // @@ -379,15 +387,18 @@ namespace build2 const target_triplet& pt, const dir_path& wd, const string& wn, const dir_path& sd, const string& sn, + const dir_path& td, bool tk, redirect&& i = redirect (redirect_type::pass), redirect&& o = redirect (redirect_type::pass), redirect&& e = redirect (redirect_type::pass)) : context (ctx), platform (pt), work_dir (wd), - sandbox_dir (sd), work_dir_name (wn), + sandbox_dir (sd), sandbox_dir_name (sn), + temp_dir (td), + temp_dir_keep (tk), in (move (i)), out (move (o)), err (move (e)) @@ -399,6 +410,7 @@ namespace build2 environment (build2::context& ctx, const target_triplet& pt, const dir_path& wd, const string& wn, + const dir_path& td, bool tk, redirect&& i = redirect (redirect_type::pass), redirect&& o = redirect (redirect_type::pass), redirect&& e = redirect (redirect_type::pass)) @@ -406,6 +418,7 @@ namespace build2 pt, wd, wn, empty_dir_path, empty_string, + td, tk, move (i), move (o), move (e)) { } @@ -425,7 +438,8 @@ namespace build2 // Register cleanup of a special file. Such files are created to // maintain the script running machinery and must be removed first, not - // to interfere with the user-defined wildcard cleanups. + // to interfere with the user-defined wildcard cleanups if the working + // and temporary directories are the same. // void clean_special (path); diff --git a/libbuild2/test/script/script.cxx b/libbuild2/test/script/script.cxx index ed9f2e7..9f8cb0b 100644 --- a/libbuild2/test/script/script.cxx +++ b/libbuild2/test/script/script.cxx @@ -64,6 +64,7 @@ namespace build2 test_tt (), wd_path (), wd_name, p != nullptr ? root.work_dir : wd_path (), sd_name, + wd_path (), true /* temp_dir_keep */, redirect (redirect_type::none), redirect (redirect_type::none), redirect (redirect_type::none)), |