From 2df57d72b65012674e6bc64dec66d9b3fd7f993b Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Mon, 14 May 2018 14:10:56 +0300 Subject: Fallback to output directory removal for --keep-out on disfigure failure --- bpkg/archive.cxx | 1 + bpkg/cfg-create.cxx | 5 +- bpkg/diagnostics.cxx | 2 + bpkg/diagnostics.hxx | 9 ++-- bpkg/manifest-utility.cxx | 120 ++++++++++++++++++---------------------------- bpkg/pkg-checkout.cxx | 5 +- bpkg/pkg-command.cxx | 25 ++++++++-- bpkg/pkg-configure.cxx | 2 +- bpkg/pkg-disfigure.cxx | 41 ++++++++++++++-- bpkg/satisfaction.cxx | 63 ++++++++++-------------- bpkg/types.hxx | 8 ++++ bpkg/utility.cxx | 114 ------------------------------------------- bpkg/utility.hxx | 37 ++++---------- bpkg/utility.txx | 98 +++++++++++++++++++++++++++++++++++++ tests/pkg-configure.test | 23 +++++++++ 15 files changed, 286 insertions(+), 267 deletions(-) create mode 100644 bpkg/utility.txx diff --git a/bpkg/archive.cxx b/bpkg/archive.cxx index cc96cd1..8ebd4cd 100644 --- a/bpkg/archive.cxx +++ b/bpkg/archive.cxx @@ -6,6 +6,7 @@ #include +#include #include using namespace std; diff --git a/bpkg/cfg-create.cxx b/bpkg/cfg-create.cxx index 04d97f4..c55623a 100644 --- a/bpkg/cfg-create.cxx +++ b/bpkg/cfg-create.cxx @@ -75,10 +75,9 @@ namespace bpkg // Run quiet. Use path representation to get canonical trailing slash. // run_b (o, - c, - "create('" + c.representation () + "'" + mods + ")", verb_b::quiet, - vars); + vars, + "create('" + c.representation () + "'" + mods + ")"); // Create .bpkg/ and its subdirectories. // diff --git a/bpkg/diagnostics.cxx b/bpkg/diagnostics.cxx index bb175d8..5dd59e5 100644 --- a/bpkg/diagnostics.cxx +++ b/bpkg/diagnostics.cxx @@ -12,6 +12,8 @@ #include #include // operator<<(ostream, process_arg) +#include + using namespace std; using namespace butl; diff --git a/bpkg/diagnostics.hxx b/bpkg/diagnostics.hxx index cbce2f2..ca277d4 100644 --- a/bpkg/diagnostics.hxx +++ b/bpkg/diagnostics.hxx @@ -5,12 +5,13 @@ #ifndef BPKG_DIAGNOSTICS_HXX #define BPKG_DIAGNOSTICS_HXX +#include // forward() + #include #include -#include -#include +#include // Note: not namespace bpkg { @@ -159,7 +160,9 @@ namespace bpkg epilogue_, type_, name_, - location (forward (f), forward (l), forward (c))); + location (std::forward (f), + std::forward (l), + std::forward (c))); } protected: diff --git a/bpkg/manifest-utility.cxx b/bpkg/manifest-utility.cxx index 0f64b30..0e0c792 100644 --- a/bpkg/manifest-utility.cxx +++ b/bpkg/manifest-utility.cxx @@ -6,8 +6,6 @@ #include #include -#include -#include // operator<<(ostream, process_path) #include #include @@ -200,92 +198,68 @@ namespace bpkg optional package_version (const common_options& o, const dir_path& d) { - const char* b (name_b (o)); + fdpipe pipe (open_pipe ()); + + process pr (start_b (o, + pipe, 2 /* stderr */, + verb_b::quiet, + "info:", + d.representation ())); + + // Shouldn't throw, unless something is severely damaged. + // + pipe.out.close (); try { - process_path pp (process::path_search (b, exec_dir)); - - fdpipe pipe (open_pipe ()); - - process pr ( - process_start_callback ( - [] (const char* const args[], size_t n) - { - if (verb >= 2) - print_process (args, n); - }, - 0 /* stdin */, pipe /* stdout */, 2 /* stderr */, - pp, - - verb < 2 - ? strings ({"-q"}) - : verb == 2 - ? strings ({"-v"}) - : strings ({"--verbose", to_string (verb)}), - - o.build_option (), - "info:", - d.representation ())); - - // Shouldn't throw, unless something is severely damaged. - // - pipe.out.close (); + optional r; - try + ifdstream is (move (pipe.in), + fdstream_mode::skip, + ifdstream::badbit); + + for (string l; !eof (getline (is, l)); ) { - optional r; + if (l.compare (0, 9, "version: ") == 0) + try + { + string v (l, 9); - ifdstream is (move (pipe.in), - fdstream_mode::skip, - ifdstream::badbit); + // An empty version indicates that the version module is not + // enabled for the project. + // + if (!v.empty ()) + r = version (v); - for (string l; !eof (getline (is, l)); ) + break; + } + catch (const invalid_argument&) { - if (l.compare (0, 9, "version: ") == 0) - try - { - string v (l, 9); - - // An empty version indicates that the version module is not - // enabled for the project. - // - if (!v.empty ()) - r = version (v); - - break; - } - catch (const invalid_argument&) - { - fail << "no package version in '" << l << "'" << - info << "produced by '" << pp << "'; use --build to override"; - } + fail << "no package version in '" << l << "'" << + info << "produced by '" << name_b (o) << "'; use --build to " + << "override"; } - - is.close (); - - if (pr.wait ()) - return r; - - // Fall through. } - catch (const io_error&) - { - if (pr.wait ()) - fail << "unable to read '" << b << "' output"; - // Fall through. - } + is.close (); - // We should only get here if the child exited with an error status. - // - assert (!pr.wait ()); + if (pr.wait ()) + return r; - fail << "unable to obtain version using '" << b << "'" << endf; + // Fall through. } - catch (const process_error& e) + catch (const io_error&) { - fail << "unable to execute '" << b << "': " << e << endf; + if (pr.wait ()) + fail << "unable to read '" << name_b (o) << "' output"; + + // Fall through. } + + // We should only get here if the child exited with an error status. + // + assert (!pr.wait ()); + + fail << "unable to obtain version using '" << name_b (o) << "'" << endf; } } diff --git a/bpkg/pkg-checkout.cxx b/bpkg/pkg-checkout.cxx index 18ce2b9..e78dd9b 100644 --- a/bpkg/pkg-checkout.cxx +++ b/bpkg/pkg-checkout.cxx @@ -198,10 +198,9 @@ namespace bpkg text << "distributing " << n << '/' << v; run_b (o, - c, - bspec, verb_b::progress, - strings ({"config.dist.root=" + c.representation ()})); + strings ({"config.dist.root=" + c.representation ()}), + bspec); mc = sha256 (o, d / manifest_file); } diff --git a/bpkg/pkg-command.cxx b/bpkg/pkg-command.cxx index b28c99a..bf7107e 100644 --- a/bpkg/pkg-command.cxx +++ b/bpkg/pkg-command.cxx @@ -28,20 +28,39 @@ namespace bpkg l4 ([&]{trace << "command: " << cmd;}); + // Set common vars on the configuration scope. + // + cstrings gvars; + strings lvars; + lvars.reserve (cvars.size ()); + + for (const string& v: cvars) + { + // Don't scope-qualify global variables. + // + if (v[0] == '!') + gvars.push_back (v.c_str ()); + + // Use path representation to get canonical trailing slash. + // + else + lvars.push_back (c.representation () + ':' + v); + } + // This one is a bit tricky: we can only update all the packages at once if // they don't have any package-specific variables. But let's try to handle // this with the same logic (being clever again). // string bspec; - auto run = - [&trace, &c, &o, &cvars, &bspec] ( const strings& vars = strings ()) + auto run = [&trace, &c, &o, &lvars, &gvars, &bspec] ( + const strings& vars = strings ()) { if (!bspec.empty ()) { bspec += ')'; l4 ([&]{trace << "buildspec: " << bspec;}); - run_b (o, c, bspec, verb_b::normal, vars, cvars); + run_b (o, verb_b::normal, gvars, lvars, vars, bspec); bspec.clear (); } }; diff --git a/bpkg/pkg-configure.cxx b/bpkg/pkg-configure.cxx index e351cfd..cfa3d37 100644 --- a/bpkg/pkg-configure.cxx +++ b/bpkg/pkg-configure.cxx @@ -162,7 +162,7 @@ namespace bpkg // try { - run_b (o, c, bspec, verb_b::quiet, vars); + run_b (o, verb_b::quiet, vars, bspec); } catch (const failed&) { diff --git a/bpkg/pkg-disfigure.cxx b/bpkg/pkg-disfigure.cxx index 4a0bd72..0e82f18 100644 --- a/bpkg/pkg-disfigure.cxx +++ b/bpkg/pkg-disfigure.cxx @@ -123,10 +123,45 @@ namespace bpkg // So in this case we just remove the output directory manually // rather then running 'b clean disfigure'. // - if (clean && p->external ()) - rm_r (out_root); + // It may also happen that we can not disfigure the external + // package' output directory (the source directory have moved, etc.). + // If that's the case, then we fallback to the output directory + // removal. + // + if (p->external ()) + { + if (!clean) + { + auto_fd dev_null (open_dev_null ()); + + // Redirect STDERR to /dev/null. Note that we don't expect + // anything to be written to STDOUT. + // + process pr (start_b (o, + 1 /* stdout */, dev_null /* stderr */, + verb_b::quiet, + bspec)); + + // If the disfigure meta-operation failed then we report the + // abnormal termination and fallback to the output directory + // removal otherwise. + // + if (!pr.wait ()) + { + const process_exit& e (*pr.exit); + + if (!e.normal ()) + fail << "process " << name_b (o) << " " << e; + + clean = true; + } + } + + if (clean) + rm_r (out_root); + } else - run_b (o, c, bspec, verb_b::quiet); + run_b (o, verb_b::quiet, bspec); } // Make sure the out directory is gone unless it is the same as src, diff --git a/bpkg/satisfaction.cxx b/bpkg/satisfaction.cxx index 9211ac0..8a61145 100644 --- a/bpkg/satisfaction.cxx +++ b/bpkg/satisfaction.cxx @@ -6,7 +6,6 @@ #include -#include #include #include @@ -100,7 +99,7 @@ namespace bpkg static version build2_version; void - satisfy_build2 (const common_options& co, + satisfy_build2 (const common_options& o, const string& pkg, const dependency& d) { @@ -110,53 +109,43 @@ namespace bpkg // if (build2_version.empty ()) { - const char* args[] = {name_b (co), "--version", nullptr}; + fdpipe pipe (open_pipe ()); - try - { - process_path pp (process::path_search (args[0], exec_dir)); + process pr (start_b (o, + pipe, 2 /* stderr */, + verb_b::quiet, + "--version")); - if (verb >= 3) - print_process (args); + // Shouldn't throw, unless something is severely damaged. + // + pipe.out.close (); - process pr (pp, args, 0, -1); // Redirect STDOUT to pipe. + string l; + try + { + ifdstream is (move (pipe.in), fdstream_mode::skip); + getline (is, l); + is.close (); - string l; - try + if (pr.wait () && l.compare (0, 7, "build2 ") == 0) { - ifdstream is (move (pr.in_ofd), fdstream_mode::skip); - getline (is, l); - is.close (); - - if (pr.wait () && l.compare (0, 7, "build2 ") == 0) + try { - try - { - build2_version = version (string (l, 7)); - } - catch (const invalid_argument&) {} // Fall through. + build2_version = version (string (l, 7)); } - - // Fall through. - } - catch (const io_error&) - { - pr.wait (); - // Fall through. + catch (const invalid_argument&) {} // Fall through. } - if (build2_version.empty ()) - fail << "unable to determine build2 version of " << args[0]; + // Fall through. } - catch (const process_error& e) + catch (const io_error&) { - error << "unable to execute " << args[0] << ": " << e; - - if (e.child) - exit (1); - - throw failed (); + pr.wait (); + // Fall through. } + + if (build2_version.empty ()) + fail << "unable to determine build2 version of " << name_b (o); } if (!satisfies (build2_version, d.constraint)) diff --git a/bpkg/types.hxx b/bpkg/types.hxx index 0433e92..e4e950d 100644 --- a/bpkg/types.hxx +++ b/bpkg/types.hxx @@ -23,6 +23,7 @@ #include #include +#include #include // compare_reference_target #include #include @@ -91,6 +92,13 @@ namespace bpkg using paths = std::vector; using dir_paths = std::vector; + // + // + using butl::process; + using butl::process_path; + using butl::process_exit; + using butl::process_error; + // // using butl::auto_fd; diff --git a/bpkg/utility.cxx b/bpkg/utility.cxx index 693c07e..a990d32 100644 --- a/bpkg/utility.cxx +++ b/bpkg/utility.cxx @@ -277,42 +277,6 @@ namespace bpkg dir_path exec_dir; - void - run (const char* args[], const dir_path& fallback) - { - try - { - process_path pp (process::path_search (args[0], fallback)); - - if (verb >= 2) - print_process (args); - - process pr (pp, args); - - if (pr.wait ()) - return; - - assert (pr.exit); - const process_exit& pe (*pr.exit); - - if (pe.normal ()) - throw failed (); // Assume the child issued diagnostics. - - diag_record dr (fail); - print_process (dr, args); - dr << " " << pe; - } - catch (const process_error& e) - { - error << "unable to execute " << args[0] << ": " << e; - - if (e.child) - exit (1); - - throw failed (); - } - } - const char* name_b (const common_options& co) { @@ -320,82 +284,4 @@ namespace bpkg ? co.build ().string ().c_str () : "b" BPKG_EXE_SUFFIX; } - - void - run_b (const common_options& co, - const dir_path& c, - const string& bspec, - verb_b v, - const strings& pvars, - const strings& cvars) - { - cstrings args {name_b (co)}; - - // Map verbosity level. If we are running quiet or at level 1, - // then run build2 quiet. Otherwise, run it at the same level - // as us. - // - string vl; - - if (verb == 0) - args.push_back ("-q"); - else if (verb == 1) - { - if (v != verb_b::normal) - { - args.push_back ("-q"); - - if (v == verb_b::progress && stderr_term) - args.push_back ("--progress"); - } - } - else if (verb == 2) - args.push_back ("-v"); - else - { - vl = to_string (verb); - args.push_back ("--verbose"); - args.push_back (vl.c_str ()); - } - - // Add user options. - // - for (const string& o: co.build_option ()) - args.push_back (o.c_str ()); - - // Add config vars. - // - strings storage; - storage.reserve (cvars.size ()); - for (const string& v: cvars) - { - // Don't scope-qualify global variables. - // - if (v[0] != '!') - { - // Use path representation to get canonical trailing slash. - // - storage.push_back (c.representation () + ':' + v); - args.push_back (storage.back ().c_str ()); - } - else - args.push_back (v.c_str ()); - } - - for (const string& v: pvars) - args.push_back (v.c_str ()); - - // Add buildspec. - // - args.push_back (bspec.c_str ()); - - args.push_back (nullptr); - - // Use our executable directory as a fallback search since normally the - // entire toolchain is installed into one directory. This way, for - // example, if we installed into /opt/build2 and run bpkg with absolute - // path (and without PATH), then bpkg will be able to find "its" b. - // - run (args, exec_dir); - } } diff --git a/bpkg/utility.hxx b/bpkg/utility.hxx index 85382ef..ae9736d 100644 --- a/bpkg/utility.hxx +++ b/bpkg/utility.hxx @@ -19,6 +19,7 @@ #include #include +#include namespace bpkg { @@ -130,34 +131,14 @@ namespace bpkg auto_fd open_dev_null (); - // Process. - // - // By default the process command line is printed for verbosity >= 2 - // (essential command lines). - // - // If fallback is specified, then this directory is searched for the - // executable as a last resort. - // - void - run (const char* args[], const dir_path& fallback = dir_path ()); - - inline void - run (cstrings& args, const dir_path& fallback = dir_path ()) - { - run (args.data (), fallback); - } - // Directory extracted from argv[0] (i.e., this process' recall directory) // or empty if there is none. Can be used as a search fallback. // extern dir_path exec_dir; // Run build2, mapping verbosity levels. If quiet is true, then run build2 - // quiet if our verbosity level is 1. Common vars (cvars) are set on the - // configuration scope. + // quiet if our verbosity level is 1. // - class common_options; - const char* name_b (const common_options&); @@ -170,13 +151,15 @@ namespace bpkg normal // Run normally (at verbosity 1). }; + template + process + start_b (const common_options&, O&& out, E&& err, verb_b, A&&... args); + + template void - run_b (const common_options&, - const dir_path& configuration, - const string& buildspec, - verb_b = verb_b::normal, - const strings& pvars = strings (), - const strings& cvars = strings ()); + run_b (const common_options&, verb_b, A&&... args); } +#include + #endif // BPKG_UTILITY_HXX diff --git a/bpkg/utility.txx b/bpkg/utility.txx new file mode 100644 index 0000000..f830478 --- /dev/null +++ b/bpkg/utility.txx @@ -0,0 +1,98 @@ +// file : bpkg/utility.txx -*- C++ -*- +// copyright : Copyright (c) 2014-2017 Code Synthesis Ltd +// license : MIT; see accompanying LICENSE file + +#include + +#include + +namespace bpkg +{ + // *_b() + // + template + process + start_b (const common_options& co, + O&& out, + E&& err, + verb_b v, + A&&... args) + { + const char* b (name_b (co)); + + try + { + // Use our executable directory as a fallback search since normally the + // entire toolchain is installed into one directory. This way, for + // example, if we installed into /opt/build2 and run bpkg with absolute + // path (and without PATH), then bpkg will be able to find "its" b. + // + process_path pp (process::path_search (b, exec_dir)); + + butl::small_vector ops; + + // Map verbosity level. If we are running quiet or at level 1, + // then run build2 quiet. Otherwise, run it at the same level + // as us. + // + string vl; + + if (verb == 0) + ops.push_back ("-q"); + else if (verb == 1) + { + if (v != verb_b::normal) + { + ops.push_back ("-q"); + + if (v == verb_b::progress && stderr_term) + ops.push_back ("--progress"); + } + } + else if (verb == 2) + ops.push_back ("-v"); + else + { + vl = to_string (verb); + ops.push_back ("--verbose"); + ops.push_back (vl.c_str ()); + } + + return process_start_callback ( + [] (const char* const args[], size_t n) + { + if (verb >= 2) + print_process (args, n); + }, + 0 /* stdin */, + forward (out), + forward (err), + pp, + ops, + co.build_option (), + forward (args)...); + } + catch (const process_error& e) + { + fail << "unable to execute " << b << ": " << e << endf; + } + } + + template + void + run_b (const common_options& co, verb_b v, A&&... args) + { + process pr ( + start_b (co, 1 /* stdout */, 2 /* stderr */, v, forward (args)...)); + + if (!pr.wait ()) + { + const process_exit& e (*pr.exit); + + if (e.normal ()) + throw failed (); // Assume the child issued diagnostics. + + fail << "process " << name_b (co) << " " << e; + } + } +} diff --git a/tests/pkg-configure.test b/tests/pkg-configure.test index 3b455ff..f950f68 100644 --- a/tests/pkg-configure.test +++ b/tests/pkg-configure.test @@ -383,3 +383,26 @@ if ($cxx.target.class != 'windows') $pkg_purge libbar 2>'purged libbar/1.3.0' } } + +: keep-out +: +{ + : fallback + : + : Test that pkg-disfigure falls back to the external package output directory + : removal if the source directory have gone. + : + { + $clone_root_cfg; + + # Configure libhello as an external package. + # + cp --no-cleanup -r $src/libhello-1.0.0 ./libhello; + $pkg_unpack -e ./libhello; + $* libhello 2>!; + + rm -r ./libhello; + $pkg_disfigure --keep-out libhello 2>'disfigured libhello/1.0.0'; + test -d cfg/libhello != 0 + } +} -- cgit v1.1