From 0e31447976e338956f4aef98930f2f28261d9983 Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Mon, 12 Nov 2018 14:38:43 +0300 Subject: Add pkg-verify --deep option --- bpkg/archive.cxx | 25 +++++++++++++++---------- bpkg/archive.hxx | 16 +++++++++++----- bpkg/pkg-build.cxx | 11 +++++++++-- bpkg/pkg-fetch.cxx | 6 +++++- bpkg/pkg-verify.cli | 16 +++++++++++++--- bpkg/pkg-verify.cxx | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++-- bpkg/pkg-verify.hxx | 10 ++++++---- bpkg/rep-create.cxx | 38 +++---------------------------------- 8 files changed, 114 insertions(+), 62 deletions(-) (limited to 'bpkg') diff --git a/bpkg/archive.cxx b/bpkg/archive.cxx index f61e7f3..d9b4265 100644 --- a/bpkg/archive.cxx +++ b/bpkg/archive.cxx @@ -30,7 +30,7 @@ namespace bpkg start_extract (const common_options& co, const path& a, const path& f, - bool err) + bool diag) { assert (!f.empty () && f.relative ()); @@ -106,17 +106,17 @@ namespace bpkg if (verb >= 2) print_process (args); - // If err is false, then redirect stderr to stdout. + // If diag is false, then redirect stderr to stdout. // - auto_fd nfd (err ? nullfd : fdnull ()); + auto_fd nfd (diag ? nullfd : fdnull ()); if (i != 0) { - dpr = process (dpp, &args[what = 0], 0, -1, (err ? 2 : nfd.get ())); - tpr = process (tpp, &args[what = i], dpr, -1, (err ? 2 : nfd.get ())); + dpr = process (dpp, &args[what = 0], 0, -1, (diag ? 2 : nfd.get ())); + tpr = process (tpp, &args[what = i], dpr, -1, (diag ? 2 : nfd.get ())); } else - tpr = process (tpp, &args[what = 0], 0, -1, (err ? 2 : nfd.get ())); + tpr = process (tpp, &args[what = 0], 0, -1, (diag ? 2 : nfd.get ())); return make_pair (move (dpr), move (tpr)); } @@ -132,10 +132,10 @@ namespace bpkg } string - extract (const common_options& o, const path& a, const path& f) + extract (const common_options& o, const path& a, const path& f, bool diag) try { - pair pr (start_extract (o, a, f)); + pair pr (start_extract (o, a, f, diag)); try { @@ -163,12 +163,17 @@ namespace bpkg // While it is reasonable to assuming the child process issued diagnostics // if exited with an error status, tar, specifically, doesn't mention the // archive name. So print the error message whatever the child exit status - // is. + // is, if the diagnostics is requested. // - fail << "unable to extract " << f << " from " << a << endf; + if (diag) + error << "unable to extract " << f << " from " << a; + + throw failed (); } catch (const process_error& e) { + // Note: this is not a "file can't be extracted" case, so no diag check. + // fail << "unable to extract " << f << " from " << a << ": " << e << endf; } } diff --git a/bpkg/archive.hxx b/bpkg/archive.hxx index 17e854a..8100870 100644 --- a/bpkg/archive.hxx +++ b/bpkg/archive.hxx @@ -20,8 +20,9 @@ namespace bpkg package_dir (const path& archive); // Start the process of extracting the specified file from the archive. If - // error is false, then redirect stderr to /dev/null (this can be used, for - // example, to suppress diagnostics). + // diag is false, then redirect stderr to /dev/null (this can be used, for + // example, to suppress diagnostics). Note that in this case process errors + // (like unable to start) are still printed. // // Return a pair of processes that form a pipe. Wait on the second first. // @@ -29,12 +30,17 @@ namespace bpkg start_extract (const common_options&, const path& archive, const path& file, - bool error = true); + bool diag = true); - // Start as above and then extract the file content as a string. + // Start as above and then extract the file content as a string. If diag is + // false, then don't issue diagnostics about the reason why the file can't + // be extracted (not present, the archive is broken, etc). // string - extract (const common_options&, const path& archive, const path& file); + extract (const common_options&, + const path& archive, + const path& file, + bool diag = true); } #endif // BPKG_ARCHIVE_HXX diff --git a/bpkg/pkg-build.cxx b/bpkg/pkg-build.cxx index 655f0de..ad61196 100644 --- a/bpkg/pkg-build.cxx +++ b/bpkg/pkg-build.cxx @@ -187,7 +187,10 @@ namespace bpkg package_manifest m ( sp->state == package_state::fetched - ? pkg_verify (options, a->absolute () ? *a : c / *a, true) + ? pkg_verify (options, + a->absolute () ? *a : c / *a, + false /* expand_values */, + true /* ignore_unknown */) : pkg_verify (sp->effective_src_root (c), true)); // Copy the possibly fixed up version from the selected package. @@ -2912,7 +2915,11 @@ namespace bpkg info << "'" << package << "' does not appear to be a valid " << "package archive: "; - package_manifest m (pkg_verify (o, a, true, diag)); + package_manifest m (pkg_verify (o, + a, + false /* expand_values */, + true /* ignore_unknown */, + diag)); // This is a package archive. // diff --git a/bpkg/pkg-fetch.cxx b/bpkg/pkg-fetch.cxx index 0ce6d9e..261bc86 100644 --- a/bpkg/pkg-fetch.cxx +++ b/bpkg/pkg-fetch.cxx @@ -162,7 +162,11 @@ namespace bpkg // Verify archive is a package and get its manifest. // - package_manifest m (pkg_verify (co, a, true)); + package_manifest m (pkg_verify (co, + a, + false /* expand_values */, + true /* ignore_unknown */)); + l4 ([&]{trace << m.name << " " << m.version;}); // Check/diagnose an already existing package. diff --git a/bpkg/pkg-verify.cli b/bpkg/pkg-verify.cli index 5432743..a22c224 100644 --- a/bpkg/pkg-verify.cli +++ b/bpkg/pkg-verify.cli @@ -22,8 +22,10 @@ namespace bpkg The \cb{pkg-verify} command verifies that the specified archive file is a valid \cb{bpkg} package. Specifically, it checks that the file's name and the top-level directory inside the archive match the canonical - \c{\i{name}\b{-}\i{version}} form and that there is a valid manifest - file in that top-level directory." + \c{\i{name}\b{-}\i{version}} form and that there is a valid manifest file + in that top-level directory. Additionally, if the \cb{--deep} option is + specified, it also checks that the files referenced by the \cb{*-file} + manifest values are present in the archive and are not empty." } class pkg_verify_options: common_options @@ -36,6 +38,11 @@ namespace bpkg invalid. Just return the error status." } + bool --deep + { + "Verify files referenced by the \cb{*-file} manifest values." + } + bool --ignore-unknown { "Ignore unknown manifest entries. By default, \cb{bpkg} will refuse to @@ -46,7 +53,10 @@ namespace bpkg bool --manifest { "Instead of printing the successful verification result in the - human-readable form, dump the package manifest to \cb{stdout}." + human-readable form, dump the package manifest to \cb{stdout}. If the + \cb{--deep} option is specified, then in the resulting manifest the + \cb{*-file} values are replaced with the contents of the referenced + files." } }; } diff --git a/bpkg/pkg-verify.cxx b/bpkg/pkg-verify.cxx index a372efa..c55fba9 100644 --- a/bpkg/pkg-verify.cxx +++ b/bpkg/pkg-verify.cxx @@ -20,7 +20,11 @@ using namespace butl; namespace bpkg { package_manifest - pkg_verify (const common_options& co, const path& af, bool iu, bool diag) + pkg_verify (const common_options& co, + const path& af, + bool ev, + bool iu, + bool diag) try { dir_path pd (package_dir (af)); @@ -62,6 +66,52 @@ namespace bpkg throw failed (); } + // Expand the *-file manifest values, if requested. + // + if (ev) + { + // Expand the description-file manifest value. + // + if (m.description && m.description->file) + { + path f (pd / m.description->path); + string s (extract (co, af, f, diag)); + + if (s.empty ()) + { + if (diag) + error << "description-file manifest value in package archive " + << af << " references empty file " << f; + + throw failed (); + } + + m.description = text_file (move (s)); + } + + // Expand the changes-file manifest values. + // + for (auto& c: m.changes) + { + if (c.file) + { + path f (pd / c.path); + string s (extract (co, af, f, diag)); + + if (s.empty ()) + { + if (diag) + error << "changes-file manifest value in package archive " + << af << " references empty file " << f; + + throw failed (); + } + + c = text_file (move (s)); + } + } + } + return m; } @@ -190,7 +240,7 @@ namespace bpkg try { package_manifest m ( - pkg_verify (o, a, o.ignore_unknown (), !o.silent ())); + pkg_verify (o, a, o.deep (), o.ignore_unknown (), !o.silent ())); if (o.manifest ()) { diff --git a/bpkg/pkg-verify.hxx b/bpkg/pkg-verify.hxx index 907b37c..a772a8f 100644 --- a/bpkg/pkg-verify.hxx +++ b/bpkg/pkg-verify.hxx @@ -17,14 +17,16 @@ namespace bpkg int pkg_verify (const pkg_verify_options&, cli::scanner& args); - // Verify archive is a valid package and return its manifest. 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. + // Verify archive is a valid package and return its manifest. If requested, + // expand the file-referencing manifest values (description, changes, etc), + // setting them to the contents of files they refer to. 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. // package_manifest pkg_verify (const common_options&, const path& archive, + bool expand_values, bool ignore_unknown, bool diag = true); diff --git a/bpkg/rep-create.cxx b/bpkg/rep-create.cxx index ad63267..16f6496 100644 --- a/bpkg/rep-create.cxx +++ b/bpkg/rep-create.cxx @@ -15,7 +15,6 @@ #include #include -#include #include #include #include @@ -99,7 +98,9 @@ namespace bpkg // Verify archive is a package and get its manifest. // path a (d / p); - package_manifest m (pkg_verify (o, a, o.ignore_unknown ())); + + package_manifest m ( + pkg_verify (o, a, true /* expand_values */, o.ignore_unknown ())); // Calculate its checksum. // @@ -112,39 +113,6 @@ namespace bpkg // m.location = a.leaf (root); - dir_path pd (m.name.string () + "-" + m.version.string ()); - - // Expand the description-file manifest value. - // - if (m.description && m.description->file) - { - path f (pd / m.description->path); - string s (extract (o, a, f)); - - if (s.empty ()) - fail << "description-file value in manifest of package archive " - << a << " references empty file " << f; - - m.description = text_file (move (s)); - } - - // Expand the changes-file manifest values. - // - for (auto& c: m.changes) - { - if (c.file) - { - path f (pd / c.path); - string s (extract (o, a, f)); - - if (s.empty ()) - fail << "changes-file value in manifest of package archive " << a - << " references empty file " << f; - - c = text_file (move (s)); - } - } - package_key k {m.name, m.version}; // Argument evaluation order. auto r (map.emplace (move (k), package_data {a, move (m)})); -- cgit v1.1