aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKaren Arutyunov <karen@codesynthesis.com>2020-05-21 22:47:10 +0300
committerBoris Kolpackov <boris@codesynthesis.com>2020-05-27 08:38:57 +0200
commitb27f36b7af5186ad66fd1afa6e7fdc742f2aa1bd (patch)
tree194e275a20eb0f3e021f71f5f4389c8a8ca8d4fa
parent84cc0fc42c6b86eb09b06c7f59a0beb94397a38a (diff)
Make build script to create special files in temporary directory
-rw-r--r--libbuild2/build/script/runner.cxx97
-rw-r--r--libbuild2/build/script/script.cxx4
-rw-r--r--libbuild2/build/script/script.hxx14
-rw-r--r--libbuild2/script/run.cxx107
-rw-r--r--libbuild2/script/script.hxx24
-rw-r--r--libbuild2/test/script/script.cxx1
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)),