diff options
| -rw-r--r-- | bpkg/pkg-build.cxx | 55 | ||||
| -rw-r--r-- | bpkg/pkg-configure.cxx | 15 | ||||
| -rw-r--r-- | bpkg/pkg-unpack.cxx | 3 | ||||
| -rw-r--r-- | bpkg/pkg-verify.cxx | 178 | ||||
| -rw-r--r-- | bpkg/pkg-verify.hxx | 42 | ||||
| -rw-r--r-- | bpkg/rep-fetch.cxx | 59 | ||||
| -rw-r--r-- | bpkg/satisfaction.cxx | 26 | ||||
| -rw-r--r-- | bpkg/satisfaction.hxx | 14 |
8 files changed, 280 insertions, 112 deletions
diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx index dd48440..29dd1c9 100644 --- a/bpkg/pkg-build.cxx +++ b/bpkg/pkg-build.cxx @@ -391,7 +391,8 @@ namespace bpkg a->absolute () ? *a : db.config_orig / *a, true /* ignore_unknown */, false /* expand_values */) - : pkg_verify (sp->effective_src_root (db.config_orig), + : pkg_verify (options, + sp->effective_src_root (db.config_orig), true /* ignore_unknown */, // Copy potentially fixed up version from selected package. [&sp] (version& v) {v = sp->version;})); @@ -1128,15 +1129,19 @@ namespace bpkg // if (dn == "build2") { - if (dp.constraint) - satisfy_build2 (options, name, dp); + if (dp.constraint && !satisfy_build2 (options, dp)) + fail << "unable to satisfy constraint (" << dp + << ") for package " << name << + info << "available build2 version is " << build2_version; continue; } else if (dn == "bpkg") { - if (dp.constraint) - satisfy_bpkg (options, name, dp); + if (dp.constraint && !satisfy_bpkg (options, dp)) + fail << "unable to satisfy constraint (" << dp + << ") for package " << name << + info << "available bpkg version is " << bpkg_version; continue; } @@ -4592,10 +4597,6 @@ namespace bpkg const char* package (pa.value.c_str ()); - // Is this a package archive? - // - bool package_arc (false); - try { path a (package); @@ -4611,14 +4612,10 @@ namespace bpkg true /* ignore_unknown */, false /* expand_values */, true /* complete_depends */, - diag)); + diag ? 2 : 1)); // This is a package archive. // - // Note that throwing failed from here on will be fatal. - // - package_arc = true; - l4 ([&]{trace << "archive '" << a << "': " << arg_string (pa);}); // Supporting this would complicate things a bit, but we may add @@ -4644,15 +4641,8 @@ namespace bpkg { // Not a valid path so cannot be an archive. } - catch (const failed& e) + catch (const not_package&) { - // If this is a valid package archive but something went wrong - // afterwards, then we are done. - // - if (package_arc) - throw; - - assert (e.code == 1); } // Is this a package directory? @@ -4666,8 +4656,6 @@ namespace bpkg size_t pn (strlen (package)); if (pn != 0 && path::traits_type::is_separator (package[pn - 1])) { - bool package_dir (false); - try { dir_path d (package); @@ -4684,6 +4672,7 @@ namespace bpkg package_manifest m ( pkg_verify ( + o, d, true /* ignore_unknown */, [&o, &d, &pvi] (version& v) @@ -4693,14 +4682,10 @@ namespace bpkg if (pvi.version) v = move (*pvi.version); }, - diag)); + diag ? 2 : 1)); // This is a package directory. // - // Note that throwing failed from here on will be fatal. - // - package_dir = true; - l4 ([&]{trace << "directory '" << d << "': " << arg_string (pa);}); @@ -4741,15 +4726,8 @@ namespace bpkg { // Not a valid path so cannot be a package directory. } - catch (const failed& e) + catch (const not_package&) { - // If this is a valid package directory but something went wrong - // afterwards, then we are done. - // - if (package_dir) - throw; - - assert (e.code == 1); } } } @@ -7261,7 +7239,8 @@ namespace bpkg assert (sp->state == package_state::unpacked); package_manifest m ( - pkg_verify (sp->effective_src_root (pdb.config_orig), + pkg_verify (o, + sp->effective_src_root (pdb.config_orig), true /* ignore_unknown */, [&sp] (version& v) {v = sp->version;})); diff --git a/bpkg/pkg-configure.cxx b/bpkg/pkg-configure.cxx index 393ed68..65718ca 100644 --- a/bpkg/pkg-configure.cxx +++ b/bpkg/pkg-configure.cxx @@ -43,16 +43,20 @@ namespace bpkg // if (n == "build2") { - if (d.constraint) - satisfy_build2 (o, package, d); + if (d.constraint && !satisfy_build2 (o, d)) + fail << "unable to satisfy constraint (" << d + << ") for package " << package << + info << "available build2 version is " << build2_version; satisfied = true; break; } else if (n == "bpkg") { - if (d.constraint) - satisfy_bpkg (o, package, d); + if (d.constraint && !satisfy_bpkg (o, d)) + fail << "unable to satisfy constraint (" << d + << ") for package " << package << + info << "available bpkg version is " << bpkg_version; satisfied = true; break; @@ -371,7 +375,8 @@ namespace bpkg l4 ([&]{trace << *p;}); - package_manifest m (pkg_verify (p->effective_src_root (c), + package_manifest m (pkg_verify (o, + p->effective_src_root (c), true /* ignore_unknown */, [&p] (version& v) {v = p->version;})); diff --git a/bpkg/pkg-unpack.cxx b/bpkg/pkg-unpack.cxx index 862feac..0087086 100644 --- a/bpkg/pkg-unpack.cxx +++ b/bpkg/pkg-unpack.cxx @@ -169,7 +169,8 @@ namespace bpkg // Verify the directory is a package and get its manifest. // package_manifest m ( - pkg_verify (d, + pkg_verify (o, + d, true /* ignore_unknown */, [&o, &d, &pvi] (version& v) { diff --git a/bpkg/pkg-verify.cxx b/bpkg/pkg-verify.cxx index aae1b43..4cfaa85 100644 --- a/bpkg/pkg-verify.cxx +++ b/bpkg/pkg-verify.cxx @@ -10,6 +10,7 @@ #include <bpkg/archive.hxx> #include <bpkg/diagnostics.hxx> +#include <bpkg/satisfaction.hxx> #include <bpkg/manifest-utility.hxx> using namespace std; @@ -17,28 +18,144 @@ using namespace butl; namespace bpkg { + vector<manifest_name_value> + pkg_verify (const common_options& co, + manifest_parser& p, + const path& what, + int diag_level) + { + manifest_name_value nv (p.next ()); + + // Make sure this is the start and we support the version. + // + if (!nv.name.empty ()) + throw manifest_parsing (p.name (), nv.name_line, nv.name_column, + "start of package manifest expected"); + + if (nv.value != "1") + throw manifest_parsing (p.name (), nv.value_line, nv.value_column, + "unsupported format version"); + + vector<manifest_name_value> r; + + // For the depends name, parse the value and if it contains the build2 or + // bpkg constraints, verify that they are satisfied. + // + // Note that if the semantics of the depends value changes we may be + // unable to parse some of them before we get to build2 or bpkg and issue + // the user-friendly diagnostics. So we are going to ignore such depends + // values. But that means that if the user made a mistake in build2/bpkg + // then we will skip them as well. This, however, is not a problem since + // the pre-parsed result will then be re-parsed (e.g., by the + // package_manifest() constructor) which will diagnose any mistakes. + // + for (nv = p.next (); !nv.empty (); nv = p.next ()) + { + if (nv.name == "depends") + try + { + dependency_alternatives da (nv.value); + + for (dependency& d: da) + { + const package_name& dn (d.name); + + if (dn != "build2" && dn != "bpkg") + continue; + + if (!da.buildtime) + { + if (diag_level != 0) + error (p.name (), nv.value_line, nv.value_column) + << dn << " dependency must be build-time"; + + throw failed (); + } + + if (da.size () != 1) + { + if (diag_level != 0) + error (p.name (), nv.value_line, nv.value_column) + << "alternatives in " << dn << " dependency"; + + throw failed (); + } + + if (dn == "build2") + { + if (d.constraint && !satisfy_build2 (co, d)) + { + if (diag_level != 0) + { + diag_record dr (error); + dr << "unable to satisfy constraint (" << d << ")"; + + if (!what.empty ()) + dr << " for package " << what; + + dr << info << "available build2 version is " << build2_version; + } + + throw failed (); + } + } + else + { + if (d.constraint && !satisfy_bpkg (co, d)) + { + if (diag_level != 0) + { + diag_record dr (error); + dr << "unable to satisfy constraint (" << d << ")"; + + if (!what.empty ()) + dr << " for package " << what; + + dr << "available bpkg version is " << bpkg_version; + } + + throw failed (); + } + } + } + } + catch (const invalid_argument&) {} // Ignore + + r.push_back (move (nv)); + } + + // Make sure this is the end. + // + nv = p.next (); + if (!nv.empty ()) + throw manifest_parsing (p.name (), nv.name_line, nv.name_column, + "single package manifest expected"); + + return r; + } + package_manifest pkg_verify (const common_options& co, const path& af, bool iu, bool ev, bool cd, - bool diag) + int diag_level) try { dir_path pd (package_dir (af)); path mf (pd / manifest_file); - // If diag is false, we need to make tar not print any diagnostics. There - // doesn't seem to be an option to suppress this and the only way is to - // redirect stderr to something like /dev/null. + // If the diag level is less than 2, we need to make tar not print any + // diagnostics. There doesn't seem to be an option to suppress this and + // the only way is to redirect stderr to something like /dev/null. // // If things go badly for tar and it starts spitting errors instead of the // manifest, the manifest parser will fail. But that's ok since we assume // that the child error is always the reason for the manifest parsing // failure. // - pair<process, process> pr (start_extract (co, af, mf, diag)); + pair<process, process> pr (start_extract (co, af, mf, diag_level == 2)); auto wait = [&pr] () {return pr.second.wait () && pr.first.wait ();}; @@ -46,7 +163,11 @@ namespace bpkg { ifdstream is (move (pr.second.in_ofd), fdstream_mode::skip); manifest_parser mp (is, mf.string ()); - package_manifest m (mp, iu, cd); + + package_manifest m (mp.name (), + pkg_verify (co, mp, af, diag_level), + iu, + cd); is.close (); if (wait ()) @@ -57,7 +178,7 @@ namespace bpkg if (pd != ed) { - if (diag) + if (diag_level != 0) error << "package archive/directory name mismatch in " << af << info << "extracted from archive '" << pd << "'" << info << "expected from manifest '" << ed << "'"; @@ -70,14 +191,14 @@ namespace bpkg if (ev) { m.load_files ( - [&pd, &co, &af, diag] (const string& n, const path& p) + [&pd, &co, &af, diag_level] (const string& n, const path& p) { path f (pd / p); - string s (extract (co, af, f, diag)); + string s (extract (co, af, f, diag_level != 0)); if (s.empty ()) { - if (diag) + if (diag_level != 0) error << n << " manifest value in package archive " << af << " references empty file " << f; @@ -101,7 +222,7 @@ namespace bpkg { if (wait ()) { - if (diag) + if (diag_level != 0) error (e.name, e.line, e.column) << e.description << info << "package archive " << af; @@ -112,7 +233,7 @@ namespace bpkg { if (wait ()) { - if (diag) + if (diag_level != 0) error << "unable to extract " << mf << " from " << af; throw failed (); @@ -128,10 +249,10 @@ namespace bpkg // diagnostics, tar, specifically, doesn't mention the archive // name. // - if (diag) + if (diag_level == 2) error << af << " does not appear to be a bpkg package"; - throw failed (); + throw not_package (); } catch (const process_error& e) { @@ -142,10 +263,11 @@ namespace bpkg } package_manifest - pkg_verify (const dir_path& d, + pkg_verify (const common_options& co, + const dir_path& d, bool iu, const function<package_manifest::translate_function>& tf, - bool diag) + int diag_level) { // Parse the manifest. // @@ -153,17 +275,21 @@ namespace bpkg if (!exists (mf)) { - if (diag) + if (diag_level == 2) error << "no manifest file in package directory " << d; - throw failed (); + throw not_package (); } try { ifdstream ifs (mf); manifest_parser mp (ifs, mf.string ()); - package_manifest m (mp, tf, iu); + + package_manifest m (mp.name (), + pkg_verify (co, mp, d, diag_level), + tf, + iu); // We used to verify package directory is <name>-<version> but it is // not clear why we should enforce it in this case (i.e., the user @@ -173,25 +299,25 @@ namespace bpkg // // if (d.leaf () != ed) // { - // if (diag) - // error << "invalid package directory name '" << d.leaf () << "'" << - // info << "expected from manifest '" << ed << "'"; + // if (diag_level != 0) + // error << "invalid package directory name '" << d.leaf () << "'" << + // info << "expected from manifest '" << ed << "'"; // - // throw failed (); + // throw failed (); // } return m; } catch (const manifest_parsing& e) { - if (diag) + if (diag_level != 0) error (e.name, e.line, e.column) << e.description; throw failed (); } catch (const io_error& e) { - if (diag) + if (diag_level != 0) error << "unable to read from " << mf << ": " << e; throw failed (); @@ -224,7 +350,7 @@ namespace bpkg o.ignore_unknown (), o.deep () /* expand_values */, o.deep () /* complete_depends */, - !o.silent ())); + o.silent () ? 0 : 2)); if (o.manifest ()) { diff --git a/bpkg/pkg-verify.hxx b/bpkg/pkg-verify.hxx index 5643692..ac231ec 100644 --- a/bpkg/pkg-verify.hxx +++ b/bpkg/pkg-verify.hxx @@ -4,6 +4,8 @@ #ifndef BPKG_PKG_VERIFY_HXX #define BPKG_PKG_VERIFY_HXX +#include <libbutl/manifest-forward.hxx> + #include <libbpkg/manifest.hxx> #include <bpkg/types.hxx> @@ -20,17 +22,27 @@ namespace bpkg // expand the file-referencing manifest values (description, changes, etc), // setting them to the contents of files they refer to, set the potentially // absent description-type value to the effective description type (see - // libbpkg/manifest.hxx), and complete the dependency constraints. Throw - // failed if invalid or if something goes wrong. If diag is false, then - // don't issue diagnostics about the reason why the package is invalid. + // libbpkg/manifest.hxx), and complete the dependency constraints. + // + // Throw not_package (derived from failed) if this doesn't look like a + // package. Throw plain failed if this does looks like a package but + // something about it is invalid or if something else goes wrong. + // + // Issue diagnostics according the diag_level as follows: // + // 0 - Suppress all errors messages except for underlying system errors. + // 1 - Suppress error messages about the reason why this is not a package. + // 2 - Suppress no error messages. + // + class not_package: public failed {}; + package_manifest pkg_verify (const common_options&, const path& archive, bool ignore_unknown, bool expand_values, bool complete_depends = true, - bool diag = true); + int diag_level = 2); // Similar to the above but verifies that a source directory is a valid // package. Always translates the package version and completes dependency @@ -39,10 +51,28 @@ namespace bpkg // itself. // package_manifest - pkg_verify (const dir_path& source, + pkg_verify (const common_options&, + const dir_path& source, bool ignore_unknown, const function<package_manifest::translate_function>&, - bool diag = true); + int diag_level = 2); + + // Pre-parse the package manifest and return the name value pairs list, + // stripping the format version and the end-of-manifest/stream pairs. Also + // verify that the package is compatible with the current toolchain and + // issue diagnostics and throw failed if it is not. Pass through the + // manifest_parsing and io_error exceptions, so that the caller can decide + // how to handle them (for example, ignore them if the manifest-printing + // process has failed, etc). + // + // To omit the package location from the diagnostics, pass an empty path as + // the what argument. + // + vector<butl::manifest_name_value> + pkg_verify (const common_options&, + butl::manifest_parser&, + const path& what, + int diag_level = 2); } #endif // BPKG_PKG_VERIFY_HXX diff --git a/bpkg/rep-fetch.cxx b/bpkg/rep-fetch.cxx index cefa799..4d1a670 100644 --- a/bpkg/rep-fetch.cxx +++ b/bpkg/rep-fetch.cxx @@ -15,6 +15,7 @@ #include <bpkg/package-odb.hxx> #include <bpkg/database.hxx> #include <bpkg/rep-remove.hxx> +#include <bpkg/pkg-verify.hxx> #include <bpkg/diagnostics.hxx> #include <bpkg/manifest-utility.hxx> @@ -219,10 +220,20 @@ 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; - package_version_infos pvs; + // Note that if the package is not compatible with the toolchain, not to + // end up with an unfriendly build2 error message (referring a line in the + // bootstrap file issued by the version module), we need to verify the + // compatibility of the package manifests prior to calling `b info`. Also + // note that we cannot create the package manifest objects at this stage, + // since we need the package versions for that. Thus, we cache the + // respective name value lists instead. + // + package_version_infos pvs; + paths mfs; + vector<vector<manifest_name_value>> nvs; { mfs.reserve (pms.size ()); + nvs.reserve (pms.size ()); dir_paths pds; pds.reserve (pms.size ()); @@ -242,6 +253,38 @@ namespace bpkg add_package_info (pm, dr); } + // Provide the context if the package compatibility verification fails. + // + auto g ( + make_exception_guard ( + [&pm, &add_package_info] () + { + diag_record dr (info); + + dr << "while retrieving information for "; + add_package_info (pm, dr); + })); + + try + { + ifdstream ifs (f); + manifest_parser mp (ifs, f.string ()); + + // Note that the package directory points to something temporary + // (e.g., .bpkg/tmp/6f746365314d/) and it's probably better to omit + // it entirely (the above exception guard will print all we've got). + // + nvs.push_back (pkg_verify (co, mp, dir_path ())); + } + catch (const manifest_parsing& e) + { + fail (e.name, e.line, e.column) << e.description; + } + catch (const io_error& e) + { + fail << "unable to read from " << f << ": " << e; + } + mfs.push_back (move (f)); pds.push_back (move (d)); } @@ -261,17 +304,13 @@ namespace bpkg assert (pm.location); - const path& f (mfs[i]); - try { - ifdstream ifs (f); - manifest_parser mp (ifs, f.string ()); - optional<version>& pv (pvs[i].version); package_manifest m ( - mp, + mfs[i].string (), + move (nvs[i]), [&pv] (version& v) { if (pv) @@ -290,10 +329,6 @@ namespace bpkg dr << e.description << info; add_package_info (pm, dr); } - catch (const io_error& e) - { - fail << "unable to read from " << f << ": " << e; - } r.first.push_back (move (pm)); r.second.push_back (move (pvs[i].info)); diff --git a/bpkg/satisfaction.cxx b/bpkg/satisfaction.cxx index 52def32..cbcb5a0 100644 --- a/bpkg/satisfaction.cxx +++ b/bpkg/satisfaction.cxx @@ -128,12 +128,10 @@ namespace bpkg return s; } - static version build2_version; + version build2_version; - void - satisfy_build2 (const common_options& o, - const package_name& pkg, - const dependency& d) + bool + satisfy_build2 (const common_options& o, const dependency& d) { assert (d.name == "build2"); @@ -180,18 +178,13 @@ namespace bpkg fail << "unable to determine build2 version of " << name_b (o); } - if (!satisfies (build2_version, d.constraint)) - fail << "unable to satisfy constraint (" << d << ") for package " - << pkg << - info << "available build2 version is " << build2_version; + return satisfies (build2_version, d.constraint); } - static version bpkg_version; + version bpkg_version; - void - satisfy_bpkg (const common_options&, - const package_name& pkg, - const dependency& d) + bool + satisfy_bpkg (const common_options&, const dependency& d) { assert (d.name == "bpkg"); @@ -200,9 +193,6 @@ namespace bpkg if (bpkg_version.empty ()) bpkg_version = version (BPKG_VERSION_STR); - if (!satisfies (bpkg_version, d.constraint)) - fail << "unable to satisfy constraint (" << d << ") for package " - << pkg << - info << "available bpkg version is " << bpkg_version; + return satisfies (bpkg_version, d.constraint); } } diff --git a/bpkg/satisfaction.hxx b/bpkg/satisfaction.hxx index 7046a92..8df4580 100644 --- a/bpkg/satisfaction.hxx +++ b/bpkg/satisfaction.hxx @@ -42,13 +42,15 @@ namespace bpkg // Special build-time dependencies. // - void - satisfy_build2 (const common_options&, - const package_name&, - const dependency&); + extern version build2_version; // Set on the first satisfy_build2() call. - void - satisfy_bpkg (const common_options&, const package_name&, const dependency&); + bool + satisfy_build2 (const common_options&, const dependency&); + + extern version bpkg_version; // Set on the first satisfy_bpkg() call. + + bool + satisfy_bpkg (const common_options&, const dependency&); } #endif // BPKG_SATISFACTION_HXX |