diff options
| -rw-r--r-- | bpkg/bpkg.cxx | 31 | ||||
| -rw-r--r-- | bpkg/package-skeleton.cxx | 707 | ||||
| -rw-r--r-- | bpkg/package-skeleton.hxx | 96 | ||||
| -rw-r--r-- | bpkg/pkg-build.cxx | 131 | ||||
| -rw-r--r-- | bpkg/pkg-configure.cxx | 44 | ||||
| -rw-r--r-- | bpkg/pkg-configure.hxx | 3 | ||||
| -rw-r--r-- | build/root.build | 3 | ||||
| -rw-r--r-- | tests/common/dependency-alternatives/t8a/fax-1.0.0.tar.gz | bin | 0 -> 584 bytes | |||
| -rw-r--r-- | tests/common/dependency-alternatives/t8a/libbiz-1.0.0.tar.gz | bin | 0 -> 347 bytes | |||
| -rw-r--r-- | tests/common/dependency-alternatives/t8a/libbox-1.0.0.tar.gz | bin | 0 -> 348 bytes | |||
| -rw-r--r-- | tests/pkg-build.testscript | 223 |
11 files changed, 1037 insertions, 201 deletions
diff --git a/bpkg/bpkg.cxx b/bpkg/bpkg.cxx index 6ec376b..03e63ea 100644 --- a/bpkg/bpkg.cxx +++ b/bpkg/bpkg.cxx @@ -16,16 +16,21 @@ // #include <libbuild2/types.hxx> #include <libbuild2/utility.hxx> +#include <libbuild2/module.hxx> #include <libbuild2/context.hxx> #include <libbuild2/scheduler.hxx> #include <libbuild2/file-cache.hxx> +#include <libbuild2/dist/init.hxx> +#include <libbuild2/test/init.hxx> +#include <libbuild2/config/init.hxx> +#include <libbuild2/install/init.hxx> + #include <libbuild2/in/init.hxx> #include <libbuild2/bin/init.hxx> #include <libbuild2/c/init.hxx> #include <libbuild2/cc/init.hxx> #include <libbuild2/cxx/init.hxx> -#include <libbuild2/bash/init.hxx> #include <libbuild2/version/init.hxx> #include <bpkg/types.hxx> @@ -419,6 +424,7 @@ init (const char* argv0, // Build system driver. // if (bsys) + try { // For now we use the same verbosity as us (equivalent to start_b() with // verb_b::normal). @@ -443,12 +449,19 @@ init (const char* argv0, nullopt /* config_sub */, nullopt /* config_guess */); - build2::bin::build2_bin_load (); - build2::cc::build2_cc_load (); - build2::c::build2_c_load (); - build2::cxx::build2_cxx_load (); - build2::version::build2_version_load (); - build2::in::build2_in_load (); + using build2::load_builtin_module; + + load_builtin_module (&build2::config::build2_config_load); + load_builtin_module (&build2::dist::build2_dist_load); + load_builtin_module (&build2::test::build2_test_load); + load_builtin_module (&build2::install::build2_install_load); + + load_builtin_module (&build2::bin::build2_bin_load); + load_builtin_module (&build2::cc::build2_cc_load); + load_builtin_module (&build2::c::build2_c_load); + load_builtin_module (&build2::cxx::build2_cxx_load); + load_builtin_module (&build2::version::build2_version_load); + load_builtin_module (&build2::in::build2_in_load); // Serial execution. // @@ -461,6 +474,10 @@ init (const char* argv0, build2_mutexes.init (build2_sched.shard_size ()); build2_fcache.init (true); } + catch (const build2::failed&) + { + throw failed (); // Assume the diagnostics has already been issued. + } return o; } diff --git a/bpkg/package-skeleton.cxx b/bpkg/package-skeleton.cxx index d789f60..cd79222 100644 --- a/bpkg/package-skeleton.cxx +++ b/bpkg/package-skeleton.cxx @@ -3,11 +3,22 @@ #include <bpkg/package-skeleton.hxx> +#include <sstream> + +#include <libbutl/manifest-parser.hxx> #include <libbutl/manifest-serializer.hxx> #include <libbuild2/types.hxx> #include <libbuild2/utility.hxx> + +#include <libbuild2/file.hxx> +#include <libbuild2/scope.hxx> #include <libbuild2/context.hxx> +#include <libbuild2/variable.hxx> +#include <libbuild2/operation.hxx> + +#include <libbuild2/lexer.hxx> +#include <libbuild2/parser.hxx> #include <bpkg/package.hxx> #include <bpkg/database.hxx> @@ -31,7 +42,6 @@ namespace bpkg package_skeleton:: package_skeleton (package_skeleton&& v) - : db_ (v.db_), available_ (v.available_) { *this = move (v); } @@ -46,10 +56,15 @@ namespace bpkg config_vars_ = move (v.config_vars_); src_root_ = move (v.src_root_); out_root_ = move (v.out_root_); - ctx_ = move (v.ctx_); created_ = v.created_; - dirty_ = v.dirty_; - reflect_ = move (v.reflect_); + ctx_ = move (v.ctx_); + rs_ = v.rs_; + reflect_names_ = move (v.reflect_names_); + reflect_vars_ = move (v.reflect_vars_); + reflect_frag_ = move (v.reflect_frag_); + + v.db_ = nullptr; + v.available_ = nullptr; } return *this; @@ -62,77 +77,548 @@ namespace bpkg config_vars_ (v.config_vars_), src_root_ (v.src_root_), out_root_ (v.out_root_), - created_ (v.created_) + created_ (v.created_), + reflect_names_ (v.reflect_names_), + reflect_vars_ (v.reflect_vars_), + reflect_frag_ (v.reflect_frag_) { - // The idea here is to create an unloaded copy but with enough state that - // it can be loaded if necessary. - - if (v.ctx_ != nullptr) - { - // @@ extract reflection - } - else - { - // @@ TODO: copy already extracted? - reflect_ = v.reflect_; - } + // The idea here is to create an "unloaded" copy but with enough state + // that it can be loaded if necessary. } package_skeleton:: package_skeleton (database& db, const available_package& ap, - const strings& cvs, - optional<dir_path> src_root) - : db_ (db), available_ (ap), config_vars_ (cvs) + strings cvs, + optional<dir_path> src_root, + optional<dir_path> out_root) + : db_ (&db), available_ (&ap), config_vars_ (move (cvs)) { // Should not be created for stubs. // - assert (available_.get ().bootstrap_build); + assert (available_->bootstrap_build); if (src_root) { src_root_ = move (*src_root); - out_root_ = dir_path (db_.get ().config_orig) /= name ().string (); + + if (out_root) + out_root_ = move (*out_root); + } + else + assert (!out_root); + } + + // Print the location of a depends value in the specified manifest file. + // + // Note that currently we only use this function for the external packages. + // We could also do something similar for normal packages by pointing to the + // manifest we have serialized. In this case we would also need to make sure + // the temp directory is not cleaned in case of an error. Maybe one day. + // + static void + depends_location (const diag_record& dr, + const path& mf, + size_t depends_index) + { + // Note that we can't do much on the manifest parsing failure and just + // skip printing the location in this case. + // + try + { + ifdstream is (mf); + manifest_parser p (is, mf.string ()); + + manifest_name_value nv (p.next ()); + if (nv.name.empty () && nv.value == "1") + { + size_t i (0); + for (nv = p.next (); !nv.empty (); nv = p.next ()) + { + if (nv.name == "depends" && i++ == depends_index) + { + dr << info (location (p.name (), + nv.value_line, + nv.value_column)) + << "depends value defined here"; + break; + } + } + } + } + catch (const manifest_parsing&) {} + catch (const io_error&) {} + } + + bool package_skeleton:: + evaluate_enable (const string& cond, size_t depends_index) + { + try + { + using namespace build2; + using build2::fail; + using build2::endf; + + scope& rs (load ()); + + istringstream is ('(' + cond + ')'); + is.exceptions (istringstream::failbit | istringstream::badbit); + + // Location is tricky: theoretically we can point to the exact position + // of an error but that would require quite hairy and expensive manifest + // re-parsing. The really bad part is that all this effort will be + // wasted in the common "no errors" cases. So instead we do this + // re-parsing lazily from the diag frame. + // + path_name in ("<depends-enable-clause>"); + uint64_t il (1); + + auto df = build2::make_diag_frame ( + [&cond, &rs, depends_index] (const diag_record& dr) + { + dr << info << "enable condition: (" << cond << ")"; + + // For external packages we have the manifest so print the location + // of the depends value in questions. + // + if (!rs.out_eq_src ()) + depends_location (dr, + rs.src_path () / manifest_file, + depends_index); + }); + + lexer l (is, in, il /* start line */); + parser p (rs.ctx); + value v (p.parse_eval (l, rs, rs, parser::pattern_mode::expand)); + + try + { + // Should evaluate to 'true' or 'false'. + // + return build2::convert<bool> (move (v)); + } + catch (const invalid_argument& e) + { + fail (build2::location (in, il)) << e << endf; + } + } + catch (const build2::failed&) + { + throw failed (); // Assume the diagnostics has already been issued. } } void package_skeleton:: + evaluate_reflect (const string& refl, size_t depends_index) + { + // The reflect configuration variables are essentially overrides that will + // be passed on the command line when we configure the package. They could + // clash with configuration variables specified by the user (config_vars_) + // and it feels like user values should take precedence. Though one could + // also argue we should diagnose this case and fail not to cause more + // confusion. + // + // It seems like the most straightforward way to achieve the desired + // semantics with the mechanisms that we have (in other words, without + // inventing another "level" of overrides) is to evaluate the reflect + // fragment after loading root.build. This way it will (1) be able to use + // variables set by root.build in conditions, (2) override default values + // of configuration variables (and those loaded from config.build), and + // (3) be overriden by configuration variables specified by the user. + // Naturally, this approach is not without a few corner cases: + // + // 1. Append in the reflect clause may not produce the desired result + // (i.e., it will append to the default value in root.build) rather + // than overriding it, as would have happen if it were a real variable + // override. + // + // config.hello.x ?= 1 # root.build + // config.hello.x += 2 # reflect clause + // + // We may also have configuration values from the previous reflect clause + // which we want to "factor in" before evaluating the next enable or + // reflect clauses (so that they can use the previously reflect values or + // values that are derived from them in root.build). It seems like we have + // two options here: either enter them as true overrides similar to + // config_vars_ or just evaluate them similar to loading config.build + // (which, BTW, we might have, in case of an external package). The big + // problem with the former approach is that it will then prevent any + // further reflect clauses from modifying the same values. + // + // So overall it feels like we have iterative/compartmentalized + // configuration process. A feedback loop, in a sense. And it's the + // responsibility of the package author (who is in control of both + // root.build and manifest) to arrange for suitable compartmentalization. + // + // BTW, a plan B would be to restrict reflect to just config vars in which + // case we could merge them with true overrides. Though how exactly would + // we do this merging is unclear. + // + try + { + // Note: similar in many respects to evaluate_enable(). + // + using namespace build2; + using build2::fail; + using build2::endf; + + scope& rs (load ()); + + istringstream is (refl); + is.exceptions (istringstream::failbit | istringstream::badbit); + + path_name in ("<depends-reflect-clause>"); + uint64_t il (1); + + auto df = build2::make_diag_frame ( + [&refl, &rs, depends_index] (const diag_record& dr) + { + // Probably safe to assume a one-line fragment contains a variable + // assignment. + // + if (refl.find ('\n') == string::npos) + dr << info << "reflect variable: " << refl; + else + dr << info << "reflect fragment:\n" + << refl; + + // For external packages we have the manifest so print the location + // of the depends value in questions. + // + if (!rs.out_eq_src ()) + depends_location (dr, + rs.src_path () / manifest_file, + depends_index); + }); + + // Note: a lot of this code is inspired by the config module. + // + + // Collect all the config.<name>.* variables on the first pass and + // filter out unchanged on the second. + // + auto& vp (rs.var_pool ()); + const variable& ns (vp.insert ("config." + name ().variable ())); + + struct value_data + { + const value* val; + size_t ver; + }; + + map<const variable*, value_data> vars; + + auto process = [&rs, &ns, &vars] (bool collect) + { + for (auto p (rs.vars.lookup_namespace (ns)); + p.first != p.second; + ++p.first) + { + const variable* var (&p.first->first.get ()); + + // This can be one of the overrides (__override, __prefix, etc), + // which we skip. + // + if (var->override ()) + continue; + + // What happens to version if overriden? A: appears to be still + // incremented! + // + const variable_map::value_data& val (p.first->second); + + if (collect) + { + vars.emplace (var, value_data {nullptr, val.version}); + } + else + { + auto i (vars.find (var)); + + if (i != vars.end ()) + { + if (i->second.ver == val.version) + vars.erase (i); // Unchanged. + else + i->second.val = &val; + } + else + vars.emplace (var, value_data {&val, 0}); + } + } + }; + + process (true); + + lexer l (is, in, il /* start line */); + parser p (rs.ctx); + p.parse_buildfile (l, &rs, rs); + + process (false); + + // Add to the map the reflect variables collected previously. + // + for (string& n: reflect_names_) + { + auto p (vars.emplace (&vp.insert (move (n)), value_data {nullptr, 0})); + + if (p.second) + { + // The value got to be there since it's set by the accumulated + // fragment we've evaluated before root.build. + // + p.first->second.val = rs.vars[p.first->first].value; + } + } + + // Re-populate everything from the map. + // + reflect_names_.clear (); + reflect_frag_.clear (); +#if 0 + reflect_vars_.clear (); +#else + reflect_vars_ = config_vars_; +#endif + + // Collect the config.<name>.* variables that were changed by this + // and previous reflect clauses. + // + // Specifically, we update both the accumulated fragment to be evaluated + // before root.build on the next load (if any) as well as the merged + // configuration variable overrides to be passed during the package + // configuration. Doing both of these now (even though one of them won't + // be needed) allows us to immediately drop the build context and + // release its memory. It also makes the implementation a bit simpler + // (see, for example, the copy constructor). + // + names storage; + for (const auto& p: vars) + { + const variable& var (*p.first); + const value& val (*p.second.val); + + reflect_names_.push_back (var.name); + + // For the accumulated fragment we always save the original and let + // the standard overriding take its course. + // + reflect_frag_ += var.name; + reflect_frag_ += " = "; + + if (val.null) + reflect_frag_ += "[null]"; + else + { + storage.clear (); + names_view nv (reverse (val, storage)); + + if (!nv.empty ()) + { + ostringstream os; + to_stream (os, nv, quote_mode::normal, '@'); + reflect_frag_ += os.str (); + } + } + + reflect_frag_ += '\n'; + + // For the accumulated overrides we have to merge user config_vars_ + // with reflect values. Essentially, we have three possibilities: + // + // 1. There is no corresponding reflect value for a user value. In + // this case we just copy over the user value. + // + // 2. There is no corresponding user value for a reflect value. In + // this case we just copy over the reflect value. + // + // 3. There are both reflect and user values. In this case we replace + // the user value with the final (overriden) value using plain + // assignment (`=`). We do it this way to cover append overrides, + // for example: + // + // config.hello.backend = foo # reflect + // config.hello.backend += bar # user + // + pair<lookup, size_t> org {lookup {val, var, rs.vars}, 1 /* depth */}; + pair<lookup, size_t> ovr; + + if (var.overrides == nullptr) + ovr = org; // Case #2. + else + { + // NOTE: see also above and below if enabling this. + // +#if 0 + // Case #3. + // + // The override can come from two places: config_vars_ or one of the + // "global" sources (environment variable, default options file; see + // load() for details). The latter can only be a global override and + // can be found (together with global overrides from config_vars_) + // in context::global_var_overrides. + // + // It feels like mixing global overrides and reflect is misguided: + // we probably don't want to rewrite it with a global override (per + // case #3 above) since it will apply globally. So let's diagnose it + // for now. + // + { + const strings& ovs (ctx_->global_var_overrides); + auto i (find_if (ovs.begin (), ovs.end (), + [&var] (const string& o) + { + // TODO: extracting name is not easy. + })); + + if (i != ovs.end ()) + { + fail << "global override for reflect clause variable " << var << + info << "global override: " << *i; + } + } + + // Ok, this override must be present in config_vars_. + // + // @@ Extracting the name from config_vars_ and similar is not easy: + // they are buildfile fragments and context actually parses them. + // + // @@ What if we have multiple overrides? + // + // @@ What if it's some scoped override or some such (e.g., all + // these .../x=y, etc). + // + // @@ Does anything change if we have an override but it does not + // apply (i.e., ovr == org && var.overrides != nullptr)? + // + // @@ Perhaps a sensible approach is to start relaxing/allowing + // this for specific, sensible cases (e.g., single unqualified + // override)? + // + // What would be the plausible scenarios for an override? + // + // 1. Append override that adds some backend or some such to the + // reflect value. + // + // 2. A reflect may enable a feature based on the dependency + // alternative selected (e.g., I see we are using Qt6 so we might + // as well enable feature X). The user may want do disable it + // with an override. + // + ovr = rs.lookup_override (var, org); +#else + fail << "command line override of reflect clause variable " << var + << endf; +#endif + } + + string s (var.name + '='); + + if (val.null) + s += "[null]"; + else + { + storage.clear (); + names_view nv (reverse (*ovr.first, storage)); + + if (!nv.empty ()) + { + // Note: we need to use command-line (effective) quoting. + // + ostringstream os; + to_stream (os, nv, quote_mode::effective, '@'); + s += os.str (); + } + } + + reflect_vars_.push_back (move (s)); + } + +#if 0 + // TODO: copy over config_vars_ that are not in the map (case #1). +#endif + + // Drop the build system state since it needd reloading (some computed + // values in root.build may depend on the new configuration values). + // + ctx_ = nullptr; + } + catch (const build2::failed&) + { + throw failed (); // Assume the diagnostics has already been issued. + } + } + + strings package_skeleton:: + collect_reflect () && + { + assert (db_ != nullptr); + + ctx_ = nullptr; // In case we only had conditions. + + db_ = nullptr; + available_ = nullptr; + + return reflect_vars_.empty () + ? move (config_vars_) + : move (reflect_vars_); + } + + build2::scope& package_skeleton:: load () { - if (ctx_ != nullptr && !dirty_) - return; + if (ctx_ != nullptr) + return *rs_; - // The overall plan is as follows: @@ TODO: revise + // The overall plan is as follows: // // 0. Create filesystem state if necessary (could have been created by // another instance, e.g., during simulation). // - // 1. If loaded but dirty, save the accumulated reflect state, and - // destroy the old state. + // 1. Load the state potentially with accumulated reflect fragment. // - // 2. Load the state potentially with accumulated reflect state. + // Creating a new context is not exactly cheap (~1.2ms debug, 0.08ms + // release) so we could try to re-use it by cleaning all the scopes other + // than the global scope (and probably some other places, like var pool). + // But we will need to carefully audit everything to make sure we don't + // miss anything (like absolute scope variable overrides being lost). So + // maybe, one day, if this really turns out to be a performance issue. // Create the skeleton filesystem state, if it doesn't exist yet. // if (!created_) { - const available_package& ap (available_); + const available_package& ap (*available_); // Note that we create the skeleton directories in the skeletons/ // subdirectory of the configuration temporary directory to make sure // they never clash with other temporary subdirectories (git // repositories, etc). // - if (src_root_.empty ()) + if (src_root_.empty () || out_root_.empty ()) { - auto i (temp_dir.find (db_.get ().config_orig)); + // Cannot be specified if src_root_ is unspecified. + // + assert (out_root_.empty ()); + + auto i (temp_dir.find (db_->config_orig)); assert (i != temp_dir.end ()); - dir_path d (i->second); + // Make sure the source and out root directories, if set, are absolute + // and normalized. + // + // Note: can never fail since the temporary directory should already + // be created and so its path should be valid. + // + dir_path d (normalize (i->second, "temporary directory")); + d /= "skeletons"; d /= name ().string () + '-' + ap.version.string (); - src_root_ = move (d); // out_root_ is the same. + if (src_root_.empty ()) + src_root_ = move (d); // out_root_ is the same. + else + out_root_ = move (d); // Don't even need to create it. } if (!exists (src_root_)) @@ -226,41 +712,130 @@ namespace bpkg created_ = true; } - // Creating a new context is not exactly cheap (~1.2ms debug, 0.08ms - // release) so we could try to re-use it by cleaning all the scopes other - // than the global scope (and probably some other places, like var pool). - // But we will need to carefully audit everything to make sure we don't - // miss anything (like absolute scope variable overrides being lost). So - // maybe, one day. - // - if (dirty_) + try { - ctx_.reset (); - dirty_ = false; - } + using namespace build2; + using build2::fail; + using build2::endf; - // Create build context. - // - // @@ BUILD2_VAR_OVR, environment/default options files? E.g., !config - // to find module... Can't we make it reusable in build2? - // - // @@ Can we release context memory early, for example, when - // collect_reflect() is called? - // + // Create build context. + // + // @@ BUILD2_VAR_OVR, environment/default options files? E.g., !config + // to find module... Can't we make it reusable in build2? + // + // @@ What is our story about build system modules? It seems we will + // only support built-in and standard pre-installed modules here + // (otherwise we would need to build all the module dependencies + // before we can evaluate any of this). Which means they cannot be + // built except during development. Maybe start parallel scheduler + // if dev build? But is it not possible someone will use installed + // bpkg to develop a module? Why don't we just start a parallel + // scheduler always (though that would be wasted overhead most of + // the time)? Or maybe provide a command line option? + + // We can reasonably assume reflect cannot have global or absolute scope + // variable overrides so we don't need to pass them to context. + // - // We can reasonably assume reflect cannot have global or absolute scope - // variable overrides so we don't need to pass them to context. - // - using namespace build2; - - ctx_.reset ( - new context (build2_sched, - build2_mutexes, - build2_fcache, - false /* match_only */, // Shouldn't matter. - false /* no_external_modules */, - false /* dry_run */, // Shouldn't matter. - false /* keep_going */, // Shouldnt' matter. - config_vars_)); + ctx_.reset ( + new context (build2_sched, + build2_mutexes, + build2_fcache, + false /* match_only */, // Shouldn't matter. + false /* no_external_modules */, + false /* dry_run */, // Shouldn't matter. + false /* keep_going */, // Shouldnt' matter. + config_vars_)); + + context& ctx (*ctx_); + + // This is essentially a subset of the steps we perform in b.cxx. See + // there for more detailed comments. + // + scope& gs (ctx.global_scope.rw ()); + + const meta_operation_info& mif (mo_perform); + const operation_info& oif (op_update); + + ctx.current_mname = mif.name; + ctx.current_oname = oif.name; + + gs.assign (ctx.var_build_meta_operation) = ctx.current_mname; + + // @@ TODO: need to set a variable indicating this is a skeleton load. + + // Note that it's ok for out_root to not exist (external package). + // + const dir_path& src_root (src_root_); + const dir_path& out_root (out_root_.empty () ? src_root_ : out_root_); + + auto rsi (create_root (ctx, out_root, src_root)); + scope& rs (*rsi->second.front ()); + + // Note: we know this project hasn't yet been bootstrapped. + // + optional<bool> altn; + value& v (bootstrap_out (rs, altn)); + + if (!v) + v = src_root; + else + assert (cast<dir_path> (v) == src_root); + + setup_root (rs, false /* forwarded */); + + bootstrap_pre (rs, altn); + bootstrap_src (rs, altn, + db_->config.relative (out_root) /* amalgamation */, + false /* subprojects */); + + create_bootstrap_outer (rs); + bootstrap_post (rs); + + assert (mif.meta_operation_pre == nullptr); + ctx.current_meta_operation (mif); + + ctx.enter_project_overrides (rs, out_root, ctx.var_overrides); + + // Load project's root.build. + // + // If we have the accumulated reflect fragment, wedge it just before + // loading root.build (but after initializing config which may load + // config.build and which we wish to override). + // + // Note that the plan for non-external packages is to extract the + // configuration and then load it with config.config.load and this + // approach should work for that case too. + // + function<void (parser&)> pre; + + if (!reflect_frag_.empty ()) + { + pre = [this, &rs] (parser& p) + { + istringstream is (reflect_frag_); + is.exceptions (istringstream::failbit | istringstream::badbit); + + // Note that the fragment is just a bunch of variable assignments + // and thus unlikely to cause any errors. + // + path_name in ("<accumulated-reflect-fragment>"); + p.parse_buildfile (is, in, &rs, rs); + }; + } + + load_root (rs, pre); + + // Setup root scope as base. + // + setup_base (rsi, out_root, src_root); + + rs_ = &rs; + return rs; + } + catch (const build2::failed&) + { + throw failed (); // Assume the diagnostics has already been issued. + } } } diff --git a/bpkg/package-skeleton.hxx b/bpkg/package-skeleton.hxx index 1a56bfb..c4c59cc 100644 --- a/bpkg/package-skeleton.hxx +++ b/bpkg/package-skeleton.hxx @@ -19,14 +19,14 @@ namespace bpkg class package_skeleton { public: - // If the package is external and will not be disfigured, then the - // existing package source root directory needs to be specified. In this - // case this source directory and the automatically deduced potentially - // non-existent out root directory will be used for build2 state loading - // instead of the newly created skeleton directory. This, in particular, - // makes sure we take into account the existing configuration variables - // while evaluating the dependency clauses (this logic is "parallel" to - // the configuration preservation in pkg-build.cxx). + // If the package is external, then the existing package source root + // directory needs to be specified (as absolute and normalized). In this + // case, if output root is specified (as absolute and normalized; normally + // <config-dir>/<package-name>), then it's used as is. Otherwise, an empty + // skeleton directory is used as output root. + // + // If the package is not external, then none of the root directories + // should be specified. // // Note that the database and available_package are expected to outlive // this object. @@ -34,70 +34,38 @@ namespace bpkg // Note also that this creates an "unloaded" skeleton and is therefore // cheap. // - // @@ Note that storing the list of configuration variables by reference - // complicates its use in pkg-build, where both the configuration and - // the optional skeleton are parts of the same copyable/moveable - // build_package object. We could probably move the configuration into - // the skeleton if create it, complicating an access to the - // configuration for the users (if the skeleton is present then get - // configuration from it, etc). Let's however keep it simple for now - // and just copy the configuration. - // package_skeleton (database&, const available_package&, - const strings& cvs, - optional<dir_path> src_root); + strings config_vars, + optional<dir_path> src_root, + optional<dir_path> out_root); - // Evaluate the enable clause. + // For the following evaluate_*() functions assume that the clause belongs + // to the specified (by index) depends value (used to print its location + // on failure for an external package). // - // @@ What can we pass as location? Maybe somehow point to manifest in the - // skeleton (will need to re-acquire the position somehow)? + // Evaluate the enable clause. // bool - evaluate_enable (const string&) - { - // @@ TMP - // - fail << "conditional dependency for package " << name () << - info << "conditional dependencies are not yet supported"; - - load (); - - // TODO - - return true; // @@ TMP - } + evaluate_enable (const string&, size_t depends_index); // Evaluate the reflect clause. // void - evaluate_reflect (const string& r) - { - load (); - - // TODO - - // @@ DEP For now we assume that the reflection, if present, contains - // a single configuration variable that assigns a literal value. - // - reflect_.push_back (r); - - // Mark the build system state as needing reloading since some computed - // values in root.build may depend on the new configuration values. - // - dirty_ = true; - } + evaluate_reflect (const string&, size_t depends_index); // Return the accumulated reflect values. // + // Note that the result is merged with config_vars and you should be used + // instead rather than in addition to config_vars. + // + // Note also that this should be the final call on this object. + // strings - collect_reflect () - { - return reflect_; - } + collect_reflect () &&; const package_name& - name () const {return available_.get ().id.name;} + name () const {return available_->id.name;} // Implementation details. // @@ -116,24 +84,26 @@ namespace bpkg // // Call this function before evaluating every clause. // - void + build2::scope& load (); private: // NOTE: remember to update move/copy constructors! // - reference_wrapper<database> db_; - reference_wrapper<const available_package> available_; + database* db_; + const available_package* available_; strings config_vars_; - dir_path src_root_; + dir_path src_root_; // Must be absolute and normalized. dir_path out_root_; // If empty, the same as src_root_. - unique_ptr<build2::context> ctx_; bool created_ = false; - bool dirty_ = false; + unique_ptr<build2::context> ctx_; + build2::scope* rs_ = nullptr; - strings reflect_; + strings reflect_names_; // Reflect configuration variable names. + strings reflect_vars_; // Reflect configuration variable overrides. + string reflect_frag_; // Reflect configuration variable fragment. }; } diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx index 73d2284..f4eef42 100644 --- a/bpkg/pkg-build.cxx +++ b/bpkg/pkg-build.cxx @@ -712,35 +712,47 @@ namespace bpkg (*action == build && (flags & build_repoint) != 0); } - // Return true if this build replaces an external package with another - // external. + // Return true if the resulting package will be configured as external. + // Optionally, if the package is external, return its absolute and + // normalized source root directory path. // bool - external () const + external (dir_path* d = nullptr) const { - if (selected == nullptr || !selected->external ()) - return false; - assert (action); if (*action == build_package::drop) return false; - bool r (false); - // If adjustment or orphan, then new and old are the same. // if (available == nullptr || available->locations.empty ()) { - r = true; + assert (selected != nullptr); + + if (selected->external ()) + { + assert (selected->src_root); + + if (d != nullptr) + *d = *selected->src_root; + + return true; + } } else { const package_location& pl (available->locations[0]); - if (pl.repository_fragment.object_id () == "") // Special root. + if (pl.repository_fragment.object_id () == "") // Special root? { - r = !exists (pl.location); // Directory case. + if (!exists (pl.location)) // Directory case? + { + if (d != nullptr) + *d = normalize (path_cast<dir_path> (pl.location), "package"); + + return true; + } } else { @@ -750,18 +762,40 @@ namespace bpkg // Note that such repository fragments are always preferred over // others (see below). // - for (const package_location& l: available->locations) + for (const package_location& pl: available->locations) { - if (l.repository_fragment.load ()->location.directory_based ()) + const repository_location& rl ( + pl.repository_fragment.load ()->location); + + if (rl.directory_based ()) { - r = true; - break; + // Note that the repository location path is always absolute for + // the directory-based repositories but the package location may + // potentially not be normalized. Thus, we normalize the + // resulting path, if requested. + // + if (d != nullptr) + *d = normalize (path_cast<dir_path> (rl.path () / pl.location), + "package"); + + return true; } } } } - return r; + return false; + } + + // If the resulting package will be configured as external, then return + // its absolute and normalized source root directory path and nullopt + // otherwise. + // + optional<dir_path> + external_dir () const + { + dir_path r; + return external (&r) ? optional<dir_path> (move (r)) : nullopt; } const version& @@ -1267,12 +1301,18 @@ namespace bpkg if (size_t n = deps.size ()) pkg.dependencies->reserve (n); + optional<dir_path> src_root (pkg.external_dir ()); + + optional<dir_path> out_root ( + src_root && !pkg.disfigure + ? dir_path (pdb.config) /= pkg.name ().string () + : optional<dir_path> ()); + pkg.skeleton = package_skeleton (pdb, *ap, pkg.config_vars, - (pkg.external () && !pkg.disfigure - ? sp->src_root - : nullopt)); + move (src_root), + move (out_root)); } dependencies& sdeps (*pkg.dependencies); @@ -1309,9 +1349,9 @@ namespace bpkg package_skeleton& skel (*pkg.skeleton); - for (size_t i (sdeps.size ()); i != deps.size (); ++i) + for (size_t di (sdeps.size ()); di != deps.size (); ++di) { - const dependency_alternatives_ex& das (deps[i]); + const dependency_alternatives_ex& das (deps[di]); // Add an empty alternatives list into the selected dependency list if // this is a toolchain build-time dependency. @@ -1339,7 +1379,7 @@ namespace bpkg { for (const dependency_alternative& da: das) { - if (!da.enable || skel.evaluate_enable (*da.enable)) + if (!da.enable || skel.evaluate_enable (*da.enable, di)) edas.push_back (da); } } @@ -2126,7 +2166,7 @@ namespace bpkg // present, and collecting its dependency builds. // bool selected (false); - auto select = [&sdeps, &sdas, &skel, &collect, &selected] + auto select = [&sdeps, &sdas, &skel, di, &collect, &selected] (const dependency_alternative& da, prebuilds&& bs) { @@ -2144,7 +2184,7 @@ namespace bpkg sdeps.push_back (move (sdas)); if (da.reflect) - skel.evaluate_reflect (*da.reflect); + skel.evaluate_reflect (*da.reflect, di); collect (move (bs)); @@ -7647,7 +7687,7 @@ namespace bpkg bool external (false); if (!simulate) { - external = p.external (); + external = sp != nullptr && sp->external () && p.external (); // Reset the keep_out flag if the package being unpacked is not // external. @@ -8090,27 +8130,29 @@ namespace bpkg t, sp, *p.dependencies, - *p.skeleton, - p.config_vars, + move (*p.skeleton), simulate, fdb); } else { - package_skeleton ps (pdb, - *ap, - p.config_vars, - (p.external () && !p.disfigure - ? sp->src_root - : nullopt)); + optional<dir_path> src_root (p.external_dir ()); + + optional<dir_path> out_root ( + src_root && !p.disfigure + ? dir_path (pdb.config) /= p.name ().string () + : optional<dir_path> ()); pkg_configure (o, pdb, t, sp, ap->dependencies, - ps, - p.config_vars, + package_skeleton (pdb, + *ap, + move (p.config_vars), + move (src_root), + move (out_root)), simulate, fdb); } @@ -8133,12 +8175,12 @@ namespace bpkg if (dap == nullptr) dap = make_available (o, pdb, sp); - package_skeleton ps (pdb, - *dap, - p.config_vars, - (p.external () && !p.disfigure - ? sp->src_root - : nullopt)); + optional<dir_path> src_root (p.external_dir ()); + + optional<dir_path> out_root ( + src_root && !p.disfigure + ? dir_path (pdb.config) /= p.name ().string () + : optional<dir_path> ()); // @@ Note that on reconfiguration the dependent looses the potential // configuration variables specified by the user on some previous @@ -8150,8 +8192,11 @@ namespace bpkg t, sp, dap->dependencies, - ps, - p.config_vars, + package_skeleton (pdb, + *dap, + move (p.config_vars), + move (src_root), + move (out_root)), simulate, fdb); } diff --git a/bpkg/pkg-configure.cxx b/bpkg/pkg-configure.cxx index 9339f75..d6f96d0 100644 --- a/bpkg/pkg-configure.cxx +++ b/bpkg/pkg-configure.cxx @@ -32,15 +32,17 @@ namespace bpkg database& db, transaction&, const dependencies& deps, - package_skeleton& ps, + package_skeleton&& ps, bool simulate, const function<find_database_function>& fdb) { package_prerequisites pps; vector<string> cvs; - for (const dependency_alternatives_ex& das: deps) + for (size_t di (0); di != deps.size (); ++di) { + const dependency_alternatives_ex& das (deps[di]); + if (das.empty () || toolchain_buildtime_dependency (o, das, ps.name ())) continue; @@ -51,7 +53,7 @@ namespace bpkg bool enabled (false); // True if there is an enabled alternative. for (const dependency_alternative& da: das) { - if (da.enable && !ps.evaluate_enable (*da.enable)) + if (da.enable && !ps.evaluate_enable (*da.enable, di)) continue; enabled = true; @@ -175,7 +177,7 @@ namespace bpkg // Evaluate the dependency alternative reflect clause, if present. // if (da.reflect) - ps.evaluate_reflect (*da.reflect); + ps.evaluate_reflect (*da.reflect, di); satisfied = true; break; @@ -190,8 +192,15 @@ namespace bpkg // if (!simulate) { - for (string& cv: ps.collect_reflect ()) - cvs.push_back (move (cv)); + strings rvs (move (ps).collect_reflect ()); + + if (cvs.empty ()) + cvs = move (rvs); + else + { + for (string& cv: rvs) + cvs.push_back (move (cv)); + } } return make_pair (move (pps), move (cvs)); @@ -203,8 +212,7 @@ namespace bpkg transaction& t, const shared_ptr<selected_package>& p, const dependencies& deps, - package_skeleton& ps, - const strings& vars, + package_skeleton&& ps, bool simulate, const function<find_database_function>& fdb) { @@ -238,7 +246,7 @@ namespace bpkg db, t, deps, - ps, + move (ps), simulate, fdb)); @@ -265,7 +273,7 @@ namespace bpkg // try { - run_b (o, verb_b::quiet, cpr.second, vars, bspec); + run_b (o, verb_b::quiet, cpr.second, bspec); } catch (const failed&) { @@ -429,18 +437,22 @@ namespace bpkg // shared_ptr<available_package> ap (make_available (o, db, p)); - package_skeleton ps (db, - *ap, - vars, - p->external () ? p->src_root : nullopt); + optional<dir_path> src_root (p->external () ? p->src_root : nullopt); + + optional<dir_path> out_root (src_root + ? dir_path (db.config) /= p->name.string () + : optional<dir_path> ()); pkg_configure (o, db, t, p, ap->dependencies, - ps, - vars, + package_skeleton (db, + *ap, + move (vars), + move (src_root), + move (out_root)), false /* simulate */); } diff --git a/bpkg/pkg-configure.hxx b/bpkg/pkg-configure.hxx index 1b92281..5d6b2eb 100644 --- a/bpkg/pkg-configure.hxx +++ b/bpkg/pkg-configure.hxx @@ -43,8 +43,7 @@ namespace bpkg transaction&, const shared_ptr<selected_package>&, const dependencies&, - package_skeleton&, - const strings& config_vars, + package_skeleton&&, bool simulate, const function<find_database_function>& = {}); diff --git a/build/root.build b/build/root.build index b32c01e..c172018 100644 --- a/build/root.build +++ b/build/root.build @@ -16,7 +16,8 @@ if ($cxx.target.system == 'win32-msvc') if ($cxx.class == 'msvc') cxx.coptions += /wd4251 /wd4275 /wd4800 elif ($cxx.id == 'gcc') - cxx.coptions += -Wno-maybe-uninitialized -Wno-free-nonheap-object # libbutl + cxx.coptions += -Wno-maybe-uninitialized -Wno-free-nonheap-object \ +-Wno-stringop-overread # libbutl cxx.poptions =+ "-I$out_root" "-I$src_root" diff --git a/tests/common/dependency-alternatives/t8a/fax-1.0.0.tar.gz b/tests/common/dependency-alternatives/t8a/fax-1.0.0.tar.gz Binary files differnew file mode 100644 index 0000000..2bf2360 --- /dev/null +++ b/tests/common/dependency-alternatives/t8a/fax-1.0.0.tar.gz diff --git a/tests/common/dependency-alternatives/t8a/libbiz-1.0.0.tar.gz b/tests/common/dependency-alternatives/t8a/libbiz-1.0.0.tar.gz Binary files differnew file mode 100644 index 0000000..06316f9 --- /dev/null +++ b/tests/common/dependency-alternatives/t8a/libbiz-1.0.0.tar.gz diff --git a/tests/common/dependency-alternatives/t8a/libbox-1.0.0.tar.gz b/tests/common/dependency-alternatives/t8a/libbox-1.0.0.tar.gz Binary files differnew file mode 100644 index 0000000..ba67cfe --- /dev/null +++ b/tests/common/dependency-alternatives/t8a/libbox-1.0.0.tar.gz diff --git a/tests/pkg-build.testscript b/tests/pkg-build.testscript index 287074c..711b5b8 100644 --- a/tests/pkg-build.testscript +++ b/tests/pkg-build.testscript @@ -137,12 +137,18 @@ # | `-- repositories.manifest # | # |-- t8a -# | |-- foo-1.0.0.tar.gz -> {libbar libbaz} ^1.0.0 -# | |-- fox-1.0.0.tar.gz -> libbar ^1.0.0 | libbaz ^1.0.0 -# | |-- fix-1.0.0.tar.gz -> libbaz ^1.0.0 | libbar ^1.0.0 # | |-- libbar-1.0.0.tar.gz # | |-- libbaz-1.0.0.tar.gz # | |-- libbaz-1.1.0.tar.gz +# | |-- libbiz-1.0.0.tar.gz +# | |-- libbox-1.0.0.tar.gz +# | |-- foo-1.0.0.tar.gz -> {libbar libbaz} ^1.0.0 +# | |-- fox-1.0.0.tar.gz -> libbar ^1.0.0 | libbaz ^1.0.0 +# | |-- fix-1.0.0.tar.gz -> libbaz ^1.0.0 | libbar ^1.0.0 +# | |-- fax-1.0.0.tar.gz -> libbar ^1.0.0 ? ($cxx.target.class == 'windows') config.fax.backend=libbar | +# | | libbaz ^1.0.0 ? ($cxx.target.class != 'windows') config.fax.backend=libbaz, +# | | libbiz ? ($config.fax.libbiz) config.fax.extras='[b\i$z]', +# | | libbox ? ($config.fax.libbox && $config.fax.backend == libbaz && $config.fax.extras == '[b\i$z]') # | `-- repositories.manifest # | # |-- t9 @@ -3585,6 +3591,217 @@ test.options += --no-progress $pkg_drop fox } } + + : enable-condition + : + { + +$cfg_create cxx $config_cxx -d cfg &cfg/*** + +$rep_add $rep/t8a && $rep_fetch + + test.arguments += --yes + + backend = ($posix ? 'libbaz' : 'libbar') + backend_dep = ($posix ? 'libbaz/1.1.0' : 'libbar/1.0.0') + backend_configured = ($posix ? 'libbaz configured 1.1.0' : 'libbar configured 1.0.0') + + : cxx-target + : + { + $clone_cfg; + + $* fax 2>>~"%EOE%"; + fetched $backend_dep + unpacked $backend_dep + fetched fax/1.0.0 + unpacked fax/1.0.0 + configured $backend_dep + configured fax/1.0.0 + %info: .+fax-1.0.0.+ is up to date% + updated fax/1.0.0 + EOE + + $pkg_status -r >>"EOO"; + !fax configured 1.0.0 + $backend_configured + EOO + + cat cfg/fax-1.0.0/build/config.build >>~"%EOO%"; + %.* + config.fax.backend = $backend + config.fax.libbiz = false + %.* + EOO + + $pkg_drop fax + } + + : config-var + : + { + $clone_cfg; + + $* config.fax.libbiz=true -- fax 2>>~"%EOE%"; + fetched $backend_dep + unpacked $backend_dep + fetched libbiz/1.0.0 + unpacked libbiz/1.0.0 + fetched fax/1.0.0 + unpacked fax/1.0.0 + configured $backend_dep + configured libbiz/1.0.0 + configured fax/1.0.0 + %info: .+fax-1.0.0.+ is up to date% + updated fax/1.0.0 + EOE + + $pkg_status -r >>"EOO"; + !fax configured 1.0.0 + $backend_configured + libbiz configured 1.0.0 + EOO + + cat cfg/fax-1.0.0/build/config.build >>~"%EOO%"; + %.* + config.fax.backend = $backend + config.fax.libbiz = true + %.* + EOO + + $pkg_drop fax + } + + : external-package + : + { + $clone_cfg; + + tar (!$posix ? --force-local : ) -xf $src/t8a/fax-1.0.0.tar.gz &fax-1.0.0/***; + mv fax-1.0.0 fax; + + $* config.fax.libbiz=true -- fax/ 2>>~"%EOE%"; + fetched $backend_dep + unpacked $backend_dep + fetched libbiz/1.0.0 + unpacked libbiz/1.0.0 + using fax/1.0.0 \(external\) + configured $backend_dep + configured libbiz/1.0.0 + configured fax/1.0.0 + %info: .+fax.+ is up to date% + updated fax/1.0.0 + EOE + + $pkg_status -r >>"EOO"; + !fax configured !1.0.0 + $backend_configured + libbiz configured 1.0.0 + EOO + + cat cfg/fax/build/config.build >>~"%EOO%"; + %.* + config.fax.backend = $backend + config.fax.libbiz = true + %.* + EOO + + # Upgrade the external package after changing its manifest and make + # sure the configuration is preserved. + # + echo '' >+fax/manifest; + + $* fax/ 2>>~%EOE%; + disfigured fax/1.0.0 + using fax/1.0.0#1 (external) + configured fax/1.0.0#1 + %info: .+fax.+ is up to date% + updated fax/1.0.0#1 + EOE + + $pkg_status -r >>"EOO"; + !fax configured !1.0.0#1 + $backend_configured + libbiz configured 1.0.0 + EOO + + cat cfg/fax/build/config.build >>~"%EOO%"; + %.* + config.fax.backend = $backend + config.fax.libbiz = true + %.* + EOO + + # While at it, test that it's ok for out root directory to not exist. + # + # Note that this testing is only meaningful when we replace an + # external package with another external (see build_package::external() + # for details). + # + echo '' >+fax/manifest; + + rm -r cfg/fax/; + + $* fax/ 2>>~%EOE%; + disfigured fax/1.0.0#1 + disfigured libbiz/1.0.0 + purged libbiz/1.0.0 + using fax/1.0.0#2 (external) + configured fax/1.0.0#2 + %info: .+fax.+ is up to date% + updated fax/1.0.0#2 + EOE + + $pkg_status -r >>"EOO"; + !fax configured !1.0.0#2 + $backend_configured + EOO + + cat cfg/fax/build/config.build >>~"%EOO%"; + %.* + config.fax.backend = $backend + config.fax.libbiz = false + %.* + EOO + + # Also tests that the depends value location is printed on the enable + # condition evaluation failure for an external package. + # + sed -i -e 's/(depends: libbiz).+/\1 ? (config.fax.libbiz = true)/' fax/manifest; + + $* fax/ 2>>~%EOE% != 0; + <depends-enable-clause>:1: error: invalid bool value: multiple names + info: enable condition: (config.fax.libbiz = true) + % .+fax.manifest:10:10: info: depends value defined here% + info: while satisfying fax/1.0.0#3 + EOE + + $pkg_drop fax + } + + : evaluate-reflect-vars + : + { + $clone_cfg; + + $* config.fax.libbox=true config.fax.libbiz=true -- fax 2>!; + + if $posix + $pkg_status -r >>EOO + !fax configured 1.0.0 + libbaz configured 1.1.0 + libbiz configured 1.0.0 + libbox configured 1.0.0 + EOO + else + $pkg_status -r >>EOO + !fax configured 1.0.0 + libbar configured 1.0.0 + libbiz configured 1.0.0 + EOO + end; + + $pkg_drop fax + } + } } } |