From 1540e984279d25e5a83894216c3610990528b95c Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Wed, 17 Apr 2019 22:41:20 +0300 Subject: Add override options to bdep-ci Namely the options are: --override, --overrides-file, --builds and --build-email. --- bdep/buildfile | 3 ++ bdep/ci-parsers.cxx | 119 ++++++++++++++++++++++++++++++++++++++++++++++++++ bdep/ci-parsers.hxx | 36 +++++++++++++++ bdep/ci-types.hxx | 22 ++++++++++ bdep/ci.cli | 64 +++++++++++++++++++++++++++ bdep/ci.cxx | 42 ++++++++++++++++++ bdep/http-service.cxx | 82 +++++++++++++++++++++++----------- bdep/http-service.hxx | 9 ++-- tests/ci.testscript | 104 +++++++++++++++++++++++++++++++++++++++++++ 9 files changed, 453 insertions(+), 28 deletions(-) create mode 100644 bdep/ci-parsers.cxx create mode 100644 bdep/ci-parsers.hxx create mode 100644 bdep/ci-types.hxx diff --git a/bdep/buildfile b/bdep/buildfile index a328629..db38734 100644 --- a/bdep/buildfile +++ b/bdep/buildfile @@ -139,6 +139,9 @@ if $cli.configured cli.cxx{new-options}: cli.options += \ --cxx-prologue "#include " + cli.cxx{ci-options}: cli.options += \ +--cxx-prologue "#include " + # Avoid generating CLI runtime and empty inline file for help topics. # cli.cxx{projects-configs}: cli.options += --suppress-cli --suppress-inline diff --git a/bdep/ci-parsers.cxx b/bdep/ci-parsers.cxx new file mode 100644 index 0000000..2d19c4b --- /dev/null +++ b/bdep/ci-parsers.cxx @@ -0,0 +1,119 @@ +// file : bdep/ci-parsers.cxx -*- C++ -*- +// copyright : Copyright (c) 2014-2019 Code Synthesis Ltd +// license : MIT; see accompanying LICENSE file + +#include + +#include // trim() + +#include + +#include // bdep::cli namespace + +namespace bdep +{ + namespace cli + { + using namespace std; + using namespace butl; + + void parser:: + parse (ci_override& r, bool& xs, scanner& s) + { + auto add = [&r] (string&& n, string&& v) + { + r.push_back ( + manifest_name_value {move (n), + move (v), + 0, 0, 0, 0, 0, 0, 0}); // Locations, etc. + }; + + string o (s.next ()); + + if (!s.more ()) + throw missing_value (o); + + string v (s.next ()); + + if (o == "--build-email") + { + add ("build-email", move (v)); + } + else if (o == "--builds") + { + add ("builds", move (v)); + } + else if (o == "--override") + { + // Validate that the value has the : form. + // + // Note that the value semantics will be verified later, with the + // package_manifest::verify_overrides() called on the final list. + // + size_t p (v.find (':')); + + if (p == string::npos) + throw invalid_value (o, v, "missing ':'"); + + string vn (v, 0, p); + string vv (v, p + 1); + + // Let's trim the name and value in case they are copied from a + // manifest file or similar. + // + trim (vn); + trim (vv); + + if (vn.empty ()) + throw invalid_value (o, v, "empty value name"); + + add (move (vn), move (vv)); + } + else if (o == "--overrides-file") + { + // Parse the manifest file into the value list. + // + // Note that the values semantics will be verified later (see above). + // + try + { + ifdstream is (v); + + // In case we end up throwing invalid_value exception, its + // description will mention the file path as an option value. + // That's why we pass the empty name to the parser constructor not + // to mention it twice. + // + manifest_parser p (is, "" /* name */); + + parse_manifest (p, r); + is.close (); + } + catch (const manifest_parsing& e) + { + throw invalid_value (o, + v, + string ("unable to parse: ") + e.what ()); + } + catch (const io_error& e) + { + // Sanitize the exception description. + // + ostringstream os; + os << "unable to read: " << e; + throw invalid_value (o, v, os.str ()); + } + } + else if (o == "--overrides") // Fake option. + { + throw unknown_option (o); + } + else + { + assert (false); // No other option is expected. + } + + xs = true; + } + } +} diff --git a/bdep/ci-parsers.hxx b/bdep/ci-parsers.hxx new file mode 100644 index 0000000..5a8f664 --- /dev/null +++ b/bdep/ci-parsers.hxx @@ -0,0 +1,36 @@ +// file : bdep/ci-parsers.hxx -*- C++ -*- +// copyright : Copyright (c) 2014-2019 Code Synthesis Ltd +// license : MIT; see accompanying LICENSE file + +// CLI parsers, included into the generated source files. +// + +#ifndef BDEP_CI_PARSERS_HXX +#define BDEP_CI_PARSERS_HXX + +#include + +namespace bdep +{ + namespace cli + { + class scanner; + + template + struct parser; + + // Accumulate package manifest override values from all the override- + // related options (--override, --overrides-file, etc), preserving their + // order. Translate each option into one or more values according to the + // option-specific semantics. + // + template <> + struct parser + { + static void + parse (ci_override&, bool&, scanner&); + }; + } +} + +#endif // BDEP_CI_PARSERS_HXX diff --git a/bdep/ci-types.hxx b/bdep/ci-types.hxx new file mode 100644 index 0000000..4b2ab6d --- /dev/null +++ b/bdep/ci-types.hxx @@ -0,0 +1,22 @@ +// file : bdep/ci-types.hxx -*- C++ -*- +// copyright : Copyright (c) 2014-2019 Code Synthesis Ltd +// license : MIT; see accompanying LICENSE file + +#ifndef BDEP_CI_TYPES_HXX +#define BDEP_CI_TYPES_HXX + +#include + +#include + +namespace bdep +{ + // This type is intended for accumulating package manifest override values + // from all the override-related options (see ci-parsers.hxx for details). + // + class ci_override: public vector + { + }; +} + +#endif // BDEP_CI_TYPES_HXX diff --git a/bdep/ci.cli b/bdep/ci.cli index 46a3488..928e884 100644 --- a/bdep/ci.cli +++ b/bdep/ci.cli @@ -3,6 +3,7 @@ // license : MIT; see accompanying LICENSE file include ; +include ; "\section=1" "\name=bdep-ci" @@ -55,6 +56,26 @@ namespace bdep \cb{git(1)}, the current branch and commit id are added as the repository URL fragment (see \l{bpkg-repository-types(1)} for details). + Some package manifest values can be overridden as part of the CI request + submission using the \cb{--override} and \cb{--overrides-file} options as + well as their \cb{--builds} and \cb{--build-email} shortcuts. This is + primarily useful for specifying alternative build configurations and/or + build notification emails. For example: + + \ + $ bdep ci --builds gcc + \ + + Note that manifest overrides override the entire value group that they + belong to. Currently, the following value groups can be overridden with + the \cb{build*-email} group overridden by default as if by specifying + an empty build email. + + \ + build-email build-{warning,error}-email + builds build-{include,exclude} + \ + While the exact interpretation of the CI request depends on the specific service, normally, the CI server will respond with a reference that can be used to query the results. See \l{brep#ci Package CI} for details on @@ -83,6 +104,49 @@ namespace bdep "Remote repository URL for the project." } + // Note: the following options are for documentation only (see --overrides + // below for details). + // + strings --override + { + ":", + "Package manifest value override. Repeat this option to override + multiple values." + } + + path --overrides-file + { + "", + "Read manifest value overrides from the specified manifest fragment + file. Repeat this option to specify multiple override files." + } + + strings --builds + { + "", + "Shortcut for \c{\b{--override\ builds:}}." + } + + string --build-email + { + "", + "Shortcut for \c{\b{--override\ build-email:}}." + } + + // All the overrides-related options are handled with a common parser and + // are collected in a single manifest value list that is accessible via + // the --overrides option accessor. The --overrides option is "fake" in + // that it only serves to collect the values in a single list while + // preserving their order. Note that for this trick (or hack, if you + // will) to work, this option should come after all the options it + // aliases. + // + ci_override --overrides | + --override | + --overrides-file | + --builds | + --build-email; + string --simulate { "", diff --git a/bdep/ci.cxx b/bdep/ci.cxx index 08193b3..8c45a11 100644 --- a/bdep/ci.cxx +++ b/bdep/ci.cxx @@ -4,6 +4,10 @@ #include +#include + +#include + #include #include @@ -144,6 +148,29 @@ namespace bdep { tracer trace ("ci"); + // Create the default override. + // + vector overrides ({ + manifest_name_value {"build-email", "", // Name and value. + 0, 0, 0, 0, 0, 0, 0}}); // Locations, etc. + + // Validate and append the specified overrides. + // + if (o.overrides_specified ()) + try + { + bpkg::package_manifest::validate_overrides (o.overrides (), + "" /* name */); + + overrides.insert (overrides.end (), + o.overrides ().begin (), + o.overrides ().end ()); + } + catch (const manifest_parsing& e) + { + fail << "invalid overrides: " << e; + } + // If we are submitting the entire project, then we have two choices: we // can list all the packages in the project or we can only do so for // packages that were initialized in the (specified) configuration(s?). @@ -268,6 +295,21 @@ namespace bdep "package", p.name.string () + '/' + p.version.string ()}); + try + { + ostringstream os; + manifest_serializer s (os, "" /* name */); + serialize_manifest (s, overrides); + + params.push_back ({parameter::file_text, "overrides", os.str ()}); + } + catch (const manifest_serialization&) + { + // Values are verified by package_manifest::validate_overrides (); + // + assert (false); + } + if (o.simulate_specified ()) params.push_back ({parameter::text, "simulate", o.simulate ()}); diff --git a/bdep/http-service.cxx b/bdep/http-service.cxx index 8b4f059..6027c9d 100644 --- a/bdep/http-service.cxx +++ b/bdep/http-service.cxx @@ -85,18 +85,30 @@ namespace bdep if (!progress) suppress_progress (); - // Convert the submit arguments to curl's --form* options. + // Convert the submit arguments to curl's --form* options and cache the + // pointer to the file_text parameter value, if present, for writing + // into curl's stdin. // strings fos; + const string* file_text (nullptr); + for (const parameter& p: params) { - fos.emplace_back (p.type == parameter::file + if (p.type == parameter::file_text) + { + assert (file_text == nullptr); + file_text = &p.value; + } + + fos.emplace_back (p.type == parameter::file || + p.type == parameter::file_text ? "--form" : "--form-string"); - fos.emplace_back (p.type == parameter::file - ? p.name + "=@" + p.value - : p.name + "=" + p.value); + fos.emplace_back ( + p.type == parameter::file ? p.name + "=@" + p.value : + p.type == parameter::file_text ? p.name + "=@-" : + p.name + "=" + p.value); } // Note that it's a bad idea to issue the diagnostics while curl is @@ -111,35 +123,43 @@ namespace bdep // // Start curl program. // - fdpipe pipe (open_pipe ()); // Text mode seems appropriate. + // Text mode seems appropriate. + // + fdpipe in_pipe (open_pipe ()); + fdpipe out_pipe (file_text != nullptr ? open_pipe () : fdpipe ()); // Note that we don't specify any default timeouts, assuming that bdep // is an interactive program and the user can always interrupt the // command (or pass the timeout with --curl-option). // - process pr (start (0 /* stdin */, - pipe /* stdout */, - 2 /* stderr */, - o.curl (), - v, - "-A", (BDEP_USER_AGENT " curl"), + process pr ( + start (file_text != nullptr ? out_pipe.in.get () : 0 /* stdin */, + in_pipe /* stdout */, + 2 /* stderr */, + o.curl (), + v, + "-A", (BDEP_USER_AGENT " curl"), - o.curl_option (), + o.curl_option (), - // Include the response headers in the output so we - // can get the status code/reason, content type, and - // the redirect location. - // - "--include", + // Include the response headers in the output so we can get the + // status code/reason, content type, and the redirect location. + // + "--include", - fos, - u.string ())); + fos, + u.string ())); // Shouldn't throw, unless something is severely damaged. // - pipe.out.close (); + in_pipe.out.close (); + + if (file_text != nullptr) + out_pipe.in.close (); + + bool io_write (false); + bool io_read (false); - bool io (false); try { // First we read the HTTP response status line and headers. At this @@ -148,10 +168,22 @@ namespace bdep // for the exception mask choice. // ifdstream is ( - move (pipe.in), + move (in_pipe.in), fdstream_mode::skip, ifdstream::badbit | ifdstream::failbit | ifdstream::eofbit); + if (file_text != nullptr) + { + ofdstream os (move (out_pipe.out)); + os << *file_text; + os.close (); + + // Indicate to the potential IO error handling that we are done with + // writing. + // + file_text = nullptr; + } + // Parse and return the HTTP status code. Return 0 if the argument is // invalid. // @@ -428,7 +460,7 @@ namespace bdep // Presumably the child process failed and issued diagnostics so let // finish() try to deal with that first. // - io = true; + (file_text != nullptr ? io_write : io_read) = true; } // Handle all parsing errors, including the manifest_parsing exception // that inherits from the runtime_error exception. @@ -455,7 +487,7 @@ namespace bdep dr << info << "reference: " << *reference; } - finish (o.curl (), pr, io); + finish (o.curl (), pr, io_read, io_write); assert (!message.empty ()); diff --git a/bdep/http-service.hxx b/bdep/http-service.hxx index 75fd603..b4eb4f2 100644 --- a/bdep/http-service.hxx +++ b/bdep/http-service.hxx @@ -5,7 +5,7 @@ #ifndef BDEP_HTTP_SERVICE_HXX #define BDEP_HTTP_SERVICE_HXX -#include +#include #include #include @@ -16,11 +16,12 @@ namespace bdep { namespace http_service { - // If type is file, then the value is a path to be uploaded. + // If type is file, then the value is a path to be uploaded. If type is + // file_text, then the value is a file content to be uploaded. // struct parameter { - enum {text, file} type; + enum {text, file, file_text} type; string name; string value; }; @@ -40,6 +41,8 @@ namespace bdep // POST method. Use the multipart/form-data content type if any files are // uploaded and application/x-www-form-urlencoded otherwise. // + // Note: currently only one file_text parameter can be specified. + // // On success, return the response manifest message and reference (if // present, see below) and the rest of the manifest values, if any. Issue // diagnostics and fail if anything goes wrong or the response manifest diff --git a/tests/ci.testscript b/tests/ci.testscript index 7bd6172..986648f 100644 --- a/tests/ci.testscript +++ b/tests/ci.testscript @@ -138,6 +138,110 @@ windows = ($cxx.target.class == 'windows') error: remote git repository URL '$repository#frag' already has fragment EOE } + + : overrides + : + { + +$clone_root_prj + +$init -C @cfg &prj-cfg/*** + + : valid + : + : Here we only test that bdep-ci does not fail for valid overrides. It + : seems to be impossible to verify the submitted overrides manifest. + : + { + $clone_prj; + + cat <=overrides.manifest; + : 1 + builds: all + EOI + + $* --overrides-file overrides.manifest \ + --build-email 'foo@example.com' \ + --override 'build-error-email: error@example.com' \ + --override 'builds: &gcc' \ + --override 'build-include: linux*' \ + --override 'build-exclude: *' 2>>~"%EOE%" + submitting to $server + %.* + %.*CI request is queued.*% + %reference: .+% + EOE + } + + : invalid-option + : + { + +$clone_prj + + : overrides-file + : + { + +$clone_prj + + : reading + : + { + $clone_prj; + + $* --overrides-file overrides.manifest 2>>~%EOE% != 0 + %error: invalid value 'overrides.manifest' for option '--overrides-file': unable to read: .+% + EOE + } + + : parsing + : + { + $clone_prj; + + cat <=overrides.manifest; + builds: all + EOI + + $* --overrides-file overrides.manifest 2>>EOE != 0 + error: invalid value 'overrides.manifest' for option '--overrides-file': unable to parse: 1:1: error: format version pair expected + EOE + } + } + + : override + : + { + $clone_prj; + + $* --override 'all' 2>>EOE != 0 + error: invalid value 'all' for option '--override': missing ':' + EOE + } + + : override-pair + : + : Each override option is valid by its own, but the resulting overrides + : manifest is invalid. + : + { + $clone_prj; + + $* --override 'builds: all' --override 'builds: default : &gcc' 2>>EOE != 0 + error: invalid overrides: invalid package builds in 'default : &gcc': unexpected underlying class set + EOE + } + + : overrides + : + : This is a fake option (see ci.cli for details). + : + { + $clone_prj; + + $* --overrides 'all' 2>>EOE != 0 + error: unknown option '--overrides' + EOE + } + } + } } : multi-pkg -- cgit v1.1