From 539d23829e2857c480882bc9a1168c0007feaf67 Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Thu, 7 Oct 2021 22:46:05 +0300 Subject: Change worker script to install tools and modules from configuration separate from build configuration --- bbot/worker/worker.cxx | 638 ++++++++++++++++++++++++++++++++++++------------- 1 file changed, 469 insertions(+), 169 deletions(-) (limited to 'bbot') diff --git a/bbot/worker/worker.cxx b/bbot/worker/worker.cxx index 4688373..957c74c 100644 --- a/bbot/worker/worker.cxx +++ b/bbot/worker/worker.cxx @@ -784,7 +784,7 @@ upload_manifest (tracer& trace, } } -static const string worker_checksum ("1"); // Logic version. +static const string worker_checksum ("2"); // Logic version. static int bbot:: build (size_t argc, const char* argv[]) @@ -806,7 +806,7 @@ build (size_t argc, const char* argv[]) // // 3. Upload the result manifest. // - // NOTE: consider updating agent_checksum if making any logic changes. + // NOTE: consider updating worker_checksum if making any logic changes. // // Note also that we are being "watched" by the startup version of us which // will upload an appropriate result in case we exit with an error. So here @@ -1174,6 +1174,11 @@ build (size_t argc, const char* argv[]) dir_path dist_root (rwd / dir_path ("dist")); dir_path dist_src (dist_root / pkg_dir); + dir_path dist_install_root (rwd / dir_path ("dist-install")); + dir_path dist_install_src (dist_install_root / pkg_dir); + + dir_path dist_installed_root (rwd / dir_path ("dist-installed")); + // Redistribute the package source directory (pkg_dir) checked out into // the directory other than the configuration directory (dist_root) and // replace it with the newly created distribution. Assume that the current @@ -1239,6 +1244,18 @@ build (size_t argc, const char* argv[]) // by the same reason, we don't install tools or modules for // non-self-hosted configurations. // + // Actually, it could make sense to build and install tools and module + // from a target configuration in this case. But that means for a + // non-self-hosted configuration a tool/module may want to test two + // things: its output build and its own build, which means we would need a + // way to control which of the two things (or both) are to be tested + // (think of two cross-compiler configurations, Emscripten and MinGW: for + // the former a source code generator would normally only want to test the + // output while for the latter -- both; maybe we could have a `cross-host` + // class, meaning that the configuration is not host itself but its target + // is). In any case, seeing that there is no way to verify such own build + // works, we ignore this for now. + // // Also note that build system modules can only have external build-time // tests (which is verified by bpkg-rep-fetch) and target packages cannot // have external build-time tests (which we verify ourselves). @@ -1304,9 +1321,20 @@ build (size_t argc, const char* argv[]) // configuration creation to take its course (since that would probably be // the most typical usage scenario). // - dir_path target_conf ("build"); - dir_path host_conf ("build-host"); - dir_path module_conf ("build-module"); + // Also note that we may need a separate target configuration to build the + // host package for installation. This is required to avoid a potential + // conflict between the main package and a tool it may try to run during + // the build. We also do the same for module packages which, while cannot + // have build-time dependencies, could have private code generators. This + // configuration needs to have the target type (so that it uses any + // build-time dependencies from build-host/module configurations). Note + // also that we currently only do this for self-hosted configuration + // (since we don't install otherwise, see above). + // + dir_path target_conf ("build"); + dir_path host_conf ("build-host"); + dir_path module_conf ("build-module"); + dir_path install_conf ("build-install"); // Main package config. // @@ -1340,12 +1368,23 @@ build (size_t argc, const char* argv[]) // bool create_module (module_pkg || (host_pkg && has_buildtime_tests)); + // Create the configuration for installing the main package of the host or + // module type, unless it's not supposed to be installed. + // + bool create_install (!target_pkg && !install_root.empty () && selfhost); + // Root configuration through which we will be configuring the cluster // (note: does not necessarily match the main package type). // // In other words, this is configuration that will be specified for // bpkg-pkg-build as the current configuration (via -d). It must be the - // configuration that links to all the other configurations. + // configuration that links to all the other configurations, except + // install. + // + // Note that the install configuration, if present, is either the + // cluster's "second root" (for a host package) or is an independent + // cluster (for a module package). In either case it needs to additionally + // be specified as a current configuration on the command line. // const dir_path& root_conf (create_target ? target_conf : create_host ? host_conf : @@ -1371,6 +1410,25 @@ build (size_t argc, const char* argv[]) // change_wd (trace, &r.log, rwd); + // If we end up with multiple current configurations (root and install) + // then when running the bpkg-pkg-build command we need to specify the + // configuration for each package explicitly via --config-uuid. + // + // Let's not generate random UUIDs but use some predefined values which + // we can easily recognize in the build logs. + // + const char* target_uuid ("00000000-0000-0000-0000-000000000001"); + const char* host_uuid ("00000000-0000-0000-0000-000000000002"); + const char* module_uuid ("00000000-0000-0000-0000-000000000003"); + const char* install_uuid ("00000000-0000-0000-0000-000000000004"); + + // Let's however distinguish the target package as a simple common case + // and simplify the configuration creation and packages configuration + // commands making them more readable in the build log. For this simple + // case only one configuration needs to be created explicitly and so it + // doesn't need the UUID. Also there is no need in any package-specific + // options for the bpkg-pkg-build command in this case. + // // Create the target configuration. // // bpkg create @@ -1389,6 +1447,7 @@ build (size_t argc, const char* argv[]) "-V", "create", "-d", target_conf, + !target_pkg ? cstrings ({"--uuid", target_uuid}) : cstrings (), step_args (modules, s, f1, f2), step_args (env_args, s, f1, f2), step_args (config_args, s, f1, f2)); @@ -1397,34 +1456,65 @@ build (size_t argc, const char* argv[]) break; } - // Create the host configuration. + // Create the host configurations. // if (create_host) { - step_id b (step_id::bpkg_create); - step_id s (step_id::bpkg_host_create); - step_id f1 (step_id::b_create); - step_id f2 (step_id::bpkg_create); + step_id b (step_id::bpkg_create); if (host_pkg && selfhost) { - // bpkg create --type host + // Create the host configuration. // - r.status |= run_bpkg ( - b, - trace, r.log, wre, - bkp_step, bkp_status, last_cmd, - "-V", - "create", - "-d", host_conf, - "--type", "host", - "--name", "host", - step_args (modules, s, f1, f2), - step_args (env_args, s, f1, f2), - step_args (config_args, s, f1, f2)); + { + step_id s (step_id::bpkg_host_create); + step_id f1 (step_id::b_create); + step_id f2 (step_id::bpkg_create); - if (!r.status) - break; + // bpkg create --type host + // + r.status |= run_bpkg ( + b, + trace, r.log, wre, + bkp_step, bkp_status, last_cmd, + "-V", + "create", + "-d", host_conf, + "--type", "host", + "--uuid", host_uuid, + step_args (modules, s, f1, f2), + step_args (env_args, s, f1, f2), + step_args (config_args, s, f1, f2)); + + if (!r.status) + break; + } + + // Create the install configuration. + // + // bpkg create + // + if (create_install) + { + step_id s (step_id::bpkg_target_create); + step_id f1 (step_id::b_create); + step_id f2 (step_id::bpkg_create); + + r.status |= run_bpkg ( + b, + trace, r.log, wre, + bkp_step, bkp_status, last_cmd, + "-V", + "create", + "-d", install_conf, + "--uuid", install_uuid, + step_args (modules, s, f1, f2), + step_args (env_args, s, f1, f2), + step_args (config_args, s, f1, f2)); + + if (!r.status) + break; + } } else { @@ -1455,37 +1545,92 @@ build (size_t argc, const char* argv[]) "--existing", "-d", host_conf, "--type", "host", - "--name", "host"); + "--uuid", host_uuid); if (!r.status) break; } } - // Create the module configuration. + // Create the module configurations. // if (create_module) { step_id b (step_id::bpkg_create); - // b create() config.config.load=~build2 [ ] - // - // Note also that we suppress warnings about unused config.* values. - // - // What if a module wants to use CLI? The current thinking is that we - // will be "whitelisting" base (i.e., those that can plausibly be used - // by multiple modules) libraries and tools for use by build system - // modules. So if and when we whitelist CLI, we will add it here, next - // to cc. + // Create the module configuration. // - cstrings eas; - cstrings cas; - string mods; + { + // b create() config.config.load=~build2 [ ] + // + // Note also that we suppress warnings about unused config.* values. + // + // What if a module wants to use CLI? The current thinking is that we + // will be "whitelisting" base (i.e., those that can plausibly be used + // by multiple modules) libraries and tools for use by build system + // modules. So if and when we whitelist CLI, we will add it here, next + // to cc. + // + string mods; + cstrings eas; + cstrings cas; + + if (module_pkg && selfhost) + { + step_id s (step_id::bpkg_module_create); + + for (const char* m: step_args (modules, s)) + { + if (!mods.empty ()) + mods += ' '; + + mods += m; + } + + eas = step_args (env_args, s); + cas = step_args (config_args, s); + } + else + mods = "cc"; + + r.status |= run_b ( + b, + trace, r.log, wre, + bkp_step, bkp_status, last_cmd, + "-V", + "create(" + module_conf.representation () + "," + mods + ")", + "config.config.load=~build2", + "config.config.persist+='config.*'@unused=drop", + eas, + cas); + + if (!r.status) + break; + + // bpkg create --existing --type build2 + // + r.status |= run_bpkg ( + b, + trace, r.log, wre, + bkp_step, bkp_status, last_cmd, + "-v", + "create", + "--existing", + "-d", module_conf, + "--type", "build2", + "--uuid", module_uuid); + + if (!r.status) + break; + } - if (module_pkg && selfhost) + // Create the install configuration. + // + if (create_install && module_pkg) { - step_id s (step_id::bpkg_module_create); + step_id s (step_id::bpkg_target_create); + string mods; for (const char* m: step_args (modules, s)) { if (!mods.empty ()) @@ -1494,41 +1639,37 @@ build (size_t argc, const char* argv[]) mods += m; } - eas = step_args (env_args, s); - cas = step_args (config_args, s); - } - else - mods = "cc"; - - r.status |= run_b ( - b, - trace, r.log, wre, - bkp_step, bkp_status, last_cmd, - "-V", - "create(" + module_conf.representation () + "," + mods + ")", - "config.config.load=~build2", - "config.config.persist+='config.*'@unused=drop", - eas, - cas); + // b create() config.config.load=~build2 [ ] + // + r.status |= run_b ( + b, + trace, r.log, wre, + bkp_step, bkp_status, last_cmd, + "-V", + "create(" + install_conf.representation () + "," + mods + ")", + "config.config.load=~build2", + "config.config.persist+='config.*'@unused=drop", + step_args (env_args, s), + step_args (config_args, s)); - if (!r.status) - break; + if (!r.status) + break; - // bpkg create --existing --type build2 - // - r.status |= run_bpkg ( - b, - trace, r.log, wre, - bkp_step, bkp_status, last_cmd, - "-v", - "create", - "--existing", - "-d", module_conf, - "--type", "build2", - "--name", "module"); + // bpkg create --existing + // + r.status |= run_bpkg ( + b, + trace, r.log, wre, + bkp_step, bkp_status, last_cmd, + "-v", + "create", + "--existing", + "-d", install_conf, + "--uuid", install_uuid); - if (!r.status) - break; + if (!r.status) + break; + } } // Link the configurations. @@ -1588,10 +1729,45 @@ build (size_t argc, const char* argv[]) break; } } + + // Link the install configuration only for the host package. Note that + // the module package may not have build-time dependencies and so + // doesn't need configurations for them. + // + if (create_install && host_pkg) + { + r.status |= run_bpkg ( + b, + trace, r.log, wre, + bkp_step, bkp_status, last_cmd, + "-v", + "link", + "-d", install_conf, + host_conf); + + if (!r.status) + break; + + if (create_module) + { + r.status |= run_bpkg ( + b, + trace, r.log, wre, + bkp_step, bkp_status, last_cmd, + "-v", + "link", + "-d", install_conf, + module_conf); + + if (!r.status) + break; + } + } } - // Fetch repositories into the main package configuration and the target - // configuration for external build-time test, if any. + // Fetch repositories into the main package configuration, the target + // configuration for external build-time tests, if any, and the install + // configuration, if present. // // bpkg add // @@ -1635,6 +1811,51 @@ build (size_t argc, const char* argv[]) break; } + if (create_install) + { + // bpkg add + // + { + step_id b (step_id::bpkg_configure_add); + step_id s (step_id::bpkg_configure_add); + + r.status |= run_bpkg ( + b, + trace, r.log, wre, + bkp_step, bkp_status, last_cmd, + "-v", + "add", + "-d", install_conf, + step_args (env_args, s), + step_args (config_args, s), + repo); + + if (!r.status) + break; + } + + // bpkg fetch + // + { + step_id b (step_id::bpkg_configure_fetch); + step_id s (step_id::bpkg_configure_fetch); + + r.status |= run_bpkg ( + b, + trace, r.log, wre, + bkp_step, bkp_status, last_cmd, + "-v", + "fetch", + "-d", install_conf, + step_args (env_args, s), + step_args (config_args, s), + trust_ops); + + if (!r.status) + break; + } + } + if (has_buildtime_tests) { // bpkg add @@ -1682,77 +1903,106 @@ build (size_t argc, const char* argv[]) // Configure all the packages using a single bpkg-pkg-build command. // - // The overall command looks like this (but some parts may be omitted): - // - // bpkg build --configure-only - // { }+ - // { }+ { ... } - // { }+ { ... } - // - - // Add the main package args. - // - // Also add the external runtime test packages here since they share the - // configuration directory with the main package. + // First, prepare the common and package arguments. // + strings common_args; strings pkg_args; - { - step_id s (target_pkg ? step_id::bpkg_target_configure_build : - host_pkg ? step_id::bpkg_host_configure_build : - step_id::bpkg_module_configure_build); + if (target_pkg) // The simple common case (see above)? + { + // The overall command looks like this: + // + // bpkg build --configure-only -- + // ... + // + step_id s (step_id::bpkg_target_configure_build); step_id f1 (step_id::b_configure); step_id f2 (step_id::bpkg_configure_build); cstrings eas (step_args (env_args, s, f1, f2)); cstrings cas (step_args (config_args, s, f1, f2)); - // Main package configuration name. + common_args.push_back ("--checkout-root"); + common_args.push_back (dist_root.string ()); + + common_args.insert (pkg_args.end (), + make_move_iterator (eas.begin ()), + make_move_iterator (eas.end ())); + + common_args.insert (pkg_args.end (), + make_move_iterator (cas.begin ()), + make_move_iterator (cas.end ())); + + pkg_args.push_back (pkg_rev); + + // Add test dependency package constraints (for example 'bar > 1.0.0'). + // + for (auto t: runtime_tests) + pkg_args.push_back (t.string ()); + } + else + { + // The overall command looks like this (but some parts may be omitted): + // + // bpkg build --configure-only -- + // { }+ + // { }+ { ... } + // { }+ + // { }+ { ... } // - string conf_name (main_pkg_conf == root_conf ? "" : - host_pkg ? "host" : - "module"); - // Add the main package. + // Add the main package args. + // + // Also add the external runtime test packages here since they share + // the configuration directory with the main package. // { - if (!eas.empty () || !cas.empty () || !conf_name.empty ()) + step_id s (target_pkg ? step_id::bpkg_target_configure_build : + host_pkg ? step_id::bpkg_host_configure_build : + step_id::bpkg_module_configure_build); + + step_id f1 (step_id::b_configure); + step_id f2 (step_id::bpkg_configure_build); + + cstrings eas (step_args (env_args, s, f1, f2)); + cstrings cas (step_args (config_args, s, f1, f2)); + + // Main package configuration name. + // + const char* conf_uuid (target_pkg ? target_uuid : + host_pkg ? host_uuid : + module_uuid); + + // Add the main package. + // { pkg_args.push_back ("{"); - if (!conf_name.empty ()) - { - pkg_args.push_back ("--config-name"); - pkg_args.push_back (conf_name); - } + pkg_args.push_back ("--config-uuid"); + pkg_args.push_back (conf_uuid); + + pkg_args.push_back ("--checkout-root"); + pkg_args.push_back (dist_root.string ()); pkg_args.insert (pkg_args.end (), eas.begin (), eas.end ()); pkg_args.insert (pkg_args.end (), cas.begin (), cas.end ()); pkg_args.push_back ("}+"); - } - pkg_args.push_back (pkg_rev); - } - - // Add the runtime test packages. - // - if (has_runtime_tests) - { - bool og (!conf_name.empty () || - bootstrap_import || - !eas.empty () || - !cas.empty ()); + pkg_args.push_back (pkg_rev); + } - if (og) + // Add the runtime test packages. + // + if (has_runtime_tests) { pkg_args.push_back ("{"); - if (!conf_name.empty ()) - { - pkg_args.push_back ("--config-name"); - pkg_args.push_back (conf_name); - } + pkg_args.push_back ("--config-uuid"); + pkg_args.push_back (conf_uuid); + + pkg_args.push_back ("--checkout-root"); + pkg_args.push_back (dist_root.string ()); if (bootstrap_import) pkg_args.push_back (*bootstrap_import); @@ -1766,43 +2016,46 @@ build (size_t argc, const char* argv[]) make_move_iterator (cas.end ())); pkg_args.push_back ("}+"); - } - // Add test dependency package constraints (for example - // 'bar > 1.0.0') and group them if there are multiple of them. - // - if (og && runtime_tests.size () != 1) - pkg_args.push_back ("{"); + // Add test dependency package constraints and group them if there + // are multiple of them. + // + if (runtime_tests.size () != 1) + pkg_args.push_back ("{"); - for (auto t: runtime_tests) - pkg_args.push_back (t.string ()); + for (auto t: runtime_tests) + pkg_args.push_back (t.string ()); - if (og && runtime_tests.size () != 1) - pkg_args.push_back ("}"); + if (runtime_tests.size () != 1) + pkg_args.push_back ("}"); + } } - } - // Add the external build-time test packages. - // - // Note that if present, they are always configured in the root - // configuration and thus don't require --config-name. - // - if (has_buildtime_tests) - { - step_id s (step_id::bpkg_target_configure_build); - step_id f (step_id::bpkg_configure_build); + // Add the main package configured in the install configuration. + // + if (create_install) + { + step_id s (step_id::bpkg_target_configure_build); + step_id f1 (step_id::b_configure); + step_id f2 (step_id::bpkg_configure_build); - cstrings eas (step_args (env_args, s, f)); - cstrings cas (step_args (config_args, s, f)); + cstrings eas (step_args (env_args, s, f1, f2)); + cstrings cas (step_args (config_args, s, f1, f2)); - bool og (bootstrap_import || !eas.empty () || !cas.empty ()); + common_args.push_back ("-d"); + common_args.push_back (install_conf.string ()); - if (og) - { pkg_args.push_back ("{"); - if (bootstrap_import) - pkg_args.push_back (*bootstrap_import); + pkg_args.push_back ("--config-uuid"); + pkg_args.push_back (install_uuid); + + // Note that we do another re-distribution (with a separate + // --checkout-root) in case the package is missing file that + // are only used during installation. + // + pkg_args.push_back ("--checkout-root"); + pkg_args.push_back (dist_install_root.string ()); pkg_args.insert (pkg_args.end (), make_move_iterator (eas.begin ()), @@ -1813,21 +2066,56 @@ build (size_t argc, const char* argv[]) make_move_iterator (cas.end ())); pkg_args.push_back ("}+"); + + pkg_args.push_back (pkg_rev); } - // Add test dependency package constraints and group them if there are - // multiple of them. + // Add the external build-time test packages. // - if (og && buildtime_tests.size () != 1) + if (has_buildtime_tests) + { + step_id s (step_id::bpkg_target_configure_build); + step_id f1 (step_id::b_configure); + step_id f2 (step_id::bpkg_configure_build); + + cstrings eas (step_args (env_args, s, f1, f2)); + cstrings cas (step_args (config_args, s, f1, f2)); + pkg_args.push_back ("{"); - // Strip the build-time mark. - // - for (auto t: buildtime_tests) - pkg_args.push_back (t.dependency::string ()); + pkg_args.push_back ("--config-uuid"); + pkg_args.push_back (target_uuid); - if (og && buildtime_tests.size () != 1) - pkg_args.push_back ("}"); + pkg_args.push_back ("--checkout-root"); + pkg_args.push_back (dist_root.string ()); + + if (bootstrap_import) + pkg_args.push_back (*bootstrap_import); + + pkg_args.insert (pkg_args.end (), + make_move_iterator (eas.begin ()), + make_move_iterator (eas.end ())); + + pkg_args.insert (pkg_args.end (), + make_move_iterator (cas.begin ()), + make_move_iterator (cas.end ())); + + pkg_args.push_back ("}+"); + + // Add test dependency package constraints and group them if there + // are multiple of them. + // + if (buildtime_tests.size () != 1) + pkg_args.push_back ("{"); + + // Strip the build-time mark. + // + for (auto t: buildtime_tests) + pkg_args.push_back (t.dependency::string ()); + + if (buildtime_tests.size () != 1) + pkg_args.push_back ("}"); + } } // Finally, configure all the packages. @@ -1845,13 +2133,13 @@ build (size_t argc, const char* argv[]) "-v", "build", "--configure-only", - "--checkout-root", dist_root, "--rebuild-checksum", tm.dependency_checksum ? *tm.dependency_checksum : "", "--yes", "-d", root_conf, step_args (env_args, s), step_args (config_args, s), + common_args, "--", pkg_args); @@ -1912,19 +2200,29 @@ build (size_t argc, const char* argv[]) break; } - change_wd (trace, &r.log, main_pkg_conf); - - // Redistribute the main package, if required (test packages will be - // handled later). + // Redistribute the main package in both build and install + // configurations, if required (test packages will be handled later). // if (exists (dist_src)) { + change_wd (trace, &r.log, main_pkg_conf); + step_id b (step_id::bpkg_configure_build); if (!redist (b, r, dist_root, pkg_dir)) break; } + if (exists (dist_install_src)) + { + change_wd (trace, &r.log, rwd / install_conf); + + step_id b (step_id::bpkg_configure_build); + + if (!redist (b, r, dist_install_root, pkg_dir)) + break; + } + rm.status |= r.status; } @@ -1933,6 +2231,8 @@ build (size_t argc, const char* argv[]) { operation_result& r (add_result ("update")); + change_wd (trace, &r.log, rwd / main_pkg_conf); + // bpkg update // step_id b (step_id::bpkg_update); @@ -2201,12 +2501,14 @@ build (size_t argc, const char* argv[]) // // 4. Uninstall the package. + install_conf = rwd / (create_install ? install_conf : main_pkg_conf); + // Install. // { operation_result& r (add_result ("install")); - change_wd (trace, &r.log, rwd / main_pkg_conf); + change_wd (trace, &r.log, install_conf); // bpkg install // @@ -2764,8 +3066,6 @@ build (size_t argc, const char* argv[]) pkg_args.push_back ("?sys:" + pkg); - dir_path dist_root (rwd / dir_path ("dist-installed")); - // Finally, configure all the test packages. // { @@ -2790,7 +3090,7 @@ build (size_t argc, const char* argv[]) "-v", "build", "--configure-only", - "--checkout-root", dist_root, + "--checkout-root", dist_installed_root, "--yes", "-d", root_conf, step_args (env_args, g), @@ -2816,7 +3116,7 @@ build (size_t argc, const char* argv[]) if (!test (r, runtime_tests, - dist_root, + dist_installed_root, true /* installed */, envvars)) break; @@ -2830,7 +3130,7 @@ build (size_t argc, const char* argv[]) if (!test (r, buildtime_tests, - dist_root, + dist_installed_root, true /* installed */, envvars)) break; @@ -2845,7 +3145,7 @@ build (size_t argc, const char* argv[]) { operation_result& r (add_result ("uninstall")); - change_wd (trace, &r.log, rwd / main_pkg_conf); + change_wd (trace, &r.log, install_conf); // bpkg uninstall // -- cgit v1.1