From b978ab110d5dd11b4efecb14830c849f89210919 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Thu, 10 Feb 2022 12:21:22 +0200 Subject: Minor tweaks to package skeleton implementation --- bpkg/package-skeleton.cxx | 37 ++++++++++++++++++------------------- bpkg/package-skeleton.hxx | 11 ++++++----- bpkg/pkg-build.cxx | 32 ++++++++++++++++---------------- bpkg/pkg-configure.cxx | 9 ++++----- 4 files changed, 44 insertions(+), 45 deletions(-) diff --git a/bpkg/package-skeleton.cxx b/bpkg/package-skeleton.cxx index a0fd419..d789f60 100644 --- a/bpkg/package-skeleton.cxx +++ b/bpkg/package-skeleton.cxx @@ -83,17 +83,17 @@ namespace bpkg const available_package& ap, const strings& cvs, optional src_root) - : db_ (db), - available_ (ap), - config_vars_ (cvs), - src_root_ (move (src_root)) + : db_ (db), available_ (ap), config_vars_ (cvs) { // Should not be created for stubs. // assert (available_.get ().bootstrap_build); - if (src_root_) + if (src_root) + { + src_root_ = move (*src_root); out_root_ = dir_path (db_.get ().config_orig) /= name ().string (); + } } void package_skeleton:: @@ -123,7 +123,7 @@ namespace bpkg // they never clash with other temporary subdirectories (git // repositories, etc). // - if (!src_root_) + if (src_root_.empty ()) { auto i (temp_dir.find (db_.get ().config_orig)); assert (i != temp_dir.end ()); @@ -132,11 +132,10 @@ namespace bpkg d /= "skeletons"; d /= name ().string () + '-' + ap.version.string (); - src_root_ = d; - out_root_ = move (d); + src_root_ = move (d); // out_root_ is the same. } - if (!exists (*src_root_)) + if (!exists (src_root_)) { // Create the buildfiles. // @@ -145,7 +144,7 @@ namespace bpkg // additional files. // { - path bf (*src_root_ / std_bootstrap_file); + path bf (src_root_ / std_bootstrap_file); mk_p (bf.directory ()); @@ -168,11 +167,12 @@ namespace bpkg save (*ap.bootstrap_build, bf); if (ap.root_build) - save (*ap.root_build, *src_root_ / std_root_file); + save (*ap.root_build, src_root_ / std_root_file); } // Create the manifest file containing the bare minimum of values - // which can potentially be required to load the build system state. + // which can potentially be required to load the build system state + // (i.e., either via the version module or manual version extraction). // { package_manifest m; @@ -181,11 +181,8 @@ namespace bpkg // 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. + // build2 dependency value, letting the version module verify the + // constraint. // // Also note that the resulting file is not quite a valid package // manifest, since it doesn't contain all the required values @@ -201,7 +198,7 @@ namespace bpkg m.dependencies.push_back (das); } - path mf (*src_root_ / manifest_file); + path mf (src_root_ / manifest_file); try { @@ -210,12 +207,14 @@ namespace bpkg m.serialize (s); os.close (); } - catch (const manifest_serialization&) + catch (const manifest_serialization& e) { // We shouldn't be creating a non-serializable manifest, since // it's crafted from the parsed values. // assert (false); + + fail << "unable to serialize " << mf << ": " << e.description; } catch (const io_error& e) { diff --git a/bpkg/package-skeleton.hxx b/bpkg/package-skeleton.hxx index 00be6a3..b26c7b9 100644 --- a/bpkg/package-skeleton.hxx +++ b/bpkg/package-skeleton.hxx @@ -22,10 +22,11 @@ namespace bpkg // 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-existing out root directory will be used for build2 state loading + // non-existent out root directory will be used for build2 state loading // instead of the newly created skeleton directory. This, in particular, - // allows to consider existing configuration variables while evaluating - // the dependency clauses. + // 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). // // Note that the database and available_package are expected to outlive // this object. @@ -125,8 +126,8 @@ namespace bpkg reference_wrapper available_; strings config_vars_; - optional src_root_; - optional out_root_; + dir_path src_root_; + dir_path out_root_; // If empty, the same as src_root_. unique_ptr ctx_; bool created_ = false; diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx index f4432b6..73d2284 100644 --- a/bpkg/pkg-build.cxx +++ b/bpkg/pkg-build.cxx @@ -1267,14 +1267,12 @@ namespace bpkg if (size_t n = deps.size ()) pkg.dependencies->reserve (n); - optional src_root; - if (pkg.external () && !pkg.disfigure) - src_root = sp->src_root; - pkg.skeleton = package_skeleton (pdb, *ap, pkg.config_vars, - move (src_root)); + (pkg.external () && !pkg.disfigure + ? sp->src_root + : nullopt)); } dependencies& sdeps (*pkg.dependencies); @@ -7663,7 +7661,7 @@ namespace bpkg // // Note that for other cases the preservation of the configuration is // still a @@ TODO (the idea is to use our config.config.{save,load} - // machinery). + // machinery). Also see "parallel" logic in package_skeleton. // // Commits the transaction. // @@ -8099,11 +8097,12 @@ namespace bpkg } else { - optional src_root; - if (p.external () && !p.disfigure) - src_root = sp->src_root; - - package_skeleton ps (pdb, *ap, p.config_vars, move (src_root)); + package_skeleton ps (pdb, + *ap, + p.config_vars, + (p.external () && !p.disfigure + ? sp->src_root + : nullopt)); pkg_configure (o, pdb, @@ -8134,11 +8133,12 @@ namespace bpkg if (dap == nullptr) dap = make_available (o, pdb, sp); - optional src_root; - if (p.external () && !p.disfigure) - src_root = sp->src_root; - - package_skeleton ps (pdb, *dap, p.config_vars, move (src_root)); + package_skeleton ps (pdb, + *dap, + p.config_vars, + (p.external () && !p.disfigure + ? sp->src_root + : nullopt)); // @@ Note that on reconfiguration the dependent looses the potential // configuration variables specified by the user on some previous diff --git a/bpkg/pkg-configure.cxx b/bpkg/pkg-configure.cxx index ff6fde6..9339f75 100644 --- a/bpkg/pkg-configure.cxx +++ b/bpkg/pkg-configure.cxx @@ -429,11 +429,10 @@ namespace bpkg // shared_ptr ap (make_available (o, db, p)); - optional src_root; - if (p->external ()) - src_root = p->src_root; - - package_skeleton ps (db, *ap, vars, move (src_root)); + package_skeleton ps (db, + *ap, + vars, + p->external () ? p->src_root : nullopt); pkg_configure (o, db, -- cgit v1.1