From d6581aa9be74e83cc689bfdaae9aaf2e78287975 Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Tue, 26 May 2020 14:55:40 +0300 Subject: Create build script temporary directory on demand --- libbuild2/build/script/lexer.cxx | 4 +- libbuild2/build/script/parser.cxx | 33 +++++++---- libbuild2/build/script/parser.hxx | 12 ++-- libbuild2/build/script/runner.cxx | 122 +++++++++++++------------------------- libbuild2/build/script/script.cxx | 95 +++++++++++++++++++++++++++-- libbuild2/build/script/script.hxx | 16 ++++- libbuild2/script/parser.hxx | 2 + libbuild2/script/run.cxx | 43 +++++++------- libbuild2/script/script.hxx | 15 ++++- libbuild2/test/script/parser.cxx | 10 +++- libbuild2/test/script/parser.hxx | 12 ++-- libbuild2/test/script/script.cxx | 29 ++++++++- libbuild2/test/script/script.hxx | 11 +++- 13 files changed, 261 insertions(+), 143 deletions(-) diff --git a/libbuild2/build/script/lexer.cxx b/libbuild2/build/script/lexer.cxx index f5a7333..7b8bdd4 100644 --- a/libbuild2/build/script/lexer.cxx +++ b/libbuild2/build/script/lexer.cxx @@ -243,7 +243,7 @@ namespace build2 lexer_mode m (st.mode); // Customized implementation that handles special variable names ($>, - // $<). + // $<, $~). // // @@ TODO: $(<), $(>): feels like this will have to somehow be // handled at the top-level lexer level. Maybe provide a @@ -254,7 +254,7 @@ namespace build2 xchar c (peek ()); - if (c != '>' && c != '<') + if (c != '>' && c != '<' && c != '~') return base_lexer::word (st, sep); get (); diff --git a/libbuild2/build/script/parser.cxx b/libbuild2/build/script/parser.cxx index b170088..648cc7b 100644 --- a/libbuild2/build/script/parser.cxx +++ b/libbuild2/build/script/parser.cxx @@ -16,12 +16,6 @@ namespace build2 { using type = token_type; - static inline bool - special_variable (const string& name) - { - return name == ">" || name == "<"; - } - // // Pre-parse. // @@ -101,11 +95,11 @@ namespace build2 { case line_type::var: { - // Check if we are trying to modify any of the special variables - // ($>, $<). + // Check if we are trying to modify any of the special variables. // if (special_variable (t.value)) - fail (t) << "attempt to set '" << t.value << "' variable"; + fail (t) << "attempt to set '" << t.value << "' special " + << "variable"; // We don't pre-enter variables. // @@ -275,6 +269,9 @@ namespace build2 { const script& s (*script_); + if (s.temp_dir) + environment_->create_temp_dir (); + runner_->enter (*environment_, s.start_loc); // Note that we rely on "small function object" optimization for the @@ -339,6 +336,14 @@ namespace build2 runner_->leave (*environment_, s.end_loc); } + // When add a special variable don't forget to update lexer::word(). + // + bool parser:: + special_variable (const string& n) noexcept + { + return n == ">" || n == "<" || n == "~"; + } + lookup parser:: lookup_variable (name&& qual, string&& name, const location& loc) { @@ -348,9 +353,15 @@ namespace build2 if (pre_parse_) { // Add the variable name skipping special variables and suppressing - // duplicates. + // duplicates. While at it, check if the script temporary directory + // is referenced and set the flag, if that's the case. // - if (!name.empty () && !special_variable (name)) + if (special_variable (name)) + { + if (name == "~") + script_->temp_dir = true; + } + else if (!name.empty ()) { auto& vars (script_->vars); diff --git a/libbuild2/build/script/parser.hxx b/libbuild2/build/script/parser.hxx index 4c16087..27e7f49 100644 --- a/libbuild2/build/script/parser.hxx +++ b/libbuild2/build/script/parser.hxx @@ -58,12 +58,6 @@ namespace build2 command_expr parse_command_line (token&, token_type&); - // Workaround for GCC 4.9 that fails to compile the base class - // protected member function call from a lambda defined in the derived - // class. - // - using build2::parser::apply_value_attributes; - // Execute. Issue diagnostics and throw failed in case of an error. // public: @@ -75,6 +69,12 @@ namespace build2 void exec_script (); + // Helpers. + // + public: + static bool + special_variable (const string&) noexcept; + // Customization hooks. // protected: diff --git a/libbuild2/build/script/runner.cxx b/libbuild2/build/script/runner.cxx index 559a6cd..315a248 100644 --- a/libbuild2/build/script/runner.cxx +++ b/libbuild2/build/script/runner.cxx @@ -3,7 +3,7 @@ #include -#include +#include // try_rmdir() #include #include @@ -17,60 +17,8 @@ namespace build2 namespace script { void default_runner:: - enter (environment& env, const location& ll) + enter (environment&, const location&) { - // 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:: @@ -96,43 +44,53 @@ namespace build2 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. + // Remove the temporary directory, if created. // - env.temp_dir.cancel (); - const dir_path& td (env.temp_dir.path); - try + if (!td.empty ()) { - // Note that the temporary directory must be empty to date. + // 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. // - rmdir_status r (try_rmdir (td)); + env.temp_dir.cancel (); - if (r != rmdir_status::success) + try { - 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); + // Note that the temporary directory must be empty to date. + // + rmdir_status r (try_rmdir (td)); + + if (r != rmdir_status::success) + { + // 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. + // + 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; } - } - catch (const system_error& e) - { - fail (ll) << "unable to remove temporary directory '" << td << "': " - << e; - } - if (verb >= 3) - text << "rmdir " << td; + if (verb >= 3) + text << "rmdir " << td; + } } void default_runner:: diff --git a/libbuild2/build/script/script.cxx b/libbuild2/build/script/script.cxx index d27a30a..e344b59 100644 --- a/libbuild2/build/script/script.cxx +++ b/libbuild2/build/script/script.cxx @@ -3,9 +3,11 @@ #include +#include + #include -#include +#include using namespace std; @@ -32,7 +34,9 @@ namespace build2 target (t), vars (context, false /* global */) { - // Set special variables. + // Set special variables. Note that the $~ variable is set later and + // only if the temporary directory is required for the script + // execution (see create_temp_dir() for details). // { // $> @@ -64,8 +68,76 @@ namespace build2 } void environment:: - set_variable (string&& nm, names&& val, const string& attrs) + create_temp_dir () + { + // 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 is also the reason why we are not using the + // build2 filesystem API that considers the dry-run mode. + // + // Note that the directory auto-removal is active. + // + dir_path& td (temp_dir.path); + + assert (td.empty ()); // Must be called once. + + try + { + td = dir_path::temp_path ("buildscript"); + } + catch (const system_error& e) + { + fail << "unable to obtain temporary directory for buildscript " + << "execution" << e; + } + + mkdir_status r; + + try + { + r = try_mkdir (td); + } + catch (const system_error& e) + { + fail << "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 << "unable to cleanup temporary directory '" << td << "': " + << e; + } + + // Set the $~ special variable. + // + value& v (assign (var_pool.insert ("~"))); + v = td; + + if (verb >= 3) + text << "mkdir " << td; + } + + void environment:: + set_variable (string&& nm, + names&& val, + const string& attrs, + const location& ll) { + // Check if we are trying to modify any of the special variables. + // + if (parser::special_variable (nm)) + fail (ll) << "attempt to set '" << nm << "' special variable"; + // Set the variable value and attributes. // const variable& var (var_pool.insert (move (nm))); @@ -80,7 +152,22 @@ namespace build2 lhs.assign (move (val), &var); else { - build2::script::parser p (context); + // If there is an error in the attributes string, our diagnostics + // will look like this: + // + // :1:1 error: unknown value attribute x + // buildfile:10:1 info: while parsing attributes '[x]' + // + // Note that the attributes parsing error is the only reason for a + // failure. + // + auto df = make_diag_frame ( + [attrs, &ll](const diag_record& dr) + { + dr << info (ll) << "while parsing attributes '" << attrs << "'"; + }); + + parser p (context); p.apply_value_attributes (&var, lhs, value (move (val)), diff --git a/libbuild2/build/script/script.hxx b/libbuild2/build/script/script.hxx index 2139f28..7d27840 100644 --- a/libbuild2/build/script/script.hxx +++ b/libbuild2/build/script/script.hxx @@ -41,7 +41,7 @@ namespace build2 // Note that the variables are not pre-entered into a pool during the // parsing phase, so the line variable pointers are NULL. // - build2::script::lines lines; + build2::script::lines lines; // Referenced ordinary (non-special) variables. // @@ -55,6 +55,10 @@ namespace build2 // small_vector vars; + // True if script references the $~ special variable. + // + bool temp_dir = false; + location start_loc; location end_loc; }; @@ -101,7 +105,15 @@ namespace build2 auto_rmdir temp_dir; virtual void - set_variable (string&& name, names&&, const string& attrs) override; + set_variable (string&& name, + names&&, + const string& attrs, + const location&) override; + + // Create the temporary directory and set the $~ variable. + // + virtual void + create_temp_dir () override; // Variables. // diff --git a/libbuild2/script/parser.hxx b/libbuild2/script/parser.hxx index d47f88e..a63ecde 100644 --- a/libbuild2/script/parser.hxx +++ b/libbuild2/script/parser.hxx @@ -40,6 +40,8 @@ namespace build2 token_type assign_kind, const path_name&); // For diagnostics. + using build2::parser::apply_value_attributes; + // Commonly used parsing functions. Issue diagnostics and throw failed // in case of an error. // diff --git a/libbuild2/script/run.cxx b/libbuild2/script/run.cxx index a48421c..64286fd 100644 --- a/libbuild2/script/run.cxx +++ b/libbuild2/script/run.cxx @@ -53,6 +53,18 @@ namespace build2 return r; } + // Return the environment temporary directory, creating it if it doesn't + // exist. + // + static inline const dir_path& + temp_dir (environment& env) + { + if (env.temp_dir.empty ()) + env.create_temp_dir (); + + return env.temp_dir; + } + // Normalize a path. Also make the relative path absolute using the // specified directory unless it is already absolute. // @@ -207,13 +219,16 @@ 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. + // Return true if the script temporary directory is not created yet (and + // so cannot contain any path), a path is not under the 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); + return env.temp_dir.empty () || + env.temp_dir_keep || + !p.sub (env.temp_dir); } // Check if the script command output matches the expected result @@ -849,26 +864,10 @@ namespace build2 cin.close (); - // If there is an error in the attributes string, our diagnostics - // will look like this: - // - // :1:1 error: unknown value attribute x - // script:10:1 info: while parsing attributes '[x]' - // - // Note that the attributes parsing error is the only reason for a - // failure. - // - auto df = make_diag_frame ( - [ats, &ll](const diag_record& dr) - { - assert (ats != nullptr); - - dr << info (ll) << "while parsing attributes '" << *ats << "'"; - }); - env.set_variable (move (vname), move (ns), - ats != nullptr ? *ats : empty_string); + ats != nullptr ? *ats : empty_string, + ll); } catch (const io_error& e) { @@ -1042,7 +1041,7 @@ namespace build2 if (ci > 0) p += "-" + to_string (ci); - return normalize (move (p), env.temp_dir, ll); + return normalize (move (p), temp_dir (env), ll); }; // If this is the first pipeline command, then open stdin descriptor diff --git a/libbuild2/script/script.hxx b/libbuild2/script/script.hxx index b887df6..b80c44a 100644 --- a/libbuild2/script/script.hxx +++ b/libbuild2/script/script.hxx @@ -373,7 +373,9 @@ namespace build2 const dir_name_view sandbox_dir; // Used by the script running machinery to create special files in it. - // Must be an absolute path. + // Must be an absolute path, unless empty. Can be empty until the + // create_temp_dir() function call, which can be used for the create-on- + // demand strategy implementation. // const dir_path& temp_dir; @@ -454,10 +456,17 @@ namespace build2 public: // Set variable value with optional (non-empty) attributes. // - // Note: see also parser::lookup_variable(). + virtual void + set_variable (string&& name, + names&&, + const string& attrs, + const location&) = 0; + + // Create the temporary directory and set the temp_dir reference target + // to its path. Must only be called if temp_dir is empty. // virtual void - set_variable (string&& name, names&&, const string& attrs) = 0; + create_temp_dir () = 0; public: virtual diff --git a/libbuild2/test/script/parser.cxx b/libbuild2/test/script/parser.cxx index c206e0a..f663c11 100644 --- a/libbuild2/test/script/parser.cxx +++ b/libbuild2/test/script/parser.cxx @@ -389,7 +389,7 @@ namespace build2 // string& n (t.value); - if (n == "*" || n == "~" || n == "@" || digit (n)) + if (special_variable (n)) fail (t) << "attempt to set '" << n << "' variable directly"; // Pre-enter the variables now while we are executing serially. @@ -1598,6 +1598,14 @@ namespace build2 // The rest. // + // When add a special variable don't forget to update lexer::word(). + // + bool parser:: + special_variable (const string& n) noexcept + { + return n == "*" || n == "~" || n == "@" || digit (n); + } + lookup parser:: lookup_variable (name&& qual, string&& name, const location& loc) { diff --git a/libbuild2/test/script/parser.hxx b/libbuild2/test/script/parser.hxx index 19d4f76..aa64943 100644 --- a/libbuild2/test/script/parser.hxx +++ b/libbuild2/test/script/parser.hxx @@ -91,12 +91,6 @@ namespace build2 command_expr parse_command_line (token&, token_type&); - // Workaround for GCC 4.9 that fails to compile the base class - // protected member function call from a lambda defined in the derived - // class. - // - using build2::parser::apply_value_attributes; - // Execute. Issue diagnostics and throw failed in case of an error. // public: @@ -110,6 +104,12 @@ namespace build2 void exec_scope_body (); + // Helpers. + // + public: + static bool + special_variable (const string&) noexcept; + // Customization hooks. // protected: diff --git a/libbuild2/test/script/script.cxx b/libbuild2/test/script/script.cxx index b56da1b..34d4723 100644 --- a/libbuild2/test/script/script.cxx +++ b/libbuild2/test/script/script.cxx @@ -8,7 +8,7 @@ #include #include -#include // parser::apply_value_attributes() +#include using namespace std; @@ -96,8 +96,16 @@ namespace build2 } void scope:: - set_variable (string&& nm, names&& val, const string& attrs) + set_variable (string&& nm, + names&& val, + const string& attrs, + const location& ll) { + // Check if we are trying to modify any of the special variables. + // + if (parser::special_variable (nm)) + fail (ll) << "attempt to set '" << nm << "' variable directly"; + // Set the variable value and attributes. Note that we need to aquire // unique lock before potentially changing the script's variable // pool. The obtained variable reference can safelly be used with no @@ -118,7 +126,22 @@ namespace build2 lhs.assign (move (val), &var); else { - build2::script::parser p (context); + // If there is an error in the attributes string, our diagnostics + // will look like this: + // + // :1:1 error: unknown value attribute x + // testscript:10:1 info: while parsing attributes '[x]' + // + // Note that the attributes parsing error is the only reason for a + // failure. + // + auto df = make_diag_frame ( + [attrs, &ll](const diag_record& dr) + { + dr << info (ll) << "while parsing attributes '" << attrs << "'"; + }); + + parser p (context); p.apply_value_attributes (&var, lhs, value (move (val)), diff --git a/libbuild2/test/script/script.hxx b/libbuild2/test/script/script.hxx index 7bdb2ac..6356501 100644 --- a/libbuild2/test/script/script.hxx +++ b/libbuild2/test/script/script.hxx @@ -91,8 +91,17 @@ namespace build2 scope_state state = scope_state::unknown; + void + set_variable (string&& name, + names&&, + const string& attrs, + const location&) override; + + // Noop since the temporary directory is a working directory and so + // is created before the scope commands execution. + // virtual void - set_variable (string&& name, names&&, const string& attrs) override; + create_temp_dir () override {assert (false);}; // Variables. // -- cgit v1.1