From d6f50b34977cead4bd1e0bd4fe49e5e5b6f2bdd3 Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Fri, 25 Nov 2022 21:46:04 +0300 Subject: Add support for package manifest build config group values override --- libbpkg/manifest.cxx | 294 +++++++++++++++++++++++++++++++++++++-------- libbpkg/manifest.hxx | 38 ++++-- tests/overrides/driver.cxx | 18 ++- tests/overrides/testscript | 165 +++++++++++++++++++++++++ 4 files changed, 457 insertions(+), 58 deletions(-) diff --git a/libbpkg/manifest.cxx b/libbpkg/manifest.cxx index c362379..b5a654c 100644 --- a/libbpkg/manifest.cxx +++ b/libbpkg/manifest.cxx @@ -3547,7 +3547,7 @@ namespace bpkg // auto build_conf = [&m] (string&& nm) -> build_package_config& { - vector& cs (m.build_configs); + small_vector& cs (m.build_configs); auto i (find_if (cs.begin (), cs.end (), [&nm] (const build_package_config& c) @@ -3583,6 +3583,8 @@ namespace bpkg optional description; optional description_type; + m.build_configs.emplace_back ("default"); + for (nv = next (); !nv.empty (); nv = next ()) { string& n (nv.name); @@ -4421,103 +4423,301 @@ namespace bpkg return r; } - void package_manifest:: - override (const vector& nvs, const string& name) + // If validate_only is true, then the package manifest is assumed to be + // default constructed and is used as a storage for convenience of the + // validation implementation. + // + static void + override (const vector& nvs, + const string& name, + package_manifest& m, + bool validate_only) { - // Reset the build constraints value sub-group on the first call. + // The first {builds, build-{include,exclude}} override value. // - bool rbc (true); - auto reset_build_constraints = [&rbc, this] () - { - if (rbc) - { - build_constraints.clear (); - rbc = false; - } - }; + const manifest_name_value* cbc (nullptr); - // Reset the builds value group on the first call. + // The first builds override value. // - bool rb (true); - auto reset_builds = [&rb, &reset_build_constraints, this] () - { - if (rb) - { - builds.clear (); - reset_build_constraints (); - rb = false; - } - }; + const manifest_name_value* cb (nullptr); - // Reset the build emails value group on the first call. + // The first {*-builds, *-build-{include,exclude}} override value. // - bool rbe (true); - auto reset_build_emails = [&rbe, this] () - { - if (rbe) - { - build_email = nullopt; - build_warning_email = nullopt; - build_error_email = nullopt; - rbe = false; - } - }; + const manifest_name_value* pbc (nullptr); + + // The first {build-*email} override value. + // + const manifest_name_value* be (nullptr); + + // List of indexes of the overridden build configurations together with + // flags which indicate if the *-builds override value was encountered for + // this configuration. + // + vector> obcs; + // Apply overrides. + // for (const manifest_name_value& nv: nvs) { + auto bad_name = [&name, &nv] (const string& d) + { + throw !name.empty () + ? parsing (name, nv.name_line, nv.name_column, d) + : parsing (d); + }; + + // Reset the build-{include,exclude} value sub-group on the first call + // but throw if any of the {*-builds, *-build-{include,exclude}} + // override values are already encountered. + // + auto reset_build_constraints = [&cbc, &pbc, &nv, &bad_name, &m] () + { + if (cbc == nullptr) + { + if (pbc != nullptr) + bad_name ('\'' + nv.name + "' override specified together with '" + + pbc->name + "' override"); + + m.build_constraints.clear (); + cbc = &nv; + } + }; + + // Reset the {builds, build-{include,exclude}} value group on the first + // call. + // + auto reset_builds = [&cb, &nv, &reset_build_constraints, &m] () + { + if (cb == nullptr) + { + reset_build_constraints (); + + m.builds.clear (); + cb = &nv; + } + }; + + // Return the reference to the package build configuration matching the + // build config-specific builds group value override, if exists. If no + // configuration matches, then throw manifest_parsing, except for the + // validate-only mode in which case just add an empty configuration with + // this name and return the reference to it. + // + // The n argument specifies the length of the configuration name in + // {*-builds, *-build-{include,exclude}} values. + // + auto build_conf = [&pbc, &cbc, &nv, &obcs, &bad_name, &m, validate_only] + (size_t n) -> build_package_config& + { + const string& nm (nv.name); + + // If this is the first build config override value, then save its + // address. But first verify that no common build constraints group + // value overrides are applied yet and throw if that's not the case. + // + if (pbc == nullptr) + { + if (cbc != nullptr) + bad_name ('\'' + nm + "' override specified together with '" + + cbc->name + "' override"); + + pbc = &nv; + } + + small_vector& cs (m.build_configs); + + // Find the build package configuration. If there is no such a + // configuration then throw, except for the validate-only mode in + // which case just add an empty configuration with this name. + // + // Note that we are using indexes rather then configuration addresses + // due to potential reallocations. + // + size_t ci (0); // Silence Clang's 'uninitialized use' warning. + { + auto i (find_if (cs.begin (), cs.end (), + [&nm, n] (const build_package_config& c) + {return nm.compare (0, n, c.name) == 0;})); + + if (i == cs.end ()) + { + string cn (nm, 0, n); + + if (validate_only) + { + ci = cs.size (); + cs.emplace_back (move (cn)); + } + else + bad_name ("cannot override '" + nm + "' value: no build " + + "package configuration '" + cn + '\''); + } + else + ci = i - cs.begin (); + } + + build_package_config& r (cs[ci]); + bool bv (nm.compare (n, nm.size () - n, "-builds") == 0); + + // If this is the first encountered + // {*-builds, *-build-{include,exclude}} override for this build + // config, then clear this config' constraints member and add an entry + // to the overridden configs list. + // + auto i (find_if (obcs.begin (), obcs.end (), + [ci] (const auto& c) {return c.first == ci;})); + + bool first (i == obcs.end ()); + + if (first) + { + r.constraints.clear (); + + obcs.push_back (make_pair (ci, bv)); + } + + // If this is the first encountered *-builds override, then also clear + // this config' builds member. + // + if (bv && (first || !i->second)) + { + r.builds.clear (); + + if (!first) + i->second = true; + } + + return r; + }; + + // Reset the {build-*email} value group on the first call. + // + auto reset_build_emails = [&be, &nv, &m] () + { + if (be == nullptr) + { + m.build_email = nullopt; + m.build_warning_email = nullopt; + m.build_error_email = nullopt; + be = &nv; + } + }; + const string& n (nv.name); if (n == "builds") { reset_builds (); - builds.push_back (parse_build_class_expr (nv, builds.empty (), name)); + + m.builds.push_back ( + parse_build_class_expr (nv, m.builds.empty (), name)); } else if (n == "build-include") { reset_build_constraints (); - build_constraints.push_back ( + m.build_constraints.push_back ( parse_build_constraint (nv, false /* exclusion */, name)); } else if (n == "build-exclude") { reset_build_constraints (); - build_constraints.push_back ( + m.build_constraints.push_back ( + parse_build_constraint (nv, true /* exclusion */, name)); + } + else if (n.size () > 7 && n.compare (n.size () - 7, 7, "-builds") == 0) + { + build_package_config& bc (build_conf (n.size () - 7)); + + bc.builds.push_back ( + parse_build_class_expr (nv, bc.builds.empty (), name)); + } + else if (n.size () > 14 && + n.compare (n.size () - 14, 14, "-build-include") == 0) + { + build_package_config& bc (build_conf (n.size () - 14)); + + bc.constraints.push_back ( + parse_build_constraint (nv, false /* exclusion */, name)); + } + else if (n.size () > 14 && + n.compare (n.size () - 14, 14, "-build-exclude") == 0) + { + build_package_config& bc (build_conf (n.size () - 14)); + + bc.constraints.push_back ( parse_build_constraint (nv, true /* exclusion */, name)); } else if (n == "build-email") { reset_build_emails (); - build_email = parse_email (nv, "build", name, true /* empty */); + m.build_email = parse_email (nv, "build", name, true /* empty */); } else if (n == "build-warning-email") { reset_build_emails (); - build_warning_email = parse_email (nv, "build warning", name); + m.build_warning_email = parse_email (nv, "build warning", name); } else if (n == "build-error-email") { reset_build_emails (); - build_error_email = parse_email (nv, "build error", name); + m.build_error_email = parse_email (nv, "build error", name); } else + bad_name ("cannot override '" + n + "' value"); + } + + // Common build constraints and build config overrides are mutually + // exclusive. + // + assert (cbc == nullptr || pbc == nullptr); + + // Now, if not in the validate-only mode, as all the potential build + // constraint overrides are applied, perform the final adjustments to the + // build config constraints. + // + if (!validate_only) + { + if (cbc != nullptr) // Common build constraints are overridden? { - string d ("cannot override '" + n + "' value"); + for (build_package_config& c: m.build_configs) + { + c.builds.clear (); + c.constraints.clear (); + } + } + else if (pbc != nullptr) // Build config constraints are overridden? + { + for (size_t i (0); i != m.build_configs.size (); ++i) + { + if (find_if (obcs.begin (), obcs.end (), + [i] (const auto& pc) {return pc.first == i;}) == + obcs.end ()) + { + build_package_config& c (m.build_configs[i]); - throw !name.empty () - ? parsing (name, nv.name_line, nv.name_column, d) - : parsing (d); + c.builds.clear (); + c.constraints.clear (); + c.builds.emplace_back ("none", "" /* comment */); + } + } } } } void package_manifest:: + override (const vector& nvs, const string& name) + { + bpkg::override (nvs, name, *this, false /* validate_only */); + } + + void package_manifest:: validate_overrides (const vector& nvs, const string& name) { package_manifest p; - p.override (nvs, name); + bpkg::override (nvs, name, p, true /* validate_only */); } static const string description_file ("description-file"); diff --git a/libbpkg/manifest.hxx b/libbpkg/manifest.hxx index ae2051e..3b75830 100644 --- a/libbpkg/manifest.hxx +++ b/libbpkg/manifest.hxx @@ -1169,7 +1169,11 @@ namespace bpkg butl::small_vector builds; std::vector build_constraints; - std::vector build_configs; + // Note that the parsing constructor adds the implied (empty) default + // configuration at the beginning of the list. Also note that serialize() + // writes no values for such a configuration. + // + butl::small_vector build_configs; // 1 for default. // If true, then this package use the alternative buildfile naming scheme // (build2/, .build2). In the manifest serialization this is encoded as @@ -1283,18 +1287,29 @@ namespace bpkg // // The specified values override the whole groups they belong to, // resetting all the group values prior to being applied. Currently, only - // the following value groups can be overridden: {build-*email} and - // {builds, build-{include,exclude}}. + // the following value groups can be overridden: + // + // {build-*email} + // {builds, build-{include,exclude}} + // {*-builds, *-build-{include,exclude}} // - // Note that the build constraints group values are overridden - // hierarchically so that the build-{include,exclude} overrides don't - // affect the builds values. + // Note that the build constraints group values (both common and build + // config-specific) are overridden hierarchically so that the + // [*-]build-{include,exclude} overrides don't affect the respective + // [*-]builds values. + // + // Also note that the common and build config-specific build constraints + // group value overrides are mutually exclusive. If the common build + // constraints are overridden, then all the build config-specific + // constraints are removed. Otherwise, if some build config-specific + // constraints are overridden, then for the remaining configs the build + // constraints are reset to `builds: none`. // // If a non-empty source name is specified, then the specified values are // assumed to also include the line/column information and the possibly - // thrown manifest_parsing exception will contain the invalid value + // thrown manifest_parsing exception will contain the invalid value's // location information. Otherwise, the exception description will refer - // to the invalid value name instead. + // to the invalid value instead. // void override (const std::vector&, @@ -1302,6 +1317,13 @@ namespace bpkg // Validate the overrides without applying them to any manifest. // + // Specifically, validate that the override values can be parsed according + // to their name semantics and that the value sequence makes sense (no + // mutually exclusive values, etc). Note, however, that the subsequent + // applying of the successfully validated overrides to a specific package + // manifest may still fail (no build config exists for specified *-builds, + // etc). + // static void validate_overrides (const std::vector&, const std::string& source_name); diff --git a/tests/overrides/driver.cxx b/tests/overrides/driver.cxx index be3e0ff..62ac7f8 100644 --- a/tests/overrides/driver.cxx +++ b/tests/overrides/driver.cxx @@ -33,7 +33,7 @@ main (int argc, char* argv[]) { vector overrides; - bool name (false); + string name; uint64_t l (1); for (int i (1); i != argc; ++i) @@ -42,7 +42,7 @@ main (int argc, char* argv[]) if (a == "-n") { - name = true; + name = "args"; } else { @@ -78,7 +78,19 @@ main (int argc, char* argv[]) try { package_manifest m (p); - m.override (overrides, name ? "args" : string ()); + m.override (overrides, name); + + // While at it, test validate_overrides(). + // + try + { + package_manifest::validate_overrides (overrides, name); + } + catch (const manifest_parsing& e) + { + assert (false); // Validation must never fail if override succeeds. + } + m.serialize (s); } catch (const manifest_parsing& e) diff --git a/tests/overrides/testscript b/tests/overrides/testscript index babe57d..95a5593 100644 --- a/tests/overrides/testscript +++ b/tests/overrides/testscript @@ -35,6 +35,10 @@ builds: default build-include: linux* build-exclude: *; Only supports Linux. + network-build-config: config.libfoo.network=true + network-builds: default + network-build-include: linux* + network-build-exclude: * EOI : 1 name: libfoo @@ -42,6 +46,7 @@ summary: Modern C++ parser license: LGPLv2 builds: gcc + network-build-config: config.libfoo.network=true EOO : build-include-exclude @@ -54,6 +59,10 @@ license: LGPLv2 builds: default build-exclude: freebsd* + network-build-config: config.libfoo.network=true + network-builds: default + network-build-include: linux* + network-build-exclude: * EOI : 1 name: libfoo @@ -63,6 +72,7 @@ builds: default build-include: linux* build-exclude: *; Only supports Linux. + network-build-config: config.libfoo.network=true EOO : builds-build-include-exclude @@ -86,6 +96,96 @@ build-exclude: *; Only supports Linux. EOO + : build-configs + : + $* 'network-builds: all' 'network-build-include: windows*' 'network-build-exclude: *' \ + 'cache-build-include: freebsd*' 'cache-build-exclude: *' 'cache-builds: legacy' \ + 'sys-build-include: linux*' 'sys-build-exclude: *' \ + 'fancy-builds: gcc' <>EOO + : 1 + name: libfoo + version: 2.0.0 + summary: Modern C++ parser + license: LGPLv2 + builds: all + build-include: linux* + build-include: macos* + build-include: freebsd* + build-exclude: * + network-build-config: config.libfoo.network=true + network-builds: default + network-build-include: linux* + network-build-exclude: * + cache-build-config: config.libfoo.cache=true + cache-builds: default + cache-build-include: macos* + cache-build-exclude: * + sys-build-config: ?sys:libcrypto + sys-builds: default + sys-build-include: freebsd* + sys-build-exclude: * + older-build-config: ?libbar/1.0.0 + older-builds: default + older-build-include: windows* + older-build-exclude: * + fancy-build-config: config.libfoo.fancy=true + fancy-builds: default + fancy-build-include: windows* + fancy-build-exclude: * + EOI + : 1 + name: libfoo + version: 2.0.0 + summary: Modern C++ parser + license: LGPLv2 + builds: all + build-include: linux* + build-include: macos* + build-include: freebsd* + build-exclude: * + default-builds: none + network-build-config: config.libfoo.network=true + network-builds: all + network-build-include: windows* + network-build-exclude: * + cache-build-config: config.libfoo.cache=true + cache-builds: legacy + cache-build-include: freebsd* + cache-build-exclude: * + sys-build-config: ?sys:libcrypto + sys-builds: default + sys-build-include: linux* + sys-build-exclude: * + older-build-config: ?libbar/1.0.0 + older-builds: none + fancy-build-config: config.libfoo.fancy=true + fancy-builds: gcc + EOO + + + : build-config-default + : + $* 'default-builds: all' 'default-build-include: windows*' 'default-build-exclude: *' <>EOO + : 1 + name: libfoo + version: 2.0.0 + summary: Modern C++ parser + license: LGPLv2 + network-build-config: config.libfoo.network=true + network-builds: all + EOI + : 1 + name: libfoo + version: 2.0.0 + summary: Modern C++ parser + license: LGPLv2 + default-builds: all + default-build-include: windows* + default-build-exclude: * + network-build-config: config.libfoo.network=true + network-builds: none + EOO + : none : $* <>EOO @@ -141,4 +241,69 @@ EOI args:2:8: error: invalid package builds: unexpected underlying class set EOE + + + : no-build-config + : + $* 'network-builds: default' <>EOE != 0 + : 1 + name: libfoo + version: 2.0.0 + summary: Modern C++ parser + license: LGPLv2 + EOI + cannot override 'network-builds' value: no build package configuration 'network' + EOE + + : config-builds-after-builds + : + $* 'builds: all' 'network-builds: default' <>EOE != 0 + : 1 + name: libfoo + version: 2.0.0 + summary: Modern C++ parser + license: LGPLv2 + network-build-config: config.libfoo.network=true + EOI + 'network-builds' override specified together with 'builds' override + EOE + + : config-builds-after-build-exclude + : + $* 'build-exclude: *' 'network-builds: default' <>EOE != 0 + : 1 + name: libfoo + version: 2.0.0 + summary: Modern C++ parser + license: LGPLv2 + network-build-config: config.libfoo.network=true + EOI + 'network-builds' override specified together with 'build-exclude' override + EOE + + : builds-after-config-builds + : + $* 'network-builds: default' 'builds: all' <>EOE != 0 + : 1 + name: libfoo + version: 2.0.0 + summary: Modern C++ parser + license: LGPLv2 + network-build-config: config.libfoo.network=true + EOI + 'builds' override specified together with 'network-builds' override + EOE + + : build-exclude-after-config-builds + : + $* 'network-builds: default' 'build-exclude: *' <>EOE != 0 + : 1 + name: libfoo + version: 2.0.0 + summary: Modern C++ parser + license: LGPLv2 + network-build-config: config.libfoo.network=true + EOI + 'build-exclude' override specified together with 'network-builds' override + EOE } -- cgit v1.1