diff options
| -rw-r--r-- | bpkg/archive.cxx | 25 | ||||
| -rw-r--r-- | bpkg/archive.hxx | 16 | ||||
| -rw-r--r-- | bpkg/pkg-build.cxx | 11 | ||||
| -rw-r--r-- | bpkg/pkg-fetch.cxx | 6 | ||||
| -rw-r--r-- | bpkg/pkg-verify.cli | 16 | ||||
| -rw-r--r-- | bpkg/pkg-verify.cxx | 54 | ||||
| -rw-r--r-- | bpkg/pkg-verify.hxx | 10 | ||||
| -rw-r--r-- | bpkg/rep-create.cxx | 38 | ||||
| -rw-r--r-- | tests/pkg-verify.testscript | 24 | ||||
| -rw-r--r-- | tests/pkg-verify/foo-2.tar.gz | bin | 277 -> 413 bytes |
10 files changed, 137 insertions, 63 deletions
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<process, process> pr (start_extract (o, a, f)); + pair<process, process> 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 <bpkg/auth.hxx> #include <bpkg/fetch.hxx> -#include <bpkg/archive.hxx> #include <bpkg/checksum.hxx> #include <bpkg/diagnostics.hxx> #include <bpkg/manifest-utility.hxx> @@ -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)})); diff --git a/tests/pkg-verify.testscript b/tests/pkg-verify.testscript index 559b612..6f12c51 100644 --- a/tests/pkg-verify.testscript +++ b/tests/pkg-verify.testscript @@ -57,7 +57,7 @@ EOE : verbose : $* $src/foo-2.tar.gz 2>>/~%EOE% != 0 - foo-2/manifest:8:1: error: unknown name 'color' in package manifest + foo-2/manifest:10:1: error: unknown name 'color' in package manifest % info: package archive .+/foo-2.tar.gz% EOE @@ -69,3 +69,25 @@ EOE : $* --ignore-unknown $src/foo-2.tar.gz 2>'valid package foo 2' } + +: manifest-expand +: +$* --deep --ignore-unknown --manifest $src/foo-2.tar.gz >>EOO + : 1 + name: foo + version: 2 + summary: The "Foo" utility + license: MIT + description: \ + This package contains the foo utility. + + \ + changes: \ + Version 2 + + * First public release. + + \ + url: http://www.example.org/foo + email: foo-users@example.org + EOO diff --git a/tests/pkg-verify/foo-2.tar.gz b/tests/pkg-verify/foo-2.tar.gz Binary files differindex cb39194..eb72635 100644 --- a/tests/pkg-verify/foo-2.tar.gz +++ b/tests/pkg-verify/foo-2.tar.gz |