From 4e9e142a6564b2a73848e735f9a1b5bb744d6a83 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Wed, 18 Aug 2021 08:35:18 +0200 Subject: Also consider subproject directory names when calculating manifest checksums --- bpkg/manifest-utility.cxx | 109 +++++++++++++++++++++++++++++++--------------- bpkg/manifest-utility.hxx | 50 ++++++++++++++++++--- bpkg/package.cxx | 3 +- bpkg/package.hxx | 17 +++++++- bpkg/pkg-build.cxx | 14 ++++-- bpkg/pkg-checkout.cxx | 2 +- bpkg/pkg-unpack.cxx | 32 ++++++++++---- bpkg/rep-fetch.cxx | 68 ++++++++++++++++++----------- bpkg/rep-fetch.hxx | 6 +++ bpkg/types.hxx | 5 +++ 10 files changed, 224 insertions(+), 82 deletions(-) diff --git a/bpkg/manifest-utility.cxx b/bpkg/manifest-utility.cxx index a4cee94..9b8cbca 100644 --- a/bpkg/manifest-utility.cxx +++ b/bpkg/manifest-utility.cxx @@ -22,6 +22,41 @@ namespace bpkg const path signature_file ("signature.manifest"); const path manifest_file ("manifest"); + vector + package_b_info (const common_options& o, const dir_paths& ds, bool ext_mods) + { + path b (name_b (o)); + + vector r; + try + { + b_info (r, + ds, + ext_mods, + verb, + [] (const char* const args[], size_t n) + { + if (verb >= 2) + print_process (args, n); + }, + b, + exec_dir, + o.build_option ()); + return r; + } + catch (const b_error& e) + { + if (e.normal ()) + throw failed (); // Assume the build2 process issued diagnostics. + + diag_record dr (fail); + dr << "unable to parse project "; + if (r.size () < ds.size ()) dr << ds[r.size ()] << ' '; + dr << "info: " << e << + info << "produced by '" << b << "'; use --build to override" << endf; + } + } + package_scheme parse_package_scheme (const char*& s) { @@ -272,52 +307,54 @@ namespace bpkg } } - vector> + package_version_infos package_versions (const common_options& o, const dir_paths& ds) { - path b (name_b (o)); + vector pis (package_b_info (o, ds, false /* ext_mods */)); + + package_version_infos r; + r.reserve (pis.size ()); + + for (const b_project_info& pi: pis) + { + // An empty version indicates that the version module is not enabled for + // the project. + // + optional v (!pi.version.empty () + ? version (pi.version.string ()) + : optional ()); + + r.push_back (package_version_info {move (v), move (pi)}); + } + + return r; + } + + string + package_checksum (const common_options& o, + const dir_path& d, + const package_info* pi) + { + path f (d / manifest_file); - vector pis; try { - b_info (pis, - ds, - false /* ext_mods */, - verb, - [] (const char* const args[], size_t n) - { - if (verb >= 2) - print_process (args, n); - }, - b, - exec_dir, - o.build_option ()); + ifdstream is (f, fdopen_mode::binary); + sha256 cs (is); - vector> r; - r.reserve (pis.size ()); + const vector& sps ( + pi != nullptr + ? pi->subprojects + : package_b_info (o, d, false /* ext_mods */).subprojects); - for (const b_project_info& pi: pis) - { - // An empty version indicates that the version module is not enabled - // for the project. - // - r.push_back (!pi.version.empty () - ? version (pi.version.string ()) - : optional ()); - } + for (const package_info::subproject& sp: sps) + cs.append (sp.path.string ()); - return r; + return cs.string (); } - catch (const b_error& e) + catch (const io_error& e) { - if (e.normal ()) - throw failed (); // Assume the build2 process issued diagnostics. - - diag_record dr (fail); - dr << "unable to parse project "; - if (pis.size () < ds.size ()) dr << ds[pis.size ()] << ' '; - dr << "info: " << e << - info << "produced by '" << b << "'; use --build to override" << endf; + fail << "unable to read from " << f << ": " << e << endf; } } } diff --git a/bpkg/manifest-utility.hxx b/bpkg/manifest-utility.hxx index b7d9b07..8ef517e 100644 --- a/bpkg/manifest-utility.hxx +++ b/bpkg/manifest-utility.hxx @@ -17,6 +17,20 @@ namespace bpkg extern const path signature_file; // signature.manifest extern const path manifest_file; // manifest + // Obtain build2 projects info for package source or output directories. + // + vector + package_b_info (const common_options&, const dir_paths&, bool ext_mods); + + // As above but return the info for a single package directory. + // + inline package_info + package_b_info (const common_options& o, const dir_path& d, bool ext_mods) + { + vector r (package_b_info (o, dir_paths ({d}), ext_mods)); + return move (r[0]); + } + // Package naming schemes. // enum class package_scheme @@ -95,10 +109,12 @@ namespace bpkg bool repository_name (const string&); - // Return the versions of packages as provided by the build2 version module. - // Return nullopt for a package if the version module is disabled for it (or - // the build2 project directory doesn't contain the manifest file). Fail if - // any of the specified directories is not a build2 project. + // Return the versions of packages as provided by the build2 version module + // together with the build2 project info the versions originate from (in + // case the caller may want to reuse it). Return nullopt as a package + // version if the version module is disabled for the package (or the build2 + // project directory doesn't contain the manifest file). Fail if any of the + // specified directories is not a build2 project. // // Note that if a package directory is under the version control, then the // resulting version may be populated with the snapshot information (see @@ -107,17 +123,37 @@ namespace bpkg // class common_options; - vector> + struct package_version_info + { + optional version; + package_info info; + }; + + using package_version_infos = vector; + + package_version_infos package_versions (const common_options&, const dir_paths&); // As above but return the version of a single package. // - inline optional + inline package_version_info package_version (const common_options& o, const dir_path& d) { - vector> r (package_versions (o, dir_paths ({d}))); + package_version_infos r (package_versions (o, dir_paths ({d}))); return move (r[0]); } + + // Caclulate the checksum of the manifest file located in the package source + // directory and the subproject set (see package::manifest_checksum). + // + // Pass the build2 project info for the package, if available, to speed up + // the call and NULL otherwise (in which case it will be queried by the + // implementation). + // + string + package_checksum (const common_options&, + const dir_path& src_dir, + const package_info*); } #endif // BPKG_MANIFEST_UTILITY_HXX diff --git a/bpkg/package.cxx b/bpkg/package.cxx index 8ad178d..e0101fb 100644 --- a/bpkg/package.cxx +++ b/bpkg/package.cxx @@ -539,6 +539,7 @@ namespace bpkg const dir_path& d, const package_name& n, const version& v, + const package_info* pi, bool check_external) { tracer trace ("package_iteration"); @@ -576,7 +577,7 @@ namespace bpkg false /* iteration */)) return nullopt; - string mc (sha256 (o, d / manifest_file)); + string mc (package_checksum (o, d, pi)); // The selected package must not be "simulated" (see pkg-build for // details). diff --git a/bpkg/package.hxx b/bpkg/package.hxx index da5c64a..cb1a401 100644 --- a/bpkg/package.hxx +++ b/bpkg/package.hxx @@ -1005,13 +1005,23 @@ namespace bpkg optional src_root; bool purge_src; - // The checksum of the manifest file located in the source directory. + // The checksum of the manifest file located in the source directory and + // the subproject set. Changes to this information should trigger the + // package version revision increment. In particular, new subprojects + // should trigger the package reconfiguration. // // Must be present if the source directory is present, unless the object // is created/updated during the package build simulation (see pkg-build // for details). Note that during the simulation the manifest may not be // available. // + // @@ Currently we don't consider subprojects recursively (would most + // likely require extension to b info, also could be a performance + // concern). + // + // @@ We should probably rename it if/when ODB add support for that for + // SQlite. + // optional manifest_checksum; // Path to the output directory of this package, if any. It is @@ -1153,6 +1163,10 @@ namespace bpkg // considered its iteration. Return the version of this iteration if that's // the case and nullopt otherwise. // + // Pass the build2 project info for the package, if available, to speed up + // the call and NULL otherwise (in which case it will be queried by the + // implementation). + // // Notes: // // - The package directory is considered an iteration of the package if this @@ -1190,6 +1204,7 @@ namespace bpkg const dir_path&, const package_name&, const version&, + const package_info*, bool check_external); // certificate diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx index f59c2c0..60bdb08 100644 --- a/bpkg/pkg-build.cxx +++ b/bpkg/pkg-build.cxx @@ -4171,14 +4171,21 @@ namespace bpkg info << "'" << package << "' does not appear to be a valid " << "package directory: "; + // For better diagnostics, let's obtain the package info after + // pkg_verify() verifies that this is a package directory. + // + package_version_info pvi; + package_manifest m ( pkg_verify ( d, true /* ignore_unknown */, - [&o, &d] (version& v) + [&o, &d, &pvi] (version& v) { - if (optional pv = package_version (o, d)) - v = move (*pv); + pvi = package_version (o, d); + + if (pvi.version) + v = move (*pvi.version); }, diag)); @@ -4208,6 +4215,7 @@ namespace bpkg d, m.name, m.version, + &pvi.info, true /* check_external */)) m.version = move (*v); diff --git a/bpkg/pkg-checkout.cxx b/bpkg/pkg-checkout.cxx index 5b69d40..94dd19b 100644 --- a/bpkg/pkg-checkout.cxx +++ b/bpkg/pkg-checkout.cxx @@ -301,7 +301,7 @@ namespace bpkg "config.dist.root='" + ord.representation () + "'", bspec); - mc = sha256 (o, d / manifest_file); + mc = package_checksum (o, d, nullptr /* package_info */); } if (p != nullptr) diff --git a/bpkg/pkg-unpack.cxx b/bpkg/pkg-unpack.cxx index 368a71c..04250f8 100644 --- a/bpkg/pkg-unpack.cxx +++ b/bpkg/pkg-unpack.cxx @@ -63,6 +63,7 @@ namespace bpkg transaction& t, package_name n, version v, + const package_info* pi, dir_path d, repository_location rl, bool purge, @@ -75,7 +76,7 @@ namespace bpkg optional mc; if (!simulate) - mc = sha256 (o, d / manifest_file); + mc = package_checksum (o, d, pi); // Make the package path absolute and normalized. If the package is inside // the configuration, use the relative path. This way we can move the @@ -160,15 +161,22 @@ namespace bpkg if (!exists (d)) fail << "package directory " << d << " does not exist"; + // For better diagnostics, let's obtain the package info after + // pkg_verify() verifies that this is a package directory. + // + package_version_info pvi; + // Verify the directory is a package and get its manifest. // package_manifest m ( pkg_verify (d, true /* ignore_unknown */, - [&o, &d] (version& v) + [&o, &d, &pvi] (version& v) { - if (optional pv = package_version (o, d)) - v = move (*pv); + pvi = package_version (o, d); + + if (pvi.version) + v = move (*pvi.version); })); l4 ([&]{trace << d << ": " << m.name << " " << m.version;}); @@ -179,8 +187,14 @@ namespace bpkg // Fix-up the package version. // - if (optional v = package_iteration ( - o, db, t, d, m.name, m.version, true /* check_external */)) + if (optional v = package_iteration (o, + db, + t, + d, + m.name, + m.version, + &pvi.info, + true /* check_external */)) m.version = move (*v); // Use the special root repository fragment as the repository fragment of @@ -191,6 +205,7 @@ namespace bpkg t, move (m.name), move (m.version), + &pvi.info, d, repository_location (), purge, @@ -256,9 +271,10 @@ namespace bpkg t, move (n), move (v), + nullptr /* package_info */, path_cast (rl.path () / pl->location), rl, - false /* purge */, + false /* purge */, simulate); } @@ -329,7 +345,7 @@ namespace bpkg fail << "unable to extract " << a << " to " << c << ": " << e; } - mc = sha256 (co, d / manifest_file); + mc = package_checksum (co, d, nullptr /* package_info */); } p->src_root = d.leaf (); // For now assuming to be in configuration. diff --git a/bpkg/rep-fetch.cxx b/bpkg/rep-fetch.cxx index 80598c5..de6e3b7 100644 --- a/bpkg/rep-fetch.cxx +++ b/bpkg/rep-fetch.cxx @@ -192,7 +192,7 @@ namespace bpkg // Parse package manifests referenced by the package directory manifests. // - static vector + static pair, vector> parse_package_manifests (const common_options& co, const dir_path& repo_dir, vector&& pms, @@ -200,8 +200,8 @@ namespace bpkg const repository_location& rl, const optional& fragment) // For diagnostics. { - auto package_info = [&rl, &fragment] (const package_manifest& pm, - diag_record& dr) + auto add_package_info = [&rl, &fragment] (const package_manifest& pm, + diag_record& dr) { dr << "package "; @@ -218,8 +218,8 @@ namespace bpkg // files and retrieve the package versions via the single `b info` call. // While at it cache the manifest paths for the future use. // - paths mfs; - vector> pvs; + paths mfs; + package_version_infos pvs; { mfs.reserve (pms.size ()); @@ -238,7 +238,7 @@ namespace bpkg { diag_record dr (fail); dr << "no manifest file for "; - package_info (pm, dr); + add_package_info (pm, dr); } mfs.push_back (move (f)); @@ -250,8 +250,9 @@ namespace bpkg // Parse package manifests, fixing up their versions. // - vector r; - r.reserve (pms.size ()); + pair, vector> r; + r.first.reserve (pms.size ()); + r.second.reserve (pms.size ()); for (size_t i (0); i != pms.size (); ++i) { @@ -266,7 +267,7 @@ namespace bpkg ifdstream ifs (f); manifest_parser mp (ifs, f.string ()); - optional& pv (pvs[i]); + optional& pv (pvs[i].version); package_manifest m ( mp, @@ -286,14 +287,15 @@ namespace bpkg { diag_record dr (fail (e.name, e.line, e.column)); dr << e.description << info; - package_info (pm, dr); + add_package_info (pm, dr); } catch (const io_error& e) { fail << "unable to read from " << f << ": " << e; } - r.emplace_back (move (pm)); + r.first.push_back (move (pm)); + r.second.push_back (move (pvs[i].info)); } return r; @@ -360,12 +362,16 @@ namespace bpkg rl, string () /* fragment */)); - fr.packages = parse_package_manifests (co, - rd, - move (pms), - iu, - rl, - empty_string /* fragment */); + pair, vector> pmi ( + parse_package_manifests (co, + rd, + move (pms), + iu, + rl, + empty_string /* fragment */)); + + fr.packages = move (pmi.first); + fr.package_infos = move (pmi.second); // Expand file-referencing package manifest values. // @@ -520,12 +526,16 @@ namespace bpkg // Parse package manifests. // - fr.packages = parse_package_manifests (co, - td, - move (pms), - iu, - rl, - fr.friendly_name); + pair, vector> pmi ( + parse_package_manifests (co, + td, + move (pms), + iu, + rl, + fr.friendly_name)); + + fr.packages = move (pmi.first); + fr.package_infos = move (pmi.second); // Expand file-referencing package manifest values checking out // submodules, if required. @@ -890,12 +900,19 @@ namespace bpkg if (exists && !full_fetch) rep_remove_package_locations (db, t, rf->name); - for (package_manifest& pm: fr.packages) + vector& pms (fr.packages); + const vector& pis (fr.package_infos); + + for (size_t i (0); i != pms.size (); ++i) { + package_manifest& pm (pms[i]); + // Fix-up the external package version iteration number. // if (rl.directory_based ()) { + assert (!pis.empty ()); + // Note that we can't check if the external package of this upstream // version and revision is already available in the configuration // until we fetch all the repositories, as some of the available @@ -909,7 +926,8 @@ namespace bpkg path_cast (rl.path () / *pm.location), pm.name, pm.version, - false /* check_external */)); + &pis[i], + false /* check_external */)); if (v) pm.version = move (*v); diff --git a/bpkg/rep-fetch.hxx b/bpkg/rep-fetch.hxx index 7905e85..2445129 100644 --- a/bpkg/rep-fetch.hxx +++ b/bpkg/rep-fetch.hxx @@ -35,6 +35,12 @@ namespace bpkg vector repositories; vector packages; + + // Empty if the build2 project info is not available for the packages. + // Currently we only retrieve it for the directory and version control + // based repositories. + // + vector package_infos; }; vector fragments; diff --git a/bpkg/types.hxx b/bpkg/types.hxx index 8e00666..6065dd8 100644 --- a/bpkg/types.hxx +++ b/bpkg/types.hxx @@ -21,6 +21,7 @@ #include +#include #include #include #include @@ -125,6 +126,10 @@ namespace bpkg using butl::default_options_entry; using butl::default_options; + // + // + using package_info = butl::b_project_info; + // Derive from ODB smart pointers to return derived database (note that the // database() functions are defined in database.hxx). // -- cgit v1.1