From 0aa8059f9b9377c7b39e4ceb2df85a4e7b6fec0f Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Thu, 10 Feb 2022 11:56:34 +0200 Subject: Add build context to package_skeleton --- bpkg/package-skeleton.cxx | 282 +++++++++++++++++++++++++++++++--------------- bpkg/package-skeleton.hxx | 35 +++--- bpkg/pkg-build.cxx | 2 +- 3 files changed, 216 insertions(+), 103 deletions(-) diff --git a/bpkg/package-skeleton.cxx b/bpkg/package-skeleton.cxx index ebcc044..a0fd419 100644 --- a/bpkg/package-skeleton.cxx +++ b/bpkg/package-skeleton.cxx @@ -5,6 +5,10 @@ #include +#include +#include +#include + #include #include #include @@ -14,6 +18,66 @@ using namespace butl; namespace bpkg { + // These are defined in bpkg.cxx and initialized in main(). + // + extern build2::scheduler build2_sched; + extern build2::global_mutexes build2_mutexes; + extern build2::file_cache build2_fcache; + + package_skeleton:: + ~package_skeleton () + { + } + + package_skeleton:: + package_skeleton (package_skeleton&& v) + : db_ (v.db_), available_ (v.available_) + { + *this = move (v); + } + + package_skeleton& package_skeleton:: + operator= (package_skeleton&& v) + { + if (this != &v) + { + db_ = v.db_; + available_ = v.available_; + 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_); + } + + return *this; + } + + package_skeleton:: + package_skeleton (const package_skeleton& v) + : db_ (v.db_), + available_ (v.available_), + config_vars_ (v.config_vars_), + src_root_ (v.src_root_), + out_root_ (v.out_root_), + created_ (v.created_) + { + // 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_; + } + } + package_skeleton:: package_skeleton (database& db, const available_package& ap, @@ -35,12 +99,10 @@ namespace bpkg void package_skeleton:: load () { - if (loaded_ && !dirty_) + if (ctx_ != nullptr && !dirty_) return; - const available_package& ap (available_); - - // The overall plan is as follows: + // The overall plan is as follows: @@ TODO: revise // // 0. Create filesystem state if necessary (could have been created by // another instance, e.g., during simulation). @@ -52,112 +114,154 @@ namespace bpkg // Create the skeleton filesystem state, if it doesn't exist yet. // - // 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_) + if (!created_) { - auto i (temp_dir.find (db_.get ().config_orig)); - assert (i != temp_dir.end ()); - - dir_path d (i->second); - d /= "skeletons"; - d /= name ().string () + '-' + ap.version.string (); - - src_root_ = d; - out_root_ = move (d); - } + const available_package& ap (available_); - if (!exists (*src_root_)) - { - // Create the buildfiles. - // - // Note that it's probably doesn't matter which naming scheme to use for - // the buildfiles, unless in the future we allow specifying additional - // files. + // 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_) { - path bf (*src_root_ / std_bootstrap_file); + auto i (temp_dir.find (db_.get ().config_orig)); + assert (i != temp_dir.end ()); - mk_p (bf.directory ()); + dir_path d (i->second); + d /= "skeletons"; + d /= name ().string () + '-' + ap.version.string (); - // Save the {bootstrap,root}.build files. - // - auto save = [] (const string& s, const path& f) - { - try - { - ofdstream os (f); - os << s; - os.close (); - } - catch (const io_error& e) - { - fail << "unable to write to " << f << ": " << e; - } - }; - - save (*ap.bootstrap_build, bf); - - if (ap.root_build) - save (*ap.root_build, *src_root_ / std_root_file); + src_root_ = d; + out_root_ = move (d); } - // Create the manifest file containing the bare minimum of values - // which can potentially be required to load the build system state. - // + if (!exists (*src_root_)) { - package_manifest m; - m.name = name (); - m.version = ap.version; - - // Note that there is no guarantee that the potential build2 - // constraint has already been verified. Thus, we also serialize the - // depends value, delegating the constraint verification to the - // version module. Also note that normally the toolchain build-time - // dependencies are specified first and, if that's the case, their - // constraints are already verified at this point and so build2 will - // not fail due to the constraint violation. + // Create the buildfiles. // - // Also note that the resulting file is not quite a valid package - // manifest, since it doesn't contain all the required values - // (summary, etc). It, however, is good enough for build2 which - // doesn't perform exhaustive manifest validation. + // Note that it's probably doesn't matter which naming scheme to use + // for the buildfiles, unless in the future we allow specifying + // additional files. // - m.dependencies.reserve (ap.dependencies.size ()); - for (const dependency_alternatives_ex& das: ap.dependencies) { - // Skip the the special (inverse) test dependencies. + path bf (*src_root_ / std_bootstrap_file); + + mk_p (bf.directory ()); + + // Save the {bootstrap,root}.build files. // - if (!das.type) - m.dependencies.push_back (das); - } + auto save = [] (const string& s, const path& f) + { + try + { + ofdstream os (f); + os << s; + os.close (); + } + catch (const io_error& e) + { + fail << "unable to write to " << f << ": " << e; + } + }; - path mf (*src_root_ / manifest_file); + save (*ap.bootstrap_build, bf); - try - { - ofdstream os (mf); - manifest_serializer s (os, mf.string ()); - m.serialize (s); - os.close (); + if (ap.root_build) + save (*ap.root_build, *src_root_ / std_root_file); } - catch (const manifest_serialization&) + + // Create the manifest file containing the bare minimum of values + // which can potentially be required to load the build system state. + // { - // We shouldn't be creating a non-serializable manifest, since it's - // crafted from the parsed values. + package_manifest m; + m.name = name (); + m.version = ap.version; + + // Note that there is no guarantee that the potential build2 + // constraint has already been verified. Thus, we also serialize the + // depends value, delegating the constraint verification to the + // version module. Also note that normally the toolchain build-time + // dependencies are specified first and, if that's the case, their + // constraints are already verified at this point and so build2 will + // not fail due to the constraint violation. // - assert (false); - } - catch (const io_error& e) - { - fail << "unable to write to " << mf << ": " << e; + // Also note that the resulting file is not quite a valid package + // manifest, since it doesn't contain all the required values + // (summary, etc). It, however, is good enough for build2 which + // doesn't perform exhaustive manifest validation. + // + m.dependencies.reserve (ap.dependencies.size ()); + for (const dependency_alternatives_ex& das: ap.dependencies) + { + // Skip the the special (inverse) test dependencies. + // + if (!das.type) + m.dependencies.push_back (das); + } + + path mf (*src_root_ / manifest_file); + + try + { + ofdstream os (mf); + manifest_serializer s (os, mf.string ()); + m.serialize (s); + os.close (); + } + catch (const manifest_serialization&) + { + // We shouldn't be creating a non-serializable manifest, since + // it's crafted from the parsed values. + // + assert (false); + } + catch (const io_error& e) + { + fail << "unable to write to " << mf << ": " << e; + } } } + + created_ = true; } - loaded_ = true; - dirty_ = false; + // 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_) + { + ctx_.reset (); + dirty_ = false; + } + + // 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? + // + + // 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_)); } } diff --git a/bpkg/package-skeleton.hxx b/bpkg/package-skeleton.hxx index 889a937..00be6a3 100644 --- a/bpkg/package-skeleton.hxx +++ b/bpkg/package-skeleton.hxx @@ -4,6 +4,8 @@ #ifndef BPKG_PACKAGE_SKELETON_HXX #define BPKG_PACKAGE_SKELETON_HXX +#include // build2::context + #include #include @@ -79,7 +81,10 @@ namespace bpkg // reflect_.push_back (r); - dirty (); + // Mark the build system state as needing reloading since some computed + // values in root.build may depend on the new configuration values. + // + dirty_ = true; } // Return the accumulated reflect values. @@ -93,6 +98,18 @@ namespace bpkg const package_name& name () const {return available_.get ().id.name;} + // Implementation details. + // + // We have to define these because context is forward-declared. Also, copy + // constructor has some special logic. + // + ~package_skeleton (); + package_skeleton (package_skeleton&&); + package_skeleton& operator= (package_skeleton&&); + + package_skeleton (const package_skeleton&); + package_skeleton& operator= (package_skeleton&) = delete; + private: // Create the skeleton if necessary and (re)load the build system state. // @@ -101,18 +118,9 @@ namespace bpkg void load (); - // Mark the build system state as needing reloading. - // - // Call this function after evaluating the reflect clause (since some - // computed values in root.build may depend on the new value). - // - void - dirty () - { - dirty_ = true; - } - private: + // NOTE: remember to update move/copy constructors! + // reference_wrapper db_; reference_wrapper available_; strings config_vars_; @@ -120,7 +128,8 @@ namespace bpkg optional src_root_; optional out_root_; - bool loaded_ = false; + unique_ptr ctx_; + bool created_ = false; bool dirty_ = false; strings reflect_; diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx index f53c14c..f4432b6 100644 --- a/bpkg/pkg-build.cxx +++ b/bpkg/pkg-build.cxx @@ -4160,7 +4160,7 @@ namespace bpkg evaluate_dependency (db, sp, nullopt /* desired */, - false /*desired_sys */, + false /* desired_sys */, db, sp, !*upgrade /* patch */, -- cgit v1.1