From 559fbe0cbdb16f0c7c3a2f33e326a3b5930fd3f3 Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Wed, 17 Apr 2019 22:46:39 +0300 Subject: Add support for overrides parameter in CI request handler --- brep/handler/ci/ci-load.in | 11 +++++- doc/cli.sh | 3 ++ doc/manual.cli | 27 +++++++++++++- load/load.cli | 7 ++++ load/load.cxx | 63 +++++++++++++++++++++++++++----- mod/external-handler.cxx | 8 ---- mod/external-handler.hxx | 7 +++- mod/mod-ci.cxx | 91 +++++++++++++++++++++++++++++++++++++++++----- mod/mod-submit.cxx | 13 +++---- mod/options.cli | 7 ++++ 10 files changed, 198 insertions(+), 39 deletions(-) diff --git a/brep/handler/ci/ci-load.in b/brep/handler/ci/ci-load.in index 0a16e8c..915f9a6 100644 --- a/brep/handler/ci/ci-load.in +++ b/brep/handler/ci/ci-load.in @@ -288,11 +288,18 @@ manifest_serializer_finish loadtab="$data_dir/loadtab" run echo "$repository $display_name cache:cache" >"$loadtab" +# Apply overrides, if uploaded. +# +if [ -f "$data_dir/overrides.manifest" ]; then + loader_options+=(--overrides-file "$data_dir/overrides.manifest") +fi + # Load the requested repository packages into the brep package database for # the tenant identified by the reference. # -run "$loader" "${loader_options[@]}" --force --shallow --tenant "$reference" \ -"$loadtab" +loader_options+=(--force --shallow --tenant "$reference") + +run "$loader" "${loader_options[@]}" "$loadtab" # Remove the no longer needed CI request data directory. # diff --git a/doc/cli.sh b/doc/cli.sh index 4f0547f..e32ee02 100755 --- a/doc/cli.sh +++ b/doc/cli.sh @@ -39,12 +39,14 @@ function compile () cli -I .. -v project="brep" -v version="$version" -v date="$date" \ --include-base-last "${o[@]}" --generate-html --html-prologue-file \ man-prologue.xhtml --html-epilogue-file man-epilogue.xhtml --html-suffix .xhtml \ +--link-regex '%bpkg([-.].+)%../../bpkg/doc/bpkg$1%' \ --link-regex '%brep(#.+)?%build2-repository-interface-manual.xhtml$1%' \ ../$n.cli cli -I .. -v project="brep" -v version="$version" -v date="$date" \ --include-base-last "${o[@]}" --generate-man --man-prologue-file \ man-prologue.1 --man-epilogue-file man-epilogue.1 --man-suffix .1 \ +--link-regex '%bpkg(#.+)?%$1%' \ --link-regex '%brep(#.+)?%$1%' \ ../$n.cli } @@ -71,6 +73,7 @@ cli -I .. \ --html-epilogue-file doc-epilogue.xhtml \ --link-regex '%b([-.].+)%../../build2/doc/b$1%' \ --link-regex '%bpkg([-.].+)%../../bpkg/doc/bpkg$1%' \ +--link-regex '%bpkg(#.+)?%../../bpkg/doc/build2-package-manager-manual.xhtml$1%' \ --output-prefix build2-repository-interface- manual.cli html2ps -f doc.html2ps:a4.html2ps -o build2-repository-interface-manual-a4.ps build2-repository-interface-manual.xhtml diff --git a/doc/manual.cli b/doc/manual.cli index 56ae05e..61ef1f8 100644 --- a/doc/manual.cli +++ b/doc/manual.cli @@ -221,6 +221,11 @@ case all the versions of this package present in the repository will be tested, or both the name and version in the \c{/} form (for example, \c{libhello/1.2.3}.| +\li|Verify the optional \c{overrides} parameter. + +The overrides parameter, if specified, must be the CI overrides manifest +upload.| + \li|Verify other parameters are valid manifest name/value pairs. The value can only contain printable ASCII characters as well as tab @@ -236,6 +241,10 @@ is created in the \c{ci-data} directory with this id as its name.| The CI request manifest is saved as \c{request.manifest} into the request subdirectory created on the previous step.| +\li|Save the CI overrides manifest into the request directory. + +If the CI overrides manifest is uploaded, then it is saved as +\c{overrides.manifest} into the request subdirectory.| \li|Invoke the CI handler program. @@ -283,7 +292,8 @@ reference: \li|Send the CI request email. If \c{ci-email} is configured, send an email to this address containing the CI -request manifest and the CI result manifest.| +request manifest, the potentially empty CI overrides manifest, and the CI +result manifest.| \li|Respond to the client. @@ -329,6 +339,21 @@ is in the ISO-8601 \c{--
T::Z} form (always UTC). Note also that \c{client-ip} can be IPv4 or IPv6. +\h#ci-overrides-manifest|CI Overrides Manifest| + +The CI overrides manifest is a package manifest fragment that should be +applied to all the packages being tested. The contained values override the +whole value groups they belong to, resetting all the group values prior to +being applied. Currently, only the following value groups can be overridden: + +\ +build-email build-{warning,error}-email +builds build-{include,exclude} +\ + +See \l{bpkg#manifest-package Package Manifest} for details on these values. + + \h#ci-result-manifest|CI Result Manifest| The CI result manifest starts with the below values and in that order diff --git a/load/load.cli b/load/load.cli index 6dc6e48..17aba5b 100644 --- a/load/load.cli +++ b/load/load.cli @@ -59,6 +59,13 @@ class options specified, then the single-tenant mode is assumed." }; + brep::path --overrides-file + { + "", + "Read package manifest overrides from the specified manifest fragment + file and apply them to packages being loaded." + } + std::string --db-user|-u { "", diff --git a/load/load.cxx b/load/load.cxx index 8e640ad..d00a1af 100644 --- a/load/load.cxx +++ b/load/load.cxx @@ -42,6 +42,8 @@ using namespace butl; using namespace bpkg; using namespace brep; +using manifest_name_values = vector; + // Operation failed, diagnostics has already been issued. // struct failed {}; @@ -339,7 +341,9 @@ repository_info (const options& lo, const string& rl, const cstrings& options) // the repository. Should be called once per repository. // static void -load_packages (const shared_ptr& rp, database& db) +load_packages (const shared_ptr& rp, + database& db, + const manifest_name_values& overrides) { // packages_timestamp other than timestamp_nonexistent signals the // repository packages are already loaded. @@ -391,7 +395,7 @@ load_packages (const shared_ptr& rp, database& db) throw failed (); } - for (auto& pm: pms) + for (package_manifest& pm: pms) { shared_ptr p ( db.find (package_id (rp->tenant, pm.name, pm.version))); @@ -405,6 +409,17 @@ load_packages (const shared_ptr& rp, database& db) { if (rp->internal) { + try + { + pm.override (overrides, "" /* name */); + } + catch (const manifest_parsing&) + { + // Overrides are already validated (see below). + // + assert (false); + } + // Create internal package object. // optional dsc; @@ -727,7 +742,9 @@ load_repositories (const shared_ptr& rp, } } - load_packages (pr, db); + // We don't apply overrides to the external packages. + // + load_packages (pr, db, manifest_name_values () /* overrides */); load_repositories (pr, db, false /* shallow */); } @@ -1054,7 +1071,7 @@ try { cerr << "error: unable to ignore broken pipe (SIGPIPE) signal: " << system_error (errno, generic_category ()) << endl; // Sanitize. - return 1; + throw failed (); } cli::argv_scanner scan (argc, argv, true); @@ -1095,14 +1112,14 @@ try { cerr << "error: configuration file expected" << endl << help_info << endl; - return 1; + throw failed (); } if (argc > 2) { cerr << "error: unexpected argument encountered" << endl << help_info << endl; - return 1; + throw failed (); } // By default the tenant is empty and assumes a single-tenant mode. Let's @@ -1114,7 +1131,35 @@ try { cerr << "error: empty tenant" << endl << help_info << endl; - return 1; + throw failed (); + } + + // Parse and validate overrides, if specified. + // + manifest_name_values overrides; + + if (ops.overrides_file_specified ()) + try + { + const string& name (ops.overrides_file ().string ()); + + ifdstream is (ops.overrides_file ()); + manifest_parser mp (is, name); + overrides = parse_manifest (mp); + is.close (); + + package_manifest::validate_overrides (overrides, name); + } + catch (const manifest_parsing& e) + { + cerr << "error: unable to parse overrides: " << e << endl; + throw failed (); + } + catch (const io_error& e) + { + cerr << "error: unable to read '" << ops.overrides_file () << "': " << e + << endl; + throw failed (); } odb::pgsql::database db ( @@ -1139,7 +1184,7 @@ try { cerr << "error: package database schema differs from the current one" << endl << " info: use brep-migrate to migrate the database" << endl; - return 1; + throw failed (); } // Load the description of all the internal repositories from the @@ -1203,7 +1248,7 @@ try move (cert), priority++)); - load_packages (r, db); + load_packages (r, db, overrides); } // On the second pass over the internal repositories we load their diff --git a/mod/external-handler.cxx b/mod/external-handler.cxx index e88e4b4..4237439 100644 --- a/mod/external-handler.cxx +++ b/mod/external-handler.cxx @@ -302,10 +302,6 @@ namespace brep // assert (n.empty () && v == "1"); - // Save the format version pair. - // - r.values.push_back (move (nv)); - // Get and verify the HTTP status. // nv = p.next (); @@ -329,10 +325,6 @@ namespace brep // for (nv = p.next (); !nv.empty (); nv = p.next ()) r.values.push_back (move (nv)); - - // Save end of manifest. - // - r.values.push_back (move (nv)); } catch (const parsing& e) { diff --git a/mod/external-handler.hxx b/mod/external-handler.hxx index 66172d0..5a1d731 100644 --- a/mod/external-handler.hxx +++ b/mod/external-handler.hxx @@ -34,8 +34,11 @@ namespace brep struct result_manifest { uint16_t status; - vector values; // Note: all values, including - // status. + + // All values, including status but excluding format version and + // end-of-manifest. + // + vector values; }; optional diff --git a/mod/mod-ci.cxx b/mod/mod-ci.cxx index f5efaac..168b57c 100644 --- a/mod/mod-ci.cxx +++ b/mod/mod-ci.cxx @@ -103,6 +103,8 @@ handle (request& rq, response& rs) using namespace bpkg; using namespace xhtml; + using parser = manifest_parser; + using parsing = manifest_parsing; using serializer = manifest_serializer; using serialization = manifest_serialization; @@ -162,7 +164,10 @@ handle (request& rq, response& rs) if (!options_->ci_data_specified ()) return respond_manifest (404, "CI request submission disabled"); - // Parse the request form data. + // Parse the request form data and verify the submission size limit. + // + // Note that the submission may include the overrides upload that we don't + // expect to be large. // const name_values& rps (rq.parameters (64 * 1024)); @@ -280,6 +285,38 @@ handle (request& rq, response& rs) return respond_manifest (400, "invalid parameter " + nv.name); } + // Parse and validate overrides, if present. + // + vector overrides; + + if (params.overrides_specified ()) + try + { + istream& is (rq.open_upload ("overrides")); + parser mp (is, "overrides"); + overrides = parse_manifest (mp); + + package_manifest::validate_overrides (overrides, mp.name ()); + } + // Note that invalid_argument (thrown by open_upload() function call) can + // mean both no overrides upload or multiple overrides uploads. + // + catch (const invalid_argument&) + { + return respond_manifest (400, "overrides upload expected"); + } + catch (const parsing& e) + { + return respond_manifest (400, + string ("unable to parse overrides: ") + + e.what ()); + } + catch (const io_error& e) + { + error << "unable to read overrides: " << e; + return respond_error (); + } + try { // Note that from now on the result manifest we respond with will contain @@ -340,7 +377,7 @@ handle (request& rq, response& rs) s.next ("id", request_id); s.next ("repository", rl.string ()); - for (const string& p: params.package()) + for (const string& p: params.package ()) { if (!p.empty ()) // Skip empty package names (see above for details). s.next ("package", p); @@ -385,6 +422,7 @@ handle (request& rq, response& rs) if (n != "repository" && n != "_" && n != "package" && + n != "overrides" && n != "simulate") s.next (n, nv.value ? *nv.value : ""); } @@ -418,6 +456,39 @@ handle (request& rq, response& rs) return respond_error (); } + // Serialize the CI overrides manifest to a stream. On the stream error pass + // through the io_error exception. + // + // Note that it can't throw the serialization exception as the override + // manifest is parsed from the stream and so verified. + // + auto ovm = [&overrides] (ostream& os, bool long_lines = false) + { + try + { + serializer s (os, "overrides", long_lines); + serialize_manifest (s, overrides); + } + catch (const serialization&) {assert (false);} // See above. + }; + + // Serialize the CI overrides manifest to the submission directory. + // + path ovf (dd / "overrides.manifest"); + + if (!overrides.empty ()) + try + { + ofdstream os (ovf); + ovm (os); + os.close (); + } + catch (const io_error& e) + { + error << "unable to write to '" << ovf << "': " << e; + return respond_error (); + } + // Given that the submission data is now successfully persisted we are no // longer in charge of removing it, except for the cases when the submission // handler terminates with an error (see below for details). @@ -493,11 +564,9 @@ handle (request& rq, response& rs) rvs.emplace_back (move (nv)); }; - add ("", "1"); // Start of manifest. add ("status", "200"); add ("message", "CI request is queued"); add ("reference", request_id); - add ("", ""); // End of manifest. } assert (!rvs.empty ()); // Produced by the handler or is implied. @@ -512,9 +581,7 @@ handle (request& rq, response& rs) try { serializer s (os, "result", long_lines); - for (const manifest_name_value& nv: rvs) - s.next (nv.name, nv.value); - + serialize_manifest (s, rvs); return true; } catch (const serialization& e) @@ -592,12 +659,18 @@ handle (request& rq, response& rs) "CI request submission (" + request_id + ")", {options_->ci_email ()}); - // Write the submission request manifest. + // Write the CI request manifest. // bool r (rqm (sm.out, true /* long_lines */)); assert (r); // The serialization succeeded once, so can't fail now. - // Write the submission result manifest. + // Write the CI overrides manifest. + // + sm.out << "\n\n"; + + ovm (sm.out, true /* long_lines */); + + // Write the CI result manifest. // sm.out << "\n\n"; diff --git a/mod/mod-submit.cxx b/mod/mod-submit.cxx index ac98268..ad1b716 100644 --- a/mod/mod-submit.cxx +++ b/mod/mod-submit.cxx @@ -12,7 +12,7 @@ #include #include #include // operator<<(ostream, process_args) -#include +#include #include #include @@ -295,8 +295,9 @@ handle (request& rq, response& rs) try { // Note that providing a meaningful prefix for temp_name() is not really - // required as the temporary directory is used by brep exclusively. However, - // using the abbreviated checksum can be helpful for troubleshooting. + // required as the temporary directory is used by brep exclusively. + // However, using the abbreviated checksum can be helpful for + // troubleshooting. // td = dir_path (options_->submit_temp () / dir_path (path::traits::temp_name (ref))); @@ -601,11 +602,9 @@ handle (request& rq, response& rs) rvs.emplace_back (move (nv)); }; - add ("", "1"); // Start of manifest. add ("status", "200"); add ("message", "package submission is queued"); add ("reference", ref); - add ("", ""); // End of manifest. } assert (!rvs.empty ()); // Produced by the handler or is implied. @@ -620,9 +619,7 @@ handle (request& rq, response& rs) try { serializer s (os, "result", long_lines); - for (const manifest_name_value& nv: rvs) - s.next (nv.name, nv.value); - + serialize_manifest (s, rvs); return true; } catch (const serialization& e) diff --git a/mod/options.cli b/mod/options.cli index 01309dc..00b88db 100644 --- a/mod/options.cli +++ b/mod/options.cli @@ -797,6 +797,13 @@ namespace brep // strings package; + // Overrides file name. Must be . + // + // Note that we don't really need this name and only check if this + // parameter is specified to detect presence of the upload. + // + string overrides; + // Submission simulation outcome. // string simulate; -- cgit v1.1