From b88ed73c1fa88976e99fa16f989920561f68f302 Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Thu, 8 Nov 2018 22:19:17 +0300 Subject: Add support for pkg-build configuration variables --- bpkg/bpkg.cxx | 89 +++++++++------- bpkg/pkg-build.cli | 19 +++- bpkg/pkg-build.cxx | 253 +++++++++++++++++++++++++++++++++------------ bpkg/pkg-configure.cli | 2 +- tests/pkg-build.testscript | 51 +++++++++ 5 files changed, 307 insertions(+), 107 deletions(-) diff --git a/bpkg/bpkg.cxx b/bpkg/bpkg.cxx index 831b742..018ca7a 100644 --- a/bpkg/bpkg.cxx +++ b/bpkg/bpkg.cxx @@ -75,6 +75,7 @@ static O init (const common_options& co, cli::group_scanner& scan, strings& args, + bool keep_sep, bool tmp) { O o; @@ -91,7 +92,9 @@ init (const common_options& co, // if (strcmp (scan.peek (), "--") == 0) { - scan.next (); + if (!keep_sep) + scan.next (); + opt = false; continue; } @@ -207,7 +210,13 @@ try const common_options& co (o); if (o.help ()) - return help (init (co, scan, argsv, false), "", nullptr); + return help (init (co, + scan, + argsv, + false /* keep_sep */, + false /* tmp */), + "", + nullptr); // The next argument should be a command. // @@ -233,7 +242,11 @@ try if (h) { - ho = init (co, scan, argsv, false); + ho = init (co, + scan, + argsv, + false /* keep_sep */, + false /* tmp */); if (args.more ()) { @@ -273,54 +286,60 @@ try // // if (cmd.pkg_verify ()) // { - // if (h) - // r = help (ho, "pkg-verify", print_bpkg_pkg_verify_usage); - // else - // r = pkg_verify (init (co, scan, argsv, TMP), args); + // if (h) + // r = help (ho, "pkg-verify", print_bpkg_pkg_verify_usage); + // else + // r = pkg_verify (init (co, + // scan, + // argsv, + // false /* keep_sep */, + // true /* tmp */), + // args); // // break; // } // -#define COMMAND_IMPL(NP, SP, CMD, TMP) \ - if (cmd.NP##CMD ()) \ - { \ - if (h) \ - r = help (ho, SP#CMD, print_bpkg_##NP##CMD##_usage); \ - else \ - r = NP##CMD (init (co, scan, argsv, TMP), args); \ - \ - break; \ +#define COMMAND_IMPL(NP, SP, CMD, SEP, TMP) \ + if (cmd.NP##CMD ()) \ + { \ + if (h) \ + r = help (ho, SP#CMD, print_bpkg_##NP##CMD##_usage); \ + else \ + r = NP##CMD (init (co, scan, argsv, SEP, TMP), \ + args); \ + \ + break; \ } // cfg-* commands // -#define CFG_COMMAND(CMD, TMP) COMMAND_IMPL(cfg_, "cfg-", CMD, TMP) +#define CFG_COMMAND(CMD, TMP) COMMAND_IMPL(cfg_, "cfg-", CMD, false, TMP) CFG_COMMAND (create, false); // Temp dir initialized manually. // pkg-* commands // -#define PKG_COMMAND(CMD) COMMAND_IMPL(pkg_, "pkg-", CMD, true) - - PKG_COMMAND (build); - PKG_COMMAND (checkout); - PKG_COMMAND (clean); - PKG_COMMAND (configure); - PKG_COMMAND (disfigure); - PKG_COMMAND (drop); - PKG_COMMAND (fetch); - PKG_COMMAND (install); - PKG_COMMAND (purge); - PKG_COMMAND (status); - PKG_COMMAND (test); - PKG_COMMAND (uninstall); - PKG_COMMAND (unpack); - PKG_COMMAND (update); - PKG_COMMAND (verify); +#define PKG_COMMAND(CMD, SEP) COMMAND_IMPL(pkg_, "pkg-", CMD, SEP, true) + + PKG_COMMAND (build, true); // Keeps the '--' separator in args. + PKG_COMMAND (checkout, false); + PKG_COMMAND (clean, false); + PKG_COMMAND (configure, false); + PKG_COMMAND (disfigure, false); + PKG_COMMAND (drop, false); + PKG_COMMAND (fetch, false); + PKG_COMMAND (install, false); + PKG_COMMAND (purge, false); + PKG_COMMAND (status, false); + PKG_COMMAND (test, false); + PKG_COMMAND (uninstall, false); + PKG_COMMAND (unpack, false); + PKG_COMMAND (update, false); + PKG_COMMAND (verify, false); // rep-* commands // -#define REP_COMMAND(CMD) COMMAND_IMPL(rep_, "rep-", CMD, true) +#define REP_COMMAND(CMD) COMMAND_IMPL(rep_, "rep-", CMD, false, true) REP_COMMAND (add); REP_COMMAND (create); diff --git a/bpkg/pkg-build.cli b/bpkg/pkg-build.cli index c370ac4..f5ece04 100644 --- a/bpkg/pkg-build.cli +++ b/bpkg/pkg-build.cli @@ -12,6 +12,7 @@ namespace bpkg { { " + @@ -19,7 +20,8 @@ namespace bpkg "\h|SYNOPSIS| - \c{\b{bpkg pkg-build}|\b{build} [] [\b{--upgrade}|\b{-u} | \b{--patch}|\b{-p}] ...\n + \c{\b{bpkg pkg-build}|\b{build} [] [\b{--upgrade}|\b{-u} | \b{--patch}|\b{-p}]\n + \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ [... \b{--}] ...\n \b{bpkg pkg-build}|\b{build} [] \ \b{--upgrade}|\b{-u} | \b{--patch}|\b{-p}} \c{ = [](([\b{:}][\b{/}])\b{,}...[\b{@}] | \n @@ -98,6 +100,14 @@ namespace bpkg \l{bpkg-pkg-unpack(1)} commands for more information on the semantics of specifying the package as an archive or a directory. + Additional configuration variables (), if any, should be + specified before packages () and should be separated with + \cb{--}. Such variables are effective only when configuring and only for + packages that were explicitly specified on the command line (they can + also be specified to apply only to specific packages using the argument + grouping mechanism discussed below). See \l{bpkg-pkg-configure(1)} for + more information on configuration variables. + By default a package that is specified explicitly on the command line is built to \i{hold}: it will not be considered for automatic removal if it no longer has any dependents. Only versions from repositories that were @@ -145,9 +155,10 @@ namespace bpkg { "\h|PKG-BUILD PACKAGE OPTIONS| - The following options can be grouped to apply to a specific \ci{pkg-spec} - as well as specified globally, in which case they apply to all the - specified packages (see \l{bpkg-argument-grouping(1)} for details)." + The following options (as well as additional configuration variables) can + be grouped to apply to a specific \ci{pkg-spec} as well as specified + globally, in which case they apply to all the specified packages (see + \l{bpkg-argument-grouping(1)} for details)." // NOTE: if adding a new option here, don't forget to also update // {validate,merge,compare,print}_options() in pkg-build.cxx! diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx index 0793195..80e14f0 100644 --- a/bpkg/pkg-build.cxx +++ b/bpkg/pkg-build.cxx @@ -7,7 +7,7 @@ #include #include #include -#include // strlen() +#include // strlen(), strcmp(), strchr() #include // cout #include // find_if() @@ -303,6 +303,11 @@ namespace bpkg // bool keep_out; + // Command line configuration variables. Only meaningful for non-system + // packages. + // + strings config_vars; + // Set of package names that caused this package to be built or adjusted. // Empty name signifies user selection. // @@ -351,12 +356,13 @@ namespace bpkg { assert (action && *action != drop); - return selected != nullptr && - selected->state == package_state::configured && - ((adjustments & adjust_reconfigure) != 0 || - (*action == build && - (selected->system () != system || - selected->version != available_version ()))); + return selected != nullptr && + selected->state == package_state::configured && + ((adjustments & adjust_reconfigure) != 0 || + (*action == build && + (selected->system () != system || + selected->version != available_version () || + (!system && !config_vars.empty ())))); } const version& @@ -380,7 +386,7 @@ namespace bpkg } // Merge constraints, required-by package names, hold_* flags, - // adjustments, and user-specified options. + // adjustments, and user-specified options/variables. // void merge (build_package&& p) @@ -389,13 +395,21 @@ namespace bpkg // assert (action && *action != drop && (!p.action || *p.action != drop)); - // Copy the user-specified options. + // Copy the user-specified options/variables. // if (p.user_selection ()) { + // We don't allow a package specified on the command line multiple + // times to have different sets of options/variables. + // + assert (!user_selection () || + (keep_out == p.keep_out && config_vars == p.config_vars)); + if (p.keep_out) keep_out = p.keep_out; + config_vars = move (p.config_vars); + // Propagate the user-selection tag. // required_by.insert (package_name ()); @@ -936,13 +950,14 @@ namespace bpkg dsp, dap, rp.second, - nullopt, // Hold package. - nullopt, // Hold version. - {}, // Constraints. + nullopt, // Hold package. + nullopt, // Hold version. + {}, // Constraints. system, - false, // Keep output directory. - {name}, // Required by (dependent). - 0}; // Adjustments. + false, // Keep output directory. + strings (), // Configuration variables. + {name}, // Required by (dependent). + 0}; // Adjustments. // Add our constraint, if we have one. // @@ -1024,13 +1039,14 @@ namespace bpkg move (sp), nullptr, nullptr, - nullopt, // Hold package. - nullopt, // Hold version. - {}, // Constraints. - false, // System package. - false, // Keep output directory. - {}, // Required by. - 0}; // Adjustments. + nullopt, // Hold package. + nullopt, // Hold version. + {}, // Constraints. + false, // System package. + false, // Keep output directory. + strings (), // Configuration variables. + {}, // Required by. + 0}; // Adjustments. auto i (map_.find (nm)); @@ -1071,12 +1087,13 @@ namespace bpkg sp, nullptr, nullptr, - nullopt, // Hold package. - nullopt, // Hold version. - {}, // Constraints. - false, // System package. - false, // Keep output directory. - {}, // Required by. + nullopt, // Hold package. + nullopt, // Hold version. + {}, // Constraints. + false, // System package. + false, // Keep output directory. + strings (), // Configuration variables. + {}, // Required by. build_package::adjust_unhold}; p.merge (move (bp)); @@ -1416,6 +1433,7 @@ namespace bpkg {}, // Constraints. system, false, // Keep output directory. + strings (), // Configuration variables. {n}, // Required by (dependency). build_package::adjust_reconfigure}; }; @@ -1566,8 +1584,9 @@ namespace bpkg bpkg::version version; // Empty if unspecified. shared_ptr selected; // NULL if not present. bool system; - bool patch; // Meaningful for an empty version. + bool patch; // Only for an empty version. bool keep_out; + strings config_vars; // Only if not system. }; using dependency_packages = vector; @@ -2261,7 +2280,8 @@ namespace bpkg session ses; // Preparse the (possibly grouped) package specs splitting them into the - // packages and location parts, and also parsing their options. + // packages and location parts, and also parsing their options and + // configuration variables. // // Also collect repository locations for the subsequent fetch, suppressing // duplicates. Note that the last repository location overrides the @@ -2272,10 +2292,54 @@ namespace bpkg string packages; repository_location location; pkg_options options; + strings config_vars; }; vector specs; { + // Parse the global configuration variables until we reach the "--" + // separator, eos or an argument. Non-empty variables list should always + // be terminated with the "--". Furthermore, argument list that contains + // anything that looks like a variable (has the '=' character) should be + // preceded with "--". + // + strings config_vars; + bool vars_separated (false); + + while (args.more ()) + { + const char* a (args.peek ()); + + // If we see the "--" separator, then we are done parsing variables. + // + if (strcmp (a, "--") == 0) + { + vars_separated = true; + args.next (); + break; + } + + // Bail out if arguments have started. We will perform the validation + // later (together with the eos case). + // + if (strchr (a, '=') == nullptr) + break; + + string v (args.next ()); + + // Make sure this is not an argument having an option group. + // + if (args.group ().more ()) + fail << "unexpected options group for configuration variable '" + << v << "'"; + + config_vars.push_back (move (v)); + } + + if (!config_vars.empty () && !vars_separated) + fail << "configuration variables must be separated from packages " + << "with '--'"; + vector locations; transaction t (db); @@ -2284,6 +2348,13 @@ namespace bpkg { string a (args.next ()); + // Make sure the argument can not be misinterpreted as a configuration + // variable. + // + if (a.find ('=') != string::npos && !vars_separated) + fail << "unexpected configuration variable '" << a << "'" << + info << "use the '--' separator to treat it as a package"; + specs.emplace_back (); pkg_spec& ps (specs.back ()); @@ -2291,9 +2362,22 @@ namespace bpkg { auto& po (ps.options); - po.parse (args.group (), - cli::unknown_mode::fail, - cli::unknown_mode::fail); + cli::scanner& ag (args.group ()); + po.parse (ag, cli::unknown_mode::fail, cli::unknown_mode::stop); + + // Merge the global and package-specific configuration variables + // (globals go first). + // + ps.config_vars = config_vars; + + while (ag.more ()) + { + string a (ag.next ()); + if (a.find ('=') == string::npos) + fail << "unexpected group argument '" << a << "'"; + + ps.config_vars.push_back (move (a)); + } // We have to manually merge global options into local since just // initializing local with global and then parsing local may end up @@ -2399,7 +2483,7 @@ namespace bpkg // Expand the package specs into individual package args, parsing them // into the package scheme, name, and version components, and also saving - // associated options. + // associated options and configuration variables. // // Note that the package specs that have no scheme and location cannot be // unambiguously distinguished from the package archive and directory @@ -2413,16 +2497,18 @@ namespace bpkg bpkg::version version; string value; pkg_options options; + strings config_vars; }; // Create the parsed package argument. // auto arg_package = [] (package_scheme sc, package_name nm, - version v, - pkg_options o) -> pkg_arg + version vr, + pkg_options os, + strings vs) -> pkg_arg { - pkg_arg r {sc, move (nm), move (v), string (), move (o)}; + pkg_arg r {sc, move (nm), move (vr), string (), move (os), move (vs)}; switch (sc) { @@ -2448,10 +2534,14 @@ namespace bpkg // Create the unparsed package argument. // - auto arg_raw = [] (string v, pkg_options o) -> pkg_arg + auto arg_raw = [] (string v, pkg_options os, strings vs) -> pkg_arg { - return pkg_arg { - package_scheme::none, package_name (), version (), move (v), move (o)}; + return pkg_arg {package_scheme::none, + package_name (), + version (), + move (v), + move (os), + move (vs)}; }; auto arg_parsed = [] (const pkg_arg& a) {return !a.name.empty ();}; @@ -2478,8 +2568,18 @@ namespace bpkg { string s (print_options (a.options, false)); - if (!s.empty ()) - r += " +{ " + s + " }"; + if (!s.empty () || !a.config_vars.empty ()) + { + r += " +{ "; + + if (!s.empty ()) + r += s + ' '; + + for (const string& v: a.config_vars) + r += v + ' '; + + r += '}'; + } } return r; @@ -2504,12 +2604,16 @@ namespace bpkg package_name n (parse_package_name (s)); version v (parse_package_version (s)); - pkg_args.push_back ( - arg_package (h, move (n), move (v), move (ps.options))); + pkg_args.push_back (arg_package (h, + move (n), + move (v), + move (ps.options), + move (ps.config_vars))); } else // Add unparsed. - pkg_args.push_back ( - arg_raw (move (ps.packages), move (ps.options))); + pkg_args.push_back (arg_raw (move (ps.packages), + move (ps.options), + move (ps.config_vars))); continue; } @@ -2600,7 +2704,7 @@ namespace bpkg // Populate the argument list with the latest package versions. // - // Don't move options as they may be reused. + // Don't move options and variables as they may be reused. // for (auto& pv: pvs) { @@ -2611,7 +2715,8 @@ namespace bpkg pkg_args.push_back (arg_package (package_scheme::none, pv.first, move (pv.second), - ps.options)); + ps.options, + ps.config_vars)); } } else // Packages with optional versions in the coma-separated list. @@ -2713,10 +2818,13 @@ namespace bpkg if (v.empty () && sc != package_scheme::sys) v = ap->version; - // Don't move options as they may be reused. + // Don't move options and variables as they may be reused. // - pkg_args.push_back ( - arg_package (sc, move (n), move (v), ps.options)); + pkg_args.push_back (arg_package (sc, + move (n), + move (v), + ps.options, + ps.config_vars)); } } } @@ -2748,11 +2856,14 @@ namespace bpkg const pkg_arg& a (r.first->second); assert (arg_parsed (a)); + // Note that the variable order may matter. + // if (!r.second && - (a.scheme != pa.scheme || - a.name != pa.name || - a.version != pa.version || - !compare_options (a.options, pa.options))) + (a.scheme != pa.scheme || + a.name != pa.name || + a.version != pa.version || + !compare_options (a.options, pa.options) || + a.config_vars != pa.config_vars)) fail << "duplicate package " << pa.name << info << "first mentioned as " << arg_string (r.first->second) << info << "second mentioned as " << arg_string (pa); @@ -2819,7 +2930,8 @@ namespace bpkg pa = arg_package (package_scheme::none, m.name, m.version, - move (pa.options)); + move (pa.options), + move (pa.config_vars)); af = root; ap = make_shared (move (m)); @@ -2897,7 +3009,8 @@ namespace bpkg pa = arg_package (package_scheme::none, m.name, m.version, - move (pa.options)); + move (pa.options), + move (pa.config_vars)); ap = make_shared (move (m)); af = root; @@ -2946,7 +3059,8 @@ namespace bpkg pa = arg_package (package_scheme::none, move (n), move (v), - move (pa.options)); + move (pa.options), + move (pa.config_vars)); } l4 ([&]{trace << "package: " << arg_string (pa);}); @@ -3058,7 +3172,8 @@ namespace bpkg move (sp), sys, pa.options.patch (), - pa.options.keep_out ()}); + pa.options.keep_out (), + move (pa.config_vars)}); continue; } @@ -3221,6 +3336,7 @@ namespace bpkg {}, // Constraints. arg_sys (pa), keep_out, + move (pa.config_vars), {package_name ()}, // Required by (command line). 0}; // Adjustments. @@ -3303,13 +3419,14 @@ namespace bpkg move (sp), move (ap), move (apr.second), - true, // Hold package. - false, // Hold version. - {}, // Constraints. - false, // System package. + true, // Hold package. + false, // Hold version. + {}, // Constraints. + false, // System package. keep_out, - {package_name ()}, // Required by (command line). - 0}; // Adjustments. + strings (), // Configuration variables. + {package_name ()}, // Required by (command line). + 0}; // Adjustments. l4 ([&]{trace << "stashing held package " << p.available_name_version ();}); @@ -3430,6 +3547,7 @@ namespace bpkg {}, // Constraints. p.system, p.keep_out, + p.config_vars, {package_name ()}, // Required by (command line). 0}; // Adjustments. @@ -3501,6 +3619,7 @@ namespace bpkg {}, // Constraints. d.system, keep_out, + strings (), // Configuration vars. {package_name ()}, // Required by (command line). 0}; // Adjustments. @@ -4386,7 +4505,7 @@ namespace bpkg if (p.system) sp = pkg_configure_system (ap->id.name, p.available_version (), t); else if (ap != nullptr) - pkg_configure (c, o, t, sp, ap->dependencies, strings (), simulate); + pkg_configure (c, o, t, sp, ap->dependencies, p.config_vars, simulate); else // Dependent. { // Must be in the unpacked state since it was disfigured on the first @@ -4395,7 +4514,7 @@ namespace bpkg assert (sp->state == package_state::unpacked); package_manifest m (pkg_verify (sp->effective_src_root (c), true)); - pkg_configure (c, o, t, sp, m.dependencies, strings (), simulate); + pkg_configure (c, o, t, sp, m.dependencies, p.config_vars, simulate); } assert (sp->state == package_state::configured); diff --git a/bpkg/pkg-configure.cli b/bpkg/pkg-configure.cli index 27977d7..fd4453d 100644 --- a/bpkg/pkg-configure.cli +++ b/bpkg/pkg-configure.cli @@ -11,7 +11,7 @@ include ; namespace bpkg { { - " ", + " ", "\h|SYNOPSIS| diff --git a/tests/pkg-build.testscript b/tests/pkg-build.testscript index 9c931bb..3834b6f 100644 --- a/tests/pkg-build.testscript +++ b/tests/pkg-build.testscript @@ -2569,6 +2569,57 @@ rep_fetch += -d cfg --auth all --trust-yes 2>! } } +: config-vars +: +{ + test.arguments += --yes + + +$cfg_create cxx "config.cxx=$config.cxx" -d cfg 2>- &cfg/*** + +cp -r $src/libhello-1.0.0 ./libhello + +$rep_add libhello --type dir + +$rep_fetch + + : vars-global-local + : + { + $clone_cfg; + + $* config.cxx.poptions=-DG -- libhello +{ config.cxx.poptions+=-DL } 2>>~%EOE%; + using libhello/1.0.0 (external) + configured libhello/1.0.0 + %(mkdir|c\+\+|ar|ld) .+%{8} + updated libhello/1.0.0 + EOE + + cat cfg/libhello/build/config.build >>~%EOO%; + %.* + config.cxx.poptions = -DG -DL + EOO + + $pkg_drop libhello + } + + : vars-options + : + { + $clone_cfg; + + $* config.cxx.poptions=-DG --fetch-timeout=60 -- libhello 2>>~%EOE%; + using libhello/1.0.0 (external) + configured libhello/1.0.0 + %(mkdir|c\+\+|ar|ld) .+%{8} + updated libhello/1.0.0 + EOE + + cat cfg/libhello/build/config.build >>~%EOO%; + %.* + config.cxx.poptions = -DG + EOO + + $pkg_drop libhello + } +} + : patch : { -- cgit v1.1