From 1fd63551d1eea54b65dd1d4cbb365c85876a1f19 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Thu, 13 Apr 2023 16:18:10 +0200 Subject: Improve diagnostics --- bpkg/pkg-build.cxx | 28 ++++++++++++---------------- bpkg/pkg-configure.cxx | 39 +++++++++++++++++++++++++++++++-------- bpkg/utility.hxx | 5 +++++ bpkg/utility.txx | 32 ++++++++++++++++++++++++++++++++ 4 files changed, 80 insertions(+), 24 deletions(-) diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx index 54f5a37..da43ef6 100644 --- a/bpkg/pkg-build.cxx +++ b/bpkg/pkg-build.cxx @@ -5371,7 +5371,7 @@ namespace bpkg vector configure_packages; configure_packages.reserve (build_pkgs.size ()); - // While at it also extract global configuration variable overrides from + // While at it also collect global configuration variable overrides from // each configure_prerequisites_result::config_variables and merge them // into configure_global_vars. // @@ -5575,42 +5575,35 @@ namespace bpkg t.commit (); - /* - if (!simulate && !cpr.config_variables.empty ()) + if (verb >= 5 && !simulate && !cpr.config_variables.empty ()) { - diag_record dr (text); + diag_record dr (trace); - dr << sp->name << pdb << ':'; + dr << sp->name << pdb << " configuration variables:"; for (const string& cv: cpr.config_variables) dr << "\n " << cv; } - */ if (!simulate) { - #ifndef BPKG_OUTPROC_CONFIGURE auto& gvs (configure_global_vars); - for (auto i (cpr.config_variables.begin ()); - i != cpr.config_variables.end (); ) + // Note that we keep global overrides in cpr.config_variables for + // diagnostics and skip them in var_override_function below. + // + for (const string& v: cpr.config_variables) { // Each package should have exactly the same set of global // overrides by construction since we don't allow package- // specific global overrides. // - string& v (*i); - if (v[0] == '!') { if (find (gvs.begin (), gvs.end (), v) == gvs.end ()) - gvs.push_back (move (v)); - - i = cpr.config_variables.erase (i); + gvs.push_back (v); } - else - ++i; } #endif // Add config.config.disfigure unless already disfigured (see the @@ -5646,6 +5639,9 @@ namespace bpkg { for (const string& v: cp.res.config_variables) { + if (v[0] == '!') // Skip global overrides (see above). + continue; + pair p ( ctx.parse_variable_override (v, i++, false /* buildspec */)); diff --git a/bpkg/pkg-configure.cxx b/bpkg/pkg-configure.cxx index ccf2f54..83ec245 100644 --- a/bpkg/pkg-configure.cxx +++ b/bpkg/pkg-configure.cxx @@ -287,6 +287,8 @@ namespace bpkg od = sp->effective_out_root (pdb.config); #ifndef BPKG_OUTPROC_CONFIGURE + // @@ + // // Use global overrides to recreate the original behavior of // not warning about unused config.import.* variables // (achived via the config.config.persist value in @@ -294,12 +296,17 @@ namespace bpkg // don't actually save the unused values anywhere, just // don't warn about them). // - // @@ Can we somehow cause a clash, say if the same package - // comes from different configurations? Yeah, we probably - // can. Maybe detect a clash somehow and "fallforward" to - // the correct behavior? + // Can we somehow cause a clash, say if the same package + // comes from different configurations? Yeah, we probably + // can. So could add it as undermined (?), detect a clash, + // and "fallforward" to the correct behavior. + // + // But we can clash with an absent value -- that is, we + // force importing from a wrong configuration where without + // any import things would have been found in the same + // amalgamation. // - vars.push_back ("!config.import." + sp->name.variable () + + vars.push_back ("config.import." + sp->name.variable () + "='" + od.representation () + '\''); #else vars.push_back ("config.import." + sp->name.variable () + @@ -533,8 +540,24 @@ namespace bpkg throw; } #else - // @@ Would be good to print the equivalent command line in -v. + // Print the out-process command line in the verbose mode. // + if (verb >= 2) + { + string bspec; + + // Use path representation to get canonical trailing slash. + // + if (src_root == out_root) + bspec = "configure('" + out_root.representation () + "')"; + else + bspec = "configure('" + + src_root.representation () + "'@'" + + out_root.representation () + "')"; + + print_b (o, verb_b::quiet, cpr.config_variables, bspec); + } + try { // Note: no bpkg::failed should be thrown from this block. @@ -585,9 +608,9 @@ namespace bpkg if (src_root != p) { - // @@ fuzzy if need this or can do as package skeleton (seeing + // @@ Fuzzy if need this or can do as package skeleton (seeing // that we know we are re-configuring). - + // ctx.new_src_root = src_root; ctx.old_src_root = move (p); p = src_root; diff --git a/bpkg/utility.hxx b/bpkg/utility.hxx index bb264ba..f36185a 100644 --- a/bpkg/utility.hxx +++ b/bpkg/utility.hxx @@ -66,6 +66,7 @@ namespace bpkg // // using butl::process_start_callback; + using butl::process_print_callback; // // @@ -254,6 +255,10 @@ namespace bpkg process_path search_b (const common_options&); + template + void + print_b (const common_options&, verb_b, A&&... args); + template process start_b (const common_options&, O&& out, E&& err, verb_b, A&&... args); diff --git a/bpkg/utility.txx b/bpkg/utility.txx index 8bdd7ec..33bb711 100644 --- a/bpkg/utility.txx +++ b/bpkg/utility.txx @@ -59,6 +59,36 @@ namespace bpkg ops.push_back ("--no-progress"); } + template + void + print_b (const common_options& co, verb_b v, A&&... args) + { + process_path pp (search_b (co)); + + small_vector ops; + + // As in start_b() below. + // + string verb_arg; + map_verb_b (co, v, ops, verb_arg); + + if (co.diag_color ()) + ops.push_back ("--diag-color"); + + if (co.no_diag_color ()) + ops.push_back ("--no-diag-color"); + + process_print_callback ( + [] (const char* const args[], size_t n) + { + print_process (args, n); + }, + pp, + ops, + co.build_option (), + forward (args)...); + } + template process start_b (const common_options& co, @@ -73,6 +103,8 @@ namespace bpkg { small_vector ops; + // NOTE: see print_b() above if changing anything here. + // // NOTE: see custom versions in system_package_manager* if adding // anything new here (search for search_b()). -- cgit v1.1