diff options
Diffstat (limited to 'libbuild2')
47 files changed, 861 insertions, 352 deletions
diff --git a/libbuild2/adhoc-rule-buildscript.cxx b/libbuild2/adhoc-rule-buildscript.cxx index b125ac5..e3ed0a4 100644 --- a/libbuild2/adhoc-rule-buildscript.cxx +++ b/libbuild2/adhoc-rule-buildscript.cxx @@ -1207,9 +1207,11 @@ namespace build2 } // Note that in case of dry run we will have an incomplete (but valid) - // database which will be updated on the next non-dry run. + // database which will be updated on the next non-dry run. Except that + // we may still end up performing a non-dry-run update due to update + // during match or load. // - if (!update || ctx.dry_run_option) + if (!update /*|| ctx.dry_run_option*/) dd.close (false /* mtime_check */); else mdb->dd = dd.close_to_reopen (); @@ -1246,8 +1248,24 @@ namespace build2 md->deferred_failure); } - if (update && dd.reading () && !ctx.dry_run_option) - dd.touch = timestamp_unknown; + // Update depdb timestamp if nothing changed. Failed that, we will keep + // re-validating the information store in depdb (see similar logic in + // cc::compile_rule). + // + if (update && dd.reading ()) + { + // What will happen if dry_run_option is true but we still end up + // performing a non-dry-run update due to update during match or + // load? In this case the target will become up-to-date and we will + // keep re-validating the cache until the depdb will get touched due + // to other reasons, which would be bad. So it feels like the least + // bad option is to keep re-touching the database on dry-run. + // +#if 0 + if (!ctx.dry_run_option) +#endif + dd.touch = timestamp_unknown; + } dd.close (false /* mtime_check */); diff --git a/libbuild2/algorithm.cxx b/libbuild2/algorithm.cxx index d2d1eb6..d1cd474 100644 --- a/libbuild2/algorithm.cxx +++ b/libbuild2/algorithm.cxx @@ -861,7 +861,7 @@ namespace build2 // bool ambig (false); - diag_record dr; + maybe_diag_record dr; for (++i; i != rs.second; ++i) { const rule_match* r1 (&*i); diff --git a/libbuild2/bash/rule.cxx b/libbuild2/bash/rule.cxx index 6e96b34..be7b2ff 100644 --- a/libbuild2/bash/rule.cxx +++ b/libbuild2/bash/rule.cxx @@ -473,7 +473,7 @@ namespace build2 { if (!*md.for_install) fail << "incompatible " << t << " build" << - info << "target already built not for install"; + info << "target already updated but not for install"; } else md.for_install = true; diff --git a/libbuild2/build/script/parser.cxx b/libbuild2/build/script/parser.cxx index 3ecf23d..95b835e 100644 --- a/libbuild2/build/script/parser.cxx +++ b/libbuild2/build/script/parser.cxx @@ -106,7 +106,7 @@ namespace build2 // name from the script operation first. // { - diag_record dr; + maybe_diag_record dr; if (!diag_name_ && diag_preamble_.empty ()) { @@ -131,7 +131,7 @@ namespace build2 << "'"; } - if (!dr.empty ()) + if (dr) { dr << info << "consider specifying it explicitly with the 'diag' " << "recipe attribute"; @@ -1529,7 +1529,7 @@ namespace build2 { // Copy the tokens and start playing. // - replay_data (replay_tokens (dl.tokens)); + replay_data (dl.tokens); token t; build2::script::token_type tt; @@ -2501,10 +2501,14 @@ namespace build2 // auto fail = [this, what, &ctx] (const auto& f) -> optional<bool> { + // Note that this test will give a false negative if this target + // ends up being updated during load or match. At least it's + // conservative. + // bool df (!ctx.match_only && !ctx.dry_run_option); - diag_record dr; - dr << error << what << ' ' << f << " not found and no rule to " + diag_record dr (error); + dr << what << ' ' << f << " not found and no rule to " << "generate it"; if (df) @@ -2822,7 +2826,7 @@ namespace build2 cmd, nullptr /* iteration_index */, li, ll, - cf, false /* last_cmd */); + cf); iss.exceptions (istream::badbit); } diff --git a/libbuild2/build/script/runner.cxx b/libbuild2/build/script/runner.cxx index 5d9764b..fc0fc05 100644 --- a/libbuild2/build/script/runner.cxx +++ b/libbuild2/build/script/runner.cxx @@ -143,7 +143,13 @@ namespace build2 (cf != nullptr && p.recall.string () == "for")); }) != expr.end ()) - build2::script::run (env, expr, ii, li, ll, cf); + { + build2::script::run (env, + expr, + ii, li, + ll, + cf, (cf != nullptr) /* replace_last_cmd */); + } else if (verb >= 2) text << expr; } diff --git a/libbuild2/build/script/script.cxx b/libbuild2/build/script/script.cxx index 0d96cc3..fba38a8 100644 --- a/libbuild2/build/script/script.cxx +++ b/libbuild2/build/script/script.cxx @@ -235,6 +235,12 @@ namespace build2 } } + void environment:: + sleep (const duration& d) + { + context.sched->sleep (d); + } + lookup environment:: lookup (const variable& var) const { diff --git a/libbuild2/build/script/script.hxx b/libbuild2/build/script/script.hxx index 08f1bf4..19f6d0b 100644 --- a/libbuild2/build/script/script.hxx +++ b/libbuild2/build/script/script.hxx @@ -189,6 +189,11 @@ namespace build2 virtual void create_temp_dir () override; + // Call the scheduler's sleep() function. + // + virtual void + sleep (const duration&) override; + // Variables. // public: diff --git a/libbuild2/cc/common.cxx b/libbuild2/cc/common.cxx index 0589c30..dfc78e7 100644 --- a/libbuild2/cc/common.cxx +++ b/libbuild2/cc/common.cxx @@ -950,22 +950,35 @@ namespace build2 const prerequisite_key& p, bool exist) const { + assert (p.scope != nullptr && (!exist || act)); + tracer trace (x, "search_library"); - assert (p.scope != nullptr && (!exist || act)); + context& ctx (p.scope->ctx); + const scope& rs (*p.scope->root_scope ()); - // Import phase 1 may pass us a user-specified path with a relative - // directory (same semantics as in lookup_import() below). + // Note: since we are searching for a (presumably) installed library, + // utility libraries do not apply. // - { - const dir_path& d (*p.tk.dir); + bool l (p.is_a<lib> ()); + const string& name (*p.tk.name); + const optional<string>& ext (l ? nullopt : p.tk.ext); // Only liba/libs. - if (!d.empty ()) - fail << "relative path in imported " << p; - } + // Import phase 1 may pass us a path specified by the user with + // config.import.<proj>.<name>.<type>. The possible cases are: + // + // 1. Empty or relative directory for liba{} and libs{} (absolute would + // be taken care of by phase 1 since these tragets are path-based). + // + // 2. Empty, relative, or absolute directory for lib{} (since it's not a + // path-based target). + // + const dir_path& dir (*p.tk.dir); - context& ctx (p.scope->ctx); - const scope& rs (*p.scope->root_scope ()); + // Same semantics as in lookup_import() below. + // + if (!dir.empty () && dir.relative ()) + fail << "relative path in imported " << p; // Here is the problem: we may be building for two different toolchains // simultaneously that use the same installed library. But our search is @@ -977,17 +990,6 @@ namespace build2 ? cpath : cast<process_path> (rs["bin.ld.path"])); - // @@ This is hairy enough to warrant a separate implementation for - // Windows. - - // Note: since we are searching for a (presumably) installed library, - // utility libraries do not apply. - // - bool l (p.is_a<lib> ()); - const optional<string>& ext (l ? nullopt : p.tk.ext); // Only liba/libs. - - const string& name (*p.tk.name); - // If this prerequisite is project-qualified do an ad hoc check for // config.import.<proj>.<name>.{liba,libs} which can be used to specify // different path (see import_search() for background). Note that for @@ -1015,7 +1017,10 @@ namespace build2 // auto lookup_import = [&rs, &act, - &name, + namev = + (p.proj + ? sanitize_identifier (name) + : string ()), projv = (p.proj ? p.proj->variable () @@ -1023,7 +1028,9 @@ namespace build2 { if (!projv.empty ()) { - string varn ("config.import." + projv + '.' + name + '.' + tt); + // Note: we know tt is liba or libs and need not to be sanitized. + // + string varn ("config.import." + projv + '.' + namev + '.' + tt); if (config::specified_config (rs, varn, true /* exact */)) { @@ -1106,14 +1113,16 @@ namespace build2 // const char* e (""); + an = dir; // Empty or absolute. + if (tsys == "win32-msvc") { - an = path (name); + an /= path (name); e = "lib"; } else { - an = path ("lib" + name); + an /= path ("lib" + name); e = "a"; } @@ -1142,14 +1151,16 @@ namespace build2 { const char* e (""); + sn = dir; + if (tsys == "win32-msvc") { - sn = path (name); + sn /= path (name); e = "dll.lib"; } else { - sn = path ("lib" + name); + sn /= path ("lib" + name); if (tsys == "darwin") e = "dylib"; else if (tsys == "mingw32") e = "dll.a"; // See search code below. @@ -1556,8 +1567,7 @@ namespace build2 // string d ("-DLIB"); - d += sanitize_identifier ( - ucase (const_cast<const string&> (t.name))); + d += sanitize_identifier (ucase (t.name)); d += '_'; d += suffix; diff --git a/libbuild2/cc/compile-rule.cxx b/libbuild2/cc/compile-rule.cxx index 29a26b5..53d38ac 100644 --- a/libbuild2/cc/compile-rule.cxx +++ b/libbuild2/cc/compile-rule.cxx @@ -1544,8 +1544,20 @@ namespace build2 // to keep re-validating the file on every subsequent dry-run as well // on the real run). // - if (u && dd.reading () && !ctx.dry_run_option) - dd.touch = timestamp_unknown; + if (u && dd.reading ()) + { + // What will happen if dry_run_option is true but we still end up + // performing a non-dry-run update due to update during match or + // load? In this case the target will become up-to-date and we will + // keep re-validating the cache until the depdb will get touched due + // to other reasons, which would be bad. So it feels like the least + // bad option is to keep re-touching the database on dry-run. + // +#if 0 + if (!ctx.dry_run_option) +#endif + dd.touch = timestamp_unknown; + } dd.close (false /* mtime_check */); md.dd = move (dd.path); @@ -2338,9 +2350,9 @@ namespace build2 if (verb > 2) { - diag_record dr; - dr << error << "header " << f << " not found and no " - << "rule to generate it"; + diag_record dr (error); + dr << "header " << f << " not found and no rule to " + << "generate it"; if (verb < 4) dr << info << "re-run with --verbose=4 for more information"; @@ -2913,8 +2925,8 @@ namespace build2 if (verb > 2) { - diag_record dr; - dr << error << "header '" << f << "' not found"; + diag_record dr (error); + dr << "header '" << f << "' not found"; if (verb < 4) dr << info << "re-run with --verbose=4 for more information"; @@ -4042,11 +4054,14 @@ namespace build2 // auto fail = [&ctx] (const auto& h) -> optional<bool> { + // Note that this test will give a false negative if this target + // ends up being updated during load or match. At least it's + // conservative. + // bool df (!ctx.match_only && !ctx.dry_run_option); - diag_record dr; - dr << error << "header " << h << " not found and no rule to " - << "generate it"; + diag_record dr (error); + dr << "header " << h << " not found and no rule to generate it"; if (df) dr << info << "failure deferred to compiler diagnostics"; @@ -4104,7 +4119,6 @@ namespace build2 this] (path hp, path bp, timestamp mt) -> optional<bool> { context& ctx (t.ctx); - bool df (!ctx.match_only && !ctx.dry_run_option); const file* ht ( enter_header (a, bs, t, li, @@ -4113,9 +4127,14 @@ namespace build2 if (ht == nullptr) // hp is still valid. { - diag_record dr; - dr << error << "header " << hp << " not found and no rule to " - << "generate it"; + // Note that this test will give a false negative if this target + // ends up being updated during load or match. At least it's + // conservative. + // + bool df (!ctx.match_only && !ctx.dry_run_option); + + diag_record dr (error); + dr << "header " << hp << " not found and no rule to generate it"; if (df) dr << info << "failure deferred to compiler diagnostics"; @@ -4964,7 +4983,7 @@ namespace build2 if (pr.wait ()) { { - diag_record dr; + maybe_diag_record dr; if (bad_error) dr << fail << "expected error exit status from " @@ -5072,7 +5091,7 @@ namespace build2 // preprocessed source files). // { - diag_record dr; + maybe_diag_record dr; if (force_gen_skip && *force_gen_skip == skip_count) { dr << @@ -5080,11 +5099,11 @@ namespace build2 info << "run the following two commands to investigate"; dr << info; - print_process (dr, args.data ()); // No pipes. + print_process (*dr, args.data ()); // No pipes. init_args ((gen = true)); dr << info << ""; - print_process (dr, args.data ()); // No pipes. + print_process (*dr, args.data ()); // No pipes. } if (dbuf.is_open ()) diff --git a/libbuild2/cc/functions.cxx b/libbuild2/cc/functions.cxx index 9d408af..0adcf5f 100644 --- a/libbuild2/cc/functions.cxx +++ b/libbuild2/cc/functions.cxx @@ -76,7 +76,9 @@ namespace build2 for (auto i (ts_ns.begin ()); i != ts_ns.end (); ++i) { name& n (*i), o; - const target& t (to_target (*bs, move (n), move (n.pair ? *++i : o))); + const target& t (to_target (*bs, + move (n), move (n.pair ? *++i : o), + true /* in_recipe */)); if (!t.matched (a)) fail << t << " is not matched" << @@ -193,7 +195,9 @@ namespace build2 for (auto i (ts_ns.begin ()); i != ts_ns.end (); ++i) { name& n (*i), o; - const target& t (to_target (*bs, move (n), move (n.pair ? *++i : o))); + const target& t (to_target (*bs, + move (n), move (n.pair ? *++i : o), + true /* in_recipe */)); bool la (false); if (li diff --git a/libbuild2/cc/init.cxx b/libbuild2/cc/init.cxx index d691bc5..64c70f8 100644 --- a/libbuild2/cc/init.cxx +++ b/libbuild2/cc/init.cxx @@ -764,7 +764,7 @@ namespace build2 perform_update_id, context::operation_callback {&compiledb_pre, &compiledb_post}); - if (ctx.load_generation > 1) + if (!ctx.phase_mutex.unlocked ()) // Interrupting load. { action a (ctx.current_action ()); diff --git a/libbuild2/cc/install-rule.cxx b/libbuild2/cc/install-rule.cxx index 46764a6..c4f924a 100644 --- a/libbuild2/cc/install-rule.cxx +++ b/libbuild2/cc/install-rule.cxx @@ -281,7 +281,7 @@ namespace build2 // if (!*md.for_install) fail << "incompatible " << t << " build" << - info << "target already built not for install"; + info << "target already updated but not for install"; } else md.for_install = true; diff --git a/libbuild2/cc/link-rule.cxx b/libbuild2/cc/link-rule.cxx index a669f37..d43e7e8 100644 --- a/libbuild2/cc/link-rule.cxx +++ b/libbuild2/cc/link-rule.cxx @@ -2598,8 +2598,8 @@ namespace build2 // if (*md->for_install != *d.for_install) fail << "incompatible " << *l << " build" << - info << "library is built " << (*md->for_install ? "" : "not ") - << "for install"; + info << "target is already updated but " + << (*md->for_install ? "" : "not ") << "for install"; } auto newer = [&d, l] () diff --git a/libbuild2/cc/msvc.cxx b/libbuild2/cc/msvc.cxx index 416df36..66223b5 100644 --- a/libbuild2/cc/msvc.cxx +++ b/libbuild2/cc/msvc.cxx @@ -511,8 +511,8 @@ namespace build2 if (!run_finish_code (args, pr, s, 2 /* verbosity */) || io) { - diag_record dr; - dr << warn << "unable to detect " << l << " library type, ignoring" << + diag_record dr (warn); + dr << "unable to detect " << l << " library type, ignoring" << info << "run the following command to investigate" << info; print_process (dr, args); return otype::e; diff --git a/libbuild2/cc/pkgconfig.cxx b/libbuild2/cc/pkgconfig.cxx index 79a38ea..d4be03b 100644 --- a/libbuild2/cc/pkgconfig.cxx +++ b/libbuild2/cc/pkgconfig.cxx @@ -693,6 +693,7 @@ namespace build2 cmp ("msxml", 5) || // msxml* cmp ("netapi32") || cmp ("normaliz") || + cmp ("ntdll") || cmp ("odbc32") || cmp ("ole32") || cmp ("oleaut32") || diff --git a/libbuild2/config/operation.cxx b/libbuild2/config/operation.cxx index 6e7ef18..5c06f3c 100644 --- a/libbuild2/config/operation.cxx +++ b/libbuild2/config/operation.cxx @@ -302,8 +302,8 @@ namespace build2 continue; } - diag_record dr; - dr << warn (on) << "saving no longer used variable " << *var; + diag_record dr (warn (on)); + dr << "saving no longer used variable " << *var; info_import (dr, var->name); if (verb >= 2) info_value (dr, v); @@ -313,8 +313,8 @@ namespace build2 { if (r.second) // warn { - diag_record dr; - dr << warn (on) << "dropping no longer used variable " << *var; + diag_record dr (warn (on)); + dr << "dropping no longer used variable " << *var; info_import (dr, var->name); info_value (dr, v); } @@ -479,9 +479,8 @@ namespace build2 // if (checked && org == ovr) { - diag_record dr; - dr << warn (on) << "saving previously inherited variable " - << var; + diag_record dr (warn (on)); + dr << "saving previously inherited variable " << var; dr << info << "because project " << *r << " no longer uses " << "it in its configuration"; @@ -1018,8 +1017,20 @@ namespace build2 if (oif->operation_pre != nullptr) oif->operation_pre (ctx, {}, true /* inner */, location ()); + // Use perform_match() instead of direct match_sync() to handle + // posthoc targets (similar to dist). + // +#if 0 phase_lock pl (ctx, run_phase::match); match_sync (action (configure_id, id), t); +#else + action a (configure_id, id); + action_targets ts {&t}; + perform_match ({}, a, ts, + 1 /* diag (failures only) */, + false /* progress */); + perform_post_operation_callbacks (ctx, a, ts, false /*failed*/); +#endif if (oif->operation_post != nullptr) oif->operation_post (ctx, {}, true /* inner */); diff --git a/libbuild2/context.cxx b/libbuild2/context.cxx index 6e4fd6f..31d218b 100644 --- a/libbuild2/context.cxx +++ b/libbuild2/context.cxx @@ -1061,6 +1061,13 @@ namespace build2 return r ? optional<bool> (s) : nullopt; } + bool run_phase_mutex:: + unlocked () const + { + mlock l (m_); + return lc_ == 0 && mc_ == 0 && ec_ == 0; + } + // C++17 deprecated uncaught_exception() so use uncaught_exceptions() if // available. // diff --git a/libbuild2/context.hxx b/libbuild2/context.hxx index 81ac970..12b11d5 100644 --- a/libbuild2/context.hxx +++ b/libbuild2/context.hxx @@ -49,6 +49,12 @@ namespace build2 optional<bool> relock (run_phase unlock, run_phase lock); + // Return true if the mutex is unlocked, meaning we are in the initial + // load phase. + // + bool + unlocked () const; + // Statistics. // public: @@ -78,7 +84,7 @@ namespace build2 // context& ctx_; - mutex m_; + mutable mutex m_; bool fail_; @@ -209,8 +215,12 @@ namespace build2 // hierarchy would "see" a new value from the newly inserted scope. // // The special load_generation value 0 indicates initialization before - // anything has been loaded. Currently, it is changed to 1 at the end - // of the context constructor. + // anything has been loaded. Currently, it is changed to 1 at the end of + // the context constructor. Note also that subsequent operations in a + // batch may trigger loading of additional buildfiles, in fact, entire new + // projects. As a result, load_generation is also incremented after each + // operation in a batch. If you need to detect the initial load in each + // operation, check that phase_mutex is unlocked. // // Note must come (and thus initialized) before the data_ member. // @@ -230,6 +240,10 @@ namespace build2 // Match only flag/level (see --{load,match}-only but also dist). // + // See also dry_run, which is in some sense a weaker version of match- + // only: the target is executed but nothing is actually being done (unless + // executed during match or load, that is). + // optional<match_only_level> match_only; // Skip booting external modules flag (see --no-external-modules). @@ -263,11 +277,22 @@ namespace build2 // // Note also that sometimes it makes sense to do a bit more than // absolutely necessary or to discard information in order to keep the - // rule logic sane. And some rules may choose to ignore this flag + // rule logic sane. And some rules may choose to ignore this flag // altogether. In this case, however, the rule should be careful not to // rely on functions (notably from filesystem) that respect this flag in // order not to end up with a job half done. // + // Finally, sometimes you may need to know during match whether there will + // be a non-dry-run execute and use the dry_run_option for that. This can + // be problematic because even when dry_run_option is true, the target may + // end up being executed in the non-dry-run mode during load or match. As + // a result, any logic that is based on dry_run_option should be capable + // of functioning correctly in the non-dry-run execute. + // + // See also match_only, which is in some sense a stronger version of + // dry-run: the target is not executed at all, again, unless during match + // or load. + // bool dry_run = false; bool dry_run_option; @@ -802,6 +827,9 @@ namespace build2 // Set current meta-operation and operation. // + // Remember to also increment load_generation for subsequent operations in + // a batch if additional buildfiles are loaded between them. + // // Note that the context instance is not to be re-used between different // meta-operations. // diff --git a/libbuild2/diagnostics.cxx b/libbuild2/diagnostics.cxx index c795e44..091d6e5 100644 --- a/libbuild2/diagnostics.cxx +++ b/libbuild2/diagnostics.cxx @@ -559,7 +559,10 @@ namespace build2 const process_env& pe, const char* const* args, size_t n) { if (pe.env ()) - dr << pe << ' '; + { + to_stream (dr.os, pe, process_env_format::cwd | process_env_format::vars); + dr << ' '; + } dr << butl::process_args {args, n}; } @@ -836,7 +839,7 @@ namespace build2 // is inseparable from any buffered diagnostics. So we prepare the record // first and then write both while holding the diagnostics stream lock. // - diag_record dr; + maybe_diag_record dr; if (!pe) { // Note: see similar code in run_finish_impl(). @@ -852,7 +855,7 @@ namespace build2 if (verb >= 1 && verb <= v) { dr << info << "command line: "; - print_process (dr, args); + print_process (*dr, args); } } } @@ -861,7 +864,7 @@ namespace build2 } void diag_buffer:: - close (diag_record&& dr) + close (maybe_diag_record&& dr) { assert (state_ != state::closed); @@ -901,7 +904,7 @@ namespace build2 args0 = nullptr; state_ = state::closed; - if (!buf.empty () || !dr.empty ()) + if (!buf.empty () || dr) { diag_stream_lock l; @@ -911,14 +914,14 @@ namespace build2 buf.clear (); } - if (!dr.empty ()) - dr.flush ([] (const butl::diag_record& r) - { - // Similar to default_writer(). - // - *diag_stream << r.os.str () << '\n'; - diag_stream->flush (); - }); + if (dr) + (*dr).flush ([] (const butl::diag_record& r) + { + // Similar to default_writer(). + // + *diag_stream << r.os.str () << '\n'; + diag_stream->flush (); + }); else diag_stream->flush (); } @@ -927,11 +930,15 @@ namespace build2 // diag_do(), etc. // string - diag_do (context& ctx, const action&) + diag_do (context& ctx, const action& a) { const meta_operation_info& m (*ctx.current_mif); const operation_info& io (*ctx.current_inner_oif); - const operation_info* oo (ctx.current_outer_oif); + const operation_info* oo (a.outer () ? ctx.current_outer_oif : nullptr); + + assert (m.id == a.meta_operation () && + io.id == a.operation () && + (a.inner () || (oo != nullptr && oo->id == a.outer_operation ()))); string r; @@ -968,11 +975,15 @@ namespace build2 } string - diag_doing (context& ctx, const action&) + diag_doing (context& ctx, const action& a) { const meta_operation_info& m (*ctx.current_mif); const operation_info& io (*ctx.current_inner_oif); - const operation_info* oo (ctx.current_outer_oif); + const operation_info* oo (a.outer () ? ctx.current_outer_oif : nullptr); + + assert (m.id == a.meta_operation () && + io.id == a.operation () && + (a.inner () || (oo != nullptr && oo->id == a.outer_operation ()))); string r; @@ -1005,11 +1016,15 @@ namespace build2 } string - diag_did (context& ctx, const action&) + diag_did (context& ctx, const action& a) { const meta_operation_info& m (*ctx.current_mif); const operation_info& io (*ctx.current_inner_oif); - const operation_info* oo (ctx.current_outer_oif); + const operation_info* oo (a.outer () ? ctx.current_outer_oif : nullptr); + + assert (m.id == a.meta_operation () && + io.id == a.operation () && + (a.inner () || (oo != nullptr && oo->id == a.outer_operation ()))); string r; @@ -1046,11 +1061,17 @@ namespace build2 } void - diag_done (ostream& os, const action&, const target& t) + diag_done (ostream& os, const action& a, const target& t) { - const meta_operation_info& m (*t.ctx.current_mif); - const operation_info& io (*t.ctx.current_inner_oif); - const operation_info* oo (t.ctx.current_outer_oif); + context& ctx (t.ctx); + + const meta_operation_info& m (*ctx.current_mif); + const operation_info& io (*ctx.current_inner_oif); + const operation_info* oo (a.outer () ? ctx.current_outer_oif : nullptr); + + assert (m.id == a.meta_operation () && + io.id == a.operation () && + (a.inner () || (oo != nullptr && oo->id == a.outer_operation ()))); // perform(update(x)) -> "x is up to date" // configure(update(x)) -> "updating x is configured" diff --git a/libbuild2/diagnostics.hxx b/libbuild2/diagnostics.hxx index ef41f22..3452ad8 100644 --- a/libbuild2/diagnostics.hxx +++ b/libbuild2/diagnostics.hxx @@ -4,6 +4,8 @@ #ifndef LIBBUILD2_DIAGNOSTICS_HXX #define LIBBUILD2_DIAGNOSTICS_HXX +#include <cstddef> // max_align_t + #include <libbutl/diagnostics.hxx> #include <libbuild2/types.hxx> @@ -447,15 +449,110 @@ namespace build2 return *this; } - diag_record () = default; + // Use maybe_diag_record below for maybe-used records. + // + diag_record () = delete; + + // Create empty record that will be used. + // + explicit + diag_record (nullptr_t): butl::diag_record () {} template <typename B> explicit - diag_record (const diag_prologue<B>& p): diag_record () { *this << p;} + diag_record (const diag_prologue<B>& p) + : butl::diag_record () + { + *this << p; + } template <typename B> explicit - diag_record (const diag_mark<B>& m): diag_record () { *this << m;} + diag_record (const diag_mark<B>& m) + : butl::diag_record () + { + *this << m; + } + }; + + // A diag_record wrapper similar to std::optional that allows creating an + // uninitialized record that may or may not be used. The initializers are + // the diagnostics marks, for example: + // + // maybe_diag_record dr; // Uninitialized. + // dr << fail << "bad"; // Initialized. + // if (dr) + // *dr << "extra info"; + // + // The reason we need this is because the std::ostringstream member of + // butl::diag_record is quite expensive to construct but to never use. + // + struct maybe_diag_record + { + maybe_diag_record (): empty_ (true) {} + + explicit operator bool () const + { + return !empty_; + } + + diag_record& + operator* () + { + return *reinterpret_cast<diag_record*> (data_); + } + + template <typename B> + diag_record& + operator<< (const diag_mark<B>& m) + { + diag_record* r; + + if (empty_) + { + r = new (&data_) diag_record (m); + empty_ = false; + } + else + *(r = reinterpret_cast<diag_record*> (data_)) << m; + + return *r; + } + + template <typename B> + diag_record& + operator<< (const diag_prologue<B>& p) + { + diag_record* r; + + if (empty_) + { + r = new (&data_) diag_record (p); + empty_ = false; + } + else + *(r = reinterpret_cast<diag_record*> (data_)) << p; + + return *r; + } + + maybe_diag_record (maybe_diag_record&&) = delete; + maybe_diag_record& operator= (maybe_diag_record&&) = delete; + + maybe_diag_record (const maybe_diag_record&) = delete; + maybe_diag_record& operator= (const maybe_diag_record&) = delete; + + ~maybe_diag_record () noexcept (false) // butl::diag_record may throw + { + if (!empty_) + reinterpret_cast<diag_record*> (data_)->~diag_record (); + } + + private: + bool empty_; + + static constexpr size_t size_ = sizeof (diag_record); + alignas (std::max_align_t) unsigned char data_[size_]; }; template <typename B> @@ -467,7 +564,7 @@ namespace build2 diag_record operator<< (const T& x) const { - diag_record r; + diag_record r (nullptr); r.append (this->indent, this->epilogue); B::operator() (r); r << x; @@ -981,7 +1078,7 @@ namespace build2 // function will throw. // void - close (diag_record&& = {}); + close (maybe_diag_record&& = {}); // Direct access to the underlying stream and buffer for custom processing // (see read() above for details). diff --git a/libbuild2/dist/operation.cxx b/libbuild2/dist/operation.cxx index 5b00980..0ed02c6 100644 --- a/libbuild2/dist/operation.cxx +++ b/libbuild2/dist/operation.cxx @@ -60,8 +60,13 @@ namespace build2 const path& arc, const dir_path& dir, const string& ext); static operation_id - dist_operation_pre (context&, const values&, operation_id o) + dist_operation_pre (context& ctx, const values&, operation_id o) { + // Note: cannot be --load-only which requires perform(update). + // + if (ctx.match_only) + fail << "--match-only specified for dist meta-operation"; + if (o != default_id) fail << "explicit operation specified for dist meta-operation"; @@ -283,6 +288,8 @@ namespace build2 action_targets ts {tgt}; { + // Signal to the rules we will not be executing. + // auto mog = make_guard ([&ctx] () {ctx.match_only = nullopt;}); ctx.match_only = match_only_level::all; diff --git a/libbuild2/dyndep.cxx b/libbuild2/dyndep.cxx index dbeb47e..9d9dfa4 100644 --- a/libbuild2/dyndep.cxx +++ b/libbuild2/dyndep.cxx @@ -40,9 +40,8 @@ namespace build2 if (!f) return nullopt; - diag_record dr; - dr << fail << what << ' ' << pt << " not found and no rule to " - << "generate it"; + diag_record dr (fail); + dr << what << ' ' << pt << " not found and no rule to generate it"; if (verb < 4) dr << info << "re-run with --verbose=4 for more information"; @@ -107,9 +106,8 @@ namespace build2 if (!f) return nullopt; - diag_record dr; - dr << fail << what << ' ' << pt << " not found and no rule to " - << "generate it"; + diag_record dr (fail); + dr << what << ' ' << pt << " not found and no rule to generate it"; if (verb < 4) dr << info << "re-run with --verbose=4 for more information"; @@ -139,7 +137,7 @@ namespace build2 action a, const target& t, size_t pts_n, const file& pt) { - diag_record dr; + const char* err (nullptr); if (pt.matched (a, memory_order_acquire)) { @@ -148,7 +146,7 @@ namespace build2 { if (pts_n == 0 || !updated_during_match (a, t, pts_n, pt)) { - dr << fail << what << ' ' << pt << " has non-noop recipe"; + err = "has non-noop recipe"; } } } @@ -157,12 +155,14 @@ namespace build2 // Note that this target could not possibly be updated during match // since it's not matched. // - dr << fail << what << ' ' << pt << " is explicitly declared as " - << "target and may have non-noop recipe"; + err = "is explicitly declared as target and may have non-noop recipe"; } - if (!dr.empty ()) - dr << info << "consider listing it as static prerequisite of " << t; + if (err != nullptr) + { + fail << what << ' ' << pt << ' ' << err << + info << "consider listing it as static prerequisite of " << t; + } } small_vector<const target_type*, 2> dyndep_rule:: diff --git a/libbuild2/file.cxx b/libbuild2/file.cxx index 8e3b2a4..14b60bd 100644 --- a/libbuild2/file.cxx +++ b/libbuild2/file.cxx @@ -2004,9 +2004,8 @@ namespace build2 if (!opt) { - diag_record dr; - dr << error (loc) << "invalid metadata signature in " << args[0] - << " output" << + diag_record dr (error (loc)); + dr << "invalid metadata signature in " << args[0] << " output" << info << "expected '" << s << "'"; if (verb >= 1 && verb <= 2) @@ -2042,8 +2041,8 @@ namespace build2 // if (!opt) { - diag_record dr; - dr << error (loc) << "unable to extract metadata from " << args[0] << + diag_record dr (error (loc)); + dr << "unable to extract metadata from " << args[0] << info << "process " << args[0] << " " << *pr.exit; if (verb >= 1 && verb <= 2) @@ -2111,14 +2110,17 @@ namespace build2 dr << info << "use config.import." << pv << " configuration variable to " << "specify its " << (qual != nullptr ? qual : "") << "project out_root"; - // Suggest ad hoc import but only if it's a path-based target (doing it - // for lib{} is very confusing). + // Suggest ad hoc import. Note that now we do it even for non-path-based + // targets since we have ad hoc import phase 2 logic for lib{}. // - if (tt != nullptr && tt->is_a<path_target> ()) + if (tt != nullptr) { - string v (tt->is_a<exe> () && (pv == tn || pn == tn) + string nv (sanitize_identifier (tn)); + + string v (tt->is_a<exe> () && pv == nv ? "config." + pv - : "config.import." + pv + '.' + tn + '.' + tt->name); + : ("config.import." + pv + '.' + nv + '.' + + sanitize_identifier (tt->name))); dr << info << "or use " << v << " configuration variable to specify " << "its " << (qual != nullptr ? qual : "") << "path"; @@ -2266,16 +2268,15 @@ namespace build2 // recognize the special config.<proj> (tool importation; we could // also handle the case where <proj> is not the same as <name> via // the config.<proj>.<name> variable). For backwards-compatibility - // reasons, it takes precedence over config.import. + // reasons, it takes precedence over config.import. Note that both + // <name> and <type> must be sanitized to a valid identifier (think + // libs{build2-foo} and cli.cxx{}). // // Note also that phase 2 import may handle these imports in an ad hoc // manner (see cc::search_library() for an example). // // Note: see import phase 2 diagnostics if changing anything here. // - // @@ How will this work for snake-case targets, say libs{build2-foo}? - // As well as for dot-separated target types, say, cli.cxx{}? - // // @@ This duality has a nasty side-effect: if we have config.<proj> // configured, then specifying config.<proj>.import has no effect // (see also a note below on priority just among these options). @@ -2333,6 +2334,8 @@ namespace build2 // const path* p (nullptr); + string valv (sanitize_identifier (tgt.value)); + if (tgt.typed ()) { bool e (tgt.type == "exe"); @@ -2341,15 +2344,18 @@ namespace build2 // overridable variable of type path. The config.<proj> we have to // type manually. // - if (e && (projv == tgt.value || proj == tgt.value)) + if (e && projv == valv) p = lookup (vp.insert<path> ("config." + projv), e); if (p == nullptr) - p = lookup (vp.insert (n + '.' + tgt.value + '.' + tgt.type), e); + { + string ttv (sanitize_identifier (tgt.type)); + p = lookup (vp.insert (n + '.' + valv + '.' + ttv), e); + } } if (p == nullptr) - p = lookup (vp.insert (n + '.' + tgt.value), false); + p = lookup (vp.insert (n + '.' + valv), false); if (p != nullptr) { @@ -2359,8 +2365,19 @@ namespace build2 { string on (move (tgt.value)); // Original name as imported. - tgt.dir = p->directory (); - tgt.value = p->leaf ().string (); + // Keep the original name if the path is (syntactically) to + // directory. + // + if (p->to_directory ()) + { + tgt.dir = path_cast<dir_path> (*p); + tgt.value = on; + } + else + { + tgt.dir = p->directory (); + tgt.value = p->leaf ().string (); + } // If the path is relative, then keep it project-qualified // assuming import phase 2 knows what to do with it. Think: @@ -2379,48 +2396,59 @@ namespace build2 tgt.proj = move (proj); else { - // Enter the target and assign its path (this will most commonly - // be some out of project file). - // - // @@ Should we check that the file actually exists (and cache - // the extracted timestamp)? Or just let things take their - // natural course? - // name n (tgt); const target_type* tt (ibase.find_target_type (n, loc).first); if (tt == nullptr) fail (loc) << "unknown target type " << n.type << " in " << n; - // Note: not using the extension extracted by find_target_type() - // to be consistent with import phase 2. - // - target& t (insert_target (trace, ctx, *tt, *p).first); - - // Load the metadata, similar to import phase 2. + // If this is not a path-based target, then delegate to import + // phase 2 as above (see cc::search_library() for an example). // - if (meta) + if (!tt->is_a<path_target> ()) { - if (exe* e = t.is_a<exe> ()) + tgt.proj = move (proj); + } + else + { + // Enter the target and assign its path (this will most + // commonly be some out of project file). + // + // @@ Should we check that the file actually exists (and cache + // the extracted timestamp)? Or just let things take their + // natural course? + // + + // Note: not using the extension extracted by + // find_target_type() to be consistent with import phase 2. + // + target& t (insert_target (trace, ctx, *tt, *p).first); + + // Load the metadata, similar to import phase 2. + // + if (meta) { - if (!e->vars[ctx.var_export_metadata].defined ()) + if (exe* e = t.is_a<exe> ()) { - optional<string> md; + if (!e->vars[ctx.var_export_metadata].defined ()) { - auto df = make_diag_frame ( - [&proj, tt, &on] (const diag_record& dr) - { - import_suggest ( - dr, proj, tt, on, false, "alternative "); - }); - - md = extract_metadata (e->process_path (), - *meta, - false /* optional */, - loc); + optional<string> md; + { + auto df = make_diag_frame ( + [&proj, tt, &on] (const diag_record& dr) + { + import_suggest ( + dr, proj, tt, on, false, "alternative "); + }); + + md = extract_metadata (e->process_path (), + *meta, + false /* optional */, + loc); + } + + parse_metadata (*e, move (*md), loc); } - - parse_metadata (*e, move (*md), loc); } } } @@ -2667,12 +2695,10 @@ namespace build2 } else { - diag_record dr; - dr << fail (loc) << "unable to determine src_root for imported "; - if (proj) - dr << *proj; - else - dr << out_root; + diag_record dr (fail (loc)); + dr << "unable to determine src_root for imported "; + if (proj) dr << *proj; + else dr << out_root; dr << info << "consider configuring " << out_root; } @@ -3250,8 +3276,8 @@ namespace build2 if (opt || exist) return nullptr; - diag_record dr; - dr << fail (loc) << "unable to import target " << pk; + diag_record dr (fail (loc)); + dr << "unable to import target " << pk; if (proj.empty ()) dr << info << "consider adding its installation location" << @@ -3298,8 +3324,8 @@ namespace build2 if (opt || exist) return nullptr; - diag_record dr; - dr << fail (loc) << "unable to import target " << ns; + diag_record dr (fail (loc)); + dr << "unable to import target " << ns; import_suggest (dr, *n.proj, nullptr, string (), meta.has_value ()); } } @@ -3377,8 +3403,8 @@ namespace build2 if (opt) return names {}; - diag_record dr; - dr << fail (loc) << "unable to import target " << n; + diag_record dr (fail (loc)); + dr << "unable to import target " << n; import_suggest (dr, *n.proj, nullptr /* tt */, n.value, false); diff --git a/libbuild2/file.hxx b/libbuild2/file.hxx index 36e4c00..ff8a821 100644 --- a/libbuild2/file.hxx +++ b/libbuild2/file.hxx @@ -30,8 +30,9 @@ namespace build2 // export.build -- export stub // export/ -- exported buildfiles // - // The build/, bootstrap/, root/, and config.build entries are in .gitignore - // as generated by bdep-new. + // The build/, /bootstrap/, /root/, and config.build entries are in + // .gitignore as generated by bdep-new (note that build/ is ignored + // recursively; see below). // // The rest of the filesystem entries are shared between the project and the // modules that it loads. In particular, if a project loads module named diff --git a/libbuild2/filesystem.hxx b/libbuild2/filesystem.hxx index 7b45a08..44f5d92 100644 --- a/libbuild2/filesystem.hxx +++ b/libbuild2/filesystem.hxx @@ -24,6 +24,9 @@ namespace build2 { using butl::entry_type; + using butl::dir_entry; + using butl::dir_iterator; + using butl::auto_rmfile; using butl::auto_rmdir; diff --git a/libbuild2/function.cxx b/libbuild2/function.cxx index 3110547..3515c8e 100644 --- a/libbuild2/function.cxx +++ b/libbuild2/function.cxx @@ -239,9 +239,9 @@ namespace build2 // No match. // - diag_record dr; + diag_record dr (fail (loc)); - dr << fail (loc) << "unmatched call to "; print_call (dr.os); + dr << "unmatched call to "; print_call (dr.os); if (all_ovls != nullptr) { @@ -284,8 +284,8 @@ namespace build2 { // Ambigous match. // - diag_record dr; - dr << fail (loc) << "ambiguous call to "; print_call (dr.os); + diag_record dr (fail (loc)); + dr << "ambiguous call to "; print_call (dr.os); for (auto f: ovls) dr << info << "candidate: " << *f; diff --git a/libbuild2/functions-json.cxx b/libbuild2/functions-json.cxx index e06d9a5..9c59c4b 100644 --- a/libbuild2/functions-json.cxx +++ b/libbuild2/functions-json.cxx @@ -295,8 +295,8 @@ namespace build2 } catch (const invalid_json_output& e) { - diag_record dr; - dr << fail << "invalid json value: " << e; + diag_record dr (fail); + dr << "invalid json value: " << e; if (e.event) dr << info << "while serializing " << to_string (*e.event); diff --git a/libbuild2/functions-name.cxx b/libbuild2/functions-name.cxx index 456f85b..cb32d09 100644 --- a/libbuild2/functions-name.cxx +++ b/libbuild2/functions-name.cxx @@ -49,7 +49,10 @@ namespace build2 } const target& - to_target (const scope& s, name&& n, name&& o) + to_target (const scope& s, + name&& n, name&& o, + bool in_recipe, + const location& loc) { // Note: help the user out and search in both out and src like a // prerequisite. @@ -62,13 +65,13 @@ namespace build2 // bool typed (n.typed ()); - diag_record dr (fail); + diag_record dr (fail (loc)); dr << "target " << (n.pair ? names {move (n), move (o)} : names {move (n)}) << " not found"; - if (!typed) + if (in_recipe && !typed) dr << info << "wrap it in ([names] ...) if this is literal target name " << "specified inside recipe"; @@ -76,12 +79,15 @@ namespace build2 } const target& - to_target (const scope& s, names&& ns) + to_target (const scope& s, names&& ns, bool in_recipe, const location& loc) { assert (ns.size () == (ns[0].pair ? 2 : 1)); name o; - return to_target (s, move (ns[0]), move (ns[0].pair ? ns[1] : o)); + return to_target (s, + move (ns[0]), move (ns[0].pair ? ns[1] : o), + in_recipe, + loc); } static bool diff --git a/libbuild2/functions-name.hxx b/libbuild2/functions-name.hxx index 34fa4b8..30fc8ad 100644 --- a/libbuild2/functions-name.hxx +++ b/libbuild2/functions-name.hxx @@ -18,13 +18,16 @@ namespace build2 // Resolve the name to target issuing diagnostics and failing if not found. // LIBBUILD2_SYMEXPORT const target& - to_target (const scope&, name&&, name&& out); + to_target (const scope&, + name&&, name&& out, + bool in_recipe, + const location& = {}); // As above but from the names vector which should contain a single name // or an out-qualified name pair (asserted). // LIBBUILD2_SYMEXPORT const target& - to_target (const scope&, names&&); + to_target (const scope&, names&&, bool in_recipe, const location& = {}); } #endif // LIBBUILD2_FUNCTIONS_NAME_HXX diff --git a/libbuild2/functions-process.cxx b/libbuild2/functions-process.cxx index 6faa798..bcd91a8 100644 --- a/libbuild2/functions-process.cxx +++ b/libbuild2/functions-process.cxx @@ -177,8 +177,8 @@ namespace build2 // While assuming that the builtin has issued the diagnostics on failure // we still print the error message (see process_finish() for details). // - diag_record dr; - dr << fail << "builtin " << bn << " " << process_exit (rs); + diag_record dr (fail); + dr << "builtin " << bn << " " << process_exit (rs); if (verb >= 1 && verb <= 2) { diff --git a/libbuild2/functions-target.cxx b/libbuild2/functions-target.cxx index c7cb50e..55baeda 100644 --- a/libbuild2/functions-target.cxx +++ b/libbuild2/functions-target.cxx @@ -35,6 +35,8 @@ namespace build2 if (s == nullptr) fail << "target.path() called out of scope"; + context& ctx (s->ctx); + // Most of the time we will have a single target so optimize for that. // small_vector<path, 1> r; @@ -42,7 +44,10 @@ namespace build2 for (auto i (ns.begin ()); i != ns.end (); ++i) { name& n (*i), o; - const target& t (to_target (*s, move (n), move (n.pair ? *++i : o))); + const target& t ( + to_target (*s, + move (n), move (n.pair ? *++i : o), + ctx.phase != run_phase::load /* in_recipe */)); if (const auto* pt = t.is_a<path_target> ()) { @@ -94,7 +99,9 @@ namespace build2 name o; const target& t ( - to_target (*s, move (ns[0]), move (ns[0].pair ? ns[1] : o))); + to_target (*s, + move (ns[0]), move (ns[0].pair ? ns[1] : o), + s->ctx.phase != run_phase::load /* in_recipe */)); if (const auto* et = t.is_a<exe> ()) { diff --git a/libbuild2/module.cxx b/libbuild2/module.cxx index 36a7ce5..9a12d01 100644 --- a/libbuild2/module.cxx +++ b/libbuild2/module.cxx @@ -188,18 +188,24 @@ namespace build2 loc, tgs); + // Note that we suppress all outcome diagnostics, even for failure since + // the extra `info: failed to update <target>` is not very useful (we + // expect an appropriate diagnostics frame explains what's going on). + // mo_perform.match ({}, /* parameters */ a, tgs, - 1, /* diag (failures only) */ + 0, /* diag (none) */ false /* progress */); mo_perform.execute ({}, /* parameters */ a, tgs, - 1, /* diag (failures only) */ + 0, /* diag (none) */ false /* progress */); + ctx.module_context->load_generation++; + assert (tgs.size () == 1); return tgs[0].as<target> (); } diff --git a/libbuild2/operation.cxx b/libbuild2/operation.cxx index 76118ea..8b28dd8 100644 --- a/libbuild2/operation.cxx +++ b/libbuild2/operation.cxx @@ -288,11 +288,14 @@ namespace build2 // Note also that the higher the increment, the less accurate our // executed during match number will be. // + // Note that we strip the outer operation from "(... during match)" + // not to repeat the same "(for <operation>)" twice. + // md.incr = stderr_term // Scale depending on output type. ? (ctx.sched->serial () ? 1 : 2) : 100; md.what1 = " targets to " + diag_do (ctx, a); - md.what2 = ' ' + diag_did (ctx, a) + " during match)"; + md.what2 = ' ' + diag_did (ctx, a.inner_action ()) + " during match)"; mg = ctx.sched->monitor ( ctx.target_count, @@ -1061,8 +1064,8 @@ namespace build2 : 0); }; - diag_record dr; - dr << info << "detected unexecuted matched targets:"; + diag_record dr (info); + dr << "detected unexecuted matched targets:"; for (const auto& pt: ctx.targets) { diff --git a/libbuild2/parser.cxx b/libbuild2/parser.cxx index 53f808c..babe4c9 100644 --- a/libbuild2/parser.cxx +++ b/libbuild2/parser.cxx @@ -2259,9 +2259,9 @@ namespace build2 if (tt != type::word) { - diag_record dr; + diag_record dr (fail (t)); - dr << fail (t) << "unterminated recipe "; + dr << "unterminated recipe "; if (kind.empty ()) dr << "block"; else dr << kind << "-block"; dr << info (st) << "recipe "; @@ -2385,9 +2385,16 @@ namespace build2 // size_t b (0), e (0); for (size_t m (0), n (text.size ()); - next_word (text, n, b, e, m, '\n', '\r'), b != n; - sloc.line++) + next_word (text, n, b, e, m, '\n', '\r'), b != n; ) { + // Treat consecutive \r\n (CRLF) as if they were a single + // delimiter. + // + if (b != 0 && text[b - 1] == '\r' && + e != n && text[e] == '\n' && + m != 2) + continue; + s.assign (text, b, e - b); if (!trim (s).empty ()) @@ -2401,6 +2408,8 @@ namespace build2 break; } } + + sloc.line++; } if (b == e) @@ -2998,8 +3007,8 @@ namespace build2 // rule-specific search) can possibly succeed so we can fail now and // with a more accurate reason. See import2(names) for background. // - diag_record dr; - dr << fail (ploc) << "unable to import target " << n; + diag_record dr (fail (ploc)); + dr << "unable to import target " << n; import_suggest (dr, *n.proj, nullptr, string (), false); } else @@ -3954,7 +3963,7 @@ namespace build2 proj = n.variable (); } - diag_record dr; + maybe_diag_record dr; do // Breakout loop. { if (!config) @@ -4058,7 +4067,7 @@ namespace build2 } while (false); - if (!dr.empty ()) + if (dr) dr << info << "expected variable name in the 'config[.**]." << (proj.empty () ? "<project>" : proj.c_str ()) << ".**' form"; } @@ -5754,9 +5763,9 @@ namespace build2 void parser:: parse_diag (token& t, type& tt) { - diag_record dr; const location l (get_location (t)); + diag_record dr (nullptr); switch (t.value[0]) { case 'f': dr << fail (l); break; @@ -6998,7 +7007,13 @@ namespace build2 // The directory/value must not be empty if we have a type. // if (d.empty () && v.empty () && !t.empty ()) - fail (loc) << "typed empty name"; + { + // We sneak +/- in type if in pattern_mode::detect (think {+{}}). + // + fail (loc) << (t == "+" ? "empty name pattern inclusion group" : + t == "-" ? "empty name pattern exclusion group" : + "typed empty name"); + } ns.emplace_back (move (p), move (d), move (t), move (v), pat); return ns.back (); @@ -7258,7 +7273,9 @@ namespace build2 (root_ != nullptr && root_->root_extra != nullptr && m.to_directory () && - exists (d.sp / m / root_->root_extra->buildignore_file))) + exists (m.relative () + ? d.sp / m / root_->root_extra->buildignore_file + : m / root_->root_extra->buildignore_file))) return !interm; // Note that we have to make copies of the extension since there will @@ -7314,9 +7331,11 @@ namespace build2 return true; }); + path pat (move (p)); + try { - path_search (path (move (p)), + path_search (pat, process, *sp, path_match_flags::follow_symlinks, @@ -7324,7 +7343,8 @@ namespace build2 } catch (const system_error& e) { - fail (l) << "unable to scan " << *sp << ": " << e; + fail (l) << "unable to scan for '" + << (pat.relative () ? *sp / pat : pat) << "': " << e; } }; @@ -7569,7 +7589,10 @@ namespace build2 // and look for some wildcards since the pattern can be the result of an // expansion (or, worse, concatenation). Thus pattern_mode::detect: we // are going to ask parse_names() to detect for us if the first name is - // a pattern. And if it is, to refrain from adding pair/dir/type. + // a pattern. And if it is, to refrain from adding pair/dir/type (note: + // for the pattern inclusions and exclusions the name's type member will + // be set to "+" and "-", respectively, and will later be cleared by + // expand_name_pattern()). // optional<const target_type*> pat_tt ( parse_names ( @@ -8313,16 +8336,42 @@ namespace build2 fail (loc) << "invalid path '" << e.path << "'"; } - count = parse_names_trailer ( - t, tt, ns, pmode, what, separators, pairn, *pp1, dp1, tp1, cross); + // Note that for a pattern inclusion group (see above) we make sure + // that the resulting patterns are simple names, passing NULL as the + // directory path (the names' type members will still be set to "+" + // thought; see the parse_names_trailer::parse() lambda + // implementation for details). + // + assert (!pinc || (tp1 != nullptr && *tp1 == "+")); - // If empty group or empty name, then this is not a pattern inclusion - // group (see above). + count = parse_names_trailer ( + t, tt, + ns, + pmode, + what, + separators, pairn, + *pp1, (!pinc ? dp1 : nullptr), tp1, + cross); + + // If empty group, then this is not a pattern inclusion group. // if (pinc) { - if (count != 0 && (count > 1 || !ns.back ().empty ())) + if (count != 0) + { + // Note that we can never end up with the empty name here. For + // example, for the below constructs the above + // parse_names_trailer() call would fail with appropriate + // diagnostics since the empty name's type will be set to "+" + // (see above for details): + // + // foo/{hxx cxx}{+{}} + // foo/{+{}} + // + assert (count > 1 || !ns.back ().empty ()); + pattern_detected (ttp); + } ppat = pinc = false; } diff --git a/libbuild2/parser.hxx b/libbuild2/parser.hxx index 3e1d0a0..b68bf7e 100644 --- a/libbuild2/parser.hxx +++ b/libbuild2/parser.hxx @@ -893,6 +893,9 @@ namespace build2 // Set the data and start playing. // + // Note that using the const reference version is better than making a + // copy since it may reuse the existing capacity. + // void replay_data (replay_tokens&& d) { @@ -905,6 +908,18 @@ namespace build2 replay_ = replay::play; } + void + replay_data (const replay_tokens& d) + { + assert (replay_ == replay::stop); + + replay_path_ = path_; // Save old path. + + replay_data_ = d; + replay_i_ = 0; + replay_ = replay::play; + } + // Implementation details, don't call directly. // replay_token diff --git a/libbuild2/script/parser.cxx b/libbuild2/script/parser.cxx index a82ccb8..30c7ff6 100644 --- a/libbuild2/script/parser.cxx +++ b/libbuild2/script/parser.cxx @@ -1668,9 +1668,9 @@ namespace build2 if (es > 255) { - diag_record dr; + diag_record dr (fail (l)); - dr << fail (l) << "expected exit status instead of "; + dr << "expected exit status instead of "; to_stream (dr.os, ns, quote_mode::normal); dr << info << "exit status is an unsigned integer less than 256"; @@ -2218,7 +2218,7 @@ namespace build2 // Copy the tokens and start playing. // - replay_data (replay_tokens (ln.tokens)); + replay_data (ln.tokens); // We don't really need to change the mode since we already know // the line type. @@ -2433,7 +2433,7 @@ namespace build2 // Prepare for the condition reevaluation. // - replay_data (replay_tokens (ln.tokens)); + replay_data (ln.tokens); next (t, tt); li = wli; } diff --git a/libbuild2/script/run.cxx b/libbuild2/script/run.cxx index f8f98c1..e669b29 100644 --- a/libbuild2/script/run.cxx +++ b/libbuild2/script/run.cxx @@ -1301,7 +1301,6 @@ namespace build2 // Terminate processes gracefully and set the terminate flag for the // pipe commands. // - diag_record dr; for (pipe_command* c (pc); c != nullptr; c = c->prev) { if (process* p = c->proc) @@ -1324,6 +1323,10 @@ namespace build2 c->terminated = true; } + // Delay any failure until we process the entire pipeline. + // + maybe_diag_record dr; + // Wait a bit for the processes to terminate and kill the remaining // ones. // @@ -1708,7 +1711,7 @@ namespace build2 const iteration_index* ii, size_t li, size_t ci, const location& ll, bool diag, - const function<command_function>& cf, bool last_cmd, + const function<command_function>& cf, bool replace_last_cmd, optional<deadline> dl = nullopt, pipe_command* prev_cmd = nullptr) { @@ -1720,7 +1723,7 @@ namespace build2 { if (cf != nullptr) { - assert (!last_cmd); // Otherwise we wouldn't be here. + assert (!replace_last_cmd); // Otherwise we wouldn't be here. // The pipeline can't be empty. // @@ -1795,8 +1798,9 @@ namespace build2 command_pipe::const_iterator nc (bc + 1); bool last (nc == ec); - // Make sure that stdout is not redirected if meant to be read (last_cmd - // is false) or cannot not be produced (last_cmd is true). + // Make sure that stdout is not redirected if meant to be read + // (replace_last_cmd is false) or cannot not be produced + // (replace_last_cmd is true). // if (last && c.out && cf != nullptr) fail (ll) << "stdout cannot be redirected"; @@ -1810,14 +1814,62 @@ namespace build2 // const string& program (c.program.recall.string ()); + // Optimize standalone true/false builtins (used in conditions, etc). We + // know they don't read/write stdin/stdout/stderr so none of the below + // complications are necessary. + // + if (resolve && (program == "true" || program == "false") && + first && last && cf == nullptr && + !c.in && !c.out && !c.err && !c.exit) + { + // Note: we can safely ignore agruments, env vars, and timeout. + // + // Don't print the true and false builtins, since they are normally + // used for the commands execution flow control (also below). + // + return program == "true"; + } + + // Standard streams. + // const redirect& in ((c.in ? *c.in : env.in).effective ()); - const redirect* out (!last || (cf != nullptr && !last_cmd) + const redirect* out (!last || (cf != nullptr && !replace_last_cmd) ? nullptr // stdout is piped. : &(c.out ? *c.out : env.out).effective ()); const redirect& err ((c.err ? *c.err : env.err).effective ()); + // If the output redirect is `none` (default for testscript) or `null` + // and this is a builtin which is known not to write to stdout, then + // redirect its stdout to stderr, unless stderr itself is redirected to + // stdout. This way we replace the expensive fdopen(/dev/null) call with + // the cheap fddup(...) call. + // + // Note that we ignore the pseudo builtins since they are handled prior + // to opening any file descriptors for stdout. + // + const redirect out_merge (redirect_type::merge, 2); + if (out != nullptr && + (out->type == redirect_type::none || + out->type == redirect_type::null) && + err.type != redirect_type::merge && + resolve && + (program == "cp" || + program == "false" || + program == "ln" || + program == "mkdir" || + program == "mv" || + program == "rm" || + program == "rmdir" || + program == "sleep" || + program == "test" || + program == "touch" || + program == "true")) + { + out = &out_merge; + } + auto process_args = [&c] () -> cstrings { return build2::process_args (c.program.recall_string (), c.arguments); @@ -1889,7 +1941,7 @@ namespace build2 if (c.out) fail (ll) << program << " builtin stdout cannot be redirected"; - if (cf != nullptr && !last_cmd) + if (cf != nullptr && !replace_last_cmd) fail (ll) << program << " builtin stdout cannot be read"; if (c.err) @@ -1964,11 +2016,14 @@ namespace build2 // If this is the first pipeline command, then open stdin descriptor // according to the redirect specified. // + // Specifically, save into ifd the descriptor of a newly created file, + // existing file, stdin descriptor duplicate, or null-device descriptor + // for the here_*_literal, file, pass, or null/none redirects, + // respectively. + // path isp; - if (!first) - assert (!c.in); // No redirect expected. - else + if (first) { // Open a file for passing to the command stdin. // @@ -2002,25 +2057,58 @@ namespace build2 break; } case redirect_type::none: - // Somehow need to make sure that the child process doesn't read - // from stdin. That is tricky to do in a portable way. Here we - // suppose that the program which (erroneously) tries to read some - // data from stdin being redirected to /dev/null fails not being - // able to read the expected data, and so the command doesn't pass - // through. - // - // @@ Obviously doesn't cover the case when the process reads - // whatever available. - // @@ Another approach could be not to redirect stdin and let the - // process to hang which can be interpreted as a command failure. - // @@ Both ways are quite ugly. Is there some better way to do - // this? - // @@ Maybe we can create a pipe, write a byte into it, close the - // writing end, and after the process terminates make sure we can - // still read this byte out? - // + { + // Somehow need to make sure that the child process doesn't read + // from stdin. That is tricky to do in a portable way. Here we + // suppose that the program which (erroneously) tries to read some + // data from stdin being redirected to /dev/null fails not being + // able to read the expected data, and so the command doesn't pass + // through. + // + // @@ Obviously doesn't cover the case when the process reads + // whatever available. + // @@ Another approach could be not to redirect stdin and let the + // process to hang which can be interpreted as a command + // failure. + // @@ Both ways are quite ugly. Is there some better way to do + // this? + // @@ Maybe we can create a pipe, write a byte into it, close the + // writing end, and after the process terminates make sure we + // can still read this byte out? + // + // Let's optimize for the builtins, which are known not to read + // any input, by not opening /dev/null but duplicating the stdin + // file descriptor instead. + // + if (resolve && + (program == "cp" || + program == "date" || + program == "echo" || + program == "false" || + program == "find" || + program == "ln" || + program == "mkdir" || + program == "mv" || + program == "rm" || + program == "rmdir" || + program == "sleep" || + program == "test" || + program == "touch" || + program == "true")) + { + try + { + ifd = fddup (0); + } + catch (const io_error& e) + { + fail (ll) << "unable to duplicate stdin: " << e; + } + + break; + } + } // Fall through. - // case redirect_type::null: { ifd = open_null (); @@ -2057,8 +2145,10 @@ namespace build2 case redirect_type::here_doc_ref: assert (false); break; } } + else + assert (!c.in); // No redirect expected. - assert (ifd.get () != -1); + assert (ifd != nullfd); // Calculate the process/builtin execution deadline. Note that we should // also consider the left-hand side processes deadlines, not to keep @@ -2092,7 +2182,7 @@ namespace build2 if (c.out) fail (ll) << "set builtin stdout cannot be redirected"; - if (cf != nullptr && !last_cmd) + if (cf != nullptr && !replace_last_cmd) fail (ll) << "set builtin stdout cannot be read"; if (c.err) @@ -2111,7 +2201,7 @@ namespace build2 // If this is the last command in the pipe and the command function is // specified for it, then call it. // - if (last && cf != nullptr && last_cmd) + if (last && cf != nullptr && replace_last_cmd) { // Must be enforced by the caller. // @@ -2152,14 +2242,16 @@ namespace build2 else pc.next = &pc; // Points to itself. - // Open a file for command output redirect if requested explicitly - // (file overwrite/append redirects) or for the purpose of the output + // Open a file for command output redirect if requested explicitly (file + // overwrite/append redirects) or for the purpose of the output // validation (none, here_*, file comparison redirects), register the // file for cleanup, return the file descriptor. Interpret trace // redirect according to the verbosity level (as null if below 2, as - // pass otherwise). Return nullfd, standard stream descriptor duplicate - // or null-device descriptor for merge, pass or null redirects - // respectively (not opening any file). + // pass otherwise). Return nullfd, standard stream descriptor duplicate, + // or null-device descriptor for merge, pass (except for the buffered + // stderr), or null redirects respectively (not opening/creating any + // file/pipe). Create the pipe and return its write end for the pass + // redirect of the buffered stderr. // auto open = [&env, &wdir, &ll, &std_path, &c, &pc] (const redirect& r, int dfd, @@ -2276,10 +2368,12 @@ namespace build2 path osp; fdpipe ofd; - // If this is the last command in the pipeline than redirect the - // command process stdout to a file. Otherwise create a pipe and - // redirect the stdout to the write-end of the pipe. The read-end will - // be passed as stdin for the next command in the pipeline. + // If this is either not the last command in the pipeline or the + // command's output needs to be read by the specified function, then + // create a pipe and redirect the stdout to the write-end of the + // pipe. The read-end will be passed as stdin for the next command in + // the pipeline or the function. Otherwise, proceed according to the + // specified redirect (see open() lambda for details). // // @@ Shouldn't we allow the here-* and file output redirects for a // command with pipelined output? Say if such redirect is present @@ -2290,16 +2384,21 @@ namespace build2 // script failures investigation and, for example, for validation // "tightening". // - if (last && out != nullptr) - ofd.out = open (*out, 1, osp); - else + if (!last || out == nullptr) { assert (!c.out); // No redirect expected. + + // Otherwise we wouldn't be here. + // + assert (!last || (cf != nullptr && !replace_last_cmd)); + ofd = open_pipe (); } + else + ofd.out = open (*out, 1, osp); // Note: may or may not open file. path esp; - auto_fd efd (open (err, 2, esp)); + auto_fd efd (open (err, 2, esp)); // Note: may or may not open file/pipe. // Merge standard streams. // @@ -2547,7 +2646,7 @@ namespace build2 // Verify the exit status and issue the diagnostics on failure. // - diag_record dr; + maybe_diag_record dr; path pr (cmd_path (cmd)); @@ -2563,16 +2662,16 @@ namespace build2 if (c->unread_stdout) { - dr << "stdout "; + *dr << "stdout "; if (c->unread_stderr) - dr << "and "; + *dr << "and "; } if (c->unread_stderr) - dr << "stderr "; + *dr << "stderr "; - dr << "not closed after exit"; + *dr << "not closed after exit"; }; // Fail if the process is terminated due to reaching the deadline. @@ -2588,7 +2687,7 @@ namespace build2 if (verb == 1) { dr << info << "command line: "; - print_process (dr, *c->args); + print_process (*dr, *c->args); } fail = true; @@ -2643,23 +2742,23 @@ namespace build2 dr << error (ll) << w << ' ' << pr << ' '; if (!exit->normal ()) - dr << *exit; + *dr << *exit; else { uint16_t ec (exit->code ()); // Make sure printed as integer. if (!valid) { - dr << "exit code " << ec << " out of 0-255 range"; + *dr << "exit code " << ec << " out of 0-255 range"; } else { if (cmd.exit) - dr << "exit code " << ec - << (cmp == exit_comparison::eq ? " != " : " == ") - << exc; + *dr << "exit code " << ec + << (cmp == exit_comparison::eq ? " != " : " == ") + << exc; else - dr << "exited with code " << ec; + *dr << "exited with code " << ec; } } @@ -2669,7 +2768,7 @@ namespace build2 if (verb == 1) { dr << info << "command line: "; - print_process (dr, *c->args); + print_process (*dr, *c->args); } if (non_empty (*c->esp, ll) && avail_on_failure (*c->esp, env)) @@ -2683,7 +2782,7 @@ namespace build2 // Print cached stderr. // - print_file (dr, *c->esp, ll); + print_file (*dr, *c->esp, ll); } else if (c->unread_stdout || c->unread_stderr) unread_output_diag (true /* main_error */); @@ -2754,7 +2853,7 @@ namespace build2 // Execute the builtin. // // Don't print the true and false builtins, since they are normally - // used for the commands execution flow control. + // used for the commands execution flow control (also above). // if (verb >= 2 && program != "true" && program != "false") print_process (args); @@ -2984,7 +3083,7 @@ namespace build2 // If/when required we could probably support the precise sleep // mode (e.g., via an option). // - env.context.sched->sleep (t); + env.sleep (t); } }; @@ -3020,7 +3119,7 @@ namespace build2 nc, ec, move (ofd.in), ii, li, ci + 1, ll, diag, - cf, last_cmd, + cf, replace_last_cmd, dl, &pc); @@ -3145,7 +3244,7 @@ namespace build2 nc, ec, move (ofd.in), ii, li, ci + 1, ll, diag, - cf, last_cmd, + cf, replace_last_cmd, dl, &pc); @@ -3198,7 +3297,7 @@ namespace build2 const iteration_index* ii, size_t li, const location& ll, bool diag, - const function<command_function>& cf, bool last_cmd) + const function<command_function>& cf, bool replace_last_cmd) { // Commands are numbered sequentially throughout the expression // starting with 1. Number 0 means the command is a single one. @@ -3243,7 +3342,7 @@ namespace build2 p.begin (), p.end (), auto_fd (), ii, li, ci, ll, print, - cf, last_cmd); + cf, replace_last_cmd); } ci += p.size (); @@ -3258,8 +3357,13 @@ namespace build2 const iteration_index* ii, size_t li, const location& ll, const function<command_function>& cf, - bool last_cmd) + bool replace_last_cmd) { + // If the last command in the pipeline is ought to be replaced, then the + // replacement function must be specified. + // + assert (!replace_last_cmd || cf != nullptr); + // Note that we don't print the expression at any verbosity level // assuming that the caller does this, potentially providing some // additional information (command type, etc). @@ -3268,7 +3372,7 @@ namespace build2 expr, ii, li, ll, true /* diag */, - cf, last_cmd)) + cf, replace_last_cmd)) throw failed (); // Assume diagnostics is already printed. } @@ -3277,15 +3381,20 @@ namespace build2 const command_expr& expr, const iteration_index* ii, size_t li, const location& ll, - const function<command_function>& cf, bool last_cmd) + const function<command_function>& cf, bool replace_last_cmd) { + // If the last command in the pipeline is ought to be replaced, then the + // replacement function must be specified. + // + assert (!replace_last_cmd || cf != nullptr); + // Note that we don't print the expression here (see above). // return run_expr (env, expr, ii, li, ll, false /* diag */, - cf, last_cmd); + cf, replace_last_cmd); } void diff --git a/libbuild2/script/run.hxx b/libbuild2/script/run.hxx index c4c2aa2..f41a3d0 100644 --- a/libbuild2/script/run.hxx +++ b/libbuild2/script/run.hxx @@ -39,7 +39,7 @@ namespace build2 // can be used in diagnostics. // // Optionally, execute the specified function at the end of the pipe, - // either after the last command or instead of it. + // potentially instead of the last command (replace_last_cmd). // void run (environment&, @@ -47,7 +47,7 @@ namespace build2 const iteration_index*, size_t index, const location&, const function<command_function>& = nullptr, - bool last_cmd = true); + bool replace_last_cmd = false); bool run_cond (environment&, @@ -55,7 +55,7 @@ namespace build2 const iteration_index*, size_t index, const location&, const function<command_function>& = nullptr, - bool last_cmd = true); + bool replace_last_cmd = false); // Perform the registered special file cleanups in the direct order and // then the regular cleanups in the reverse order. diff --git a/libbuild2/script/script.hxx b/libbuild2/script/script.hxx index f5bd69a..76c4010 100644 --- a/libbuild2/script/script.hxx +++ b/libbuild2/script/script.hxx @@ -588,6 +588,11 @@ namespace build2 virtual void create_temp_dir () = 0; + // Used as an implementation of the sleep builtin. + // + virtual void + sleep (const duration&) = 0; + public: virtual ~environment () = default; diff --git a/libbuild2/test/rule.cxx b/libbuild2/test/rule.cxx index 28eb35b..b3a73ce 100644 --- a/libbuild2/test/rule.cxx +++ b/libbuild2/test/rule.cxx @@ -745,7 +745,9 @@ namespace build2 // auto term_pipe = [&timed_wait] (pipe_process* pp) { - diag_record dr; + // Delay the failure until we process the entire pipeline. + // + maybe_diag_record dr; // Terminate processes gracefully and set the terminate flag for // them. @@ -962,7 +964,7 @@ namespace build2 // if (!fail) { - diag_record dr; + maybe_diag_record dr; // Note that there can be a race, so that the process we have // terminated due to reaching the deadline has in fact exited @@ -983,14 +985,14 @@ namespace build2 if (!pe) { - dr << "terminated: execution timeout expired"; + *dr << "terminated: execution timeout expired"; if (p->unread_stderr) dr << error << "stderr not closed after exit"; } else if (!pe->normal () || pe->code () != 0) { - dr << *pe; + *dr << *pe; if (p->unread_stderr) dr << error << "stderr not closed after exit"; @@ -999,7 +1001,7 @@ namespace build2 { assert (p->unread_stderr); - dr << "stderr not closed after exit"; + *dr << "stderr not closed after exit"; } if (verb == 1) @@ -1009,9 +1011,9 @@ namespace build2 for (pipe_process* p (b); p != nullptr; p = p->next) { if (p != b) - dr << " | "; + *dr << " | "; - print_process (dr, p->args); + print_process (*dr, p->args); } } } diff --git a/libbuild2/test/script/runner.cxx b/libbuild2/test/script/runner.cxx index 98d6868..54009f6 100644 --- a/libbuild2/test/script/runner.cxx +++ b/libbuild2/test/script/runner.cxx @@ -182,7 +182,11 @@ namespace build2 }); ++sp.exec_level; - build2::script::run (sp, expr, ii, li, ll, cf); + build2::script::run (sp, + expr, + ii, li, + ll, + cf, (cf != nullptr) /* replace_last_cmd */); --sp.exec_level; } diff --git a/libbuild2/test/script/script.cxx b/libbuild2/test/script/script.cxx index 7862120..176daf3 100644 --- a/libbuild2/test/script/script.cxx +++ b/libbuild2/test/script/script.cxx @@ -181,6 +181,12 @@ namespace build2 : exported_vars; } + void scope:: + sleep (const duration& d) + { + context.sched->sleep (d); + } + // script_base // script_base:: diff --git a/libbuild2/test/script/script.hxx b/libbuild2/test/script/script.hxx index 9409b01..d1e0f53 100644 --- a/libbuild2/test/script/script.hxx +++ b/libbuild2/test/script/script.hxx @@ -140,6 +140,11 @@ namespace build2 virtual void create_temp_dir () override {assert (false);}; + // Call the scheduler's sleep() function. + // + virtual void + sleep (const duration&) override; + // Return true if this is a test program path. // // Note that currently the test program is only specified via the test diff --git a/libbuild2/utility.cxx b/libbuild2/utility.cxx index ae7c9b0..83947b2 100644 --- a/libbuild2/utility.cxx +++ b/libbuild2/utility.cxx @@ -364,8 +364,8 @@ namespace build2 // // Note: make sure keep the above trace if decide not to print. // - diag_record dr; - dr << error (loc) << "process " << args[0] << " " << pe; + diag_record dr (error (loc)); + dr << "process " << args[0] << " " << pe; if (verb >= 1 && verb <= v) { diff --git a/libbuild2/utility.hxx b/libbuild2/utility.hxx index f4fd7bc..99d6806 100644 --- a/libbuild2/utility.hxx +++ b/libbuild2/utility.hxx @@ -73,7 +73,9 @@ namespace build2 using butl::icase_compare_string; using butl::icase_compare_c_string; using butl::lcase; + using butl::make_lcase; using butl::ucase; + using butl::make_ucase; using butl::alpha; using butl::alnum; using butl::digit; @@ -82,6 +84,7 @@ namespace build2 using butl::trim; using butl::next_word; using butl::sanitize_identifier; + using butl::make_sanitized_identifier; using butl::sanitize_strlit; using butl::make_guard; diff --git a/libbuild2/variable.cxx b/libbuild2/variable.cxx index 0abc360..086fa7a 100644 --- a/libbuild2/variable.cxx +++ b/libbuild2/variable.cxx @@ -1872,8 +1872,8 @@ namespace build2 // // Note: the same diagnostics as in $json.serialize(). // - diag_record dr; - dr << fail << "invalid json value: " << e; + diag_record dr (fail); + dr << "invalid json value: " << e; if (e.event) dr << info << "while serializing " << to_string (*e.event); @@ -2099,8 +2099,8 @@ namespace build2 // We will likely be trying to interpret a member name as an integer // due to the incorrect value type so issue appropriate diagnostics. // - diag_record dr; - dr << fail (sloc) << "invalid json value subscript: " << e; + diag_record dr (fail (sloc)); + dr << "invalid json value subscript: " << e; if (jv != nullptr && jv->type != json_type::object) dr << info << "json value type is " << jv->type; diff --git a/libbuild2/variable.txx b/libbuild2/variable.txx index 6e00f89..1d27ec7 100644 --- a/libbuild2/variable.txx +++ b/libbuild2/variable.txx @@ -135,30 +135,34 @@ namespace build2 { size_t n (ns.size ()); - diag_record dr; - if (value_traits<T>::empty_value ? n <= 1 : n == 1) + try { - try + if (value_traits<T>::empty_value ? n <= 1 : n == 1) { + // Throws invalid argument with complete diagnostics, in which case ns + // is not moved from. + // value_traits<T>::assign ( v, (n == 0 ? T () : value_traits<T>::convert (move (ns.front ()), nullptr))); } - catch (const invalid_argument& e) - { - dr << fail << e; - } + else + throw invalid_argument (""); } - else - dr << fail << "invalid " << value_traits<T>::value_type.name - << " value: " << (n == 0 ? "empty" : "multiple names"); - - if (!dr.empty ()) + catch (const invalid_argument& e) { + diag_record dr (fail); + + if (*e.what () != '\0') + dr << e; + else + dr << "invalid " << value_traits<T>::value_type.name << " value: " + << (n == 0 ? "empty value" : "multiple names"); + if (var != nullptr) - dr << " in variable " << var->name; + dr << " in variable " << var->name; // Note: appended to above line. dr << info << "while converting '" << ns << "'"; } @@ -170,30 +174,34 @@ namespace build2 { size_t n (ns.size ()); - diag_record dr; - if (value_traits<T>::empty_value ? n <= 1 : n == 1) + try { - try + if (value_traits<T>::empty_value ? n <= 1 : n == 1) { + // Throws invalid argument with complete diagnostics, in which case ns + // is not moved from. + // value_traits<T>::append ( v, (n == 0 ? T () : value_traits<T>::convert (move (ns.front ()), nullptr))); } - catch (const invalid_argument& e) - { - dr << fail << e; - } + else + throw invalid_argument (""); } - else - dr << fail << "invalid " << value_traits<T>::value_type.name - << " value: " << (n == 0 ? "empty" : "multiple names"); - - if (!dr.empty ()) + catch (const invalid_argument& e) { + diag_record dr (fail); + + if (*e.what () != '\0') + dr << e; + else + dr << "invalid " << value_traits<T>::value_type.name << " value: " + << (n == 0 ? "empty value" : "multiple names"); + if (var != nullptr) - dr << " in variable " << var->name; + dr << " in variable " << var->name; // Note: appended to above line. dr << info << "while converting '" << ns << "'"; } @@ -205,30 +213,34 @@ namespace build2 { size_t n (ns.size ()); - diag_record dr; - if (value_traits<T>::empty_value ? n <= 1 : n == 1) + try { - try + if (value_traits<T>::empty_value ? n <= 1 : n == 1) { + // Throws invalid argument with complete diagnostics, in which case ns + // is not moved from. + // value_traits<T>::prepend ( v, (n == 0 ? T () : value_traits<T>::convert (move (ns.front ()), nullptr))); } - catch (const invalid_argument& e) - { - dr << fail << e; - } + else + throw invalid_argument (""); } - else - dr << fail << "invalid " << value_traits<T>::value_type.name - << " value: " << (n == 0 ? "empty" : "multiple names"); - - if (!dr.empty ()) + catch (const invalid_argument& e) { + diag_record dr (fail); + + if (*e.what () != '\0') + dr << e; + else + dr << "invalid " << value_traits<T>::value_type.name << " value: " + << (n == 0 ? "empty value" : "multiple names"); + if (var != nullptr) - dr << " in variable " << var->name; + dr << " in variable " << var->name; // Note: appended to above line. dr << info << "while converting '" << ns << "'"; } |